FEATURE: Add timezone to core user_options (#8380)

* Add timezone to user_options table

* Also migrate existing timezone values from UserCustomField,
  which is where the discourse-calendar plugin is storing them

* Allow user to change their core timezone from Profile

* Auto guess & set timezone on login & invite accept & signup

* Serialize user_options.timezone for group members. this is so discourse-group-timezones can access the core user timezone, as it is being removed in discourse-calendar.

* Annotate user_option with timezone

* Validate timezone values

app/assets/javascripts/discourse/controllers/invites-show.js.es6

@@ -82,7 +82,8 @@ export default Controller.extend(
             username: this.accountUsername,
             name: this.accountName,
             password: this.accountPassword,
-            user_custom_fields: userCustomFields
+            user_custom_fields: userCustomFields,
+            timezone: moment.tz.guess()
           }
         })
           .then(result => {

app/assets/javascripts/discourse/controllers/login.js.es6

@@ -122,7 +122,8 @@ export default Controller.extend(ModalFunctionality, {
           password: this.loginPassword,
           second_factor_token: this.secondFactorToken,
           second_factor_method: this.secondFactorMethod,
-          security_key_credential: this.securityKeyCredential
+          security_key_credential: this.securityKeyCredential,
+          timezone: moment.tz.guess()
         }
       }).then(
         result => {

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

@@ -18,7 +18,8 @@ export default Controller.extend(PreferencesTabController, {
       "user_fields",
       "profile_background_upload_url",
       "card_background_upload_url",
-      "date_of_birth"
+      "date_of_birth",
+      "timezone"
     ];
   },
 

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

@@ -302,7 +302,8 @@ const User = RestModel.extend({
       "homepage_id",
       "hide_profile_and_presence",
       "text_size",
-      "title_count_mode"
+      "title_count_mode",
+      "timezone"
     ];
 
     if (fields) {
@@ -901,7 +902,8 @@ User.reopenClass(Singleton, {
         username: attrs.accountUsername,
         password_confirmation: attrs.accountPasswordConfirm,
         challenge: attrs.accountChallenge,
-        user_fields: attrs.userFields
+        user_fields: attrs.userFields,
+        timezone: moment.tz.guess()
       },
       type: "POST"
     });

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

@@ -1,5 +1,10 @@
 import RestrictedUserRoute from "discourse/routes/restricted-user";
 
 export default RestrictedUserRoute.extend({
-  showFooter: true
+  showFooter: true,
+  setupController(controller, model) {
+    model.user_option.timezone =
+      model.user_option.timezone || moment.tz.guess();
+    controller.set("model", model);
+  }
 });

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

@@ -7,6 +7,11 @@
 </div>
 {{/if}}
 
+<div class="control-group pref-timezone">
+  <label class="control-label">{{i18n 'user.timezone'}}</label>
+  {{timezone-input value=model.user_option.timezone onSelect=(action (mut model.user_option.timezone)) class="input-xxlarge"}}
+</div>
+
 <div class="control-group pref-location">
   <label class="control-label">{{i18n 'user.location'}}</label>
   <div class="controls">

app/controllers/groups_controller.rb

@@ -269,7 +269,7 @@ class GroupsController < ApplicationController
       end
     end
 
-    users = users.select('users.*, group_users.created_at as added_at')
+    users = users.joins(:user_option).select('users.*, user_options.timezone, group_users.created_at as added_at')
 
     members = users
       .order('NOT group_users.owner')

app/controllers/invites_controller.rb

@@ -40,7 +40,7 @@ class InvitesController < ApplicationController
 
   def perform_accept_invitation
     params.require(:id)
-    params.permit(:username, :name, :password, user_custom_fields: {})
+    params.permit(:username, :name, :password, :timezone, user_custom_fields: {})
     invite = Invite.find_by(invite_key: params[:id])
 
     if invite.present?
@@ -48,6 +48,7 @@ class InvitesController < ApplicationController
         user = invite.redeem(username: params[:username], name: params[:name], password: params[:password], user_custom_fields: params[:user_custom_fields], ip_address: request.remote_ip)
         if user.present?
           log_on_user(user) if user.active?
+          user.update_timezone_if_missing(params[:timezone])
           post_process_invite(user)
         end
 

app/controllers/session_controller.rb

@@ -540,6 +540,7 @@ class SessionController < ApplicationController
 
   def login(user)
     session.delete(ACTIVATE_USER_KEY)
+    user.update_timezone_if_missing(params[:timezone])
     log_on_user(user)
 
     if payload = cookies.delete(:sso_payload)

app/controllers/users_controller.rb

@@ -128,7 +128,7 @@ class UsersController < ApplicationController
       end
     end
 
-    json_result(user, serializer: UserSerializer, additional_errors: [:user_profile]) do |u|
+    json_result(user, serializer: UserSerializer, additional_errors: [:user_profile, :user_option]) do |u|
       updater = UserUpdater.new(current_user, user)
       updater.update(attributes.permit!)
     end
@@ -387,7 +387,7 @@ class UsersController < ApplicationController
 
     params[:locale] ||= I18n.locale unless current_user
 
-    new_user_params = user_params
+    new_user_params = user_params.except(:timezone)
     user = User.unstage(new_user_params)
     user = User.new(new_user_params) if user.nil?
 
@@ -435,6 +435,7 @@ class UsersController < ApplicationController
     if user.save
       authentication.finish
       activation.finish
+      user.update_timezone_if_missing(params[:timezone])
 
       secure_session[HONEYPOT_KEY] = nil
       secure_session[CHALLENGE_KEY] = nil

app/models/user.rb

@@ -670,6 +670,15 @@ class User < ActiveRecord::Base
     create_visit_record!(date) unless visit_record_for(date)
   end
 
+  def update_timezone_if_missing(timezone)
+    return if timezone.blank? || !TimezoneValidator.valid?(timezone)
+
+    # we only want to update the user's timezone if they have not set it themselves
+    UserOption
+      .where(user_id: self.id, timezone: nil)
+      .update_all(timezone: timezone)
+  end
+
   def update_posts_read!(num_posts, opts = {})
     now = opts[:at] || Time.zone.now
     _retry = opts[:retry] || false

app/models/user_option.rb

@@ -42,6 +42,7 @@ class UserOption < ActiveRecord::Base
   validates :text_size_key, inclusion: { in: UserOption.text_sizes.values }
   validates :email_level, inclusion: { in: UserOption.email_level_types.values }
   validates :email_messages_level, inclusion: { in: UserOption.email_level_types.values }
+  validates :timezone, timezone: true
 
   def set_defaults
     self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode
@@ -224,6 +225,7 @@ end
 #  email_messages_level             :integer          default(0), not null
 #  title_count_mode_key             :integer          default(0), not null
 #  enable_defer                     :boolean          default(FALSE), not null
+#  timezone                         :string
 #
 # Indexes
 #

app/serializers/group_user_serializer.rb

@@ -7,7 +7,8 @@ class GroupUserSerializer < BasicUserSerializer
              :title,
              :last_posted_at,
              :last_seen_at,
-             :added_at
+             :added_at,
+             :timezone
 
   def include_added_at
     object.respond_to? :added_at

app/serializers/user_option_serializer.rb

@@ -27,7 +27,8 @@ class UserOptionSerializer < ApplicationSerializer
              :hide_profile_and_presence,
              :text_size,
              :text_size_seq,
-             :title_count_mode
+             :title_count_mode,
+             :timezone
 
   def auto_track_topics_after_msecs
     object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs

app/services/user_updater.rb

@@ -40,7 +40,8 @@ class UserUpdater
     :homepage_id,
     :hide_profile_and_presence,
     :text_size,
-    :title_count_mode
+    :title_count_mode,
+    :timezone
   ]
 
   def initialize(actor, user)

config/locales/client.en.yml

@@ -821,6 +821,7 @@ en:
       collapse_profile: "Collapse"
       bookmarks: "Bookmarks"
       bio: "About me"
+      timezone: "Timezone"
       invited_by: "Invited By"
       trust_level: "Trust Level"
       notifications: "Notifications"

config/locales/server.en.yml

@@ -163,6 +163,7 @@ en:
       inclusion: is not included in the list
       invalid: is invalid
       is_invalid: "seems unclear, is it a complete sentence?"
+      invalid_timezone: "'%{tz}' is not a valid timezone"
       contains_censored_words: "contains the following censored words: %{censored_words}"
       less_than: must be less than %{count}
       less_than_or_equal_to: must be less than or equal to %{count}

db/migrate/20191120015344_add_timezone_to_user_options.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddTimezoneToUserOptions < ActiveRecord::Migration[6.0]
+  def up
+    add_column :user_options, :timezone, :string
+    execute(
+      <<-SQL
+        UPDATE user_options
+        SET timezone = ucf.value
+        FROM user_custom_fields AS ucf
+        WHERE ucf.user_id = user_options.user_id AND ucf.name = 'timezone'
+      SQL
+    )
+  end
+
+  def down
+    remove_column :user_options, :timezone
+  end
+end

lib/validators/timezone_validator.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class TimezoneValidator < ActiveModel::EachValidator
+  def self.valid?(value)
+    ok = ActiveSupport::TimeZone[value].present?
+    Rails.logger.warn("Invalid timezone '#{value}' detected!") if !ok
+    ok
+  end
+
+  def self.error_message(value)
+    I18n.t("errors.messages.invalid_timezone", tz: value)
+  end
+
+  def validate_each(record, attribute, value)
+    return if value.blank? || TimezoneValidator.valid?(value)
+    record.errors.add(
+      attribute,
+      :timezone,
+      message: TimezoneValidator.error_message(value)
+    )
+  end
+end

spec/lib/validators/timezone_validator_spec.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe TimezoneValidator do
+  describe "#valid?" do
+    context "when timezone is ok" do
+      it "returns true" do
+        expect(described_class.valid?("Australia/Brisbane")).to eq(true)
+      end
+    end
+
+    context "when timezone is not ok" do
+      it "returns false" do
+        expect(described_class.valid?("Mars")).to eq(false)
+      end
+    end
+  end
+
+  describe "#validate_each" do
+    let(:record) { Fabricate(:active_user).user_option }
+
+    context "when timezone is ok" do
+      it "adds no errors to the record" do
+        record.timezone = "Australia/Melbourne"
+        record.save
+        expect(record.errors.full_messages.empty?).to eq(true)
+      end
+    end
+
+    context "when timezone is blank" do
+      it "adds no errors to the record" do
+        record.timezone = nil
+        record.save
+        expect(record.errors.full_messages.empty?).to eq(true)
+      end
+    end
+
+    context "when timezone is not ok" do
+      it "adds errors to the record" do
+        record.timezone = "Mars"
+        record.save
+        expect(record.errors.full_messages).to include(
+          "Timezone 'Mars' is not a valid timezone"
+        )
+      end
+    end
+  end
+end

spec/models/user_spec.rb

@@ -917,6 +917,48 @@ describe User do
     end
   end
 
+  describe "update_timezone_if_missing" do
+    let(:timezone) { nil }
+
+    it "does nothing if timezone is nil" do
+      user.update_timezone_if_missing(timezone)
+      expect(user.reload.user_option.timezone).to eq(nil)
+    end
+
+    context "if timezone is provided" do
+      context "if the timezone is valid" do
+        let(:timezone) { "Australia/Melbourne" }
+        context "if no timezone exists on user option" do
+          it "sets the timezone for the user" do
+            user.update_timezone_if_missing(timezone)
+            expect(user.reload.user_option.timezone).to eq(timezone)
+          end
+        end
+      end
+
+      context "if the timezone is not valid" do
+        let(:timezone) { "Jupiter" }
+        context "if no timezone exists on user option" do
+          it "does not set the timezone for the user" do
+            user.update_timezone_if_missing(timezone)
+            expect(user.reload.user_option.timezone).to eq(nil)
+          end
+        end
+      end
+
+      context "if a timezone already exists on user option" do
+        before do
+          user.user_option.update_attribute(:timezone, "America/Denver")
+        end
+
+        it "does not update the timezone" do
+          user.update_timezone_if_missing(timezone)
+          expect(user.reload.user_option.timezone).to eq("America/Denver")
+        end
+      end
+    end
+  end
+
   describe "last_seen_at" do
     fab!(:user) { Fabricate(:user) }
 

spec/requests/invites_controller_spec.rb

@@ -290,6 +290,16 @@ describe InvitesController do
             expect(json["success"]).to eq(true)
             expect(json["redirect_to"]).to eq(topic.relative_url)
           end
+
+          context "if a timezone guess is provided" do
+            it "sets the timezone of the user in user_options" do
+              put "/invites/show/#{invite.invite_key}.json", params: { timezone: "Australia/Melbourne" }
+              expect(response.status).to eq(200)
+              invite.reload
+              user = User.find(invite.user_id)
+              expect(user.user_option.timezone).to eq("Australia/Melbourne")
+            end
+          end
         end
 
         context 'failure' do

spec/requests/session_controller_spec.rb

@@ -1048,6 +1048,16 @@ RSpec.describe SessionController do
           expect(user.user_auth_tokens.count).to eq(1)
           expect(UserAuthToken.hash_token(cookies[:_t])).to eq(user.user_auth_tokens.first.auth_token)
         end
+
+        context "when timezone param is provided" do
+          it "sets the user_option timezone for the user" do
+            post "/session.json", params: {
+              login: user.username, password: 'myawesomepassword', timezone: "Australia/Melbourne"
+            }
+            expect(response.status).to eq(200)
+            expect(user.reload.user_option.timezone).to eq("Australia/Melbourne")
+          end
+        end
       end
 
       context 'when user has 2-factor logins' do

spec/requests/users_controller_spec.rb

@@ -702,6 +702,22 @@ describe UsersController do
         post_user
         expect(User.find_by(username: @user.username).locale).to eq('fr')
       end
+
+      context "when timezone is provided as a guess on signup" do
+        let(:post_user_params) do
+          { name: @user.name,
+            username: @user.username,
+            password: "strongpassword",
+            email: @user.email,
+            timezone: "Australia/Brisbane" }
+        end
+
+        it "sets the timezone" do
+          post_user
+          expect(response.status).to eq(200)
+          expect(User.find_by(username: @user.username).user_option.timezone).to eq("Australia/Brisbane")
+        end
+      end
     end
 
     context 'when creating a non active user (unconfirmed email)' do