From 3811b8aa4c9fc08bf84d5da55b7927f4e300b9b5 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 16 Feb 2016 12:25:01 -0500 Subject: [PATCH 1/6] `withPluginAPI` shim to updated plugins will not raise errors --- app/assets/javascripts/discourse/lib/plugin-api.js.es6 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 index b7a7ece9129..f5d66d738a6 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 +++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 @@ -15,3 +15,8 @@ export function decorateCooked(container, cb) { decorate(container.lookupFactory('view:embedded-post'), 'didInsertElement', cb); decorate(container.lookupFactory('view:user-stream'), 'didInsertElement', cb); } + +// Will be backported so plugins in the new format will not raise errors +export function withPluginApi(version) { + console.warn(`Plugin API v${version} is not supported`); +} From 63b9d1c64588bb2d7a3fe3c84741edf0cd836a5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 16 Feb 2016 18:29:23 +0100 Subject: [PATCH 2/6] FIX: sends an email notifcation when a user's post is linked --- app/controllers/admin/email_templates_controller.rb | 3 ++- app/mailers/user_notifications.rb | 7 +++++++ app/models/user_email_observer.rb | 4 ++++ config/locales/server.en.yml | 12 ++++++++++++ spec/models/user_email_observer_spec.rb | 8 ++++++++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/email_templates_controller.rb b/app/controllers/admin/email_templates_controller.rb index 9d872c99842..ccbb3eaf692 100644 --- a/app/controllers/admin/email_templates_controller.rb +++ b/app/controllers/admin/email_templates_controller.rb @@ -26,7 +26,8 @@ class Admin::EmailTemplatesController < Admin::AdminController "user_notifications.user_invited_to_private_message_pm", "user_notifications.user_invited_to_topic", "user_notifications.user_mentioned", "user_notifications.user_posted", "user_notifications.user_posted_pm", - "user_notifications.user_quoted", "user_notifications.user_replied"] + "user_notifications.user_quoted", "user_notifications.user_replied", + "user_notifications.user_linked"] end def show diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 51729427816..2f4423cb146 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -110,6 +110,13 @@ class UserNotifications < ActionMailer::Base notification_email(user, opts) end + def user_linked(user, opts) + opts[:allow_reply_by_email] = true + opts[:use_site_subject] = true + opts[:show_category_in_subject] = true + notification_email(user, opts) + end + def user_mentioned(user, opts) opts[:allow_reply_by_email] = true opts[:use_site_subject] = true diff --git a/app/models/user_email_observer.rb b/app/models/user_email_observer.rb index db9dbdc73ef..31128235ef9 100644 --- a/app/models/user_email_observer.rb +++ b/app/models/user_email_observer.rb @@ -28,6 +28,10 @@ class UserEmailObserver < ActiveRecord::Observer enqueue :user_replied end + def linked + enqueue :user_linked + end + def private_message enqueue_private(:user_private_message) end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2e7ef476879..281d5365467 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2095,6 +2095,18 @@ en: --- %{respond_instructions} + user_linked: + subject_template: "[%{site_name}] %{topic_title}" + text_body_template: | + %{header_instructions} + + %{message} + + %{context} + + --- + %{respond_instructions} + user_mentioned: subject_template: "[%{site_name}] %{topic_title}" text_body_template: | diff --git a/spec/models/user_email_observer_spec.rb b/spec/models/user_email_observer_spec.rb index 0f1150c4dd1..c0b8da164ed 100644 --- a/spec/models/user_email_observer_spec.rb +++ b/spec/models/user_email_observer_spec.rb @@ -99,6 +99,14 @@ describe UserEmailObserver do include_examples "enqueue_public" end + context 'user_linked' do + let(:type) { :user_linked } + let(:delay) { SiteSetting.email_time_window_mins.minutes } + let!(:notification) { create_notification(11) } + + include_examples "enqueue_public" + end + context 'user_posted' do let(:type) { :user_posted } let(:delay) { SiteSetting.email_time_window_mins.minutes } From 63cda2262307d43160de0a84d538a646cd288723 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 16 Feb 2016 16:43:59 -0500 Subject: [PATCH 3/6] Upgrade `withPluginApi` to support non-api callbacks --- .../discourse/lib/plugin-api.js.es6 | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 index f5d66d738a6..5a700db5f2a 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 +++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 @@ -16,7 +16,29 @@ export function decorateCooked(container, cb) { decorate(container.lookupFactory('view:user-stream'), 'didInsertElement', cb); } -// Will be backported so plugins in the new format will not raise errors -export function withPluginApi(version) { +// This is backported so plugins in the new format will not raise errors +// +// To upgrade your plugin for backwards compatibility, you can add code in this +// form: +// +// function newApiCode(api) { +// // api.xyz(); +// } +// +// function oldCode() { +// // your pre-PluginAPI code goes here. You will be able to delete this +// // code once the `PluginAPI` has been rolled out to all versions of +// // Discourse you want to support. +// } +// +// // `newApiCode` will use API version 0.1, if no API support then +// // `oldCode` will be called +// withPluginApi('0.1', newApiCode, { noApi: oldCode }); +// +export function withPluginApi(version, apiCodeCallback, opts) { console.warn(`Plugin API v${version} is not supported`); + + if (opts && opts.noApi) { + return opts.noApi(); + } } From 3829c78526d2a552ff61767f81694650151cac6b Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 17 Feb 2016 15:46:19 +1100 Subject: [PATCH 4/6] 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 --- .../javascripts/discourse/models/user.js.es6 | 32 ++++---- .../discourse/templates/user/preferences.hbs | 26 +++---- app/controllers/email_controller.rb | 9 ++- .../notify_mailing_list_subscribers.rb | 3 +- app/jobs/regular/user_email.rb | 6 +- app/jobs/scheduled/enqueue_digest_emails.rb | 4 +- app/mailers/user_notifications.rb | 2 +- app/models/user.rb | 62 ++------------- app/models/user_email_observer.rb | 4 +- app/models/user_option.rb | 29 +++++++ app/serializers/current_user_serializer.rb | 23 +++++- app/serializers/user_option_serializer.rb | 20 +++++ app/serializers/user_serializer.rb | 23 ++---- app/services/anonymous_shadow_creator.rb | 7 +- app/services/user_anonymizer.rb | 13 ++-- app/services/user_updater.rb | 24 ++++-- db/fixtures/009_users.rb | 8 +- db/migrate/20160225050317_add_user_options.rb | 76 +++++++++++++++++++ lib/guardian/post_guardian.rb | 2 +- spec/components/guardian_spec.rb | 4 +- spec/controllers/email_controller_spec.rb | 29 ++++--- spec/jobs/user_email_spec.rb | 7 +- spec/models/user_email_observer_spec.rb | 4 +- spec/models/user_spec.rb | 76 +++++++++++-------- spec/serializers/user_serializer_spec.rb | 21 ++++- .../services/anonymous_shadow_creator_spec.rb | 3 + spec/services/user_anonymizer_spec.rb | 14 ++-- spec/services/user_updater_spec.rb | 23 ++++-- 28 files changed, 363 insertions(+), 191 deletions(-) create mode 100644 app/models/user_option.rb create mode 100644 app/serializers/user_option_serializer.rb create mode 100644 db/migrate/20160225050317_add_user_options.rb diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index fd702adff76..a26f5e10e7a 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -147,25 +147,29 @@ const User = RestModel.extend({ 'location', 'name', 'locale', - 'email_digests', - 'email_direct', - 'email_always', - 'email_private_messages', - 'dynamic_favicon', - 'digest_after_days', 'new_topic_duration_minutes', - 'external_links_in_new_tab', - 'mailing_list_mode', - 'enable_quoting', - 'disable_jump_reply', 'custom_fields', 'user_fields', 'muted_usernames', 'profile_background', - 'card_background', - 'automatically_unpin_topics' + 'card_background' ); + [ '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 => { let cats = this.get(s + 'Categories').map(c => c.get('id')); // HACK: denote lack of categories @@ -174,7 +178,7 @@ const User = RestModel.extend({ }); 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. @@ -184,7 +188,7 @@ const User = RestModel.extend({ type: 'PUT' }).then(result => { 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); }).finally(() => { this.set('isSaving', false); diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index 636227b757b..4c65524e82b 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -169,17 +169,17 @@
{{#if canReceiveDigest}} - {{preference-checkbox labelKey="user.email_digests.title" checked=model.email_digests}} - {{#if model.email_digests}} + {{preference-checkbox labelKey="user.email_digests.title" checked=model.user_option.email_digests}} + {{#if model.user_option.email_digests}}
- {{combo-box valueAttribute="value" content=digestFrequencies value=model.digest_after_days}} + {{combo-box valueAttribute="value" content=digestFrequencies value=model.user_option.digest_after_days}}
{{/if}} {{/if}} - {{preference-checkbox labelKey="user.email_private_messages" checked=model.email_private_messages}} - {{preference-checkbox labelKey="user.email_direct" checked=model.email_direct}} - {{preference-checkbox labelKey="user.mailing_list_mode" checked=model.mailing_list_mode}} - {{preference-checkbox labelKey="user.email_always" checked=model.email_always}} + {{preference-checkbox labelKey="user.email_private_messages" checked=model.user_option.email_private_messages}} + {{preference-checkbox labelKey="user.email_direct" checked=model.user_option.email_direct}} + {{preference-checkbox labelKey="user.mailing_list_mode" checked=model.user_option.mailing_list_mode}} + {{preference-checkbox labelKey="user.email_always" checked=model.user_option.email_always}}
{{#if siteSettings.email_time_window_mins}} @@ -209,12 +209,12 @@ {{combo-box valueAttribute="value" content=autoTrackDurations value=model.auto_track_topics_after_msecs}}
- {{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.external_links_in_new_tab}} - {{preference-checkbox labelKey="user.enable_quoting" checked=model.enable_quoting}} - {{preference-checkbox labelKey="user.dynamic_favicon" checked=model.dynamic_favicon}} - {{preference-checkbox labelKey="user.disable_jump_reply" checked=model.disable_jump_reply}} + {{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.user_option.enable_quoting}} + {{preference-checkbox labelKey="user.dynamic_favicon" checked=model.user_option.dynamic_favicon}} + {{preference-checkbox labelKey="user.disable_jump_reply" checked=model.user_option.disable_jump_reply}} {{#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}} {{plugin-outlet "user-custom-preferences"}} @@ -254,7 +254,7 @@ {{#if siteSettings.automatically_unpin_topics}}
- {{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}}
{{/if}} diff --git a/app/controllers/email_controller.rb b/app/controllers/email_controller.rb index 4e0ff301af7..22bbec05103 100644 --- a/app/controllers/email_controller.rb +++ b/app/controllers/email_controller.rb @@ -25,9 +25,12 @@ class EmailController < ApplicationController end 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 - @user.update_column(:email_digests, false) + @user.user_option.update_column(:email_digests, false) end @success = true @@ -36,7 +39,7 @@ class EmailController < ApplicationController def resubscribe @user = DigestUnsubscribeKey.user_for_key(params[:key]) raise Discourse::NotFound unless @user.present? - @user.update_column(:email_digests, true) + @user.user_option.update_column(:email_digests, true) end end diff --git a/app/jobs/regular/notify_mailing_list_subscribers.rb b/app/jobs/regular/notify_mailing_list_subscribers.rb index c78e78f3e63..29436e42327 100644 --- a/app/jobs/regular/notify_mailing_list_subscribers.rb +++ b/app/jobs/regular/notify_mailing_list_subscribers.rb @@ -11,7 +11,8 @@ module Jobs users = 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( SELECT 1 FROM topic_users tu diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 6830ec2e64a..ff38409c1be 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -62,7 +62,7 @@ module Jobs 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 = false if user.email_always || user.staged + seen_recently = false if user.user_option.email_always || user.staged email_args = {} @@ -85,14 +85,14 @@ module Jobs email_args[:notification_type] = email_args[:notification_type].to_s end - if user.mailing_list_mode? && + if user.user_option.mailing_list_mode? && !post.topic.private_message? && 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 return [nil, nil] end - unless user.email_always? + unless user.user_option.email_always? if (notification && notification.read?) || (post && post.seen?(user)) return skip_message(I18n.t('email_log.notification_already_read')) end diff --git a/app/jobs/scheduled/enqueue_digest_emails.rb b/app/jobs/scheduled/enqueue_digest_emails.rb index 40cf0976d95..ea74f98ed28 100644 --- a/app/jobs/scheduled/enqueue_digest_emails.rb +++ b/app/jobs/scheduled/enqueue_digest_emails.rb @@ -15,8 +15,10 @@ module Jobs def target_user_ids # Users who want to receive emails and haven't been emailed in the last day query = User.real - .where(email_digests: true, active: true, staged: false) + .where(active: true, staged: false) + .joins(:user_option) .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_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})") diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 2f4423cb146..52db55f8ab6 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -321,7 +321,7 @@ class UserNotifications < ActionMailer::Base context: context, username: username, 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, allow_reply_by_email: allow_reply_by_email, use_site_subject: use_site_subject, diff --git a/app/models/user.rb b/app/models/user.rb index a1be4c16b97..5634cbeaf38 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,6 +38,7 @@ class User < ActiveRecord::Base has_many :user_archived_messages, dependent: :destroy + has_one :user_option, dependent: :destroy has_one :user_avatar, dependent: :destroy has_one :facebook_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_user_stat + after_create :create_user_option after_create :create_user_profile after_create :ensure_in_trust_level_group after_create :automatic_group_membership @@ -123,7 +125,7 @@ class User < ActiveRecord::Base # TODO-PERF: There is no indexes on any of these # 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 :not_blocked, -> { where(blocked: false) } scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) } @@ -911,6 +913,10 @@ class User < ActiveRecord::Base stat.save! end + def create_user_option + UserOption.create(user_id: id) + end + def create_email_token email_tokens.create(email: email) end @@ -965,21 +971,8 @@ class User < ActiveRecord::Base end 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_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... true @@ -1031,26 +1024,6 @@ class User < ActiveRecord::Base 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| define_method("set_default_other_#{s}") do @@ -1058,16 +1031,6 @@ class User < ActiveRecord::Base 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 # == Schema Information @@ -1090,14 +1053,11 @@ end # last_seen_at :datetime # admin :boolean default(FALSE), not null # last_emailed_at :datetime -# email_digests :boolean not null # trust_level :integer not null # email_private_messages :boolean default(TRUE) -# email_direct :boolean default(TRUE), not null # approved :boolean default(FALSE), not null # approved_by_id :integer # approved_at :datetime -# digest_after_days :integer # previous_visit_at :datetime # suspended_at :datetime # suspended_till :datetime @@ -1107,24 +1067,16 @@ end # flag_level :integer default(0), not null # ip_address :inet # new_topic_duration_minutes :integer -# external_links_in_new_tab :boolean not null -# enable_quoting :boolean default(TRUE), not null # moderator :boolean default(FALSE) # blocked :boolean default(FALSE) -# dynamic_favicon :boolean default(FALSE), not null # title :string(255) # uploaded_avatar_id :integer -# email_always :boolean default(FALSE), not null -# mailing_list_mode :boolean default(FALSE), not null # primary_group_id :integer # locale :string(10) # registration_ip_address :inet # 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 # staged :boolean default(FALSE), not null -# automatically_unpin_topics :boolean default(TRUE) # # Indexes # diff --git a/app/models/user_email_observer.rb b/app/models/user_email_observer.rb index 31128235ef9..d153d01b504 100644 --- a/app/models/user_email_observer.rb +++ b/app/models/user_email_observer.rb @@ -64,12 +64,12 @@ class UserEmailObserver < ActiveRecord::Observer EMAILABLE_POST_TYPES ||= Set.new [Post.types[:regular], Post.types[:whisper]] def enqueue(type, delay=default_delay) - return unless notification.user.email_direct? + return unless notification.user.user_option.email_direct? perform_enqueue(type, delay) end 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) end diff --git a/app/models/user_option.rb b/app/models/user_option.rb new file mode 100644 index 00000000000..3dac2ded9e0 --- /dev/null +++ b/app/models/user_option.rb @@ -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 diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index c576025cd5c..5cf912d4734 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -31,7 +31,8 @@ class CurrentUserSerializer < BasicUserSerializer :is_anonymous, :post_queue_new_count, :show_queued_posts, - :read_faq + :read_faq, + :automatically_unpin_topics def include_site_flagged_posts_count? object.staff? @@ -49,6 +50,26 @@ class CurrentUserSerializer < BasicUserSerializer object.user_stat.topic_reply_count 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 PostAction.flagged_posts_count end diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb new file mode 100644 index 00000000000..77c9d68540a --- /dev/null +++ b/app/serializers/user_option_serializer.rb @@ -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 diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 480fbd9c612..1ad96ce2405 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -61,40 +61,33 @@ class UserSerializer < BasicUserSerializer :uploaded_avatar_id, :badge_count, :has_title_badges, - :edit_history_public, :custom_fields, :user_fields, :topic_post_count, :pending_count, - :profile_view_count, - :automatically_unpin_topics + :profile_view_count has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges 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, :can_be_deleted, :can_delete_all_posts private_attributes :locale, - :email_digests, - :email_private_messages, - :email_direct, - :email_always, - :digest_after_days, - :mailing_list_mode, :auto_track_topics_after_msecs, :new_topic_duration_minutes, - :external_links_in_new_tab, - :dynamic_favicon, - :enable_quoting, :muted_category_ids, :tracked_category_ids, :watched_category_ids, :private_messages_stats, - :disable_jump_reply, :system_avatar_upload_id, :system_avatar_template, :gravatar_avatar_upload_id, @@ -322,10 +315,6 @@ class UserSerializer < BasicUserSerializer object.badges.where(allow_title: true).count > 0 end - def include_edit_history_public? - can_edit && !SiteSetting.edit_history_visible_to_public - end - def user_fields object.user_fields end diff --git a/app/services/anonymous_shadow_creator.rb b/app/services/anonymous_shadow_creator.rb index 18b7ecceaf3..08c1d6a3380 100644 --- a/app/services/anonymous_shadow_creator.rb +++ b/app/services/anonymous_shadow_creator.rb @@ -40,11 +40,14 @@ class AnonymousShadowCreator active: true, trust_level: 1, trust_level_locked: true, - email_private_messages: false, - email_digests: false, 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.activate diff --git a/app/services/user_anonymizer.rb b/app/services/user_anonymizer.rb index 48699b176a7..e9ff162dbc6 100644 --- a/app/services/user_anonymizer.rb +++ b/app/services/user_anonymizer.rb @@ -23,14 +23,17 @@ class UserAnonymizer @user.name = nil @user.date_of_birth = 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.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.destroy if profile @user.create_user_profile diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index c6f236d2b32..e028a0d14b0 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -6,18 +6,19 @@ class UserUpdater muted_category_ids: :muted } - USER_ATTR = [ - :email_digests, + OPTION_ATTR = [ :email_always, + :mailing_list_mode, + :email_digests, :email_direct, :email_private_messages, :external_links_in_new_tab, :enable_quoting, :dynamic_favicon, - :mailing_list_mode, :disable_jump_reply, :edit_history_public, :automatically_unpin_topics, + :digest_after_days ] def initialize(actor, user) @@ -36,7 +37,6 @@ class UserUpdater user.name = attributes.fetch(:name) { user.name } 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] user.auto_track_topics_after_msecs = attributes[:auto_track_topics_after_msecs].to_i @@ -56,9 +56,19 @@ class UserUpdater end end - USER_ATTR.each do |attribute| + + save_options = false + OPTION_ATTR.each do |attribute| 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 @@ -72,7 +82,7 @@ class UserUpdater update_muted_users(attributes[:muted_usernames]) end - user_profile.save && user.save + (!save_options || user.user_option.save) && user_profile.save && user.save end end diff --git a/db/fixtures/009_users.rb b/db/fixtures/009_users.rb index 7effb73775a..e6c7486b96d 100644 --- a/db/fixtures/009_users.rb +++ b/db/fixtures/009_users.rb @@ -16,12 +16,15 @@ User.seed do |u| u.active = true u.admin = true u.moderator = true - u.email_direct = false u.approved = true - u.email_private_messages = false u.trust_level = TrustLevel[4] end +UserOption.where(user_id: -1).update_all( + email_private_messages: false, + email_direct: false +) + Group.user_trust_level_change!(-1, TrustLevel[4]) # User for the smoke tests @@ -49,3 +52,4 @@ if ENV["SMOKE"] == "1" et.confirmed = true end end + diff --git a/db/migrate/20160225050317_add_user_options.rb b/db/migrate/20160225050317_add_user_options.rb new file mode 100644 index 00000000000..13df057cda4 --- /dev/null +++ b/db/migrate/20160225050317_add_user_options.rb @@ -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 < Date: Wed, 17 Feb 2016 17:47:47 +1100 Subject: [PATCH 5/6] FEATURE: start tracking information about migrations that run This commit adds a new tracking table that lets us know - When a migration ran - What version Discourse was at - How long it took - What version Rails was at The built in tracking in Rails is very limited, does not track this info --- ...0225050318_add_schema_migration_details.rb | 30 +++++++++++ .../schema_migration_details.rb | 54 +++++++++++++++++++ .../schema_migration_details_spec.rb | 32 +++++++++++ 3 files changed, 116 insertions(+) create mode 100644 db/migrate/20000225050318_add_schema_migration_details.rb create mode 100644 lib/freedom_patches/schema_migration_details.rb create mode 100644 spec/components/freedom_patches/schema_migration_details_spec.rb diff --git a/db/migrate/20000225050318_add_schema_migration_details.rb b/db/migrate/20000225050318_add_schema_migration_details.rb new file mode 100644 index 00000000000..dbf4959b173 --- /dev/null +++ b/db/migrate/20000225050318_add_schema_migration_details.rb @@ -0,0 +1,30 @@ +class AddSchemaMigrationDetails < ActiveRecord::Migration + def up + # schema_migrations table is way too thin, does not give info about + # duration of migration or the date it happened, this migration together with the + # monkey patch adds a lot of information to the migration table + + create_table :schema_migration_details do |t| + t.string :version, null: false + t.string :name + t.string :hostname + t.string :git_version + t.string :rails_version + t.integer :duration + t.string :direction # this really should be a pg enum type but annoying to wire up for little gain + t.datetime :created_at, null: false + end + + add_index :schema_migration_details, [:version] + + execute("INSERT INTO schema_migration_details(version, created_at) + SELECT version, current_timestamp + FROM schema_migrations + ORDER BY version + ") + end + + def down + drop_table :schema_migration_details + end +end diff --git a/lib/freedom_patches/schema_migration_details.rb b/lib/freedom_patches/schema_migration_details.rb new file mode 100644 index 00000000000..bbae956c11a --- /dev/null +++ b/lib/freedom_patches/schema_migration_details.rb @@ -0,0 +1,54 @@ +module FreedomPatches + module SchemaMigrationDetails + def exec_migration(conn, direction) + rval = nil + + time = Benchmark.measure do + rval=super + end + + sql = < 0 + expect(info.git_version).to eq Discourse.git_version + expect(info.direction).to eq "up" + expect(info.rails_version).to eq Rails.version + expect(info.filename).to eq migration.filename + expect(info.name).to eq "awesome_migration" + end +end From 6912aa9fd92dbe38a1e3c1c40d1e624f368792ab Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 17 Feb 2016 18:08:05 +1100 Subject: [PATCH 6/6] Remove superflous columns from the users table --- db/fixtures/009_users.rb | 34 +++++++++++++++++++ ...225050318_allow_defaults_on_users_table.rb | 10 ++++++ 2 files changed, 44 insertions(+) create mode 100644 db/migrate/20160225050318_allow_defaults_on_users_table.rb diff --git a/db/fixtures/009_users.rb b/db/fixtures/009_users.rb index e6c7486b96d..4c5f1187540 100644 --- a/db/fixtures/009_users.rb +++ b/db/fixtures/009_users.rb @@ -27,6 +27,40 @@ UserOption.where(user_id: -1).update_all( Group.user_trust_level_change!(-1, TrustLevel[4]) +# 60 minutes after our migration runs we need to exectue this code... +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' + ) AND + name = 'AllowDefaultsOnUsersTable' AND + created_at < (current_timestamp at time zone 'UTC' - interval '#{duration} minutes') + ").to_a.length > 0 + + + User.transaction do + STDERR.puts "Removing superflous user columns!" + %w[ + 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 + ].each do |column| + User.exec_sql("ALTER TABLE users DROP column #{column}") + end + + end +end + # User for the smoke tests if ENV["SMOKE"] == "1" smoke_user = User.seed do |u| diff --git a/db/migrate/20160225050318_allow_defaults_on_users_table.rb b/db/migrate/20160225050318_allow_defaults_on_users_table.rb new file mode 100644 index 00000000000..999e275fdd0 --- /dev/null +++ b/db/migrate/20160225050318_allow_defaults_on_users_table.rb @@ -0,0 +1,10 @@ +class AllowDefaultsOnUsersTable < ActiveRecord::Migration + def up + # we need to temporarily change table a bit to ensure we can insert new records + change_column :users, :email_digests, :boolean, null: false, default: true + change_column :users, :external_links_in_new_tab, :boolean, null: false, default: false + end + + def down + end +end