- <%= link_to preview.title, proposal_url(preview.proposal, anchor: "tab-notifications") %>
+
-
-
<%= t("admin.shared.content") %>
-
- <%= preview.body %>
-
+
+
+
<%= t("admin.shared.content") %>
+
+ <%= preview.body %>
+
+
+
+ <%= link_to t("admin.system_emails.preview_pending.moderate_pending"),
+ admin_system_email_moderate_pending_path(system_email_id: 'proposal_notification_digest',
+ id: preview.id),
+ method: :put,
+ class: "button hollow float-right" %>
+
<% end %>
diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml
index 7871b377b..259ea7c44 100644
--- a/config/locales/en/admin.yml
+++ b/config/locales/en/admin.yml
@@ -67,6 +67,7 @@ en:
on_debates: Debates
on_proposals: Proposals
on_users: Users
+ on_system_emails: System emails
title: Moderator activity
type: Type
no_activity: There are no moderators activity.
@@ -655,6 +656,9 @@ en:
action: Preview Pending
preview_of: Preview of %{name}
pending_to_be_sent: This is the content pending to be sent
+ moderate_pending: Moderate notification send
+ send_pending: Send pending
+ send_pending_notification: Pending notifications sent succesfully
proposal_notification_digest:
title: Proposal Notification Digest
description: Gathers all proposal notifications for an user in a single message, to avoid too much emails.
diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml
index c6761b568..00cd907a3 100644
--- a/config/locales/es/admin.yml
+++ b/config/locales/es/admin.yml
@@ -67,6 +67,7 @@ es:
on_debates: Debates
on_proposals: Propuestas
on_users: Usuarios
+ on_system_emails: Emails del sistema
title: Actividad de los Moderadores
type: Tipo
no_activity: No hay actividad de moderadores.
@@ -656,6 +657,9 @@ es:
action: Previsualizar Pendientes
preview_of: Vista previa de %{name}
pending_to_be_sent: Este es todo el contenido pendiente de enviar
+ moderate_pending: Moderar envío de notificación
+ send_pending : Enviar pendientes
+ send_pending_notification: Notificaciones pendientes enviadas correctamente
proposal_notification_digest:
title: Resumen de Notificationes de Propuestas
description: Reune todas las notificaciones de propuestas en un único mensaje, para evitar demasiados emails.
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index 150d1b28a..31138e48c 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -168,6 +168,8 @@ namespace :admin do
resources :system_emails, only: [:index] do
get :view
get :preview_pending
+ put :moderate_pending
+ put :send_pending
end
resources :emails_download, only: :index do
diff --git a/db/migrate/20180517141120_add_hidden_at_to_newsletters.rb b/db/migrate/20180517141120_add_hidden_at_to_newsletters.rb
new file mode 100644
index 000000000..5e1b4b8cf
--- /dev/null
+++ b/db/migrate/20180517141120_add_hidden_at_to_newsletters.rb
@@ -0,0 +1,5 @@
+class AddHiddenAtToNewsletters < ActiveRecord::Migration
+ def change
+ add_column :newsletters, :hidden_at, :datetime
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 878c16869..142990d93 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -682,6 +682,7 @@ ActiveRecord::Schema.define(version: 20180711224810) do
t.date "sent_at"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
+ t.datetime "hidden_at"
end
create_table "notifications", force: :cascade do |t|
diff --git a/lib/email_digest.rb b/lib/email_digest.rb
index 511d137a6..502c68f41 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.present? && user.email.match(/\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i)
+ end
+
end
diff --git a/lib/tasks/emails.rake b/lib/tasks/emails.rake
deleted file mode 100644
index 3afa81449..000000000
--- a/lib/tasks/emails.rake
+++ /dev/null
@@ -1,17 +0,0 @@
-namespace :emails do
-
- desc "Sends email digest of proposal notifications to each user"
- task digest: :environment do
- User.email_digest.find_each do |user|
- email_digest = EmailDigest.new(user)
- begin
- email_digest.deliver
- email_digest.mark_as_emailed
- rescue
- user.increment_counter(:failed_email_digests_count)
- user.save
- end
- end
- end
-
-end
diff --git a/spec/features/admin/activity_spec.rb b/spec/features/admin/activity_spec.rb
index 5529cb464..9cfd5f9b8 100644
--- a/spec/features/admin/activity_spec.rb
+++ b/spec/features/admin/activity_spec.rb
@@ -331,4 +331,22 @@ feature 'Admin activity' do
end
end
+ context "System emails" do
+ scenario "Shows moderation activity on system emails" do
+ proposal = create(:proposal, title: 'Proposal A')
+ proposal_notification = create(:proposal_notification, proposal: proposal,
+ title: 'Proposal A Title',
+ body: 'Proposal A Notification Body')
+ proposal_notification.moderate_system_email(@admin.user)
+
+ visit admin_activity_path
+
+ within("#activity_#{Activity.last.id}") do
+ expect(page).to have_content(proposal_notification.title)
+ expect(page).to have_content("Hidden")
+ expect(page).to have_content(@admin.user.username)
+ end
+ end
+ end
+
end
diff --git a/spec/features/admin/system_emails_spec.rb b/spec/features/admin/system_emails_spec.rb
index dd14e87bc..239e6183a 100644
--- a/spec/features/admin/system_emails_spec.rb
+++ b/spec/features/admin/system_emails_spec.rb
@@ -65,6 +65,49 @@ feature "System Emails" do
expect(page).to have_link('Proposal B Title', href: proposal_url(proposal_b,
anchor: 'tab-notifications'))
end
+
+ scenario "#moderate_pending" do
+ proposal1 = create(:proposal, title: 'Proposal A')
+ proposal2 = create(:proposal, title: 'Proposal B')
+ proposal_notification1 = create(:proposal_notification, proposal: proposal1,
+ title: 'Proposal A Title',
+ body: 'Proposal A Notification Body')
+ proposal_notification2 = create(:proposal_notification, proposal: proposal2)
+ create(:notification, notifiable: proposal_notification1, emailed_at: nil)
+ create(:notification, notifiable: proposal_notification2, emailed_at: nil)
+
+ visit admin_system_email_preview_pending_path('proposal_notification_digest')
+
+ within("#proposal_notification_#{proposal_notification1.id}") do
+ click_on "Moderate notification send"
+ end
+
+ visit admin_system_email_preview_pending_path('proposal_notification_digest')
+
+ expect(Notification.count).to equal(1)
+ expect(Activity.last.actionable_type).to eq('ProposalNotification')
+ expect(page).not_to have_content("Proposal A Title")
+ end
+
+ scenario "#send_pending" do
+ proposal = create(:proposal)
+ proposal_notification = create(:proposal_notification, proposal: proposal,
+ title: 'Proposal A Title',
+ body: 'Proposal A Notification Body')
+ voter = create(:user, :level_two)
+ create(:notification, notifiable: proposal_notification, user: voter, emailed_at: nil)
+ create(:follow, user: voter, followable: proposal)
+
+ visit admin_system_emails_path
+
+ click_on "Send pending"
+
+ email = open_last_email
+ expect(email).to deliver_to(voter)
+ expect(email).to have_body_text(proposal_notification.body)
+
+ expect(page).to have_content("Pending notifications sent succesfully")
+ end
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..f596375b9 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 CONSUL")
+ 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..c5477610d 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,29 @@ 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
+
+ it "returns false if email does not exist" do
+ user = create(:user)
+ user.update_attribute(:email, nil)
+
+ email_digest = described_class.new(user)
+ expect(email_digest.valid_email?).to be(false)
+ end
+ end
+
end
diff --git a/spec/models/activity_spec.rb b/spec/models/activity_spec.rb
index 902320ec6..d048a6bb8 100644
--- a/spec/models/activity_spec.rb
+++ b/spec/models/activity_spec.rb
@@ -7,12 +7,14 @@ describe Activity do
expect(build(:activity, actionable: create(:debate))).to be_valid
expect(build(:activity, actionable: create(:comment))).to be_valid
expect(build(:activity, actionable: create(:user))).to be_valid
+ expect(build(:activity, actionable: create(:newsletter))).to be_valid
end
it "is a valid only with allowed actions" do
expect(build(:activity, action: "hide")).to be_valid
expect(build(:activity, action: "block")).to be_valid
expect(build(:activity, action: "restore")).to be_valid
+ expect(build(:activity, action: "email")).to be_valid
expect(build(:activity, action: "dissapear")).not_to be_valid
end
diff --git a/spec/models/newsletter_spec.rb b/spec/models/newsletter_spec.rb
index bd42e9177..61abb3daf 100644
--- a/spec/models/newsletter_spec.rb
+++ b/spec/models/newsletter_spec.rb
@@ -63,4 +63,95 @@ describe Newsletter do
expect(newsletter.list_of_recipient_emails).not_to include('erased_user@consul.dev')
end
end
+
+ describe "#deliver" do
+ let!(:proposals) { Array.new(3) { create(:proposal) } }
+ let!(:debate) { create(:debate) }
+
+ let!(:recipients) { proposals.map(&:author).map(&:email) }
+ let!(:newsletter) { create(:newsletter, segment_recipient: "proposal_authors") }
+
+ before do
+ reset_mailer
+ Delayed::Worker.delay_jobs = true
+ end
+
+ after do
+ Delayed::Worker.delay_jobs = false
+ end
+
+ it "sends an email with the newsletter to every recipient" do
+ newsletter.deliver
+
+ recipients.each do |recipient|
+ email = Mailer.newsletter(newsletter, recipient)
+ expect(email).to deliver_to(recipient)
+ end
+
+ Delayed::Job.all.map(&:invoke_job)
+ expect(ActionMailer::Base.deliveries.count).to eq(3)
+ end
+
+ it "sends emails in batches" do
+ allow(newsletter).to receive(:batch_size).and_return(1)
+
+ newsletter.deliver
+
+ expect(Delayed::Job.count).to eq(3)
+ end
+
+ it "sends batches in time intervals" do
+ allow(newsletter).to receive(:batch_size).and_return(1)
+ allow(newsletter).to receive(:batch_interval).and_return(1.second)
+ allow(newsletter).to receive(:first_batch_run_at).and_return(Time.current)
+
+ newsletter.deliver
+
+ now = newsletter.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
+
+ it "logs users that have received the newsletter" do
+ newsletter.deliver
+
+ expect(Activity.count).to eq(3)
+
+ recipients.each do |email|
+ user = User.where(email: email).first
+ activity = Activity.where(user: user).first
+
+ expect(activity.user_id).to eq(user.id)
+ expect(activity.action).to eq("email")
+ expect(activity.actionable).to eq(newsletter)
+ end
+ end
+
+ it "skips invalid emails" do
+ Proposal.destroy_all
+
+ valid_email = "john@gmail.com"
+ invalid_email = "john@gmail..com"
+
+ valid_email_user = create(:user, email: valid_email)
+ proposal = create(:proposal, author: valid_email_user)
+
+ invalid_email_user = create(:user, email: invalid_email)
+ proposal = create(:proposal, author: invalid_email_user)
+
+ newsletter.deliver
+
+ expect(Activity.count).to eq(1)
+ expect(Activity.first.user_id).to eq(valid_email_user.id)
+ expect(Activity.first.action).to eq("email")
+ expect(Activity.first.actionable).to eq(newsletter)
+ end
+
+ end
end
diff --git a/spec/models/proposal_notification_spec.rb b/spec/models/proposal_notification_spec.rb
index 60021d95c..d7ba529c4 100644
--- a/spec/models/proposal_notification_spec.rb
+++ b/spec/models/proposal_notification_spec.rb
@@ -152,5 +152,21 @@ describe ProposalNotification do
end
+ describe "#moderate_system_email" do
+ let(:admin) { create(:administrator) }
+ let(:proposal) { create(:proposal) }
+ let(:proposal_notification) { build(:proposal_notification, proposal: proposal) }
+ let(:notification) { create(:notification, notifiable: proposal_notification) }
+
+ it "removes all notifications related to the proposal notification" do
+ proposal_notification.moderate_system_email(admin.user)
+ expect(Notification.all.count).to be(0)
+ end
+
+ it "records the moderation action in the Activity table" do
+ proposal_notification.moderate_system_email(admin.user)
+ expect(Activity.last.actionable_type).to eq('ProposalNotification')
+ end
+ end
end
end