diff --git a/app/controllers/admin/system_emails_controller.rb b/app/controllers/admin/system_emails_controller.rb index 626680f43..7a20877d5 100644 --- a/app/controllers/admin/system_emails_controller.rb +++ b/app/controllers/admin/system_emails_controller.rb @@ -24,6 +24,19 @@ class Admin::SystemEmailsController < Admin::BaseController end end + def moderate_pending + ProposalNotification.find(params[:id]).moderate_system_email(current_user) + + redirect_to admin_system_email_preview_pending_path("proposal_notification_digest") + end + + def send_pending + Notification.send_pending + + flash[:notice] = t("admin.system_emails.preview_pending.send_pending_notification") + redirect_to admin_system_emails_path + end + private def load_system_email diff --git a/app/models/notification.rb b/app/models/notification.rb index dacedb762..f99cbc1ca 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -68,4 +68,29 @@ class Notification < ActiveRecord::Base end end + def self.send_pending + run_at = first_batch_run_at + User.email_digest.find_in_batches(batch_size: batch_size) do |users| + users.each do |user| + email_digest = EmailDigest.new(user) + email_digest.deliver(run_at) + end + run_at += batch_interval + end + end + + private + + def self.batch_size + 10000 + end + + def self.batch_interval + 20.minutes + end + + def self.first_batch_run_at + Time.current + end + end diff --git a/lib/email_digest.rb b/lib/email_digest.rb index 511d137a6..b20ab5a6d 100644 --- a/lib/email_digest.rb +++ b/lib/email_digest.rb @@ -14,9 +14,11 @@ class EmailDigest notifications.any? end - def deliver - if pending_notifications? - Mailer.proposal_notification_digest(user, notifications.to_a).deliver_later + + def deliver(run_at) + if valid_email? && pending_notifications? + Mailer.delay(run_at: run_at).proposal_notification_digest(user, notifications.to_a) + mark_as_emailed end end @@ -25,4 +27,8 @@ class EmailDigest user.update(failed_email_digests_count: 0) end + def valid_email? + user.email.match(/\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i) + end + end diff --git a/spec/features/emails_spec.rb b/spec/features/emails_spec.rb index 91f8b31b6..bbb92230f 100644 --- a/spec/features/emails_spec.rb +++ b/spec/features/emails_spec.rb @@ -294,7 +294,7 @@ feature 'Emails' do notification3 = create_proposal_notification(proposal3) email_digest = EmailDigest.new(user) - email_digest.deliver + email_digest.deliver(Time.current) email_digest.mark_as_emailed email = open_last_email @@ -327,6 +327,26 @@ feature 'Emails' do expect(notification2.emailed_at).to be end + scenario "notifications moderated are not sent" do + user = create(:user, email_digest: true) + proposal = create(:proposal) + proposal_notification = create(:proposal_notification, proposal: proposal) + notification = create(:notification, notifiable: proposal_notification) + + reset_mailer + + proposal_notification.moderate_system_email(create(:administrator).user) + + email_digest = EmailDigest.new(user) + email_digest.deliver(Time.current) + email_digest.mark_as_emailed + + expect { open_last_email }.to raise_error "No email has been sent!" + end + + xscenario "Delete all Notifications included in the digest after email sent" do + end + end context "User invites" do diff --git a/spec/features/notifications_spec.rb b/spec/features/notifications_spec.rb index 482d38b5a..807a644e6 100644 --- a/spec/features/notifications_spec.rb +++ b/spec/features/notifications_spec.rb @@ -182,4 +182,67 @@ feature "Notifications" do end end + describe "#send_pending" do + let!(:user1) { create(:user) } + let!(:user2) { create(:user) } + let!(:user3) { create(:user) } + let!(:proposal_notification) { create(:proposal_notification) } + let!(:notification1) { create(:notification, notifiable: proposal_notification, user: user1) } + let!(:notification2) { create(:notification, notifiable: proposal_notification, user: user2) } + let!(:notification3) { create(:notification, notifiable: proposal_notification, user: user3) } + + before do + reset_mailer + Delayed::Worker.delay_jobs = true + end + + after do + Delayed::Worker.delay_jobs = false + end + + it "sends pending proposal notifications" do + Delayed::Worker.delay_jobs = false + Notification.send_pending + + email = open_last_email + expect(email).to have_subject("Proposal notifications in Decide Madrid") + end + + it "sends emails in batches" do + Notification.send_pending + + expect(Delayed::Job.count).to eq(3) + end + + it "sends batches in time intervals" do + allow(Notification).to receive(:batch_size).and_return(1) + allow(Notification).to receive(:batch_interval).and_return(1.second) + allow(Notification).to receive(:first_batch_run_at).and_return(Time.current) + + remove_users_without_pending_notifications + + Notification.send_pending + + now = Notification.first_batch_run_at + first_batch_run_at = now.change(usec: 0) + second_batch_run_at = (now + 1.second).change(usec: 0) + third_batch_run_at = (now + 2.seconds).change(usec: 0) + + expect(Delayed::Job.count).to eq(3) + expect(Delayed::Job.first.run_at.change(usec: 0)).to eq(first_batch_run_at) + expect(Delayed::Job.second.run_at.change(usec: 0)).to eq(second_batch_run_at) + expect(Delayed::Job.third.run_at.change(usec: 0)).to eq(third_batch_run_at) + end + + end + + def remove_users_without_pending_notifications + users_without_notifications.each { |user| user.destroy } + end + + def users_without_notifications + User.all.select { |user| user.notifications.not_emailed + .where(notifiable_type: 'ProposalNotification').blank? } + end + end diff --git a/spec/lib/email_digests_spec.rb b/spec/lib/email_digests_spec.rb index 7992c3706..2622b76e7 100644 --- a/spec/lib/email_digests_spec.rb +++ b/spec/lib/email_digests_spec.rb @@ -75,7 +75,7 @@ describe EmailDigest do reset_mailer email_digest = described_class.new(user) - email_digest.deliver + email_digest.deliver(Time.current) email = open_last_email expect(email).to have_subject("Proposal notifications in CONSUL") @@ -89,7 +89,7 @@ describe EmailDigest do reset_mailer email_digest = described_class.new(user) - email_digest.deliver + email_digest.deliver(Time.current) expect(all_emails.count).to eq(0) end @@ -137,4 +137,21 @@ describe EmailDigest do end + describe "#valid_email?" do + + it "returns a MatchData if email is valid" do + user = create(:user, email: 'valid_email@email.com') + + email_digest = described_class.new(user) + expect(email_digest.valid_email?).to be_a(MatchData) + end + + it "returns nil if email is invalid" do + user = create(:user, email: 'invalid_email@email..com') + + email_digest = described_class.new(user) + expect(email_digest.valid_email?).to be(nil) + end + end + end