Add UserSegments#user_segment_emails helper method
Why: Both Newsletters and Email Downloads need the same logic: To extract the emails from all the users in the segment that have newsletter flag active, removing all empty email values. How: 1- UserSegments#user_segment_emails holds that repeated logic and is used on both Newsletter & EmailDownload. 2- Rename Newsletter#list_of_recipients to list_of_recipient_emails as it is more descriptive. There is no need to pass entire Users around, only the emails are needed at Mailer#newsletter method. 3- Cleanup Newsletter#list_of_recipient_emails model spec scenario
This commit is contained in:
@@ -13,6 +13,6 @@ class Admin::EmailsDownloadController < Admin::BaseController
|
|||||||
private
|
private
|
||||||
|
|
||||||
def users_segment_emails_csv(users_segment)
|
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
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -48,8 +48,8 @@ class Admin::NewslettersController < Admin::BaseController
|
|||||||
@newsletter = Newsletter.find(params[:id])
|
@newsletter = Newsletter.find(params[:id])
|
||||||
|
|
||||||
if @newsletter.valid?
|
if @newsletter.valid?
|
||||||
@newsletter.list_of_recipients.each do |recipient|
|
@newsletter.list_of_recipient_emails.each do |recipient_email|
|
||||||
Mailer.newsletter(@newsletter, recipient).deliver_later
|
Mailer.newsletter(@newsletter, recipient_email).deliver_later
|
||||||
end
|
end
|
||||||
|
|
||||||
@newsletter.update(sent_at: Time.current)
|
@newsletter.update(sent_at: Time.current)
|
||||||
|
|||||||
@@ -126,9 +126,9 @@ class Mailer < ApplicationMailer
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def newsletter(newsletter, recipient)
|
def newsletter(newsletter, recipient_email)
|
||||||
@newsletter = newsletter
|
@newsletter = newsletter
|
||||||
@email_to = recipient.email
|
@email_to = recipient_email
|
||||||
|
|
||||||
mail(to: @email_to, from: @newsletter.from, subject: @newsletter.subject)
|
mail(to: @email_to, from: @newsletter.from, subject: @newsletter.subject)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -8,8 +8,8 @@ class Newsletter < ActiveRecord::Base
|
|||||||
|
|
||||||
validates_format_of :from, :with => /@/
|
validates_format_of :from, :with => /@/
|
||||||
|
|
||||||
def list_of_recipients
|
def list_of_recipient_emails
|
||||||
UserSegments.send(segment_recipient).newsletter.select {|user| !user.email.nil? } if valid_segment_recipient?
|
UserSegments.user_segment_emails(segment_recipient) if valid_segment_recipient?
|
||||||
end
|
end
|
||||||
|
|
||||||
def valid_segment_recipient?
|
def valid_segment_recipient?
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
<h2><%= t("admin.newsletters.show.title") %></h2>
|
<h2><%= t("admin.newsletters.show.title") %></h2>
|
||||||
|
|
||||||
<% 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 %>
|
||||||
|
|
||||||
<div class="small-12 column">
|
<div class="small-12 column">
|
||||||
<div class="callout highlight">
|
<div class="callout highlight">
|
||||||
|
|||||||
@@ -37,6 +37,10 @@ class UserSegments
|
|||||||
author_ids(current_budget_investments.winners.pluck(:author_id).uniq)
|
author_ids(current_budget_investments.winners.pluck(:author_id).uniq)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def self.user_segment_emails(users_segment)
|
||||||
|
UserSegments.send(users_segment).newsletter.pluck(:email).compact
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def self.current_budget_investments
|
def self.current_budget_investments
|
||||||
|
|||||||
@@ -133,7 +133,7 @@ feature "Admin newsletter emails" do
|
|||||||
|
|
||||||
click_link "Send"
|
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?")
|
page.accept_confirm("Are you sure you want to send this newsletter to #{total_users} users?")
|
||||||
|
|
||||||
expect(page).to have_content "Newsletter sent successfully"
|
expect(page).to have_content "Newsletter sent successfully"
|
||||||
|
|||||||
@@ -47,21 +47,20 @@ describe Newsletter do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#list_of_recipients' do
|
describe '#list_of_recipient_emails' do
|
||||||
|
|
||||||
before do
|
before do
|
||||||
create(:user, newsletter: true, email: 'newsletter_user@consul.dev')
|
create(:user, newsletter: true, email: 'newsletter_user@consul.dev')
|
||||||
no_news_user = create(:user, newsletter: false, email: 'no_news_user@consul.dev')
|
create(:user, newsletter: false, email: 'no_news_user@consul.dev')
|
||||||
erased_user = create(:user, email: 'erased_user@consul.dev')
|
create(:user, email: 'erased_user@consul.dev').erase
|
||||||
erased_user.erase
|
|
||||||
newsletter.update(segment_recipient: 'all_users')
|
newsletter.update(segment_recipient: 'all_users')
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns list of recipients excluding users with disabled newsletter' do
|
it 'returns list of recipients excluding users with disabled newsletter' do
|
||||||
expect(newsletter.list_of_recipients.count).to eq(1)
|
expect(newsletter.list_of_recipient_emails.count).to eq(1)
|
||||||
expect(newsletter.list_of_recipients.map(&:email)).to include('newsletter_user@consul.dev')
|
expect(newsletter.list_of_recipient_emails).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_recipient_emails).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).not_to include('erased_user@consul.dev')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user