diff --git a/app/controllers/admin/emails_download_controller.rb b/app/controllers/admin/emails_download_controller.rb index a1725677d..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).compact.join(',') + 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 75c4c0af9..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.select {|user| !user.email.nil? } 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/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 e7e56ee06..bd42e9177 100644 --- a/spec/models/newsletter_spec.rb +++ b/spec/models/newsletter_spec.rb @@ -47,21 +47,20 @@ describe Newsletter do end end - describe '#list_of_recipients' do + describe '#list_of_recipient_emails' do before do create(:user, newsletter: true, email: 'newsletter_user@consul.dev') - no_news_user = create(:user, newsletter: false, email: 'no_news_user@consul.dev') - erased_user = create(:user, email: 'erased_user@consul.dev') - erased_user.erase + 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.map(&:email)).to include('newsletter_user@consul.dev') - expect(newsletter.list_of_recipients.map(&:email)).not_to include('no_news_user@consul.dev') - expect(newsletter.list_of_recipients.map(&:email)).not_to include('erased_user@consul.dev') + 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