diff --git a/Gemfile b/Gemfile index 12fa31a0af6..3f5a46fb761 100644 --- a/Gemfile +++ b/Gemfile @@ -64,7 +64,7 @@ gem 'aws-sdk', require: false gem 'excon', require: false gem 'unf', require: false -gem 'email_reply_trimmer', '0.0.6' +gem 'email_reply_trimmer', '0.0.8' # note: for image_optim to correctly work you need to follow # https://github.com/toy/image_optim diff --git a/Gemfile.lock b/Gemfile.lock index 55d8c3d124b..a614c28a89b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -76,7 +76,7 @@ GEM docile (1.1.5) domain_name (0.5.25) unf (>= 0.0.5, < 1.0.0) - email_reply_trimmer (0.0.6) + email_reply_trimmer (0.0.8) ember-data-source (1.0.0.beta.16.1) ember-source (~> 1.8) ember-handlebars-template (0.1.5) @@ -411,7 +411,7 @@ DEPENDENCIES byebug certified discourse-qunit-rails - email_reply_trimmer (= 0.0.6) + email_reply_trimmer (= 0.0.8) ember-rails ember-source (= 1.12.2) excon diff --git a/app/assets/javascripts/discourse/controllers/preferences.js.es6 b/app/assets/javascripts/discourse/controllers/preferences.js.es6 index 1efdaff99be..5231e4fa430 100644 --- a/app/assets/javascripts/discourse/controllers/preferences.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences.js.es6 @@ -62,6 +62,12 @@ export default Ember.Controller.extend(CanCheckEmails, { return this.siteSettings.available_locales.split('|').map(s => ({ name: s, value: s })); }, + previousRepliesOptions: [ + {name: I18n.t('user.email_previous_replies.always'), value: 0}, + {name: I18n.t('user.email_previous_replies.unless_emailed'), value: 1}, + {name: I18n.t('user.email_previous_replies.never'), value: 2} + ], + digestFrequencies: [{ name: I18n.t('user.email_digests.daily'), value: 1 }, { name: I18n.t('user.email_digests.every_three_days'), value: 3 }, { name: I18n.t('user.email_digests.weekly'), value: 7 }, diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index c57574431e7..9eeb9f87b35 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -159,6 +159,7 @@ const User = RestModel.extend({ 'email_digests', 'email_direct', 'email_private_messages', + 'email_previous_replies', 'dynamic_favicon', 'enable_quoting', 'disable_jump_reply', diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index 06bab9cbf45..6e8c026a4d3 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -176,6 +176,10 @@ {{/if}} {{/if}} +
+ + {{combo-box valueAttribute="value" content=previousRepliesOptions value=model.user_option.email_previous_replies}} +
{{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}} @@ -188,6 +192,9 @@ {{i18n 'user.email.frequency_immediately'}} {{/if}} + + +
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 62653c427e2..920cc34955d 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -28,28 +28,7 @@ class Admin::GroupsController < Admin::AdminController if group.present? users = (params[:users] || []).map {|u| u.downcase} user_ids = User.where("username_lower in (:users) OR email IN (:users)", users: users).pluck(:id) - - if user_ids.present? - Group.exec_sql("INSERT INTO group_users - (group_id, user_id, created_at, updated_at) - SELECT #{group.id}, - u.id, - CURRENT_TIMESTAMP, - CURRENT_TIMESTAMP - FROM users AS u - WHERE u.id IN (#{user_ids.join(', ')}) - AND NOT EXISTS(SELECT 1 FROM group_users AS gu - WHERE gu.user_id = u.id AND - gu.group_id = #{group.id})") - - if group.primary_group? - User.where(id: user_ids).update_all(primary_group_id: group.id) - end - - if group.title.present? - User.where(id: user_ids).update_all(title: group.title) - end - end + group.bulk_add(user_ids) if user_ids.present? end render json: success_json diff --git a/app/jobs/scheduled/enqueue_digest_emails.rb b/app/jobs/scheduled/enqueue_digest_emails.rb index ea74f98ed28..9c10f695730 100644 --- a/app/jobs/scheduled/enqueue_digest_emails.rb +++ b/app/jobs/scheduled/enqueue_digest_emails.rb @@ -19,8 +19,8 @@ module Jobs .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_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * user_options.digest_after_days)") + .where("COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * user_options.digest_after_days)") .where("COALESCE(last_seen_at, '2010-01-01') >= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * #{SiteSetting.delete_digest_email_after_days})") # If the site requires approval, make sure the user is approved diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 52db55f8ab6..e8def9ecce6 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -193,6 +193,11 @@ class UserNotifications < ActionMailer::Base end def self.get_context_posts(post, topic_user) + user_option = topic_user.try(:user).try(:user_option) + if user_option && (user_option.email_previous_replies == UserOption.previous_replies_type[:never]) + return [] + end + allowed_post_types = [Post.types[:regular]] allowed_post_types << Post.types[:whisper] if topic_user.try(:user).try(:staff?) @@ -204,7 +209,7 @@ class UserNotifications < ActionMailer::Base .order('created_at desc') .limit(SiteSetting.email_posts_context) - if topic_user && topic_user.last_emailed_post_number + if topic_user && topic_user.last_emailed_post_number && user_option.try(:email_previous_replies) == UserOption.previous_replies_type[:unless_emailed] context_posts = context_posts.where("post_number > ?", topic_user.last_emailed_post_number) end diff --git a/app/models/group.rb b/app/models/group.rb index a508335a77a..148b1e9e8d1 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -335,6 +335,31 @@ class Group < ActiveRecord::Base self.find_by(incoming_email: Email.downcase(email)) end + def bulk_add(user_ids) + if user_ids.present? + Group.exec_sql("INSERT INTO group_users + (group_id, user_id, created_at, updated_at) + SELECT #{self.id}, + u.id, + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP + FROM users AS u + WHERE u.id IN (#{user_ids.join(', ')}) + AND NOT EXISTS(SELECT 1 FROM group_users AS gu + WHERE gu.user_id = u.id AND + gu.group_id = #{self.id})") + + if self.primary_group? + User.where(id: user_ids).update_all(primary_group_id: self.id) + end + + if self.title.present? + User.where(id: user_ids).update_all(title: self.title) + end + end + true + end + protected def name_format_validator diff --git a/app/models/previous_replies_site_setting.rb b/app/models/previous_replies_site_setting.rb new file mode 100644 index 00000000000..eac55ae5232 --- /dev/null +++ b/app/models/previous_replies_site_setting.rb @@ -0,0 +1,22 @@ +require_dependency 'enum_site_setting' + +class PreviousRepliesSiteSetting < EnumSiteSetting + + def self.valid_value?(val) + val.to_i.to_s == val.to_s && + values.any? { |v| v[:value] == val.to_i } + end + + def self.values + @values ||= [ + { name: 'user.email_previous_replies.always', value: 0 }, + { name: 'user.email_previous_replies.unless_emailed', value: 1 }, + { name: 'user.email_previous_replies.never', value: 2 }, + ] + end + + def self.translate_names? + true + end + +end diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 46d7e673079..e6384f061b2 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -5,12 +5,17 @@ class UserOption < ActiveRecord::Base after_save :update_tracked_topics + def self.previous_replies_type + @previous_replies_type ||= Enum.new(always: 0, unless_emailed: 1, never: 2) + end + 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.email_previous_replies = SiteSetting.default_email_previous_replies self.enable_quoting = SiteSetting.default_other_enable_quoting self.external_links_in_new_tab = SiteSetting.default_other_external_links_in_new_tab diff --git a/app/models/username_validator.rb b/app/models/username_validator.rb index 1ef41e17efa..0dbae4bae6e 100644 --- a/app/models/username_validator.rb +++ b/app/models/username_validator.rb @@ -71,7 +71,7 @@ class UsernameValidator def username_first_char_valid? return unless errors.empty? if username[0] =~ /\W/ - self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric') + self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric_or_underscore') end end diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index 440587a4d42..bf43b41a36f 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -13,7 +13,8 @@ class UserOptionSerializer < ApplicationSerializer :automatically_unpin_topics, :edit_history_public, :auto_track_topics_after_msecs, - :new_topic_duration_minutes + :new_topic_duration_minutes, + :email_previous_replies def include_edit_history_public? diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index b3b8e9031c7..3b24efff08f 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -20,7 +20,8 @@ class UserUpdater :automatically_unpin_topics, :digest_after_days, :new_topic_duration_minutes, - :auto_track_topics_after_msecs + :auto_track_topics_after_msecs, + :email_previous_replies ] def initialize(actor, user) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 89a2331567c..b476120000c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -627,6 +627,11 @@ en: website: "Web Site" email_settings: "Email" + email_previous_replies: + title: "Include previous replies" + unless_emailed: "unless previously sent" + always: "always" + never: "never" email_digests: title: "When I don't visit here, send an email digest of what's new:" daily: "daily" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5a4011301a5..e42d71a2adc 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1240,6 +1240,7 @@ en: default_email_direct: "Send an email when someone quotes/replies to/mentions or invites the user by default." default_email_mailing_list_mode: "Send an email for every new post by default." default_email_always: "Send an email notification even when the user is active by default." + default_email_previous_replies: "Include previous replies in emails by default." default_other_new_topic_duration_minutes: "Global default condition for which a topic is considered new." default_other_auto_track_topics_after_msecs: "Global default time before a topic is automatically tracked." @@ -1394,8 +1395,8 @@ en: characters: "must only include numbers, letters and underscores" unique: "must be unique" blank: "must be present" - must_begin_with_alphanumeric: "must begin with a letter or number or an underscore" - must_end_with_alphanumeric: "must end with a letter or number or an underscore" + must_begin_with_alphanumeric_or_underscore: "must begin with a letter, a number or an underscore" + must_end_with_alphanumeric: "must end with a letter or a number" must_not_contain_two_special_chars_in_seq: "must not contain a sequence of 2 or more special chars (.-_)" must_not_end_with_confusing_suffix: "must not end with a confusing suffix like .json or .png etc." email: diff --git a/config/site_settings.yml b/config/site_settings.yml index b95667162aa..b456e86ca8b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1061,6 +1061,9 @@ user_preferences: default_email_direct: true default_email_mailing_list_mode: false default_email_always: false + default_email_previous_replies: + enum: 'PreviousRepliesSiteSetting' + default: 1 default_other_new_topic_duration_minutes: enum: 'NewTopicDurationSiteSetting' diff --git a/db/migrate/20160225050320_add_email_previous_replies_to_user_options.rb b/db/migrate/20160225050320_add_email_previous_replies_to_user_options.rb new file mode 100644 index 00000000000..5f9aec18a03 --- /dev/null +++ b/db/migrate/20160225050320_add_email_previous_replies_to_user_options.rb @@ -0,0 +1,5 @@ +class AddEmailPreviousRepliesToUserOptions < ActiveRecord::Migration + def change + add_column :user_options, :email_previous_replies, :integer, null: false, default: 1 + end +end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 947dd9a466b..a58c2d16f53 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -7,12 +7,12 @@ describe UserNotifications do describe "#get_context_posts" do it "does not include hidden/deleted/user_deleted posts in context" do post1 = create_post - post2 = Fabricate(:post, topic: post1.topic, deleted_at: 1.day.ago) - post3 = Fabricate(:post, topic: post1.topic, user_deleted: true) - post4 = Fabricate(:post, topic: post1.topic, hidden: true) - post5 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:moderator_action]) - post6 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:small_action]) - post7 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:whisper]) + _post2 = Fabricate(:post, topic: post1.topic, deleted_at: 1.day.ago) + _post3 = Fabricate(:post, topic: post1.topic, user_deleted: true) + _post4 = Fabricate(:post, topic: post1.topic, hidden: true) + _post5 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:moderator_action]) + _post6 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:small_action]) + _post7 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:whisper]) last = Fabricate(:post, topic: post1.topic) # default is only post #1 @@ -21,6 +21,26 @@ describe UserNotifications do tu = TopicUser.new(topic: post1.topic, user: build(:moderator)) expect(UserNotifications.get_context_posts(last, tu).count).to eq(2) end + + it "allows users to control context" do + post1 = create_post + _post2 = Fabricate(:post, topic: post1.topic) + post3 = Fabricate(:post, topic: post1.topic) + + user = Fabricate(:user) + TopicUser.change(user.id, post1.topic_id, last_emailed_post_number: 1) + topic_user = TopicUser.find_by(user_id: user.id, topic_id: post1.topic_id) + # to avoid reloads after update_columns + user = topic_user.user + expect(UserNotifications.get_context_posts(post3, topic_user).count).to eq(1) + + user.user_option.update_columns(email_previous_replies: UserOption.previous_replies_type[:never]) + expect(UserNotifications.get_context_posts(post3, topic_user).count).to eq(0) + + user.user_option.update_columns(email_previous_replies: UserOption.previous_replies_type[:always]) + expect(UserNotifications.get_context_posts(post3, topic_user).count).to eq(2) + + end end describe ".signup" do