diff --git a/app/controllers/admin/emails_download_controller.rb b/app/controllers/admin/emails_download_controller.rb
index 179240358..2d6c2f2ef 100644
--- a/app/controllers/admin/emails_download_controller.rb
+++ b/app/controllers/admin/emails_download_controller.rb
@@ -13,6 +13,6 @@ class Admin::EmailsDownloadController < Admin::BaseController
private
def users_segment_emails_csv(users_segment)
- UserSegments.send(users_segment).newsletter.pluck(:email).to_csv
+ UserSegments.user_segment_emails(users_segment).join(',')
end
end
diff --git a/app/controllers/admin/newsletters_controller.rb b/app/controllers/admin/newsletters_controller.rb
index 12522927c..966556082 100644
--- a/app/controllers/admin/newsletters_controller.rb
+++ b/app/controllers/admin/newsletters_controller.rb
@@ -48,8 +48,8 @@ class Admin::NewslettersController < Admin::BaseController
@newsletter = Newsletter.find(params[:id])
if @newsletter.valid?
- @newsletter.list_of_recipients.each do |recipient|
- Mailer.newsletter(@newsletter, recipient).deliver_later
+ @newsletter.list_of_recipient_emails.each do |recipient_email|
+ Mailer.newsletter(@newsletter, recipient_email).deliver_later
end
@newsletter.update(sent_at: Time.current)
diff --git a/app/mailers/mailer.rb b/app/mailers/mailer.rb
index a46e2866a..2cdb494c3 100644
--- a/app/mailers/mailer.rb
+++ b/app/mailers/mailer.rb
@@ -126,9 +126,9 @@ class Mailer < ApplicationMailer
end
end
- def newsletter(newsletter, recipient)
+ def newsletter(newsletter, recipient_email)
@newsletter = newsletter
- @email_to = recipient.email
+ @email_to = recipient_email
mail(to: @email_to, from: @newsletter.from, subject: @newsletter.subject)
end
diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb
index 4ac6d59af..8c3a83cfa 100644
--- a/app/models/newsletter.rb
+++ b/app/models/newsletter.rb
@@ -8,8 +8,8 @@ class Newsletter < ActiveRecord::Base
validates_format_of :from, :with => /@/
- def list_of_recipients
- UserSegments.send(segment_recipient).newsletter if valid_segment_recipient?
+ def list_of_recipient_emails
+ UserSegments.user_segment_emails(segment_recipient) if valid_segment_recipient?
end
def valid_segment_recipient?
diff --git a/app/views/admin/newsletters/show.html.erb b/app/views/admin/newsletters/show.html.erb
index 35f6d86b8..55cc6aabe 100644
--- a/app/views/admin/newsletters/show.html.erb
+++ b/app/views/admin/newsletters/show.html.erb
@@ -2,7 +2,7 @@
<%= t("admin.newsletters.show.title") %>
-<% recipients_count = @newsletter.valid_segment_recipient? ? @newsletter.list_of_recipients.count : 0 %>
+<% recipients_count = @newsletter.valid_segment_recipient? ? @newsletter.list_of_recipient_emails.count : 0 %>
diff --git a/lib/user_segments.rb b/lib/user_segments.rb
index 1505e08e7..548775c39 100644
--- a/lib/user_segments.rb
+++ b/lib/user_segments.rb
@@ -37,6 +37,10 @@ class UserSegments
author_ids(current_budget_investments.winners.pluck(:author_id).uniq)
end
+ def self.user_segment_emails(users_segment)
+ UserSegments.send(users_segment).newsletter.pluck(:email).compact
+ end
+
private
def self.current_budget_investments
diff --git a/spec/features/admin/emails/emails_download_spec.rb b/spec/features/admin/emails/emails_download_spec.rb
index ec3908307..8a92337b0 100644
--- a/spec/features/admin/emails/emails_download_spec.rb
+++ b/spec/features/admin/emails/emails_download_spec.rb
@@ -2,29 +2,48 @@ require 'rails_helper'
feature "Admin download user emails" do
+ let(:admin_user) { create(:user, newsletter: false, email: 'admin@consul.dev') }
+
background do
- admin = create(:administrator)
- login_as(admin.user)
+ create(:administrator, user: admin_user)
+ login_as(admin_user)
end
- context "Index" do
- scenario "returns the selected users segment csv file" do
- user1 = create(:user)
- user2 = create(:user)
+ context "Download only emails from segment users with newsletter flag & present email " do
+ before do
+ create(:user, email: 'user@consul.dev')
+
+ create(:administrator, user: create(:user, newsletter: true, email: 'admin_news1@consul.dev'))
+ create(:administrator, user: create(:user, newsletter: true, email: 'admin_news2@consul.dev'))
+
+ create(:administrator, user: create(:user, newsletter: false, email: 'no_news@consul.dev'))
+
+ admin_without_email = create(:user, newsletter: true, email: 'no_email@consul.dev')
+ create(:administrator, user: admin_without_email)
+ admin_without_email.update_attribute(:email, nil)
+ end
+
+ scenario "returns the selected users segment csv file" do
visit admin_emails_download_index_path
within('#admin_download_emails') do
- select 'All users', from: 'users_segment'
+ select 'Administrators', from: 'users_segment'
click_button 'Download emails list'
end
header = page.response_headers['Content-Disposition']
expect(header).to match /^attachment/
- expect(header).to match /filename="All users.csv"$/
+ expect(header).to match /filename="Administrators.csv"$/
- expect(page).to have_content(user1.email)
- expect(page).to have_content(user2.email)
+ file_contents = page.body.split(',')
+ expect(file_contents.count).to eq(2)
+ expect(file_contents).to include('admin_news1@consul.dev')
+ expect(file_contents).to include('admin_news2@consul.dev')
+ expect(file_contents).not_to include('admin@consul.dev')
+ expect(file_contents).not_to include('user@consul.dev')
+ expect(file_contents).not_to include('no_news@consul.dev')
+ expect(file_contents).not_to include('no_email@consul.dev')
end
end
end
diff --git a/spec/features/admin/emails/newsletters_spec.rb b/spec/features/admin/emails/newsletters_spec.rb
index b953c140a..1f12d05ee 100644
--- a/spec/features/admin/emails/newsletters_spec.rb
+++ b/spec/features/admin/emails/newsletters_spec.rb
@@ -133,7 +133,7 @@ feature "Admin newsletter emails" do
click_link "Send"
- total_users = newsletter.list_of_recipients.count
+ total_users = newsletter.list_of_recipient_emails.count
page.accept_confirm("Are you sure you want to send this newsletter to #{total_users} users?")
expect(page).to have_content "Newsletter sent successfully"
diff --git a/spec/models/newsletter_spec.rb b/spec/models/newsletter_spec.rb
index 74e0d855c..bd42e9177 100644
--- a/spec/models/newsletter_spec.rb
+++ b/spec/models/newsletter_spec.rb
@@ -47,20 +47,20 @@ describe Newsletter do
end
end
- describe '#list_of_recipients' do
- let(:erased_user) { create(:user, username: 'erased_user') }
+ describe '#list_of_recipient_emails' do
before do
- create(:user, newsletter: true, username: 'newsletter_user')
- create(:user, newsletter: false)
- erased_user.erase
+ create(:user, newsletter: true, email: 'newsletter_user@consul.dev')
+ create(:user, newsletter: false, email: 'no_news_user@consul.dev')
+ create(:user, email: 'erased_user@consul.dev').erase
newsletter.update(segment_recipient: 'all_users')
end
it 'returns list of recipients excluding users with disabled newsletter' do
- expect(newsletter.list_of_recipients.count).to eq(1)
- expect(newsletter.list_of_recipients.first.username).to eq('newsletter_user')
- expect(newsletter.list_of_recipients).not_to include(erased_user)
+ expect(newsletter.list_of_recipient_emails.count).to eq(1)
+ expect(newsletter.list_of_recipient_emails).to include('newsletter_user@consul.dev')
+ expect(newsletter.list_of_recipient_emails).not_to include('no_news_user@consul.dev')
+ expect(newsletter.list_of_recipient_emails).not_to include('erased_user@consul.dev')
end
end
end