PERF: move 3 more option columns out of the user table

This commit is contained in:
Sam 2016-02-18 16:57:22 +11:00
parent f9c5cded6f
commit f0e942f647
22 changed files with 278 additions and 250 deletions

View file

@ -141,13 +141,11 @@ const User = RestModel.extend({
save() { save() {
const data = this.getProperties( const data = this.getProperties(
'auto_track_topics_after_msecs',
'bio_raw', 'bio_raw',
'website', 'website',
'location', 'location',
'name', 'name',
'locale', 'locale',
'new_topic_duration_minutes',
'custom_fields', 'custom_fields',
'user_fields', 'user_fields',
'muted_usernames', 'muted_usernames',
@ -165,7 +163,9 @@ const User = RestModel.extend({
'enable_quoting', 'enable_quoting',
'disable_jump_reply', 'disable_jump_reply',
'automatically_unpin_topics', 'automatically_unpin_topics',
'digest_after_days' 'digest_after_days',
'new_topic_duration_minutes',
'auto_track_topics_after_msecs'
].forEach(s => { ].forEach(s => {
data[s] = this.get(`user_option.${s}`); data[s] = this.get(`user_option.${s}`);
}); });

View file

@ -201,12 +201,12 @@
<div class="controls controls-dropdown"> <div class="controls controls-dropdown">
<label>{{i18n 'user.new_topic_duration.label'}}</label> <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>
<div class="controls controls-dropdown"> <div class="controls controls-dropdown">
<label>{{i18n 'user.auto_track_topics'}}</label> <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> </div>
{{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.user_option.external_links_in_new_tab}} {{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.user_option.external_links_in_new_tab}}

View file

@ -4,7 +4,7 @@ module Jobs
def execute(args) def execute(args)
if user = User.find_by(id: args[:user_id]) 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 end
end end

View file

@ -104,9 +104,9 @@ class TopicTrackingState
def self.treat_as_new_topic_clause def self.treat_as_new_topic_clause
User.where("GREATEST(CASE User.where("GREATEST(CASE
WHEN COALESCE(u.new_topic_duration_minutes, :default_duration) = :always THEN u.created_at WHEN COALESCE(uo.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) 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(u.new_topic_duration_minutes, :default_duration)) ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(uo.new_topic_duration_minutes, :default_duration))
END, us.new_since, :min_date)", END, us.new_since, :min_date)",
now: DateTime.now, now: DateTime.now,
last_visit: User::NewTopicDuration::LAST_VISIT, last_visit: User::NewTopicDuration::LAST_VISIT,
@ -169,6 +169,7 @@ class TopicTrackingState
FROM topics FROM topics
JOIN users u on u.id = :user_id JOIN users u on u.id = :user_id
JOIN user_stats AS us ON us.user_id = u.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 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 LEFT JOIN topic_users tu ON tu.topic_id = topics.id AND tu.user_id = u.id
WHERE u.id = :user_id AND WHERE u.id = :user_id AND

View file

@ -104,7 +104,7 @@ class TopicUser < ActiveRecord::Base
if rows == 0 if rows == 0
now = DateTime.now 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 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) 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. # 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 # 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={}) def update_last_read(user, topic_id, post_number, msecs, opts={})
return if post_number.blank? return if post_number.blank?
msecs = 0 if msecs.to_i < 0 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 # ... user visited the topic but did not read the posts
# #
# 86400000 = 1 day # 86400000 = 1 day
rows = exec_sql("UPDATE topic_users rows = exec_sql(UPDATE_TOPIC_USER_SQL,args).values
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
if rows.length == 1 if rows.length == 1
before = rows[0][1].to_i before = rows[0][1].to_i
@ -206,7 +207,7 @@ class TopicUser < ActiveRecord::Base
if rows.length == 0 if rows.length == 0
# The user read at least one post in a topic that they haven't viewed before. # The user read at least one post in a topic that they haven't viewed before.
args[:new_status] = notification_levels[:regular] 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] args[:new_status] = notification_levels[:tracking]
end end
TopicTrackingState.publish_read(topic_id, post_number, user.id, args[:new_status]) TopicTrackingState.publish_read(topic_id, post_number, user.id, args[:new_status])

View file

@ -77,8 +77,6 @@ class User < ActiveRecord::Base
after_initialize :add_trust_level after_initialize :add_trust_level
before_create :set_default_user_preferences
after_create :create_email_token after_create :create_email_token
after_create :create_user_stat after_create :create_user_stat
after_create :create_user_option after_create :create_user_option
@ -90,7 +88,6 @@ class User < ActiveRecord::Base
before_save :update_username_lower before_save :update_username_lower
before_save :ensure_password_is_hashed before_save :ensure_password_is_hashed
after_save :update_tracked_topics
after_save :clear_global_notice_if_needed after_save :clear_global_notice_if_needed
after_save :refresh_avatar after_save :refresh_avatar
after_save :badge_grant after_save :badge_grant
@ -626,20 +623,6 @@ class User < ActiveRecord::Base
Promotion.new(self).change_trust_level!(level, opts) Promotion.new(self).change_trust_level!(level, opts)
end 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 def readable_name
return "#{name} (#{username})" if name.present? && name != username return "#{name} (#{username})" if name.present? && name != username
username username
@ -730,51 +713,6 @@ class User < ActiveRecord::Base
.exists? .exists?
end 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 def refresh_avatar
return if @import_mode return if @import_mode
@ -880,11 +818,6 @@ class User < ActiveRecord::Base
end end
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 def clear_global_notice_if_needed
if admin && SiteSetting.has_login_hint if admin && SiteSetting.has_login_hint
SiteSetting.has_login_hint = false SiteSetting.has_login_hint = false
@ -970,14 +903,6 @@ class User < ActiveRecord::Base
end end
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 def set_default_categories_preferences
values = [] values = []
@ -1025,12 +950,6 @@ class User < ActiveRecord::Base
end 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 end
# == Schema Information # == Schema Information
@ -1054,7 +973,6 @@ end
# admin :boolean default(FALSE), not null # admin :boolean default(FALSE), not null
# last_emailed_at :datetime # last_emailed_at :datetime
# trust_level :integer not null # trust_level :integer not null
# email_private_messages :boolean default(TRUE)
# approved :boolean default(FALSE), not null # approved :boolean default(FALSE), not null
# approved_by_id :integer # approved_by_id :integer
# approved_at :datetime # approved_at :datetime
@ -1062,11 +980,9 @@ end
# suspended_at :datetime # suspended_at :datetime
# suspended_till :datetime # suspended_till :datetime
# date_of_birth :date # date_of_birth :date
# auto_track_topics_after_msecs :integer
# views :integer default(0), not null # views :integer default(0), not null
# flag_level :integer default(0), not null # flag_level :integer default(0), not null
# ip_address :inet # ip_address :inet
# new_topic_duration_minutes :integer
# moderator :boolean default(FALSE) # moderator :boolean default(FALSE)
# blocked :boolean default(FALSE) # blocked :boolean default(FALSE)
# title :string(255) # title :string(255)
@ -1074,7 +990,6 @@ end
# primary_group_id :integer # primary_group_id :integer
# locale :string(10) # locale :string(10)
# registration_ip_address :inet # registration_ip_address :inet
# last_redirected_to_top_at :datetime
# trust_level_locked :boolean default(FALSE), not null # trust_level_locked :boolean default(FALSE), not null
# staged :boolean default(FALSE), not null # staged :boolean default(FALSE), not null
# #

View file

@ -3,6 +3,8 @@ class UserOption < ActiveRecord::Base
belongs_to :user belongs_to :user
before_create :set_defaults before_create :set_defaults
after_save :update_tracked_topics
def set_defaults def set_defaults
self.email_always = SiteSetting.default_email_always self.email_always = SiteSetting.default_email_always
self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode 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.disable_jump_reply = SiteSetting.default_other_disable_jump_reply
self.edit_history_public = SiteSetting.default_other_edit_history_public 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 if SiteSetting.default_email_digest_frequency.to_i <= 0
self.email_digests = false self.email_digests = false
@ -26,4 +31,70 @@ class UserOption < ActiveRecord::Base
true true
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 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 end

View file

@ -70,6 +70,14 @@ class CurrentUserSerializer < BasicUserSerializer
object.user_option.automatically_unpin_topics object.user_option.automatically_unpin_topics
end 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 def site_flagged_posts_count
PostAction.flagged_posts_count PostAction.flagged_posts_count
end end
@ -103,7 +111,7 @@ class CurrentUserSerializer < BasicUserSerializer
end end
def include_redirected_to_top? def include_redirected_to_top?
object.redirected_to_top.present? object.user_option.redirected_to_top.present?
end end
def custom_fields def custom_fields

View file

@ -45,7 +45,7 @@ class ListableTopicSerializer < BasicTopicSerializer
def seen def seen
return true if !scope || !scope.user return true if !scope || !scope.user
return true if object.user_data && !object.user_data.last_read_post_number.nil? 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 false
end end

View file

@ -11,10 +11,21 @@ class UserOptionSerializer < ApplicationSerializer
:disable_jump_reply, :disable_jump_reply,
:digest_after_days, :digest_after_days,
:automatically_unpin_topics, :automatically_unpin_topics,
:edit_history_public :edit_history_public,
:auto_track_topics_after_msecs,
:new_topic_duration_minutes
def include_edit_history_public? def include_edit_history_public?
!SiteSetting.edit_history_visible_to_public !SiteSetting.edit_history_visible_to_public
end 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 end

View file

@ -82,8 +82,6 @@ class UserSerializer < BasicUserSerializer
:can_delete_all_posts :can_delete_all_posts
private_attributes :locale, private_attributes :locale,
:auto_track_topics_after_msecs,
:new_topic_duration_minutes,
:muted_category_ids, :muted_category_ids,
:tracked_category_ids, :tracked_category_ids,
:watched_category_ids, :watched_category_ids,
@ -253,14 +251,6 @@ class UserSerializer < BasicUserSerializer
### PRIVATE ATTRIBUTES ### 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 def muted_category_ids
CategoryUser.lookup(object, :muted).pluck(:category_id) CategoryUser.lookup(object, :muted).pluck(:category_id)
end end

View file

@ -18,7 +18,9 @@ class UserUpdater
:disable_jump_reply, :disable_jump_reply,
:edit_history_public, :edit_history_public,
:automatically_unpin_topics, :automatically_unpin_topics,
:digest_after_days :digest_after_days,
:new_topic_duration_minutes,
:auto_track_topics_after_msecs
] ]
def initialize(actor, user) def initialize(actor, user)
@ -38,14 +40,6 @@ class UserUpdater
user.name = attributes.fetch(:name) { user.name } user.name = attributes.fetch(:name) { user.name }
user.locale = attributes.fetch(:locale) { user.locale } 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) if guardian.can_grant_title?(user)
user.title = attributes.fetch(:title) { user.title } user.title = attributes.fetch(:title) { user.title }
end end

View file

@ -32,9 +32,9 @@ duration = Rails.env.production? ? 60 : 0
if User.exec_sql("SELECT 1 FROM schema_migration_details if User.exec_sql("SELECT 1 FROM schema_migration_details
WHERE EXISTS( WHERE EXISTS(
SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS 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 ) AND
name = 'AllowDefaultsOnUsersTable' AND name = 'MoveTrackingOptionsToUserOptions' AND
created_at < (current_timestamp at time zone 'UTC' - interval '#{duration} minutes') created_at < (current_timestamp at time zone 'UTC' - interval '#{duration} minutes')
").to_a.length > 0 ").to_a.length > 0
@ -54,7 +54,10 @@ if User.exec_sql("SELECT 1 FROM schema_migration_details
edit_history_public edit_history_public
automatically_unpin_topics automatically_unpin_topics
digest_after_days 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}") User.exec_sql("ALTER TABLE users DROP column IF EXISTS #{column}")
end end

View file

@ -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

View file

@ -331,7 +331,7 @@ class TopicQuery
def new_results(options={}) def new_results(options={})
# TODO does this make sense or should it be ordered on created_at # TODO does this make sense or should it be ordered on created_at
# it is ordering on bumped_at now # 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_topics(result, @user)
result = remove_muted_categories(result, @user, exclude: options[:category]) result = remove_muted_categories(result, @user, exclude: options[:category])

View file

@ -294,8 +294,8 @@ describe TopicQuery do
context 'user with auto_track_topics list_unread' do context 'user with auto_track_topics list_unread' do
before do before do
user.auto_track_topics_after_msecs = 0 user.user_option.auto_track_topics_after_msecs = 0
user.save user.user_option.save
end end
it 'only contains the partially read topic' do 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]) expect(topic_query.list_new.topics).to eq([new_topic])
user.new_topic_duration_minutes = 5 user.user_option.new_topic_duration_minutes = 5
user.save user.user_option.save
new_topic.created_at = 10.minutes.ago new_topic.created_at = 10.minutes.ago
new_topic.save new_topic.save
expect(topic_query.list_new.topics).to eq([]) expect(topic_query.list_new.topics).to eq([])
@ -561,8 +561,8 @@ describe TopicQuery do
let!(:fully_read_archived) { Fabricate(:post, user: creator).topic } let!(:fully_read_archived) { Fabricate(:post, user: creator).topic }
before do before do
user.auto_track_topics_after_msecs = 0 user.user_option.auto_track_topics_after_msecs = 0
user.save user.user_option.save
TopicUser.update_last_read(user, partially_read.id, 0, 0) 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.id, 1, 0)
TopicUser.update_last_read(user, fully_read_closed.id, 1, 0) TopicUser.update_last_read(user, fully_read_closed.id, 1, 0)

View file

@ -0,0 +1,2 @@
Fabricator(:user_option) do
end

View file

@ -48,7 +48,12 @@ describe TopicUser do
let(:topic_creator_user) { TopicUser.get(topic, topic.user) } let(:topic_creator_user) { TopicUser.get(topic, topic.user) }
let(:post) { Fabricate(:post, topic: topic, user: 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(:topic_new_user) { TopicUser.get(topic, new_user)}
let(:yesterday) { DateTime.now.yesterday } let(:yesterday) { DateTime.now.yesterday }
@ -68,15 +73,15 @@ describe TopicUser do
describe 'notifications' do describe 'notifications' do
it 'should be set to tracking if auto_track_topics is enabled' 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 ensure_topic_user
expect(TopicUser.get(topic, user).notification_level).to eq(TopicUser.notification_levels[:tracking]) expect(TopicUser.get(topic, user).notification_level).to eq(TopicUser.notification_levels[:tracking])
end end
it 'should reset regular topics to tracking topics if auto track is changed' do it 'should reset regular topics to tracking topics if auto track is changed' do
ensure_topic_user ensure_topic_user
user.auto_track_topics_after_msecs = 0 user.user_option.auto_track_topics_after_msecs = 0
user.save user.user_option.save
expect(topic_user.notification_level).to eq(TopicUser.notification_levels[:tracking]) expect(topic_user.notification_level).to eq(TopicUser.notification_levels[:tracking])
end end

View file

@ -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

View file

@ -986,95 +986,6 @@ describe User do
end end
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 describe "automatic avatar creation" do
it "sets a system avatar for new users" 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.edit_history_public).to eq(true)
expect(options.automatically_unpin_topics).to eq(false) expect(options.automatically_unpin_topics).to eq(false)
expect(options.email_direct).to eq(false) expect(options.email_direct).to eq(false)
expect(options.new_topic_duration_minutes).to eq(-1)
expect(user.new_topic_duration_minutes).to eq(-1) expect(options.auto_track_topics_after_msecs).to eq(0)
expect(user.auto_track_topics_after_msecs).to eq(0)
expect(CategoryUser.lookup(user, :watching).pluck(:category_id)).to eq([1]) expect(CategoryUser.lookup(user, :watching).pluck(:category_id)).to eq([1])
expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([2]) expect(CategoryUser.lookup(user, :tracking).pluck(:category_id)).to eq([2])

View file

@ -19,6 +19,8 @@ describe UserSerializer do
it "serializes options correctly" do it "serializes options correctly" do
# so we serialize more stuff # so we serialize more stuff
SiteSetting.edit_history_visible_to_public = false 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 = Fabricate.build(:user,
user_profile: Fabricate.build(:user_profile), 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 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][: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
end end

View file

@ -46,13 +46,18 @@ describe UserUpdater do
updater.update(bio_raw: 'my new bio', updater.update(bio_raw: 'my new bio',
email_always: 'true', email_always: 'true',
mailing_list_mode: 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 user.reload
expect(user.user_profile.bio_raw).to eq 'my new bio' expect(user.user_profile.bio_raw).to eq 'my new bio'
expect(user.user_option.email_always).to eq true expect(user.user_option.email_always).to eq true
expect(user.user_option.mailing_list_mode).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.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 end
context 'when update succeeds' do context 'when update succeeds' do