From f0e942f6474ffa41be835e2deda9ebdac2f6c93c Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 18 Feb 2016 16:57:22 +1100
Subject: [PATCH] PERF: move 3 more option columns out of the user table
---
.../javascripts/discourse/models/user.js.es6 | 6 +-
.../discourse/templates/user/preferences.hbs | 4 +-
app/jobs/regular/update_top_redirection.rb | 2 +-
app/models/topic_tracking_state.rb | 7 +-
app/models/topic_user.rb | 55 +++++------
app/models/user.rb | 85 -----------------
app/models/user_option.rb | 71 ++++++++++++++
app/serializers/current_user_serializer.rb | 10 +-
app/serializers/listable_topic_serializer.rb | 2 +-
app/serializers/user_option_serializer.rb | 13 ++-
app/serializers/user_serializer.rb | 10 --
app/services/user_updater.rb | 12 +--
db/fixtures/009_users.rb | 9 +-
...9_move_tracking_options_to_user_options.rb | 16 ++++
lib/topic_query.rb | 2 +-
spec/components/topic_query_spec.rb | 12 +--
spec/fabricators/user_option_fabricator.rb | 2 +
spec/models/topic_user_spec.rb | 13 ++-
spec/models/user_option_spec.rb | 92 ++++++++++++++++++
spec/models/user_spec.rb | 94 +------------------
spec/serializers/user_serializer_spec.rb | 4 +
spec/services/user_updater_spec.rb | 7 +-
22 files changed, 278 insertions(+), 250 deletions(-)
create mode 100644 db/migrate/20160225050319_move_tracking_options_to_user_options.rb
create mode 100644 spec/fabricators/user_option_fabricator.rb
create mode 100644 spec/models/user_option_spec.rb
diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6
index 516f7b1f7..c57574431 100644
--- a/app/assets/javascripts/discourse/models/user.js.es6
+++ b/app/assets/javascripts/discourse/models/user.js.es6
@@ -141,13 +141,11 @@ const User = RestModel.extend({
save() {
const data = this.getProperties(
- 'auto_track_topics_after_msecs',
'bio_raw',
'website',
'location',
'name',
'locale',
- 'new_topic_duration_minutes',
'custom_fields',
'user_fields',
'muted_usernames',
@@ -165,7 +163,9 @@ const User = RestModel.extend({
'enable_quoting',
'disable_jump_reply',
'automatically_unpin_topics',
- 'digest_after_days'
+ 'digest_after_days',
+ 'new_topic_duration_minutes',
+ 'auto_track_topics_after_msecs'
].forEach(s => {
data[s] = this.get(`user_option.${s}`);
});
diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs
index 4c65524e8..06bab9cbf 100644
--- a/app/assets/javascripts/discourse/templates/user/preferences.hbs
+++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs
@@ -201,12 +201,12 @@
<div class="controls controls-dropdown">
<label>{{i18n 'user.new_topic_duration.label'}}</label>
- {{combo-box valueAttribute="value" content=considerNewTopicOptions value=model.new_topic_duration_minutes}}
+ {{combo-box valueAttribute="value" content=considerNewTopicOptions value=model.user_option.new_topic_duration_minutes}}
</div>
<div class="controls controls-dropdown">
<label>{{i18n 'user.auto_track_topics'}}</label>
- {{combo-box valueAttribute="value" content=autoTrackDurations value=model.auto_track_topics_after_msecs}}
+ {{combo-box valueAttribute="value" content=autoTrackDurations value=model.user_option.auto_track_topics_after_msecs}}
</div>
{{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.user_option.external_links_in_new_tab}}
diff --git a/app/jobs/regular/update_top_redirection.rb b/app/jobs/regular/update_top_redirection.rb
index 496f86f58..776cdfca7 100644
--- a/app/jobs/regular/update_top_redirection.rb
+++ b/app/jobs/regular/update_top_redirection.rb
@@ -4,7 +4,7 @@ module Jobs
def execute(args)
if user = User.find_by(id: args[:user_id])
- user.update_column(:last_redirected_to_top_at, args[:redirected_at])
+ user.user_option.update_column(:last_redirected_to_top_at, args[:redirected_at])
end
end
end
diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb
index 5f96c2e26..7ffc6ad1e 100644
--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@@ -104,9 +104,9 @@ class TopicTrackingState
def self.treat_as_new_topic_clause
User.where("GREATEST(CASE
- WHEN COALESCE(u.new_topic_duration_minutes, :default_duration) = :always THEN u.created_at
- WHEN COALESCE(u.new_topic_duration_minutes, :default_duration) = :last_visit THEN COALESCE(u.previous_visit_at,u.created_at)
- ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(u.new_topic_duration_minutes, :default_duration))
+ WHEN COALESCE(uo.new_topic_duration_minutes, :default_duration) = :always THEN u.created_at
+ WHEN COALESCE(uo.new_topic_duration_minutes, :default_duration) = :last_visit THEN COALESCE(u.previous_visit_at,u.created_at)
+ ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(uo.new_topic_duration_minutes, :default_duration))
END, us.new_since, :min_date)",
now: DateTime.now,
last_visit: User::NewTopicDuration::LAST_VISIT,
@@ -169,6 +169,7 @@ class TopicTrackingState
FROM topics
JOIN users u on u.id = :user_id
JOIN user_stats AS us ON us.user_id = u.id
+ JOIN user_options AS uo ON uo.user_id = u.id
JOIN categories c ON c.id = topics.category_id
LEFT JOIN topic_users tu ON tu.topic_id = topics.id AND tu.user_id = u.id
WHERE u.id = :user_id AND
diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb
index 25298af1f..1a5023782 100644
--- a/app/models/topic_user.rb
+++ b/app/models/topic_user.rb
@@ -104,7 +104,7 @@ class TopicUser < ActiveRecord::Base
if rows == 0
now = DateTime.now
- auto_track_after = User.select(:auto_track_topics_after_msecs).find_by(id: user_id).try(:auto_track_topics_after_msecs)
+ auto_track_after = UserOption.where(user_id: user_id).pluck(:auto_track_topics_after_msecs).first
auto_track_after ||= SiteSetting.default_other_auto_track_topics_after_msecs
if auto_track_after >= 0 && auto_track_after <= (attrs[:total_msecs_viewed].to_i || 0)
@@ -140,6 +140,31 @@ class TopicUser < ActiveRecord::Base
# Update the last read and the last seen post count, but only if it doesn't exist.
# This would be a lot easier if psql supported some kind of upsert
+ UPDATE_TOPIC_USER_SQL = "UPDATE topic_users
+ SET
+ last_read_post_number = GREATEST(:post_number, tu.last_read_post_number),
+ highest_seen_post_number = t.highest_post_number,
+ total_msecs_viewed = LEAST(tu.total_msecs_viewed + :msecs,86400000),
+ notification_level =
+ case when tu.notifications_reason_id is null and (tu.total_msecs_viewed + :msecs) >
+ coalesce(uo.auto_track_topics_after_msecs,:threshold) and
+ coalesce(uo.auto_track_topics_after_msecs, :threshold) >= 0 then
+ :tracking
+ else
+ tu.notification_level
+ end
+ FROM topic_users tu
+ join topics t on t.id = tu.topic_id
+ join users u on u.id = :user_id
+ join user_options uo on uo.user_id = :user_id
+ WHERE
+ tu.topic_id = topic_users.topic_id AND
+ tu.user_id = topic_users.user_id AND
+ tu.topic_id = :topic_id AND
+ tu.user_id = :user_id
+ RETURNING
+ topic_users.notification_level, tu.notification_level old_level, tu.last_read_post_number
+ "
def update_last_read(user, topic_id, post_number, msecs, opts={})
return if post_number.blank?
msecs = 0 if msecs.to_i < 0
@@ -160,31 +185,7 @@ class TopicUser < ActiveRecord::Base
# ... user visited the topic but did not read the posts
#
# 86400000 = 1 day
- rows = exec_sql("UPDATE topic_users
- SET
- last_read_post_number = GREATEST(:post_number, tu.last_read_post_number),
- highest_seen_post_number = t.highest_post_number,
- total_msecs_viewed = LEAST(tu.total_msecs_viewed + :msecs,86400000),
- notification_level =
- case when tu.notifications_reason_id is null and (tu.total_msecs_viewed + :msecs) >
- coalesce(u.auto_track_topics_after_msecs,:threshold) and
- coalesce(u.auto_track_topics_after_msecs, :threshold) >= 0 then
- :tracking
- else
- tu.notification_level
- end
- FROM topic_users tu
- join topics t on t.id = tu.topic_id
- join users u on u.id = :user_id
- WHERE
- tu.topic_id = topic_users.topic_id AND
- tu.user_id = topic_users.user_id AND
- tu.topic_id = :topic_id AND
- tu.user_id = :user_id
- RETURNING
- topic_users.notification_level, tu.notification_level old_level, tu.last_read_post_number
- ",
- args).values
+ rows = exec_sql(UPDATE_TOPIC_USER_SQL,args).values
if rows.length == 1
before = rows[0][1].to_i
@@ -206,7 +207,7 @@ class TopicUser < ActiveRecord::Base
if rows.length == 0
# The user read at least one post in a topic that they haven't viewed before.
args[:new_status] = notification_levels[:regular]
- if (user.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs) == 0
+ if (user.user_option.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs) == 0
args[:new_status] = notification_levels[:tracking]
end
TopicTrackingState.publish_read(topic_id, post_number, user.id, args[:new_status])
diff --git a/app/models/user.rb b/app/models/user.rb
index 5634cbeaf..71ee83831 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -77,8 +77,6 @@ class User < ActiveRecord::Base
after_initialize :add_trust_level
- before_create :set_default_user_preferences
-
after_create :create_email_token
after_create :create_user_stat
after_create :create_user_option
@@ -90,7 +88,6 @@ class User < ActiveRecord::Base
before_save :update_username_lower
before_save :ensure_password_is_hashed
- after_save :update_tracked_topics
after_save :clear_global_notice_if_needed
after_save :refresh_avatar
after_save :badge_grant
@@ -626,20 +623,6 @@ class User < ActiveRecord::Base
Promotion.new(self).change_trust_level!(level, opts)
end
- def treat_as_new_topic_start_date
- duration = new_topic_duration_minutes || SiteSetting.default_other_new_topic_duration_minutes.to_i
- times = [case duration
- when User::NewTopicDuration::ALWAYS
- created_at
- when User::NewTopicDuration::LAST_VISIT
- previous_visit_at || user_stat.new_since
- else
- duration.minutes.ago
- end, user_stat.new_since, Time.at(SiteSetting.min_new_topics_time).to_datetime]
-
- times.max
- end
-
def readable_name
return "#{name} (#{username})" if name.present? && name != username
username
@@ -730,51 +713,6 @@ class User < ActiveRecord::Base
.exists?
end
- def should_be_redirected_to_top
- redirected_to_top.present?
- end
-
- def redirected_to_top
- # redirect is enabled
- return unless SiteSetting.redirect_users_to_top_page
- # top must be in the top_menu
- return unless SiteSetting.top_menu =~ /(^|\|)top(\||$)/i
- # not enough topics
- return unless period = SiteSetting.min_redirected_to_top_period
-
- if !seen_before? || (trust_level == 0 && !redirected_to_top_yet?)
- update_last_redirected_to_top!
- return {
- reason: I18n.t('redirected_to_top_reasons.new_user'),
- period: period
- }
- elsif last_seen_at < 1.month.ago
- update_last_redirected_to_top!
- return {
- reason: I18n.t('redirected_to_top_reasons.not_seen_in_a_month'),
- period: period
- }
- end
-
- # don't redirect to top
- nil
- end
-
- def redirected_to_top_yet?
- last_redirected_to_top_at.present?
- end
-
- def update_last_redirected_to_top!
- key = "user:#{id}:update_last_redirected_to_top"
- delay = SiteSetting.active_user_rate_limit_secs
-
- # only update last_redirected_to_top_at once every minute
- return unless $redis.setnx(key, "1")
- $redis.expire(key, delay)
-
- # delay the update
- Jobs.enqueue_in(delay / 2, :update_top_redirection, user_id: self.id, redirected_at: Time.zone.now)
- end
def refresh_avatar
return if @import_mode
@@ -880,11 +818,6 @@ class User < ActiveRecord::Base
end
end
- def update_tracked_topics
- return unless auto_track_topics_after_msecs_changed?
- TrackedTopicsUpdater.new(id, auto_track_topics_after_msecs).call
- end
-
def clear_global_notice_if_needed
if admin && SiteSetting.has_login_hint
SiteSetting.has_login_hint = false
@@ -970,14 +903,6 @@ class User < ActiveRecord::Base
end
end
- def set_default_user_preferences
- set_default_other_new_topic_duration_minutes
- set_default_other_auto_track_topics_after_msecs
-
- # needed, otherwise the callback chain is broken...
- true
- end
-
def set_default_categories_preferences
values = []
@@ -1025,12 +950,6 @@ class User < ActiveRecord::Base
end
- %w{new_topic_duration_minutes auto_track_topics_after_msecs}.each do |s|
- define_method("set_default_other_#{s}") do
- self.send("#{s}=", SiteSetting.send("default_other_#{s}").to_i) if has_attribute?(s)
- end
- end
-
end
# == Schema Information
@@ -1054,7 +973,6 @@ end
# admin :boolean default(FALSE), not null
# last_emailed_at :datetime
# trust_level :integer not null
-# email_private_messages :boolean default(TRUE)
# approved :boolean default(FALSE), not null
# approved_by_id :integer
# approved_at :datetime
@@ -1062,11 +980,9 @@ end
# suspended_at :datetime
# suspended_till :datetime
# date_of_birth :date
-# auto_track_topics_after_msecs :integer
# views :integer default(0), not null
# flag_level :integer default(0), not null
# ip_address :inet
-# new_topic_duration_minutes :integer
# moderator :boolean default(FALSE)
# blocked :boolean default(FALSE)
# title :string(255)
@@ -1074,7 +990,6 @@ end
# primary_group_id :integer
# locale :string(10)
# registration_ip_address :inet
-# last_redirected_to_top_at :datetime
# trust_level_locked :boolean default(FALSE), not null
# staged :boolean default(FALSE), not null
#
diff --git a/app/models/user_option.rb b/app/models/user_option.rb
index 3dac2ded9..46d7e6730 100644
--- a/app/models/user_option.rb
+++ b/app/models/user_option.rb
@@ -3,6 +3,8 @@ class UserOption < ActiveRecord::Base
belongs_to :user
before_create :set_defaults
+ after_save :update_tracked_topics
+
def set_defaults
self.email_always = SiteSetting.default_email_always
self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode
@@ -16,6 +18,9 @@ class UserOption < ActiveRecord::Base
self.disable_jump_reply = SiteSetting.default_other_disable_jump_reply
self.edit_history_public = SiteSetting.default_other_edit_history_public
+ self.new_topic_duration_minutes = SiteSetting.default_other_new_topic_duration_minutes
+ self.auto_track_topics_after_msecs = SiteSetting.default_other_auto_track_topics_after_msecs
+
if SiteSetting.default_email_digest_frequency.to_i <= 0
self.email_digests = false
@@ -26,4 +31,70 @@ class UserOption < ActiveRecord::Base
true
end
+
+ def update_tracked_topics
+ return unless auto_track_topics_after_msecs_changed?
+ TrackedTopicsUpdater.new(id, auto_track_topics_after_msecs).call
+ end
+
+ def redirected_to_top_yet?
+ last_redirected_to_top_at.present?
+ end
+
+ def update_last_redirected_to_top!
+ key = "user:#{id}:update_last_redirected_to_top"
+ delay = SiteSetting.active_user_rate_limit_secs
+
+ # only update last_redirected_to_top_at once every minute
+ return unless $redis.setnx(key, "1")
+ $redis.expire(key, delay)
+
+ # delay the update
+ Jobs.enqueue_in(delay / 2, :update_top_redirection, user_id: self.id, redirected_at: Time.zone.now)
+ end
+
+ def should_be_redirected_to_top
+ redirected_to_top.present?
+ end
+
+ def redirected_to_top
+ # redirect is enabled
+ return unless SiteSetting.redirect_users_to_top_page
+ # top must be in the top_menu
+ return unless SiteSetting.top_menu =~ /(^|\|)top(\||$)/i
+ # not enough topics
+ return unless period = SiteSetting.min_redirected_to_top_period
+
+ if !user.seen_before? || (user.trust_level == 0 && !redirected_to_top_yet?)
+ update_last_redirected_to_top!
+ return {
+ reason: I18n.t('redirected_to_top_reasons.new_user'),
+ period: period
+ }
+ elsif user.last_seen_at < 1.month.ago
+ update_last_redirected_to_top!
+ return {
+ reason: I18n.t('redirected_to_top_reasons.not_seen_in_a_month'),
+ period: period
+ }
+ end
+
+ # don't redirect to top
+ nil
+ end
+
+ def treat_as_new_topic_start_date
+ duration = new_topic_duration_minutes || SiteSetting.default_other_new_topic_duration_minutes.to_i
+ times = [case duration
+ when User::NewTopicDuration::ALWAYS
+ user.created_at
+ when User::NewTopicDuration::LAST_VISIT
+ user.previous_visit_at || user.user_stat.new_since
+ else
+ duration.minutes.ago
+ end, user.user_stat.new_since, Time.at(SiteSetting.min_new_topics_time).to_datetime]
+
+ times.max
+ end
+
end
diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb
index 5cf912d47..2c916e475 100644
--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@@ -70,6 +70,14 @@ class CurrentUserSerializer < BasicUserSerializer
object.user_option.automatically_unpin_topics
end
+ def should_be_redirected_to_top
+ object.user_option.should_be_redirected_to_top
+ end
+
+ def redirected_to_top
+ object.user_option.redirected_to_top
+ end
+
def site_flagged_posts_count
PostAction.flagged_posts_count
end
@@ -103,7 +111,7 @@ class CurrentUserSerializer < BasicUserSerializer
end
def include_redirected_to_top?
- object.redirected_to_top.present?
+ object.user_option.redirected_to_top.present?
end
def custom_fields
diff --git a/app/serializers/listable_topic_serializer.rb b/app/serializers/listable_topic_serializer.rb
index 609bf1d72..3589d7a23 100644
--- a/app/serializers/listable_topic_serializer.rb
+++ b/app/serializers/listable_topic_serializer.rb
@@ -45,7 +45,7 @@ class ListableTopicSerializer < BasicTopicSerializer
def seen
return true if !scope || !scope.user
return true if object.user_data && !object.user_data.last_read_post_number.nil?
- return true if object.created_at < scope.user.treat_as_new_topic_start_date
+ return true if object.created_at < scope.user.user_option.treat_as_new_topic_start_date
false
end
diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb
index 77c9d6854..440587a4d 100644
--- a/app/serializers/user_option_serializer.rb
+++ b/app/serializers/user_option_serializer.rb
@@ -11,10 +11,21 @@ class UserOptionSerializer < ApplicationSerializer
:disable_jump_reply,
:digest_after_days,
:automatically_unpin_topics,
- :edit_history_public
+ :edit_history_public,
+ :auto_track_topics_after_msecs,
+ :new_topic_duration_minutes
def include_edit_history_public?
!SiteSetting.edit_history_visible_to_public
end
+
+ def auto_track_topics_after_msecs
+ object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs
+ end
+
+ def new_topic_duration_minutes
+ object.new_topic_duration_minutes || SiteSetting.default_other_new_topic_duration_minutes
+ end
+
end
diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb
index 1ad96ce24..730a23da7 100644
--- a/app/serializers/user_serializer.rb
+++ b/app/serializers/user_serializer.rb
@@ -82,8 +82,6 @@ class UserSerializer < BasicUserSerializer
:can_delete_all_posts
private_attributes :locale,
- :auto_track_topics_after_msecs,
- :new_topic_duration_minutes,
:muted_category_ids,
:tracked_category_ids,
:watched_category_ids,
@@ -253,14 +251,6 @@ class UserSerializer < BasicUserSerializer
### PRIVATE ATTRIBUTES
###
- def auto_track_topics_after_msecs
- object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs
- end
-
- def new_topic_duration_minutes
- object.new_topic_duration_minutes || SiteSetting.default_other_new_topic_duration_minutes
- end
-
def muted_category_ids
CategoryUser.lookup(object, :muted).pluck(:category_id)
end
diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb
index e028a0d14..b3b8e9031 100644
--- a/app/services/user_updater.rb
+++ b/app/services/user_updater.rb
@@ -18,7 +18,9 @@ class UserUpdater
:disable_jump_reply,
:edit_history_public,
:automatically_unpin_topics,
- :digest_after_days
+ :digest_after_days,
+ :new_topic_duration_minutes,
+ :auto_track_topics_after_msecs
]
def initialize(actor, user)
@@ -38,14 +40,6 @@ class UserUpdater
user.name = attributes.fetch(:name) { user.name }
user.locale = attributes.fetch(:locale) { user.locale }
- if attributes[:auto_track_topics_after_msecs]
- user.auto_track_topics_after_msecs = attributes[:auto_track_topics_after_msecs].to_i
- end
-
- if attributes[:new_topic_duration_minutes]
- user.new_topic_duration_minutes = attributes[:new_topic_duration_minutes].to_i
- end
-
if guardian.can_grant_title?(user)
user.title = attributes.fetch(:title) { user.title }
end
diff --git a/db/fixtures/009_users.rb b/db/fixtures/009_users.rb
index ccdcaceac..d65c8f2ef 100644
--- a/db/fixtures/009_users.rb
+++ b/db/fixtures/009_users.rb
@@ -32,9 +32,9 @@ duration = Rails.env.production? ? 60 : 0
if User.exec_sql("SELECT 1 FROM schema_migration_details
WHERE EXISTS(
SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS
- WHERE table_name = 'users' AND column_name = 'enable_quoting'
+ WHERE table_name = 'users' AND column_name = 'last_redirected_to_top_at'
) AND
- name = 'AllowDefaultsOnUsersTable' AND
+ name = 'MoveTrackingOptionsToUserOptions' AND
created_at < (current_timestamp at time zone 'UTC' - interval '#{duration} minutes')
").to_a.length > 0
@@ -54,7 +54,10 @@ if User.exec_sql("SELECT 1 FROM schema_migration_details
edit_history_public
automatically_unpin_topics
digest_after_days
- ].each do |column|
+ auto_track_topics_after_msecs
+ new_topic_duration_minutes
+ last_redirected_to_top_at
+].each do |column|
User.exec_sql("ALTER TABLE users DROP column IF EXISTS #{column}")
end
diff --git a/db/migrate/20160225050319_move_tracking_options_to_user_options.rb b/db/migrate/20160225050319_move_tracking_options_to_user_options.rb
new file mode 100644
index 000000000..87dabf399
--- /dev/null
+++ b/db/migrate/20160225050319_move_tracking_options_to_user_options.rb
@@ -0,0 +1,16 @@
+class MoveTrackingOptionsToUserOptions < ActiveRecord::Migration
+ def change
+ add_column :user_options, :auto_track_topics_after_msecs, :integer
+ add_column :user_options, :new_topic_duration_minutes, :integer
+ add_column :user_options, :last_redirected_to_top_at, :datetime
+
+ execute <<SQL
+ UPDATE user_options
+ SET auto_track_topics_after_msecs = users.auto_track_topics_after_msecs,
+ new_topic_duration_minutes = users.new_topic_duration_minutes,
+ last_redirected_to_top_at = users.last_redirected_to_top_at
+ FROM users
+ WHERE users.id = user_options.user_id
+SQL
+ end
+end
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index 598f7ad8f..cdd5abbb4 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -331,7 +331,7 @@ class TopicQuery
def new_results(options={})
# TODO does this make sense or should it be ordered on created_at
# it is ordering on bumped_at now
- result = TopicQuery.new_filter(default_results(options.reverse_merge(:unordered => true)), @user.treat_as_new_topic_start_date)
+ result = TopicQuery.new_filter(default_results(options.reverse_merge(:unordered => true)), @user.user_option.treat_as_new_topic_start_date)
result = remove_muted_topics(result, @user)
result = remove_muted_categories(result, @user, exclude: options[:category])
diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb
index 44f219d47..fad4faaea 100644
--- a/spec/components/topic_query_spec.rb
+++ b/spec/components/topic_query_spec.rb
@@ -294,8 +294,8 @@ describe TopicQuery do
context 'user with auto_track_topics list_unread' do
before do
- user.auto_track_topics_after_msecs = 0
- user.save
+ user.user_option.auto_track_topics_after_msecs = 0
+ user.user_option.save
end
it 'only contains the partially read topic' do
@@ -360,8 +360,8 @@ describe TopicQuery do
expect(topic_query.list_new.topics).to eq([new_topic])
- user.new_topic_duration_minutes = 5
- user.save
+ user.user_option.new_topic_duration_minutes = 5
+ user.user_option.save
new_topic.created_at = 10.minutes.ago
new_topic.save
expect(topic_query.list_new.topics).to eq([])
@@ -561,8 +561,8 @@ describe TopicQuery do
let!(:fully_read_archived) { Fabricate(:post, user: creator).topic }
before do
- user.auto_track_topics_after_msecs = 0
- user.save
+ user.user_option.auto_track_topics_after_msecs = 0
+ user.user_option.save
TopicUser.update_last_read(user, partially_read.id, 0, 0)
TopicUser.update_last_read(user, fully_read.id, 1, 0)
TopicUser.update_last_read(user, fully_read_closed.id, 1, 0)
diff --git a/spec/fabricators/user_option_fabricator.rb b/spec/fabricators/user_option_fabricator.rb
new file mode 100644
index 000000000..f42ddaec9
--- /dev/null
+++ b/spec/fabricators/user_option_fabricator.rb
@@ -0,0 +1,2 @@
+Fabricator(:user_option) do
+end
diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb
index 9f9f5651a..f51b90b33 100644
--- a/spec/models/topic_user_spec.rb
+++ b/spec/models/topic_user_spec.rb
@@ -48,7 +48,12 @@ describe TopicUser do
let(:topic_creator_user) { TopicUser.get(topic, topic.user) }
let(:post) { Fabricate(:post, topic: topic, user: user) }
- let(:new_user) { Fabricate(:user, auto_track_topics_after_msecs: 1000) }
+ let(:new_user) {
+ u = Fabricate(:user)
+ u.user_option.update_columns(auto_track_topics_after_msecs: 1000)
+ u
+ }
+
let(:topic_new_user) { TopicUser.get(topic, new_user)}
let(:yesterday) { DateTime.now.yesterday }
@@ -68,15 +73,15 @@ describe TopicUser do
describe 'notifications' do
it 'should be set to tracking if auto_track_topics is enabled' do
- user.update_column(:auto_track_topics_after_msecs, 0)
+ user.user_option.update_column(:auto_track_topics_after_msecs, 0)
ensure_topic_user
expect(TopicUser.get(topic, user).notification_level).to eq(TopicUser.notification_levels[:tracking])
end
it 'should reset regular topics to tracking topics if auto track is changed' do
ensure_topic_user
- user.auto_track_topics_after_msecs = 0
- user.save
+ user.user_option.auto_track_topics_after_msecs = 0
+ user.user_option.save
expect(topic_user.notification_level).to eq(TopicUser.notification_levels[:tracking])
end
diff --git a/spec/models/user_option_spec.rb b/spec/models/user_option_spec.rb
new file mode 100644
index 000000000..e3144989a
--- /dev/null
+++ b/spec/models/user_option_spec.rb
@@ -0,0 +1,92 @@
+require 'rails_helper'
+require_dependency 'user_option'
+
+describe UserOption do
+
+ describe "should_be_redirected_to_top" do
+ let!(:user) { Fabricate(:user) }
+
+ it "should be redirected to top when there is a reason to" do
+ user.user_option.expects(:redirected_to_top).returns({ reason: "42" })
+ expect(user.user_option.should_be_redirected_to_top).to eq(true)
+ end
+
+ it "should not be redirected to top when there is no reason to" do
+ user.user_option.expects(:redirected_to_top).returns(nil)
+ expect(user.user_option.should_be_redirected_to_top).to eq(false)
+ end
+
+ end
+
+ describe ".redirected_to_top" do
+ let!(:user) { Fabricate(:user) }
+
+ it "should have no reason when `SiteSetting.redirect_users_to_top_page` is disabled" do
+ SiteSetting.expects(:redirect_users_to_top_page).returns(false)
+ expect(user.user_option.redirected_to_top).to eq(nil)
+ end
+
+ context "when `SiteSetting.redirect_users_to_top_page` is enabled" do
+ before { SiteSetting.expects(:redirect_users_to_top_page).returns(true) }
+
+ it "should have no reason when top is not in the `SiteSetting.top_menu`" do
+ SiteSetting.expects(:top_menu).returns("latest")
+ expect(user.user_option.redirected_to_top).to eq(nil)
+ end
+
+ context "and when top is in the `SiteSetting.top_menu`" do
+ before { SiteSetting.expects(:top_menu).returns("latest|top") }
+
+ it "should have no reason when there are not enough topics" do
+ SiteSetting.expects(:min_redirected_to_top_period).returns(nil)
+ expect(user.user_option.redirected_to_top).to eq(nil)
+ end
+
+ context "and there are enough topics" do
+
+ before { SiteSetting.expects(:min_redirected_to_top_period).returns(:monthly) }
+
+ describe "a new user" do
+ before do
+ user.stubs(:trust_level).returns(0)
+ user.stubs(:last_seen_at).returns(5.minutes.ago)
+ end
+
+ it "should have a reason for the first visit" do
+ expect(user.user_option.redirected_to_top).to eq({
+ reason: I18n.t('redirected_to_top_reasons.new_user'),
+ period: :monthly
+ })
+ end
+
+ it "should not have a reason for next visits" do
+ user.user_option.expects(:last_redirected_to_top_at).returns(10.minutes.ago)
+ user.user_option.expects(:update_last_redirected_to_top!).never
+
+ expect(user.user_option.redirected_to_top).to eq(nil)
+ end
+ end
+
+ describe "an older user" do
+ before { user.stubs(:trust_level).returns(1) }
+
+ it "should have a reason when the user hasn't been seen in a month" do
+ user.last_seen_at = 2.months.ago
+ user.user_option.expects(:update_last_redirected_to_top!).once
+
+ expect(user.user_option.redirected_to_top).to eq({
+ reason: I18n.t('redirected_to_top_reasons.not_seen_in_a_month'),
+ period: :monthly
+ })
+ end
+
+ end
+
+ end
+
+ end
+
+ end
+
+ end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 5acf8bfb6..dfbabbe03 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -986,95 +986,6 @@ describe User do
end
end
- describe "should_be_redirected_to_top" do
- let!(:user) { Fabricate(:user) }
-
- it "should be redirected to top when there is a reason to" do
- user.expects(:redirected_to_top).returns({ reason: "42" })
- expect(user.should_be_redirected_to_top).to eq(true)
- end
-
- it "should not be redirected to top when there is no reason to" do
- user.expects(:redirected_to_top).returns(nil)
- expect(user.should_be_redirected_to_top).to eq(false)
- end
-
- end
-
- describe ".redirected_to_top" do
- let!(:user) { Fabricate(:user) }
-
- it "should have no reason when `SiteSetting.redirect_users_to_top_page` is disabled" do
- SiteSetting.expects(:redirect_users_to_top_page).returns(false)
- expect(user.redirected_to_top).to eq(nil)
- end
-
- context "when `SiteSetting.redirect_users_to_top_page` is enabled" do
- before { SiteSetting.expects(:redirect_users_to_top_page).returns(true) }
-
- it "should have no reason when top is not in the `SiteSetting.top_menu`" do
- SiteSetting.expects(:top_menu).returns("latest")
- expect(user.redirected_to_top).to eq(nil)
- end
-
- context "and when top is in the `SiteSetting.top_menu`" do
- before { SiteSetting.expects(:top_menu).returns("latest|top") }
-
- it "should have no reason when there are not enough topics" do
- SiteSetting.expects(:min_redirected_to_top_period).returns(nil)
- expect(user.redirected_to_top).to eq(nil)
- end
-
- context "and there are enough topics" do
-
- before { SiteSetting.expects(:min_redirected_to_top_period).returns(:monthly) }
-
- describe "a new user" do
- before do
- user.stubs(:trust_level).returns(0)
- user.stubs(:last_seen_at).returns(5.minutes.ago)
- end
-
- it "should have a reason for the first visit" do
- user.expects(:last_redirected_to_top_at).returns(nil)
- user.expects(:update_last_redirected_to_top!).once
-
- expect(user.redirected_to_top).to eq({
- reason: I18n.t('redirected_to_top_reasons.new_user'),
- period: :monthly
- })
- end
-
- it "should not have a reason for next visits" do
- user.expects(:last_redirected_to_top_at).returns(10.minutes.ago)
- user.expects(:update_last_redirected_to_top!).never
-
- expect(user.redirected_to_top).to eq(nil)
- end
- end
-
- describe "an older user" do
- before { user.stubs(:trust_level).returns(1) }
-
- it "should have a reason when the user hasn't been seen in a month" do
- user.last_seen_at = 2.months.ago
- user.expects(:update_last_redirected_to_top!).once
-
- expect(user.redirected_to_top).to eq({
- reason: I18n.t('redirected_to_top_reasons.not_seen_in_a_month'),
- period: :monthly
- })
- end
-
- end
-
- end
-
- end
-
- end
-
- end
describe "automatic avatar creation" do
it "sets a system avatar for new users" do
@@ -1281,9 +1192,8 @@ describe User do
expect(options.edit_history_public).to eq(true)
expect(options.automatically_unpin_topics).to eq(false)
expect(options.email_direct).to eq(false)
-
- expect(user.new_topic_duration_minutes).to eq(-1)
- expect(user.auto_track_topics_after_msecs).to eq(0)
+ expect(options.new_topic_duration_minutes).to eq(-1)
+ expect(options.auto_track_topics_after_msecs).to eq(0)
expect(CategoryUser.lookup(user, :watching).pluck(:category_id)).to eq([1])
expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([2])
diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb
index 3a9db1a23..16320fdfb 100644
--- a/spec/serializers/user_serializer_spec.rb
+++ b/spec/serializers/user_serializer_spec.rb
@@ -19,6 +19,8 @@ describe UserSerializer do
it "serializes options correctly" do
# so we serialize more stuff
SiteSetting.edit_history_visible_to_public = false
+ SiteSetting.default_other_auto_track_topics_after_msecs = 0
+ SiteSetting.default_other_new_topic_duration_minutes = 60*24
user = Fabricate.build(:user,
user_profile: Fabricate.build(:user_profile),
@@ -29,6 +31,8 @@ describe UserSerializer do
json = UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json
expect(json[:user_option][:edit_history_public]).to eq(true)
+ expect(json[:user_option][:new_topic_duration_minutes]).to eq(60*24)
+ expect(json[:user_option][:auto_track_topics_after_msecs]).to eq(0)
end
end
diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb
index f94d2bb9e..7866206bf 100644
--- a/spec/services/user_updater_spec.rb
+++ b/spec/services/user_updater_spec.rb
@@ -46,13 +46,18 @@ describe UserUpdater do
updater.update(bio_raw: 'my new bio',
email_always: 'true',
mailing_list_mode: true,
- digest_after_days: "8")
+ digest_after_days: "8",
+ new_topic_duration_minutes: 100,
+ auto_track_topics_after_msecs: 101
+ )
user.reload
expect(user.user_profile.bio_raw).to eq 'my new bio'
expect(user.user_option.email_always).to eq true
expect(user.user_option.mailing_list_mode).to eq true
expect(user.user_option.digest_after_days).to eq 8
+ expect(user.user_option.new_topic_duration_minutes).to eq 100
+ expect(user.user_option.auto_track_topics_after_msecs).to eq 101
end
context 'when update succeeds' do