Move newsletter User scope outside UserSegments

Why:

UserSegments are not only used for Newsletters or Email downloads, but
also for internal Global Notifications. We don't want to have that scope
hardcoded inside UserSegments as users that have opted-out from the
newsletter should still be recipients of global notifications.

How:

Removing the scope from the UserSegments `all_users` method that acts as
base for all the other segments. Including that `newsletter` scope only
on the places that is relevant:
* When listing recipients for a newsletter
* When downloading a listing emails that can be newsletter recipients

Also updated relevant tests
This commit is contained in:
Bertocq
2018-02-20 18:57:20 +01:00
parent eb4a446a9a
commit bdbb32e824
5 changed files with 24 additions and 10 deletions

View File

@@ -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).pluck(:email).to_csv UserSegments.send(users_segment).newsletter.pluck(:email).to_csv
end end
end end

View File

@@ -14,7 +14,7 @@ class Newsletter < ActiveRecord::Base
validates_format_of :from, :with => /@/ validates_format_of :from, :with => /@/
def list_of_recipients def list_of_recipients
UserSegments.send(segment_recipient) UserSegments.send(segment_recipient).newsletter
end end
def draft? def draft?

View File

@@ -7,7 +7,7 @@ class UserSegments
winner_investment_authors) winner_investment_authors)
def self.all_users def self.all_users
User.newsletter.active User.active
end end
def self.proposal_authors def self.proposal_authors

View File

@@ -6,15 +6,11 @@ describe UserSegments do
let(:user3) { create(:user) } let(:user3) { create(:user) }
describe "#all_users" do describe "#all_users" do
it "returns all active users with newsletter enabled" do it "returns all active users enabled" do
active_user1 = create(:user, newsletter: true) active_user = create(:user)
active_user2 = create(:user, newsletter: true)
active_user3 = create(:user, newsletter: false)
erased_user = create(:user, erased_at: Time.current) erased_user = create(:user, erased_at: Time.current)
expect(described_class.all_users).to include active_user1 expect(described_class.all_users).to include active_user
expect(described_class.all_users).to include active_user2
expect(described_class.all_users).not_to include active_user3
expect(described_class.all_users).not_to include erased_user expect(described_class.all_users).not_to include erased_user
end end
end end

View File

@@ -17,6 +17,11 @@ describe Newsletter do
expect(newsletter).not_to be_valid expect(newsletter).not_to be_valid
end end
it 'is not valid with an inexistent user segment for segment_recipient' do
newsletter.segment_recipient = 'invalid_user_segment_name'
expect(newsletter).not_to be_valid
end
it 'is not valid without a from' do it 'is not valid without a from' do
newsletter.from = nil newsletter.from = nil
expect(newsletter).not_to be_valid expect(newsletter).not_to be_valid
@@ -31,4 +36,17 @@ describe Newsletter do
newsletter.from = "this_is_not_an_email" newsletter.from = "this_is_not_an_email"
expect(newsletter).not_to be_valid expect(newsletter).not_to be_valid
end end
describe '#list_of_recipients' do
before do
create(:user, newsletter: true, username: 'newsletter_user')
create(:user, newsletter: false)
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')
end
end
end end