PERF: shift most user options out of the user table

As it stands we load up user records quite frequently on the topic pages,
this in turn pulls all the columns for the users being selected, just to
discard them after they are loaded

New structure keeps all options in a discrete table, this is better organised
and allows us to easily add more column without worrying about bloating the
user table
This commit is contained in:
Sam 2016-02-17 15:46:19 +11:00
parent 63cda22623
commit 3829c78526
28 changed files with 363 additions and 191 deletions

View File

@ -147,25 +147,29 @@ const User = RestModel.extend({
'location', 'location',
'name', 'name',
'locale', 'locale',
'email_digests',
'email_direct',
'email_always',
'email_private_messages',
'dynamic_favicon',
'digest_after_days',
'new_topic_duration_minutes', 'new_topic_duration_minutes',
'external_links_in_new_tab',
'mailing_list_mode',
'enable_quoting',
'disable_jump_reply',
'custom_fields', 'custom_fields',
'user_fields', 'user_fields',
'muted_usernames', 'muted_usernames',
'profile_background', 'profile_background',
'card_background', 'card_background'
'automatically_unpin_topics'
); );
[ 'email_always',
'mailing_list_mode',
'external_links_in_new_tab',
'email_digests',
'email_direct',
'email_private_messages',
'dynamic_favicon',
'enable_quoting',
'disable_jump_reply',
'automatically_unpin_topics',
'digest_after_days'
].forEach(s => {
data[s] = this.get(`user_option.${s}`);
});
['muted','watched','tracked'].forEach(s => { ['muted','watched','tracked'].forEach(s => {
let cats = this.get(s + 'Categories').map(c => c.get('id')); let cats = this.get(s + 'Categories').map(c => c.get('id'));
// HACK: denote lack of categories // HACK: denote lack of categories
@ -174,7 +178,7 @@ const User = RestModel.extend({
}); });
if (!Discourse.SiteSettings.edit_history_visible_to_public) { if (!Discourse.SiteSettings.edit_history_visible_to_public) {
data['edit_history_public'] = this.get('edit_history_public'); data['edit_history_public'] = this.get('user_option.edit_history_public');
} }
// TODO: We can remove this when migrated fully to rest model. // TODO: We can remove this when migrated fully to rest model.
@ -184,7 +188,7 @@ const User = RestModel.extend({
type: 'PUT' type: 'PUT'
}).then(result => { }).then(result => {
this.set('bio_excerpt', result.user.bio_excerpt); this.set('bio_excerpt', result.user.bio_excerpt);
const userProps = this.getProperties('enable_quoting', 'external_links_in_new_tab', 'dynamic_favicon'); const userProps = Em.getProperties(this.get('user-option'),'enable_quoting', 'external_links_in_new_tab', 'dynamic_favicon');
Discourse.User.current().setProperties(userProps); Discourse.User.current().setProperties(userProps);
}).finally(() => { }).finally(() => {
this.set('isSaving', false); this.set('isSaving', false);

View File

@ -169,17 +169,17 @@
<div class="control-group pref-email-settings"> <div class="control-group pref-email-settings">
<label class="control-label">{{i18n 'user.email_settings'}}</label> <label class="control-label">{{i18n 'user.email_settings'}}</label>
{{#if canReceiveDigest}} {{#if canReceiveDigest}}
{{preference-checkbox labelKey="user.email_digests.title" checked=model.email_digests}} {{preference-checkbox labelKey="user.email_digests.title" checked=model.user_option.email_digests}}
{{#if model.email_digests}} {{#if model.user_option.email_digests}}
<div class='controls controls-dropdown'> <div class='controls controls-dropdown'>
{{combo-box valueAttribute="value" content=digestFrequencies value=model.digest_after_days}} {{combo-box valueAttribute="value" content=digestFrequencies value=model.user_option.digest_after_days}}
</div> </div>
{{/if}} {{/if}}
{{/if}} {{/if}}
{{preference-checkbox labelKey="user.email_private_messages" checked=model.email_private_messages}} {{preference-checkbox labelKey="user.email_private_messages" checked=model.user_option.email_private_messages}}
{{preference-checkbox labelKey="user.email_direct" checked=model.email_direct}} {{preference-checkbox labelKey="user.email_direct" checked=model.user_option.email_direct}}
<span class="pref-mailing-list-mode">{{preference-checkbox labelKey="user.mailing_list_mode" checked=model.mailing_list_mode}}</span> <span class="pref-mailing-list-mode">{{preference-checkbox labelKey="user.mailing_list_mode" checked=model.user_option.mailing_list_mode}}</span>
{{preference-checkbox labelKey="user.email_always" checked=model.email_always}} {{preference-checkbox labelKey="user.email_always" checked=model.user_option.email_always}}
<div class='instructions'> <div class='instructions'>
{{#if siteSettings.email_time_window_mins}} {{#if siteSettings.email_time_window_mins}}
@ -209,12 +209,12 @@
{{combo-box valueAttribute="value" content=autoTrackDurations value=model.auto_track_topics_after_msecs}} {{combo-box valueAttribute="value" content=autoTrackDurations value=model.auto_track_topics_after_msecs}}
</div> </div>
{{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.external_links_in_new_tab}} {{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.user_option.external_links_in_new_tab}}
{{preference-checkbox labelKey="user.enable_quoting" checked=model.enable_quoting}} {{preference-checkbox labelKey="user.enable_quoting" checked=model.user_option.enable_quoting}}
{{preference-checkbox labelKey="user.dynamic_favicon" checked=model.dynamic_favicon}} {{preference-checkbox labelKey="user.dynamic_favicon" checked=model.user_option.dynamic_favicon}}
{{preference-checkbox labelKey="user.disable_jump_reply" checked=model.disable_jump_reply}} {{preference-checkbox labelKey="user.disable_jump_reply" checked=model.user_option.disable_jump_reply}}
{{#unless siteSettings.edit_history_visible_to_public}} {{#unless siteSettings.edit_history_visible_to_public}}
{{preference-checkbox labelKey="user.edit_history_public" checked=model.edit_history_public}} {{preference-checkbox labelKey="user.edit_history_public" checked=model.user_option.edit_history_public}}
{{/unless}} {{/unless}}
{{plugin-outlet "user-custom-preferences"}} {{plugin-outlet "user-custom-preferences"}}
@ -254,7 +254,7 @@
{{#if siteSettings.automatically_unpin_topics}} {{#if siteSettings.automatically_unpin_topics}}
<div class="control-group topics"> <div class="control-group topics">
<label class="control-label">{{i18n 'categories.topics'}}</label> <label class="control-label">{{i18n 'categories.topics'}}</label>
{{preference-checkbox labelKey="user.automatically_unpin_topics" checked=model.automatically_unpin_topics}} {{preference-checkbox labelKey="user.automatically_unpin_topics" checked=model.user_option.automatically_unpin_topics}}
</div> </div>
{{/if}} {{/if}}

View File

@ -25,9 +25,12 @@ class EmailController < ApplicationController
end end
if params[:from_all] if params[:from_all]
@user.update_columns(email_digests: false, email_direct: false, email_private_messages: false, email_always: false) @user.user_option.update_columns(email_always: false,
email_digests: false,
email_direct: false,
email_private_messages: false)
else else
@user.update_column(:email_digests, false) @user.user_option.update_column(:email_digests, false)
end end
@success = true @success = true
@ -36,7 +39,7 @@ class EmailController < ApplicationController
def resubscribe def resubscribe
@user = DigestUnsubscribeKey.user_for_key(params[:key]) @user = DigestUnsubscribeKey.user_for_key(params[:key])
raise Discourse::NotFound unless @user.present? raise Discourse::NotFound unless @user.present?
@user.update_column(:email_digests, true) @user.user_option.update_column(:email_digests, true)
end end
end end

View File

@ -11,7 +11,8 @@ module Jobs
users = users =
User.activated.not_blocked.not_suspended.real User.activated.not_blocked.not_suspended.real
.where(mailing_list_mode: true) .joins(:user_option)
.where(user_options: {mailing_list_mode: true})
.where('NOT EXISTS( .where('NOT EXISTS(
SELECT 1 SELECT 1
FROM topic_users tu FROM topic_users tu

View File

@ -62,7 +62,7 @@ module Jobs
return if user.staged && type == :digest return if user.staged && type == :digest
seen_recently = (user.last_seen_at.present? && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago) seen_recently = (user.last_seen_at.present? && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago)
seen_recently = false if user.email_always || user.staged seen_recently = false if user.user_option.email_always || user.staged
email_args = {} email_args = {}
@ -85,14 +85,14 @@ module Jobs
email_args[:notification_type] = email_args[:notification_type].to_s email_args[:notification_type] = email_args[:notification_type].to_s
end end
if user.mailing_list_mode? && if user.user_option.mailing_list_mode? &&
!post.topic.private_message? && !post.topic.private_message? &&
NOTIFICATIONS_SENT_BY_MAILING_LIST.include?(email_args[:notification_type]) NOTIFICATIONS_SENT_BY_MAILING_LIST.include?(email_args[:notification_type])
# no need to log a reason when the mail was already sent via the mailing list job # no need to log a reason when the mail was already sent via the mailing list job
return [nil, nil] return [nil, nil]
end end
unless user.email_always? unless user.user_option.email_always?
if (notification && notification.read?) || (post && post.seen?(user)) if (notification && notification.read?) || (post && post.seen?(user))
return skip_message(I18n.t('email_log.notification_already_read')) return skip_message(I18n.t('email_log.notification_already_read'))
end end

View File

@ -15,8 +15,10 @@ module Jobs
def target_user_ids def target_user_ids
# Users who want to receive emails and haven't been emailed in the last day # Users who want to receive emails and haven't been emailed in the last day
query = User.real query = User.real
.where(email_digests: true, active: true, staged: false) .where(active: true, staged: false)
.joins(:user_option)
.not_suspended .not_suspended
.where(user_options: {email_digests: true})
.where("COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)") .where("COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)")
.where("COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)") .where("COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)")
.where("COALESCE(last_seen_at, '2010-01-01') >= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * #{SiteSetting.delete_digest_email_after_days})") .where("COALESCE(last_seen_at, '2010-01-01') >= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * #{SiteSetting.delete_digest_email_after_days})")

View File

@ -321,7 +321,7 @@ class UserNotifications < ActionMailer::Base
context: context, context: context,
username: username, username: username,
add_unsubscribe_link: !user.staged, add_unsubscribe_link: !user.staged,
add_unsubscribe_via_email_link: user.mailing_list_mode, add_unsubscribe_via_email_link: user.user_option.mailing_list_mode,
unsubscribe_url: post.topic.unsubscribe_url, unsubscribe_url: post.topic.unsubscribe_url,
allow_reply_by_email: allow_reply_by_email, allow_reply_by_email: allow_reply_by_email,
use_site_subject: use_site_subject, use_site_subject: use_site_subject,

View File

@ -38,6 +38,7 @@ class User < ActiveRecord::Base
has_many :user_archived_messages, dependent: :destroy has_many :user_archived_messages, dependent: :destroy
has_one :user_option, dependent: :destroy
has_one :user_avatar, dependent: :destroy has_one :user_avatar, dependent: :destroy
has_one :facebook_user_info, dependent: :destroy has_one :facebook_user_info, dependent: :destroy
has_one :twitter_user_info, dependent: :destroy has_one :twitter_user_info, dependent: :destroy
@ -80,6 +81,7 @@ class User < ActiveRecord::Base
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_profile after_create :create_user_profile
after_create :ensure_in_trust_level_group after_create :ensure_in_trust_level_group
after_create :automatic_group_membership after_create :automatic_group_membership
@ -123,7 +125,7 @@ class User < ActiveRecord::Base
# TODO-PERF: There is no indexes on any of these # TODO-PERF: There is no indexes on any of these
# and NotifyMailingListSubscribers does a select-all-and-loop # and NotifyMailingListSubscribers does a select-all-and-loop
# may want to create an index on (active, blocked, suspended_till, mailing_list_mode)? # may want to create an index on (active, blocked, suspended_till)?
scope :blocked, -> { where(blocked: true) } scope :blocked, -> { where(blocked: true) }
scope :not_blocked, -> { where(blocked: false) } scope :not_blocked, -> { where(blocked: false) }
scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) } scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) }
@ -911,6 +913,10 @@ class User < ActiveRecord::Base
stat.save! stat.save!
end end
def create_user_option
UserOption.create(user_id: id)
end
def create_email_token def create_email_token
email_tokens.create(email: email) email_tokens.create(email: email)
end end
@ -965,21 +971,8 @@ class User < ActiveRecord::Base
end end
def set_default_user_preferences def set_default_user_preferences
set_default_email_digest_frequency
set_default_email_private_messages
set_default_email_direct
set_default_email_mailing_list_mode
set_default_email_always
set_default_other_new_topic_duration_minutes set_default_other_new_topic_duration_minutes
set_default_other_auto_track_topics_after_msecs set_default_other_auto_track_topics_after_msecs
set_default_other_external_links_in_new_tab
set_default_other_enable_quoting
set_default_other_dynamic_favicon
set_default_other_disable_jump_reply
set_default_other_edit_history_public
set_default_topics_automatic_unpin
# needed, otherwise the callback chain is broken... # needed, otherwise the callback chain is broken...
true true
@ -1031,26 +1024,6 @@ class User < ActiveRecord::Base
end end
end end
def set_default_email_digest_frequency
if has_attribute?(:email_digests)
if SiteSetting.default_email_digest_frequency.to_i <= 0
self.email_digests = false
else
self.email_digests = true
self.digest_after_days ||= SiteSetting.default_email_digest_frequency.to_i if has_attribute?(:digest_after_days)
end
end
end
def set_default_email_mailing_list_mode
self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode if has_attribute?(:mailing_list_mode)
end
%w{private_messages direct always}.each do |s|
define_method("set_default_email_#{s}") do
self.send("email_#{s}=", SiteSetting.send("default_email_#{s}")) if has_attribute?("email_#{s}")
end
end
%w{new_topic_duration_minutes auto_track_topics_after_msecs}.each do |s| %w{new_topic_duration_minutes auto_track_topics_after_msecs}.each do |s|
define_method("set_default_other_#{s}") do define_method("set_default_other_#{s}") do
@ -1058,16 +1031,6 @@ class User < ActiveRecord::Base
end end
end end
%w{external_links_in_new_tab enable_quoting dynamic_favicon disable_jump_reply edit_history_public}.each do |s|
define_method("set_default_other_#{s}") do
self.send("#{s}=", SiteSetting.send("default_other_#{s}")) if has_attribute?(s)
end
end
def set_default_topics_automatic_unpin
self.automatically_unpin_topics = SiteSetting.default_topics_automatic_unpin
end
end end
# == Schema Information # == Schema Information
@ -1090,14 +1053,11 @@ end
# last_seen_at :datetime # last_seen_at :datetime
# admin :boolean default(FALSE), not null # admin :boolean default(FALSE), not null
# last_emailed_at :datetime # last_emailed_at :datetime
# email_digests :boolean not null
# trust_level :integer not null # trust_level :integer not null
# email_private_messages :boolean default(TRUE) # email_private_messages :boolean default(TRUE)
# email_direct :boolean default(TRUE), not null
# 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
# digest_after_days :integer
# previous_visit_at :datetime # previous_visit_at :datetime
# suspended_at :datetime # suspended_at :datetime
# suspended_till :datetime # suspended_till :datetime
@ -1107,24 +1067,16 @@ end
# 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 # new_topic_duration_minutes :integer
# external_links_in_new_tab :boolean not null
# enable_quoting :boolean default(TRUE), not null
# moderator :boolean default(FALSE) # moderator :boolean default(FALSE)
# blocked :boolean default(FALSE) # blocked :boolean default(FALSE)
# dynamic_favicon :boolean default(FALSE), not null
# title :string(255) # title :string(255)
# uploaded_avatar_id :integer # uploaded_avatar_id :integer
# email_always :boolean default(FALSE), not null
# mailing_list_mode :boolean default(FALSE), not null
# 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 # last_redirected_to_top_at :datetime
# disable_jump_reply :boolean default(FALSE), not null
# edit_history_public :boolean default(FALSE), not null
# 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
# automatically_unpin_topics :boolean default(TRUE)
# #
# Indexes # Indexes
# #

View File

@ -64,12 +64,12 @@ class UserEmailObserver < ActiveRecord::Observer
EMAILABLE_POST_TYPES ||= Set.new [Post.types[:regular], Post.types[:whisper]] EMAILABLE_POST_TYPES ||= Set.new [Post.types[:regular], Post.types[:whisper]]
def enqueue(type, delay=default_delay) def enqueue(type, delay=default_delay)
return unless notification.user.email_direct? return unless notification.user.user_option.email_direct?
perform_enqueue(type, delay) perform_enqueue(type, delay)
end end
def enqueue_private(type, delay=private_delay) def enqueue_private(type, delay=private_delay)
return unless notification.user.email_private_messages? return unless notification.user.user_option.email_private_messages?
perform_enqueue(type, delay) perform_enqueue(type, delay)
end end

29
app/models/user_option.rb Normal file
View File

@ -0,0 +1,29 @@
class UserOption < ActiveRecord::Base
self.primary_key = :user_id
belongs_to :user
before_create :set_defaults
def set_defaults
self.email_always = SiteSetting.default_email_always
self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode
self.email_direct = SiteSetting.default_email_direct
self.automatically_unpin_topics = SiteSetting.default_topics_automatic_unpin
self.email_private_messages = SiteSetting.default_email_private_messages
self.enable_quoting = SiteSetting.default_other_enable_quoting
self.external_links_in_new_tab = SiteSetting.default_other_external_links_in_new_tab
self.dynamic_favicon = SiteSetting.default_other_dynamic_favicon
self.disable_jump_reply = SiteSetting.default_other_disable_jump_reply
self.edit_history_public = SiteSetting.default_other_edit_history_public
if SiteSetting.default_email_digest_frequency.to_i <= 0
self.email_digests = false
else
self.email_digests = true
self.digest_after_days ||= SiteSetting.default_email_digest_frequency.to_i
end
true
end
end

View File

@ -31,7 +31,8 @@ class CurrentUserSerializer < BasicUserSerializer
:is_anonymous, :is_anonymous,
:post_queue_new_count, :post_queue_new_count,
:show_queued_posts, :show_queued_posts,
:read_faq :read_faq,
:automatically_unpin_topics
def include_site_flagged_posts_count? def include_site_flagged_posts_count?
object.staff? object.staff?
@ -49,6 +50,26 @@ class CurrentUserSerializer < BasicUserSerializer
object.user_stat.topic_reply_count object.user_stat.topic_reply_count
end end
def enable_quoting
object.user_option.enable_quoting
end
def disable_jump_reply
object.user_option.disable_jump_reply
end
def external_links_in_new_tab
object.user_option.external_links_in_new_tab
end
def dynamic_favicon
object.user_option.dynamic_favicon
end
def automatically_unpin_topics
object.user_option.automatically_unpin_topics
end
def site_flagged_posts_count def site_flagged_posts_count
PostAction.flagged_posts_count PostAction.flagged_posts_count
end end

View File

@ -0,0 +1,20 @@
class UserOptionSerializer < ApplicationSerializer
attributes :user_id,
:email_always,
:mailing_list_mode,
:email_digests,
:email_private_messages,
:email_direct,
:external_links_in_new_tab,
:dynamic_favicon,
:enable_quoting,
:disable_jump_reply,
:digest_after_days,
:automatically_unpin_topics,
:edit_history_public
def include_edit_history_public?
!SiteSetting.edit_history_visible_to_public
end
end

View File

@ -61,40 +61,33 @@ class UserSerializer < BasicUserSerializer
:uploaded_avatar_id, :uploaded_avatar_id,
:badge_count, :badge_count,
:has_title_badges, :has_title_badges,
:edit_history_public,
:custom_fields, :custom_fields,
:user_fields, :user_fields,
:topic_post_count, :topic_post_count,
:pending_count, :pending_count,
:profile_view_count, :profile_view_count
:automatically_unpin_topics
has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_one :invited_by, embed: :object, serializer: BasicUserSerializer
has_many :groups, embed: :object, serializer: BasicGroupSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer
has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges
has_one :card_badge, embed: :object, serializer: BadgeSerializer has_one :card_badge, embed: :object, serializer: BadgeSerializer
has_one :user_option, embed: :object, serializer: UserOptionSerializer
def include_user_option?
can_edit
end
staff_attributes :post_count, staff_attributes :post_count,
:can_be_deleted, :can_be_deleted,
:can_delete_all_posts :can_delete_all_posts
private_attributes :locale, private_attributes :locale,
:email_digests,
:email_private_messages,
:email_direct,
:email_always,
:digest_after_days,
:mailing_list_mode,
:auto_track_topics_after_msecs, :auto_track_topics_after_msecs,
:new_topic_duration_minutes, :new_topic_duration_minutes,
:external_links_in_new_tab,
:dynamic_favicon,
:enable_quoting,
:muted_category_ids, :muted_category_ids,
:tracked_category_ids, :tracked_category_ids,
:watched_category_ids, :watched_category_ids,
:private_messages_stats, :private_messages_stats,
:disable_jump_reply,
:system_avatar_upload_id, :system_avatar_upload_id,
:system_avatar_template, :system_avatar_template,
:gravatar_avatar_upload_id, :gravatar_avatar_upload_id,
@ -322,10 +315,6 @@ class UserSerializer < BasicUserSerializer
object.badges.where(allow_title: true).count > 0 object.badges.where(allow_title: true).count > 0
end end
def include_edit_history_public?
can_edit && !SiteSetting.edit_history_visible_to_public
end
def user_fields def user_fields
object.user_fields object.user_fields
end end

View File

@ -40,11 +40,14 @@ class AnonymousShadowCreator
active: true, active: true,
trust_level: 1, trust_level: 1,
trust_level_locked: true, trust_level_locked: true,
email_private_messages: false,
email_digests: false,
created_at: 1.day.ago # bypass new user restrictions created_at: 1.day.ago # bypass new user restrictions
) )
shadow.user_option.update_columns(
email_private_messages: false,
email_digests: false
)
shadow.email_tokens.update_all(confirmed: true) shadow.email_tokens.update_all(confirmed: true)
shadow.activate shadow.activate

View File

@ -23,14 +23,17 @@ class UserAnonymizer
@user.name = nil @user.name = nil
@user.date_of_birth = nil @user.date_of_birth = nil
@user.title = nil @user.title = nil
@user.email_digests = false
@user.email_private_messages = false
@user.email_direct = false
@user.email_always = false
@user.mailing_list_mode = false
@user.uploaded_avatar_id = nil @user.uploaded_avatar_id = nil
@user.save @user.save
options = @user.user_option
options.email_always = false
options.mailing_list_mode = false
options.email_digests = false
options.email_private_messages = false
options.email_direct = false
options.save
profile = @user.user_profile profile = @user.user_profile
profile.destroy if profile profile.destroy if profile
@user.create_user_profile @user.create_user_profile

View File

@ -6,18 +6,19 @@ class UserUpdater
muted_category_ids: :muted muted_category_ids: :muted
} }
USER_ATTR = [ OPTION_ATTR = [
:email_digests,
:email_always, :email_always,
:mailing_list_mode,
:email_digests,
:email_direct, :email_direct,
:email_private_messages, :email_private_messages,
:external_links_in_new_tab, :external_links_in_new_tab,
:enable_quoting, :enable_quoting,
:dynamic_favicon, :dynamic_favicon,
:mailing_list_mode,
:disable_jump_reply, :disable_jump_reply,
:edit_history_public, :edit_history_public,
:automatically_unpin_topics, :automatically_unpin_topics,
:digest_after_days
] ]
def initialize(actor, user) def initialize(actor, user)
@ -36,7 +37,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 }
user.digest_after_days = attributes.fetch(:digest_after_days) { user.digest_after_days }
if attributes[:auto_track_topics_after_msecs] if attributes[:auto_track_topics_after_msecs]
user.auto_track_topics_after_msecs = attributes[:auto_track_topics_after_msecs].to_i user.auto_track_topics_after_msecs = attributes[:auto_track_topics_after_msecs].to_i
@ -56,9 +56,19 @@ class UserUpdater
end end
end end
USER_ATTR.each do |attribute|
save_options = false
OPTION_ATTR.each do |attribute|
if attributes[attribute].present? if attributes[attribute].present?
user.send("#{attribute}=", attributes[attribute] == 'true') save_options = true
if [true,false].include?(user.user_option.send(attribute))
val = attributes[attribute].to_s == 'true'
user.user_option.send("#{attribute}=", val)
else
user.user_option.send("#{attribute}=", attributes[attribute])
end
end end
end end
@ -72,7 +82,7 @@ class UserUpdater
update_muted_users(attributes[:muted_usernames]) update_muted_users(attributes[:muted_usernames])
end end
user_profile.save && user.save (!save_options || user.user_option.save) && user_profile.save && user.save
end end
end end

View File

@ -16,12 +16,15 @@ User.seed do |u|
u.active = true u.active = true
u.admin = true u.admin = true
u.moderator = true u.moderator = true
u.email_direct = false
u.approved = true u.approved = true
u.email_private_messages = false
u.trust_level = TrustLevel[4] u.trust_level = TrustLevel[4]
end end
UserOption.where(user_id: -1).update_all(
email_private_messages: false,
email_direct: false
)
Group.user_trust_level_change!(-1, TrustLevel[4]) Group.user_trust_level_change!(-1, TrustLevel[4])
# User for the smoke tests # User for the smoke tests
@ -49,3 +52,4 @@ if ENV["SMOKE"] == "1"
et.confirmed = true et.confirmed = true
end end
end end

View File

@ -0,0 +1,76 @@
class AddUserOptions < ActiveRecord::Migration
def up
create_table :user_options, id: false do |t|
t.integer :user_id, null: false
t.boolean :email_always, null: false, default: false
t.boolean :mailing_list_mode, null: false, default: false
t.boolean :email_digests
t.boolean :email_direct, null: false, default: true
t.boolean :email_private_messages, null: false, default: true
t.boolean :external_links_in_new_tab, null: false, default: false
t.boolean :enable_quoting, null: false, default: true
t.boolean :dynamic_favicon, null: false, default: false
t.boolean :disable_jump_reply, null: false, default: false
t.boolean :edit_history_public, null: false, default: false
t.boolean :automatically_unpin_topics, null: false, default: true
t.integer :digest_after_days
end
add_index :user_options, [:user_id], unique: true
execute <<SQL
INSERT INTO user_options (
user_id,
email_always,
mailing_list_mode,
email_digests,
email_direct,
email_private_messages,
external_links_in_new_tab,
enable_quoting,
dynamic_favicon,
disable_jump_reply,
edit_history_public,
automatically_unpin_topics,
digest_after_days
)
SELECT id,
email_always,
mailing_list_mode,
email_digests,
email_direct,
email_private_messages,
external_links_in_new_tab,
enable_quoting,
dynamic_favicon,
disable_jump_reply,
edit_history_public,
automatically_unpin_topics,
digest_after_days
FROM users
SQL
# these can not be removed until a bit later
# if we remove them now all currently running unicorns will start erroring out
#
# remove_column :users, :email_always
# remove_column :users, :mailing_list_mode
# remove_column :users, :email_digests
# remove_column :users, :email_direct
# remove_column :users, :email_private_messages
# remove_column :users, :external_links_in_new_tab
# remove_column :users, :enable_quoting
# remove_column :users, :dynamic_favicon
# remove_column :users, :disable_jump_reply
# remove_column :users, :edit_history_public
# remove_column :users, :automatically_unpin_topics
# remove_column :users, :digest_after_days
end
def down
# we can not move backwards here cause columns
# get removed an hour after the migration
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -157,7 +157,7 @@ module PostGuardian
return false unless post return false unless post
if !post.hidden if !post.hidden
return true if post.wiki || SiteSetting.edit_history_visible_to_public || post.user.try(:edit_history_public) return true if post.wiki || SiteSetting.edit_history_visible_to_public || (post.user && post.user.user_option.edit_history_public)
end end
authenticated? && authenticated? &&

View File

@ -510,7 +510,7 @@ describe Guardian do
it 'is true if the author has public edit history' do it 'is true if the author has public edit history' do
public_post_revision = Fabricate(:post_revision) public_post_revision = Fabricate(:post_revision)
public_post_revision.post.user.edit_history_public = true public_post_revision.post.user.user_option.edit_history_public = true
expect(Guardian.new.can_see?(public_post_revision)).to be_truthy expect(Guardian.new.can_see?(public_post_revision)).to be_truthy
end end
end end
@ -533,7 +533,7 @@ describe Guardian do
it 'is true if the author has public edit history' do it 'is true if the author has public edit history' do
public_post_revision = Fabricate(:post_revision) public_post_revision = Fabricate(:post_revision)
public_post_revision.post.user.edit_history_public = true public_post_revision.post.user.user_option.edit_history_public = true
expect(Guardian.new.can_see?(public_post_revision)).to be_truthy expect(Guardian.new.can_see?(public_post_revision)).to be_truthy
end end
end end

View File

@ -21,7 +21,11 @@ describe EmailController do
context '.resubscribe' do context '.resubscribe' do
let(:user) { Fabricate(:user, email_digests: false) } let(:user) {
user = Fabricate(:user)
user.user_option.update_columns(email_digests: false)
user
}
let(:key) { DigestUnsubscribeKey.create_key_for(user) } let(:key) { DigestUnsubscribeKey.create_key_for(user) }
context 'with a valid key' do context 'with a valid key' do
@ -31,7 +35,7 @@ describe EmailController do
end end
it 'subscribes the user' do it 'subscribes the user' do
expect(user.email_digests).to eq(true) expect(user.user_option.email_digests).to eq(true)
end end
end end
@ -39,7 +43,11 @@ describe EmailController do
context '.unsubscribe' do context '.unsubscribe' do
let(:user) { Fabricate(:user, email_digests: true, email_direct: true, email_private_messages: true, email_always: true) } let(:user) {
user = Fabricate(:user)
user.user_option.update_columns(email_always: true, email_digests: true, email_direct: true, email_private_messages: true)
user
}
let(:key) { DigestUnsubscribeKey.create_key_for(user) } let(:key) { DigestUnsubscribeKey.create_key_for(user) }
context 'from confirm unsubscribe email' do context 'from confirm unsubscribe email' do
@ -49,10 +57,11 @@ describe EmailController do
end end
it 'unsubscribes from all emails' do it 'unsubscribes from all emails' do
expect(user.email_digests).to eq false options = user.user_option
expect(user.email_direct).to eq false expect(options.email_digests).to eq false
expect(user.email_private_messages).to eq false expect(options.email_direct).to eq false
expect(user.email_always).to eq false expect(options.email_private_messages).to eq false
expect(options.email_always).to eq false
end end
end end
@ -63,7 +72,7 @@ describe EmailController do
end end
it 'unsubscribes the user' do it 'unsubscribes the user' do
expect(user.email_digests).to eq(false) expect(user.user_option.email_digests).to eq(false)
end end
it "sets the appropriate instance variables" do it "sets the appropriate instance variables" do
@ -90,7 +99,7 @@ describe EmailController do
end end
it 'does not unsubscribe the user' do it 'does not unsubscribe the user' do
expect(user.email_digests).to eq(true) expect(user.user_option.email_digests).to eq(true)
end end
it 'sets the appropriate instance variables' do it 'sets the appropriate instance variables' do
@ -108,7 +117,7 @@ describe EmailController do
end end
it 'unsubscribes the user' do it 'unsubscribes the user' do
expect(user.email_digests).to eq(false) expect(user.user_option.email_digests).to eq(false)
end end
it 'sets the appropriate instance variables' do it 'sets the appropriate instance variables' do

View File

@ -55,7 +55,8 @@ describe Jobs::UserEmail do
end end
it "does send an email to a user that's been recently seen but has email_always set" do it "does send an email to a user that's been recently seen but has email_always set" do
user.update_attributes(last_seen_at: 9.minutes.ago, email_always: true) user.update_attributes(last_seen_at: 9.minutes.ago)
user.user_option.update_attributes(email_always: true)
Email::Sender.any_instance.expects(:send) Email::Sender.any_instance.expects(:send)
Jobs::UserEmail.new.execute(type: :user_replied, user_id: user.id, post_id: post.id) Jobs::UserEmail.new.execute(type: :user_replied, user_id: user.id, post_id: post.id)
end end
@ -188,13 +189,13 @@ describe Jobs::UserEmail do
it "does send the email if the notification has been seen but the user is set for email_always" do it "does send the email if the notification has been seen but the user is set for email_always" do
Email::Sender.any_instance.expects(:send) Email::Sender.any_instance.expects(:send)
notification.update_column(:read, true) notification.update_column(:read, true)
user.update_column(:email_always, true) user.user_option.update_column(:email_always, true)
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
end end
it "doesn't send the mail if the user is using mailing list mode" do it "doesn't send the mail if the user is using mailing list mode" do
Email::Sender.any_instance.expects(:send).never Email::Sender.any_instance.expects(:send).never
user.update_column(:mailing_list_mode, true) user.user_option.update_column(:mailing_list_mode, true)
# sometimes, we pass the notification_id # sometimes, we pass the notification_id
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id)
# other times, we only pass the type of notification # other times, we only pass the type of notification

View File

@ -51,7 +51,7 @@ describe UserEmailObserver do
include_examples "enqueue" include_examples "enqueue"
it "doesn't enqueue a job if the user has mention emails disabled" do it "doesn't enqueue a job if the user has mention emails disabled" do
notification.user.expects(:email_direct?).returns(false) notification.user.user_option.update_columns(email_direct: false)
Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
UserEmailObserver.process_notification(notification) UserEmailObserver.process_notification(notification)
end end
@ -61,7 +61,7 @@ describe UserEmailObserver do
include_examples "enqueue" include_examples "enqueue"
it "doesn't enqueue a job if the user has private message emails disabled" do it "doesn't enqueue a job if the user has private message emails disabled" do
notification.user.expects(:email_private_messages?).returns(false) notification.user.user_option.update_columns(email_private_messages: false)
Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
UserEmailObserver.process_notification(notification) UserEmailObserver.process_notification(notification)
end end

View File

@ -152,15 +152,15 @@ describe User do
it "is properly initialized" do it "is properly initialized" do
expect(subject.approved_at).to be_blank expect(subject.approved_at).to be_blank
expect(subject.approved_by_id).to be_blank expect(subject.approved_by_id).to be_blank
expect(subject.email_private_messages).to eq(true)
expect(subject.email_direct).to eq(true)
end end
context 'after_save' do context 'after_save' do
before { subject.save } before { subject.save }
it "has an email token" do it "has correct settings" do
expect(subject.email_tokens).to be_present expect(subject.email_tokens).to be_present
expect(subject.user_option.email_private_messages).to eq(true)
expect(subject.user_option.email_direct).to eq(true)
end end
end end
@ -1246,50 +1246,62 @@ describe User do
context "when user preferences are overriden" do context "when user preferences are overriden" do
before do before do
SiteSetting.stubs(:default_email_digest_frequency).returns(1) # daily SiteSetting.default_email_digest_frequency = 1 # daily
SiteSetting.stubs(:default_email_private_messages).returns(false) SiteSetting.default_email_private_messages = false
SiteSetting.stubs(:default_email_direct).returns(false) SiteSetting.default_email_direct = false
SiteSetting.stubs(:default_email_mailing_list_mode).returns(true) SiteSetting.default_email_mailing_list_mode = true
SiteSetting.stubs(:default_email_always).returns(true) SiteSetting.default_email_always = true
SiteSetting.stubs(:default_other_new_topic_duration_minutes).returns(-1) # not viewed SiteSetting.default_other_new_topic_duration_minutes = -1 # not viewed
SiteSetting.stubs(:default_other_auto_track_topics_after_msecs).returns(0) # immediately SiteSetting.default_other_auto_track_topics_after_msecs = 0 # immediately
SiteSetting.stubs(:default_other_external_links_in_new_tab).returns(true) SiteSetting.default_other_external_links_in_new_tab = true
SiteSetting.stubs(:default_other_enable_quoting).returns(false) SiteSetting.default_other_enable_quoting = false
SiteSetting.stubs(:default_other_dynamic_favicon).returns(true) SiteSetting.default_other_dynamic_favicon = true
SiteSetting.stubs(:default_other_disable_jump_reply).returns(true) SiteSetting.default_other_disable_jump_reply = true
SiteSetting.stubs(:default_other_edit_history_public).returns(true) SiteSetting.default_other_edit_history_public = true
SiteSetting.stubs(:default_topics_automatic_unpin).returns(false) SiteSetting.default_topics_automatic_unpin = false
SiteSetting.stubs(:default_categories_watching).returns("1") SiteSetting.default_categories_watching = "1"
SiteSetting.stubs(:default_categories_tracking).returns("2") SiteSetting.default_categories_tracking = "2"
SiteSetting.stubs(:default_categories_muted).returns("3") SiteSetting.default_categories_muted = "3"
end end
it "has overriden preferences" do it "has overriden preferences" do
user = Fabricate(:user) user = Fabricate(:user)
options = user.user_option
expect(user.digest_after_days).to eq(1) expect(options.email_always).to eq(true)
expect(user.email_private_messages).to eq(false) expect(options.mailing_list_mode).to eq(true)
expect(user.email_direct).to eq(false) expect(options.digest_after_days).to eq(1)
expect(user.mailing_list_mode).to eq(true) expect(options.email_private_messages).to eq(false)
expect(user.email_always).to eq(true) expect(options.external_links_in_new_tab).to eq(true)
expect(options.enable_quoting).to eq(false)
expect(options.dynamic_favicon).to eq(true)
expect(options.disable_jump_reply).to eq(true)
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.new_topic_duration_minutes).to eq(-1)
expect(user.auto_track_topics_after_msecs).to eq(0) expect(user.auto_track_topics_after_msecs).to eq(0)
expect(user.external_links_in_new_tab).to eq(true)
expect(user.enable_quoting).to eq(false)
expect(user.dynamic_favicon).to eq(true)
expect(user.disable_jump_reply).to eq(true)
expect(user.edit_history_public).to eq(true)
expect(user.automatically_unpin_topics).to eq(false)
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])
expect(CategoryUser.lookup(user, :muted).pluck(:category_id)).to eq([3]) expect(CategoryUser.lookup(user, :muted).pluck(:category_id)).to eq([3])
end end
end
context UserOption do
it "Creates a UserOption row when a user record is created and destroys once done" do
user = Fabricate(:user)
expect(user.user_option.email_always).to eq(false)
user_id = user.id
user.destroy!
expect(UserOption.find_by(user_id: user_id)).to eq(nil)
end
end end

View File

@ -15,6 +15,24 @@ describe UserSerializer do
end end
end end
context "as current user" do
it "serializes options correctly" do
# so we serialize more stuff
SiteSetting.edit_history_visible_to_public = false
user = Fabricate.build(:user,
user_profile: Fabricate.build(:user_profile),
user_option: UserOption.new(edit_history_public: true),
user_stat: UserStat.new
)
json = UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json
expect(json[:user_option][:edit_history_public]).to eq(true)
end
end
context "with a user" do context "with a user" do
let(:user) { Fabricate.build(:user, user_profile: Fabricate.build(:user_profile) ) } let(:user) { Fabricate.build(:user, user_profile: Fabricate.build(:user_profile) ) }
let(:serializer) { UserSerializer.new(user, scope: Guardian.new, root: false) } let(:serializer) { UserSerializer.new(user, scope: Guardian.new, root: false) }
@ -26,7 +44,7 @@ describe UserSerializer do
context "with `enable_names` true" do context "with `enable_names` true" do
before do before do
SiteSetting.stubs(:enable_names?).returns(true) SiteSetting.enable_names = true
end end
it "has a name" do it "has a name" do
@ -34,6 +52,7 @@ describe UserSerializer do
end end
end end
context "with `enable_names` false" do context "with `enable_names` false" do
before do before do
SiteSetting.stubs(:enable_names?).returns(false) SiteSetting.stubs(:enable_names?).returns(false)

View File

@ -30,6 +30,9 @@ describe AnonymousShadowCreator do
freeze_time 4.minutes.from_now freeze_time 4.minutes.from_now
shadow3 = AnonymousShadowCreator.get(user) shadow3 = AnonymousShadowCreator.get(user)
expect(shadow3.user_option.email_digests).to eq(false)
expect(shadow3.user_option.email_private_messages).to eq(false)
expect(shadow2.id).not_to eq(shadow3.id) expect(shadow2.id).not_to eq(shadow3.id)
end end

View File

@ -19,13 +19,17 @@ describe UserAnonymizer do
end end
it "turns off all notifications" do it "turns off all notifications" do
user.user_option.update_columns(
email_always: true
)
make_anonymous make_anonymous
user.reload user.reload
expect(user.email_digests).to eq(false) expect(user.user_option.email_digests).to eq(false)
expect(user.email_private_messages).to eq(false) expect(user.user_option.email_private_messages).to eq(false)
expect(user.email_direct).to eq(false) expect(user.user_option.email_direct).to eq(false)
expect(user.email_always).to eq(false) expect(user.user_option.email_always).to eq(false)
expect(user.mailing_list_mode).to eq(false) expect(user.user_option.mailing_list_mode).to eq(false)
end end
it "resets profile to default values" do it "resets profile to default values" do

View File

@ -32,26 +32,33 @@ describe UserUpdater do
describe '#update' do describe '#update' do
it 'saves user' do it 'saves user' do
user = Fabricate(:user, name: 'Billy Bob') user = Fabricate(:user, name: 'Billy Bob')
updater = described_class.new(acting_user, user) updater = UserUpdater.new(acting_user, user)
updater.update(name: 'Jim Tom') updater.update(name: 'Jim Tom')
expect(user.reload.name).to eq 'Jim Tom' expect(user.reload.name).to eq 'Jim Tom'
end end
it 'updates bio' do it 'updates various fields' do
user = Fabricate(:user) user = Fabricate(:user)
updater = described_class.new(acting_user, user) updater = UserUpdater.new(acting_user, user)
updater.update(bio_raw: 'my new bio') updater.update(bio_raw: 'my new bio',
email_always: 'true',
mailing_list_mode: true,
digest_after_days: "8")
user.reload
expect(user.reload.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.mailing_list_mode).to eq true
expect(user.user_option.digest_after_days).to eq 8
end end
context 'when update succeeds' do context 'when update succeeds' do
it 'returns true' do it 'returns true' do
user = Fabricate(:user) user = Fabricate(:user)
updater = described_class.new(acting_user, user) updater = UserUpdater.new(acting_user, user)
expect(updater.update).to be_truthy expect(updater.update).to be_truthy
end end
@ -61,7 +68,7 @@ describe UserUpdater do
it 'returns false' do it 'returns false' do
user = Fabricate(:user) user = Fabricate(:user)
user.stubs(save: false) user.stubs(save: false)
updater = described_class.new(acting_user, user) updater = UserUpdater.new(acting_user, user)
expect(updater.update).to be_falsey expect(updater.update).to be_falsey
end end
@ -73,7 +80,7 @@ describe UserUpdater do
guardian = stub guardian = stub
guardian.stubs(:can_grant_title?).with(user).returns(true) guardian.stubs(:can_grant_title?).with(user).returns(true)
Guardian.stubs(:new).with(acting_user).returns(guardian) Guardian.stubs(:new).with(acting_user).returns(guardian)
updater = described_class.new(acting_user, user) updater = UserUpdater.new(acting_user, user)
updater.update(title: 'Minion') updater.update(title: 'Minion')