diff --git a/app/assets/javascripts/discourse/controllers/preferences/emails.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/emails.js.es6 index db25accf5a1..74b1d76f67a 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/emails.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/emails.js.es6 @@ -30,7 +30,6 @@ export default Ember.Controller.extend(PreferencesTabController, { @computed() mailingListModeOptions() { return [ - {name: I18n.t('user.mailing_list_mode.daily'), value: 0}, {name: this.get('frequencyEstimate'), value: 1}, {name: I18n.t('user.mailing_list_mode.individual_no_echo'), value: 2} ]; diff --git a/app/jobs/scheduled/enqueue_mailing_list_emails.rb b/app/jobs/scheduled/enqueue_mailing_list_emails.rb deleted file mode 100644 index 1f4562dffdb..00000000000 --- a/app/jobs/scheduled/enqueue_mailing_list_emails.rb +++ /dev/null @@ -1,29 +0,0 @@ -module Jobs - - class EnqueueMailingListEmails < Jobs::Scheduled - every 1.hour - - def execute(args) - return if SiteSetting.disable_mailing_list_mode? - target_user_ids.each do |user_id| - Jobs.enqueue(:user_email, type: :mailing_list, user_id: user_id) - end - end - - def target_user_ids - # Users who want to receive daily mailing list emails - User.real - .activated - .not_suspended - .not_blocked - .joins(:user_option) - .where(staged: false, user_options: { mailing_list_mode: true, mailing_list_mode_frequency: 0 }) - .where("#{!SiteSetting.must_approve_users?} OR approved OR moderator OR admin") - .where("date_part('hour', first_seen_at) = date_part('hour', CURRENT_TIMESTAMP)") # where the hour of first_seen_at is the same as the current hour - .where("COALESCE(first_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - '23 HOURS'::INTERVAL") # don't send unless you've been around for a day already - .pluck(:id) - end - - end - -end diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 0f91088de55..d96d0ec8074 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -75,39 +75,6 @@ class UserNotifications < ActionMailer::Base end end - # This is the "mailing list summary email" - def mailing_list(user, opts={}) - @since = opts[:since] || 1.day.ago - @since_formatted = short_date(@since) - - topics = Topic - .joins(:posts) - .includes(:posts) - .for_digest(user, 100.years.ago) - .where("posts.created_at > ?", @since) - - unless user.staff? - topics = topics.where("posts.post_type <> ?", Post.types[:whisper]) - end - - @new_topics = topics.where("topics.created_at > ?", @since).uniq - @existing_topics = topics.where("topics.created_at <= ?", @since).uniq - @topics = topics.uniq - - return if @topics.empty? - - build_summary_for(user) - opts = { - from_alias: I18n.t('user_notifications.mailing_list.from', site_name: SiteSetting.title), - subject: I18n.t('user_notifications.mailing_list.subject_template', email_prefix: @email_prefix, date: @date), - mailing_list_mode: true, - add_unsubscribe_link: true, - unsubscribe_url: "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}", - } - - apply_notification_styles(build_email(@user.email, opts)) - end - def digest(user, opts={}) build_summary_for(user) min_date = opts[:since] || user.last_emailed_at || user.last_seen_at || 1.month.ago diff --git a/app/models/mailing_list_mode_site_setting.rb b/app/models/mailing_list_mode_site_setting.rb index 9a8241c1f04..ea0161de827 100644 --- a/app/models/mailing_list_mode_site_setting.rb +++ b/app/models/mailing_list_mode_site_setting.rb @@ -8,7 +8,6 @@ class MailingListModeSiteSetting < EnumSiteSetting def self.values @values ||= [ - { name: 'user.mailing_list_mode.daily', value: 0 }, { name: 'user.mailing_list_mode.individual', value: 1 }, { name: 'user.mailing_list_mode.individual_no_echo', value: 2 } ] diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 7ef3b5262af..0a1cb1cdabb 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -155,8 +155,8 @@ end # email_previous_replies :integer default(2), not null # email_in_reply_to :boolean default(TRUE), not null # like_notification_frequency :integer default(1), not null +# mailing_list_mode_frequency :integer default(1), not null # include_tl0_in_digests :boolean default(FALSE) -# mailing_list_mode_frequency :integer default(0), not null # # Indexes # diff --git a/app/views/user_notifications/mailing_list.html.erb b/app/views/user_notifications/mailing_list.html.erb deleted file mode 100644 index 076204dd2fe..00000000000 --- a/app/views/user_notifications/mailing_list.html.erb +++ /dev/null @@ -1,79 +0,0 @@ -
- - - - - - - -
- - <% if logo_url.blank? %> - <%= SiteSetting.title %> - <% else %> - - <% end %> -
-
- <%= raw(t 'user_notifications.mailing_list.why', site_link: html_site_link(@anchor_color), date: @since_formatted) %> -
-
- -
-

<%= t('user_notifications.mailing_list.new_topics') %>

-
    - <% @new_topics.each do |topic| %> - <%= mailing_list_topic(topic, topic.posts.length) %> - <% end %> -
-

<%= t('user_notifications.mailing_list.topic_updates') %>

-
    - <% @existing_topics.each do |topic| %> - <%= mailing_list_topic(topic, topic.posts.length) %> - <% end %> -
-
- - <% @topics.each do |topic| %> - - - - <%- unless SiteSetting.private_email? %> - - - - - - - <% end %> - <% end %> -
-

- <%= email_topic_link(topic) %> -

-
- <% topic.posts.each do |post| %> -
- -

- - <%- if show_username_on_post(post) %> - <%= post.user.username %> - <% end %> - - <%- if show_name_on_post(post) %> - - <%= post.user.name %> - <% end %> - - - - - <%= I18n.l(post.created_at, format: :long) %> -

- <%= raw format_for_email(post, false) %> -
-
- <% end %> - <%= t('user_notifications.mailing_list.view_this_topic') %> -
- <%= t('user_notifications.mailing_list.back_to_top') %> -
diff --git a/app/views/user_notifications/mailing_list.text.erb b/app/views/user_notifications/mailing_list.text.erb deleted file mode 100644 index 90f63dbfd0d..00000000000 --- a/app/views/user_notifications/mailing_list.text.erb +++ /dev/null @@ -1,33 +0,0 @@ -<%- site_link = raw(@markdown_linker.create(@site_name, '/')) %> -<%= raw(t 'user_notifications.mailing_list.why', site_link: site_link, date: @since_formatted) %> - -<%- if @new_topics.present? -%> - ### <%= t 'user_notifications.mailing_list.new_topics' %> - <%- @new_topics.each do |topic| %> - <%= mailing_list_topic_text(topic) %> - <%- end -%> -<%- end -%> - -<%- if @existing_topics.present? -%> - ### <%= t 'user_notifications.mailing_list.new_topics' %> - <%- @existing_topics.each do |topic| -%> - <%= mailing_list_topic_text(topic) %> - <%= topic.posts.length %> - <%- end -%> -<%- end -%> - --------------------------------------------------------------------------------- - -<%- unless SiteSetting.private_email? %> - <%- @topics.each do |topic| %> - ### <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %> - - <%- topic.posts.each do |post| -%> - <%= post.user.name || post.user.username %> - <%= post.created_at %> - -------------------------------------------------------------------------------- - <%= post.raw %> - - - <%- end -%> - <%- end %> -<%- end %> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0d2c312851f..7bc3e113dd6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -599,7 +599,6 @@ en: instructions: | This setting overrides the activity summary.
Muted topics and categories are not included in these emails. - daily: "Send daily updates" individual: "Send an email for every new post" individual_no_echo: "Send an email for every new post except my own" many_per_day: "Send me an email for every new post (about {{dailyEmailEstimate}} per day)" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ab86c9070ad..3191c59848c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2594,15 +2594,6 @@ en: above_footer: '' below_footer: '' - mailing_list: - why: "All activity on %{site_link} for %{date}" - subject_template: "[%{email_prefix}] Summary for %{date}" - unsubscribe: "This summary is sent daily due to mailing list mode being enabled. To unsubscribe %{unsubscribe_link}." - from: "%{site_name} summary" - new_topics: "New topics" - topic_updates: "Topic updates" - view_this_topic: "View this topic" - back_to_top: "Back to top" forgot_password: title: "Forgot Password" subject_template: "[%{email_prefix}] Password reset" diff --git a/db/migrate/20170505035229_migrate_mailing_list_daily_updates_users_to_daily_summary.rb b/db/migrate/20170505035229_migrate_mailing_list_daily_updates_users_to_daily_summary.rb new file mode 100644 index 00000000000..418bfe2e95b --- /dev/null +++ b/db/migrate/20170505035229_migrate_mailing_list_daily_updates_users_to_daily_summary.rb @@ -0,0 +1,11 @@ +class MigrateMailingListDailyUpdatesUsersToDailySummary < ActiveRecord::Migration + def change + change_column_default :user_options, :mailing_list_mode_frequency, 1 + + UserOption.exec_sql(<<~SQL) + UPDATE user_options + SET digest_after_minutes = 1440, email_digests = 't', mailing_list_mode = 'f' + WHERE mailing_list_mode_frequency = 0 AND mailing_list_mode + SQL + end +end diff --git a/spec/jobs/enqueue_mailing_list_emails_spec.rb b/spec/jobs/enqueue_mailing_list_emails_spec.rb deleted file mode 100644 index 23a9c6eae45..00000000000 --- a/spec/jobs/enqueue_mailing_list_emails_spec.rb +++ /dev/null @@ -1,142 +0,0 @@ -require 'rails_helper' -require_dependency 'jobs/base' - -describe Jobs::EnqueueMailingListEmails do - - describe '#target_users' do - - context 'unapproved users' do - let!(:unapproved_user) { Fabricate(:active_user, approved: false, first_seen_at: 24.hours.ago) } - - before do - @original_value = SiteSetting.must_approve_users - SiteSetting.must_approve_users = true - end - - after do - SiteSetting.must_approve_users = @original_value - end - - it 'should enqueue the right emails' do - unapproved_user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0) - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(false) - - # As a moderator - unapproved_user.update_column(:moderator, true) - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(true) - - # As an admin - unapproved_user.update_attributes(admin: true, moderator: false) - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(true) - - # As an approved user - unapproved_user.update_attributes(admin: false, moderator: false, approved: true ) - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(true) - end - end - - context 'staged users' do - let!(:staged_user) { Fabricate(:active_user, staged: true, last_emailed_at: 1.year.ago, last_seen_at: 1.year.ago) } - - it "doesn't return staged users" do - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(staged_user.id)).to eq(false) - end - end - - context "inactive user" do - let!(:inactive_user) { Fabricate(:user, active: false) } - - it "doesn't return users who have been emailed recently" do - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(inactive_user.id)).to eq(false) - end - end - - context "suspended user" do - let!(:suspended_user) { Fabricate(:user, suspended_till: 1.week.from_now, suspended_at: 1.day.ago) } - - it "doesn't return users who are suspended" do - expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(suspended_user.id)).to eq(false) - end - end - - context 'users with mailing list mode on' do - let(:user) { Fabricate(:active_user, first_seen_at: 24.hours.ago) } - let(:user_option) { user.user_option } - subject { Jobs::EnqueueMailingListEmails.new.target_user_ids } - before do - user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0) - end - - it "returns a user whose first_seen_at matches the current hour" do - expect(subject).to include user.id - end - - it "returns a user seen multiple days ago" do - user.update(first_seen_at: 72.hours.ago) - expect(subject).to include user.id - end - - it "doesn't return a user who has never been seen" do - user.update(first_seen_at: nil) - expect(subject).to_not include user.id - end - - it "doesn't return users with mailing list mode off" do - user_option.update(mailing_list_mode: false) - expect(subject).to_not include user.id - end - - it "doesn't return users with mailing list mode set to 'individual'" do - user_option.update(mailing_list_mode_frequency: 1) - expect(subject).to_not include user.id - end - - it "doesn't return users with mailing list mode set to 'individual_excluding_own'" do - user_option.update(mailing_list_mode_frequency: 2) - expect(subject).to_not include user.id - end - - it "doesn't return a user who has received the mailing list summary earlier" do - user.update(first_seen_at: 5.hours.ago) - expect(subject).to_not include user.id - end - - it "doesn't return a user who was first seen today" do - user.update(first_seen_at: 2.minutes.ago) - expect(subject).to_not include user.id - end - end - - end - - describe '#execute' do - - let(:user) { Fabricate(:user) } - - context "mailing list emails are enabled" do - before do - Jobs::EnqueueMailingListEmails.any_instance.expects(:target_user_ids).returns([user.id]) - end - - it "enqueues the mailing list email job" do - Jobs.expects(:enqueue).with(:user_email, type: :mailing_list, user_id: user.id) - Jobs::EnqueueMailingListEmails.new.execute({}) - end - end - - context "mailing list emails are disabled" do - before do - Jobs::EnqueueMailingListEmails.any_instance.expects(:target_user_ids).never - end - - it "does not enqueue the mailing list email job" do - SiteSetting.disable_mailing_list_mode = true - Jobs.expects(:enqueue).with(:user_email, type: :mailing_list, user_id: user.id).never - Jobs::EnqueueMailingListEmails.new.execute({}) - end - end - - end - - -end diff --git a/spec/jobs/notify_mailing_list_subscribers_spec.rb b/spec/jobs/notify_mailing_list_subscribers_spec.rb index b4419e3059e..dc732b86a8c 100644 --- a/spec/jobs/notify_mailing_list_subscribers_spec.rb +++ b/spec/jobs/notify_mailing_list_subscribers_spec.rb @@ -79,11 +79,6 @@ describe Jobs::NotifyMailingListSubscribers do include_examples "no emails" end - context "to an user who has frequency set to 'daily'" do - before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 0) } - include_examples "no emails" - end - context "to an user who has frequency set to 'always'" do before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 1) } include_examples "one email" diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 43c8918bbf6..9b1ed8d9a79 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -79,94 +79,6 @@ describe UserNotifications do end - describe '.mailing_list' do - subject { UserNotifications.mailing_list(user) } - - context "without new posts" do - it "doesn't send the email" do - expect(subject.to).to be_blank - end - end - - context "with new posts" do - let(:user) { Fabricate(:user) } - let(:topic) { Fabricate(:topic, user: user) } - let!(:new_post) { Fabricate(:post, topic: topic, created_at: 2.hours.ago, raw: "Feel the Bern") } - let!(:old_post) { Fabricate(:post, topic: topic, created_at: 25.hours.ago, raw: "Make America Great Again") } - let(:old_topic) { Fabricate(:topic, user: user, created_at: 10.days.ago) } - let(:new_post_in_old_topic) { Fabricate(:post, topic: old_topic, created_at: 2.hours.ago, raw: "Yes We Can") } - let(:stale_post) { Fabricate(:post, topic: old_topic, created_at: 2.days.ago, raw: "A New American Century") } - - it "works" do - expect(subject.to).to eq([user.email]) - expect(subject.subject).to be_present - expect(subject.from).to eq([SiteSetting.notification_email]) - expect(subject.html_part.body.to_s).to include topic.title - expect(subject.text_part.body.to_s).to be_present - expect(subject.header["List-Unsubscribe"].to_s).to match(/\/email\/unsubscribe\/\h{64}/) - end - - it "includes posts less than 24 hours old" do - expect(subject.html_part.body.to_s).to include new_post.cooked - end - - it "does not include posts older than 24 hours old" do - expect(subject.html_part.body.to_s).to_not include old_post.cooked - end - - it "includes topics created over 24 hours ago which have new posts" do - new_post_in_old_topic - expect(subject.html_part.body.to_s).to include old_topic.title - expect(subject.html_part.body.to_s).to include new_post_in_old_topic.cooked - expect(subject.html_part.body.to_s).to_not include stale_post.cooked - end - - it "includes multiple topics" do - new_post_in_old_topic - expect(subject.html_part.body.to_s).to include topic.title - expect(subject.html_part.body.to_s).to include old_topic.title - end - - it "does not include topics not updated for the past 24 hours" do - stale_post - expect(subject.html_part.body.to_s).to_not include old_topic.title - expect(subject.html_part.body.to_s).to_not include stale_post.cooked - end - - it "includes email_prefix in email subject instead of site title" do - SiteSetting.email_prefix = "Try Discourse" - SiteSetting.title = "Discourse Meta" - - expect(subject.subject).to match(/Try Discourse/) - expect(subject.subject).not_to match(/Discourse Meta/) - end - - it "excludes whispers" do - new_post_in_old_topic - whisper = Fabricate(:post, topic: old_topic, created_at: 1.hour.ago, raw: "This is secret", post_type: Post.types[:whisper]) - expect(subject.html_part.body.to_s).to include old_topic.title - expect(subject.html_part.body.to_s).to_not include whisper.cooked - end - - it "includes whispers for staff" do - user.admin = true; user.save! - new_post_in_old_topic - whisper = Fabricate(:post, topic: old_topic, created_at: 1.hour.ago, raw: "This is secret", post_type: Post.types[:whisper]) - expect(subject.html_part.body.to_s).to include old_topic.title - expect(subject.html_part.body.to_s).to include whisper.cooked - end - - it "hides details for private email" do - SiteSetting.private_email = true - - expect(subject.html_part.body.to_s).not_to include(topic.title) - expect(subject.html_part.body.to_s).not_to include(topic.slug) - expect(subject.text_part.body.to_s).not_to include(topic.title) - expect(subject.text_part.body.to_s).not_to include(topic.slug) - end - end - end - describe '.digest' do subject { UserNotifications.digest(user) } diff --git a/spec/models/mailing_list_mode_site_setting_spec.rb b/spec/models/mailing_list_mode_site_setting_spec.rb index 498e5137fbc..88267fde4d5 100644 --- a/spec/models/mailing_list_mode_site_setting_spec.rb +++ b/spec/models/mailing_list_mode_site_setting_spec.rb @@ -3,15 +3,17 @@ require 'rails_helper' describe MailingListModeSiteSetting do describe 'valid_value?' do it 'returns true for a valid value as an int' do - expect(MailingListModeSiteSetting.valid_value?(0)).to eq(true) + expect(MailingListModeSiteSetting.valid_value?(1)).to eq(true) end it 'returns true for a valid value as a string' do - expect(MailingListModeSiteSetting.valid_value?('0')).to eq(true) + expect(MailingListModeSiteSetting.valid_value?('1')).to eq(true) end it 'returns false for an out of range value' do - expect(MailingListModeSiteSetting.valid_value?(3)).to eq(false) + [0, 3].each do |value| + expect(MailingListModeSiteSetting.valid_value?(value)).to eq(false) + end end end end