FIX: Improve user timezone saving (#9230)

Based on issues identified in https://meta.discourse.org/t/improved-bookmarks-with-reminders/144542/20

* Implement the resolvedTimezone() function on the user model where we return the user's timezone if it has been set, or we guess it using moment and save it to the user using an update call if it has not yet been set. This covers the cases of users who do not log out/in often who will not get their timezone set via login. This also makes sure the guess + save is done in a non-obtrusive way not on every page -- only when it is needed.

* Before if a user's timezone was blank when they visited their profile page we were autofilling the dropdown with the guessed timezone from moment. However this was confusing as it would appear you have that timezone saved in the DB when you really didn't. Now we do not autofill the dropdown and added a button to automatically guess the current timezone to make everything more explicit.

app/assets/javascripts/discourse/controllers/bookmark.js

@@ -45,7 +45,7 @@ export default Controller.extend(ModalFunctionality, {
       customReminderTime: null,
       lastCustomReminderDate: null,
       lastCustomReminderTime: null,
-      userTimezone: this.currentUser.timezone
+      userTimezone: this.currentUser.resolvedTimezone()
     });
 
     this.loadLastUsedCustomReminderDatetime();

app/assets/javascripts/discourse/controllers/preferences/profile.js

@@ -73,6 +73,10 @@ export default Controller.extend({
       );
     },
 
+    useCurrentTimezone() {
+      this.model.set("user_option.timezone", moment.tz.guess());
+    },
+
     save() {
       this.set("saved", false);
 
@@ -95,7 +99,7 @@ export default Controller.extend({
           // update the timezone in memory so we can use the new
           // one if we change routes without reloading the user
           if (this.currentUser.id === this.model.id) {
-            this.currentUser.timezone = this.model.user_option.timezone;
+            this.currentUser.changeTimezone(this.model.user_option.timezone);
           }
 
           cookAsync(model.get("bio_raw"))

app/assets/javascripts/discourse/models/bookmark.js

@@ -113,7 +113,7 @@ const Bookmark = RestModel.extend({
   formattedReminder(bookmarkReminderAt) {
     const currentUser = PreloadStore.get("currentUser");
     return moment
-      .tz(bookmarkReminderAt, currentUser.timezone || moment.tz.guess())
+      .tz(bookmarkReminderAt, currentUser.resolvedTimezone())
       .format(I18n.t("dates.long_with_year"));
   },
 

app/assets/javascripts/discourse/models/user.js

@@ -840,10 +840,40 @@ const User = RestModel.extend({
       !secondFactorEnabled &&
       (enforce === "all" || (enforce === "staff" && staff))
     );
+  },
+
+  resolvedTimezone() {
+    if (this._timezone) {
+      return this._timezone;
+    }
+
+    this.changeTimezone(moment.tz.guess());
+    ajax(userPath(this.username + ".json"), {
+      type: "PUT",
+      dataType: "json",
+      data: { timezone: this._timezone }
+    });
+
+    return this._timezone;
+  },
+
+  changeTimezone(tz) {
+    this._timezone = tz;
   }
 });
 
 User.reopenClass(Singleton, {
+  munge(json) {
+    // timezone should not be directly accessed, use
+    // resolvedTimezone() and changeTimezone(tz)
+    if (!json._timezone) {
+      json._timezone = json.timezone;
+      delete json.timezone;
+    }
+
+    return json;
+  },
+
   // Find a `User` for a given username.
   findByUsername(username, options) {
     const user = User.create({ username: username });
@@ -852,7 +882,7 @@ User.reopenClass(Singleton, {
 
   // TODO: Use app.register and junk Singleton
   createCurrent() {
-    const userJson = PreloadStore.get("currentUser");
+    let userJson = PreloadStore.get("currentUser");
 
     if (userJson && userJson.primary_group_id) {
       const primaryGroup = userJson.groups.find(
@@ -864,6 +894,7 @@ User.reopenClass(Singleton, {
     }
 
     if (userJson) {
+      userJson = User.munge(userJson);
       const store = Discourse.__container__.lookup("service:store");
       return store.createRecord("user", userJson);
     }

app/assets/javascripts/discourse/routes/preferences-profile.js

@@ -1,13 +1,8 @@
 import RestrictedUserRoute from "discourse/routes/restricted-user";
-import { set } from "@ember/object";
 
 export default RestrictedUserRoute.extend({
   showFooter: true,
   setupController(controller, model) {
-    if (!model.user_option.timezone) {
-      set(model, "user_option.timezone", moment.tz.guess());
-    }
-
     controller.set("model", model);
   }
 });

app/assets/javascripts/discourse/templates/preferences/profile.hbs

@@ -14,6 +14,7 @@
     onChange=(action (mut model.user_option.timezone))
     class="input-xxlarge"
   }}
+  {{d-button icon="globe" label="user.use_current_timezone" action=(action "useCurrentTimezone") }}
 </div>
 
 <div class="control-group pref-location">

app/assets/javascripts/discourse/widgets/post-menu.js

@@ -315,7 +315,7 @@ registerButton("bookmarkWithReminder", (attrs, state, siteSettings) => {
 
     if (attrs.bookmarkReminderAt) {
       let reminderAtDate = moment(attrs.bookmarkReminderAt).tz(
-        Discourse.currentUser.timezone
+        Discourse.currentUser.resolvedTimezone()
       );
       title = "bookmarks.created_with_reminder";
       titleOptions = {

app/assets/stylesheets/common/base/user.scss

@@ -724,3 +724,7 @@
     }
   }
 }
+
+.timezone-input {
+  margin-bottom: 0.5em;
+}

config/locales/client.en.yml

@@ -846,6 +846,7 @@ en:
         clear:
           title: "Clear"
           warning: "Are you sure you want to clear your featured topic?"
+      use_current_timezone: "Use Current Timezone"
       profile_hidden: "This user's public profile is hidden."
       expand_profile: "Expand"
       collapse_profile: "Collapse"

test/javascripts/controllers/bookmark-test.js.es6

@@ -1,10 +1,11 @@
-import { currentUser } from "helpers/qunit-helpers";
+import { logIn } from "helpers/qunit-helpers";
+import User from "discourse/models/user";
 let BookmarkController;
 
 moduleFor("controller:bookmark", {
   beforeEach() {
-    Discourse.currentUser = currentUser();
-    BookmarkController = this.subject({ currentUser: Discourse.currentUser });
+    logIn();
+    BookmarkController = this.subject({ currentUser: User.current() });
   },
 
   afterEach() {
@@ -119,7 +120,7 @@ QUnit.test(
   function(assert) {
     let dt = moment.tz(
       "2019-12-11T11:37:16",
-      BookmarkController.currentUser.timezone
+      BookmarkController.currentUser.resolvedTimezone()
     );
 
     assert.equal(
@@ -181,14 +182,23 @@ QUnit.test(
   }
 );
 
-QUnit.test(
-  "userHasTimezoneSet updates true/false based on whether the current user timezone is set globally",
-  function(assert) {
-    Discourse.currentUser.timezone = null;
-    BookmarkController.onShow();
-    assert.equal(BookmarkController.userHasTimezoneSet, false);
-    Discourse.currentUser.timezone = "Australia/Brisbane";
-    BookmarkController.onShow();
-    assert.equal(BookmarkController.userHasTimezoneSet, true);
-  }
-);
+QUnit.test("user timezone updates when the modal is shown", function(assert) {
+  User.current().changeTimezone(null);
+  let stub = sandbox.stub(moment.tz, "guess").returns("Europe/Moscow");
+  BookmarkController.onShow();
+  assert.equal(BookmarkController.userHasTimezoneSet, true);
+  assert.equal(
+    BookmarkController.userTimezone,
+    "Europe/Moscow",
+    "the user does not have their timezone set and a timezone is guessed"
+  );
+  User.current().changeTimezone("Australia/Brisbane");
+  BookmarkController.onShow();
+  assert.equal(BookmarkController.userHasTimezoneSet, true);
+  assert.equal(
+    BookmarkController.userTimezone,
+    "Australia/Brisbane",
+    "the user does their timezone set"
+  );
+  stub.restore();
+});

test/javascripts/models/user-test.js.es6

@@ -1,5 +1,7 @@
 import User from "discourse/models/user";
 import Group from "discourse/models/group";
+import * as ajaxlib from "discourse/lib/ajax";
+import pretender from "helpers/create-pretender";
 
 QUnit.module("model:user");
 
@@ -66,3 +68,39 @@ QUnit.test("canMangeGroup", assert => {
     "a group owner should be able to manage the group"
   );
 });
+
+QUnit.test("resolvedTimezone", assert => {
+  const tz = "Australia/Brisbane";
+  let user = User.create({ timezone: tz, username: "chuck" });
+  let stub = sandbox.stub(moment.tz, "guess").returns("America/Chicago");
+
+  pretender.put("/u/chuck.json", () => {
+    return [200, { "Content-Type": "application/json" }, {}];
+  });
+
+  let spy = sandbox.spy(ajaxlib, "ajax");
+  assert.equal(
+    user.resolvedTimezone(),
+    tz,
+    "if the user already has a timezone return it"
+  );
+  assert.ok(
+    spy.notCalled,
+    "if the user already has a timezone do not call AJAX update"
+  );
+  user = User.create({ username: "chuck" });
+  assert.equal(
+    user.resolvedTimezone(),
+    "America/Chicago",
+    "if the user has no timezone guess it with moment"
+  );
+  assert.ok(
+    spy.calledWith("/u/chuck.json", {
+      type: "PUT",
+      dataType: "json",
+      data: { timezone: "America/Chicago" }
+    }),
+    "if the user has no timezone save it with an AJAX update"
+  );
+  stub.restore();
+});