FIX: CurrentUser now must be passed to resolveTimezone and user card local time issues (#9734)

FIX: CurrentUser now must be passed to resolveTimezone and user card local time issues (#9734)

  • This is to prevent user’s timezones being changed accidentally e.g. by admin looking at a user
  • This problem only occurred via the user card, however the user card was still calling userTimezone even if the setting to display user time in card was disabled
diff --git a/app/assets/javascripts/discourse/app/components/user-card-contents.js b/app/assets/javascripts/discourse/app/components/user-card-contents.js
index 6295456..7b92e86 100644
--- a/app/assets/javascripts/discourse/app/components/user-card-contents.js
+++ b/app/assets/javascripts/discourse/app/components/user-card-contents.js
@@ -72,7 +72,10 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
 
   @discourseComputed("user")
   userTimezone(user) {
-    return user.resolvedTimezone();
+    if (!this.showUserLocalTime) {
+      return;
+    }
+    return user.resolvedTimezone(this.currentUser);
   },
 
   @discourseComputed("userTimezone")
diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js
index 7d86ed2..4e9e6c3 100644
--- a/app/assets/javascripts/discourse/app/controllers/bookmark.js
+++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js
@@ -73,7 +73,7 @@ export default Controller.extend(ModalFunctionality, {
       customReminderTime: this._defaultCustomReminderTime(),
       lastCustomReminderDate: null,
       lastCustomReminderTime: null,
-      userTimezone: this.currentUser.resolvedTimezone(),
+      userTimezone: this.currentUser.resolvedTimezone(this.currentUser),
       showOptions: false,
       options: {},
       model: this.model || {}
diff --git a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
index 7751be3..54d0853 100644
--- a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
+++ b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
@@ -109,7 +109,7 @@ export default {
             return I18n.t("bookmarked.help.unbookmark_with_reminder", {
               reminder_at: formattedReminderTime(
                 bookmark_reminder_at,
-                currentUser.resolvedTimezone()
+                currentUser.resolvedTimezone(currentUser)
               )
             });
           }
diff --git a/app/assets/javascripts/discourse/app/models/bookmark.js b/app/assets/javascripts/discourse/app/models/bookmark.js
index 44daca7..fd88264 100644
--- a/app/assets/javascripts/discourse/app/models/bookmark.js
+++ b/app/assets/javascripts/discourse/app/models/bookmark.js
@@ -113,7 +113,7 @@ const Bookmark = RestModel.extend({
   formattedReminder(bookmarkReminderAt, currentUser) {
     return formattedReminderTime(
       bookmarkReminderAt,
-      currentUser.resolvedTimezone()
+      currentUser.resolvedTimezone(currentUser)
     ).capitalize();
   },
 
diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js
index 6f4cf5d..fc98218 100644
--- a/app/assets/javascripts/discourse/app/models/user.js
+++ b/app/assets/javascripts/discourse/app/models/user.js
@@ -849,17 +849,21 @@ const User = RestModel.extend({
     );
   },
 
-  resolvedTimezone() {
-    if (this._timezone) {
+  resolvedTimezone(currentUser) {
+    if (this.hasSavedTimezone()) {
       return this._timezone;
     }
 
-    this.changeTimezone(moment.tz.guess());
-    ajax(userPath(this.username + ".json"), {
-      type: "PUT",
-      dataType: "json",
-      data: { timezone: this._timezone }
-    });
+    // only change the timezone and save it if we are
+    // looking at our own user
+    if (currentUser.id === this.id) {
+      this.changeTimezone(moment.tz.guess());
+      ajax(userPath(this.username + ".json"), {
+        type: "PUT",
+        dataType: "json",
+        data: { timezone: this._timezone }
+      });
+    }
 
     return this._timezone;
   },
@@ -868,6 +872,13 @@ const User = RestModel.extend({
     this._timezone = tz;
   },
 
+  hasSavedTimezone() {
+    if (this._timezone) {
+      return true;
+    }
+    return false;
+  },
+
   calculateMutedIds(notificationLevel, id, type) {
     const muted_ids = this.get(type);
     if (notificationLevel === NotificationLevels.MUTED) {
diff --git a/app/assets/javascripts/discourse/app/widgets/post-menu.js b/app/assets/javascripts/discourse/app/widgets/post-menu.js
index 376772c..00c8597 100644
--- a/app/assets/javascripts/discourse/app/widgets/post-menu.js
+++ b/app/assets/javascripts/discourse/app/widgets/post-menu.js
@@ -302,7 +302,7 @@ registerButton("bookmark", attrs => {
     if (attrs.bookmarkReminderAt) {
       let formattedReminder = formattedReminderTime(
         attrs.bookmarkReminderAt,
-        Discourse.currentUser.resolvedTimezone()
+        Discourse.currentUser.resolvedTimezone(Discourse.currentUser)
       );
       title = "bookmarks.created_with_reminder";
       titleOptions.date = formattedReminder;
diff --git a/test/javascripts/acceptance/bookmarks-test.js b/test/javascripts/acceptance/bookmarks-test.js
index 762cd1e..83781f7 100644
--- a/test/javascripts/acceptance/bookmarks-test.js
+++ b/test/javascripts/acceptance/bookmarks-test.js
@@ -205,7 +205,7 @@ test("Editing a bookmark", async assert => {
   mockSuccessfulBookmarkPost(assert);
 
   await visit("/t/internationalization-localization/280");
-  let now = moment.tz(loggedInUser().resolvedTimezone());
+  let now = moment.tz(loggedInUser().resolvedTimezone(loggedInUser()));
   let tomorrow = now.add(1, "day").format("YYYY-MM-DD");
   await openBookmarkModal();
   await fillIn("input#bookmark-name", "Test name");
@@ -234,7 +234,7 @@ test("Editing a bookmark that has a Later Today reminder, and it is before 6pm t
   mockSuccessfulBookmarkPost(assert);
   let clock = fakeTime(
     "2020-05-04T13:00:00",
-    loggedInUser().resolvedTimezone()
+    loggedInUser().resolvedTimezone(loggedInUser())
   );
   await timeStep(clock, () =>
     visit("/t/internationalization-localization/280")
diff --git a/test/javascripts/acceptance/user-card-test.js b/test/javascripts/acceptance/user-card-test.js
index f15bbd3..8ba3c95 100644
--- a/test/javascripts/acceptance/user-card-test.js
+++ b/test/javascripts/acceptance/user-card-test.js
@@ -64,6 +64,29 @@ QUnit.test("user card local time", async assert => {
   );
 });
 
+QUnit.test(
+  "user card local time - does not update timezone for another user",
+  async assert => {
+    User.current().changeTimezone("Australia/Brisbane");
+    let cardResponse = _.clone(userFixtures["/u/charlie/card.json"]);
+    delete cardResponse.user.timezone;
+
+    pretender.get("/u/charlie/card.json", () => [
+      200,
+      { "Content-Type": "application/json" },
+      cardResponse
+    ]);
+
+    await visit("/t/internationalization-localization/280");
+    await click("a[data-user-card=charlie]:first");
+
+    assert.not(
+      exists(".user-card .local-time"),
+      "it does not show the local time if the user card returns a null/undefined timezone for another user"
+    );
+  }
+);
+
 acceptance("User Card", { loggedIn: true });
 
 QUnit.test("user card", async assert => {
diff --git a/test/javascripts/controllers/bookmark-test.js b/test/javascripts/controllers/bookmark-test.js
index 246d10d..0474215 100644
--- a/test/javascripts/controllers/bookmark-test.js
+++ b/test/javascripts/controllers/bookmark-test.js
@@ -114,7 +114,9 @@ QUnit.test(
   function(assert) {
     let dt = moment.tz(
       "2019-12-11T11:37:16",
-      BookmarkController.currentUser.resolvedTimezone()
+      BookmarkController.currentUser.resolvedTimezone(
+        BookmarkController.currentUser
+      )
     );
 
     assert.equal(
@@ -208,7 +210,9 @@ QUnit.test(
       moment
         .tz(
           "2028-12-12 08:00",
-          BookmarkController.currentUser.resolvedTimezone()
+          BookmarkController.currentUser.resolvedTimezone(
+            BookmarkController.currentUser
+          )
         )
         .toString(),
       "the custom date and time are parsed correctly with default time"
diff --git a/test/javascripts/models/user-test.js b/test/javascripts/models/user-test.js

[... diff too long, it was truncated ...]

GitHub sha: 12d4d51d

This commit appears in #9734 which was merged by martin.

I dislike that we have to use Discourse.currentUser here. Could you follow up and pass currentUser through to the builder so that it can use that instead?

1 Like

@eviltrout I did this in this PR, and made it available for plugin post menu buttons too :slight_smile: