diff --git a/app/assets/javascripts/discourse/controllers/invites-show.js.es6 b/app/assets/javascripts/discourse/controllers/invites-show.js.es6 index f6e0ced4fd9..9be1de123e3 100644 --- a/app/assets/javascripts/discourse/controllers/invites-show.js.es6 +++ b/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 => { diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 701d4102568..7fd028cced2 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/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 => { diff --git a/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 index f726f87e17e..764dbb29dc3 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 +++ b/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" ]; }, diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 830e9a399ac..9b92f4bd030 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/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" }); diff --git a/app/assets/javascripts/discourse/routes/preferences-profile.js.es6 b/app/assets/javascripts/discourse/routes/preferences-profile.js.es6 index 713d79e4207..7b794ef681b 100644 --- a/app/assets/javascripts/discourse/routes/preferences-profile.js.es6 +++ b/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); + } }); diff --git a/app/assets/javascripts/discourse/templates/preferences/profile.hbs b/app/assets/javascripts/discourse/templates/preferences/profile.hbs index 3d5a2bcc750..7bd63f09302 100644 --- a/app/assets/javascripts/discourse/templates/preferences/profile.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/profile.hbs @@ -7,6 +7,11 @@ {{/if}} +
+ + {{timezone-input value=model.user_option.timezone onSelect=(action (mut model.user_option.timezone)) class="input-xxlarge"}} +
+
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 1ca58471ce1..36e3b7b2dad 100644 --- a/app/controllers/groups_controller.rb +++ b/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') diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 7a62d95a56e..a1bbedc5ce1 100644 --- a/app/controllers/invites_controller.rb +++ b/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 diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 7f93b365fb9..0bddc5322b1 100644 --- a/app/controllers/session_controller.rb +++ b/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) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5a113ca717a..80847938291 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index a7426820110..e90315db6a9 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/models/user_option.rb b/app/models/user_option.rb index a5e583f453c..9756f3d0deb 100644 --- a/app/models/user_option.rb +++ b/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 # diff --git a/app/serializers/group_user_serializer.rb b/app/serializers/group_user_serializer.rb index 4b79988877a..aa191cf18d4 100644 --- a/app/serializers/group_user_serializer.rb +++ b/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 diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index 4a8a5a1d0ca..4c78c7b5bda 100644 --- a/app/serializers/user_option_serializer.rb +++ b/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 diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index c5d539e1db4..44c262d8f29 100644 --- a/app/services/user_updater.rb +++ b/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) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5deaca25d53..bbd442acd99 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c93fb0cb285..6c20fb1c126 100644 --- a/config/locales/server.en.yml +++ b/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} diff --git a/db/migrate/20191120015344_add_timezone_to_user_options.rb b/db/migrate/20191120015344_add_timezone_to_user_options.rb new file mode 100644 index 00000000000..8284842cf6c --- /dev/null +++ b/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 diff --git a/lib/validators/timezone_validator.rb b/lib/validators/timezone_validator.rb new file mode 100644 index 00000000000..93ff1a2d78a --- /dev/null +++ b/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 diff --git a/spec/lib/validators/timezone_validator_spec.rb b/spec/lib/validators/timezone_validator_spec.rb new file mode 100644 index 00000000000..4fec4b53ba9 --- /dev/null +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f34a895b444..7815fa778f2 100644 --- a/spec/models/user_spec.rb +++ b/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) } diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 23c7f173b0f..8af1c741eaf 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/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 diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index f554e0fe46d..4d75c201f42 100644 --- a/spec/requests/session_controller_spec.rb +++ b/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 diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 276901b6d12..9a7323a1b4e 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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