diff --git a/app/controllers/admin/emails_download_controller.rb b/app/controllers/admin/emails_download_controller.rb index 3bafa7668..b05ad83b7 100644 --- a/app/controllers/admin/emails_download_controller.rb +++ b/app/controllers/admin/emails_download_controller.rb @@ -4,7 +4,7 @@ class Admin::EmailsDownloadController < Admin::BaseController def generate_csv users_segment = params[:users_segment] - filename = t("admin.segment_recipient.#{users_segment}") + filename = UserSegments.segment_name(users_segment) csv_file = users_segment_emails_csv(users_segment) send_data csv_file, filename: "#{filename}.csv" diff --git a/app/helpers/user_segments_helper.rb b/app/helpers/user_segments_helper.rb index 72925c6b1..3f4d5e2e1 100644 --- a/app/helpers/user_segments_helper.rb +++ b/app/helpers/user_segments_helper.rb @@ -1,15 +1,11 @@ module UserSegmentsHelper def user_segments_options - UserSegments::SEGMENTS.map do |user_segment_name| - [t("admin.segment_recipient.#{user_segment_name}"), user_segment_name] + UserSegments.segments.map do |user_segment_name| + [segment_name(user_segment_name), user_segment_name] end end def segment_name(user_segment) - if user_segment && UserSegments.respond_to?(user_segment) - I18n.t("admin.segment_recipient.#{user_segment}") - else - I18n.t("admin.segment_recipient.invalid_recipients_segment") - end + UserSegments.segment_name(user_segment) || I18n.t("admin.segment_recipient.invalid_recipients_segment") end end diff --git a/app/models/admin_notification.rb b/app/models/admin_notification.rb index 52c741c93..7b5e3b74e 100644 --- a/app/models/admin_notification.rb +++ b/app/models/admin_notification.rb @@ -13,11 +13,11 @@ class AdminNotification < ApplicationRecord before_validation :complete_link_url def list_of_recipients - UserSegments.send(segment_recipient) if valid_segment_recipient? + UserSegments.recipients(segment_recipient) if valid_segment_recipient? end def valid_segment_recipient? - segment_recipient && UserSegments.respond_to?(segment_recipient) + UserSegments.valid_segment?(segment_recipient) end def draft? diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb index 7e8955931..d4095be1e 100644 --- a/app/models/newsletter.rb +++ b/app/models/newsletter.rb @@ -15,7 +15,7 @@ class Newsletter < ApplicationRecord end def valid_segment_recipient? - segment_recipient && UserSegments.respond_to?(segment_recipient) + UserSegments.valid_segment?(segment_recipient) end def draft? diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 2cf5a6e35..b5932360a 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -81,6 +81,7 @@ search: - app/ - db/pages/ - db/dev_seeds/ + - lib/ ## Root directories for relative keys resolution. # relative_roots: diff --git a/db/dev_seeds/newsletters.rb b/db/dev_seeds/newsletters.rb index 3105a108e..e196f8a0a 100644 --- a/db/dev_seeds/newsletters.rb +++ b/db/dev_seeds/newsletters.rb @@ -14,7 +14,7 @@ section "Creating Newsletters" do 5.times do |n| Newsletter.create!( subject: "Newsletter subject #{n}", - segment_recipient: UserSegments::SEGMENTS.sample, + segment_recipient: UserSegments.segments.sample, from: "no-reply@consul.dev", body: newsletter_body.sample, sent_at: [Time.current, nil].sample diff --git a/lib/user_segments.rb b/lib/user_segments.rb index efa0d51a4..7e4e8a154 100644 --- a/lib/user_segments.rb +++ b/lib/user_segments.rb @@ -1,13 +1,19 @@ class UserSegments - SEGMENTS = %w[all_users - administrators - all_proposal_authors - proposal_authors - investment_authors - feasible_and_undecided_investment_authors - selected_investment_authors - winner_investment_authors - not_supported_on_current_budget].freeze + def self.segments + %w[all_users + administrators + all_proposal_authors + proposal_authors + investment_authors + feasible_and_undecided_investment_authors + selected_investment_authors + winner_investment_authors + not_supported_on_current_budget] + geozones.keys + end + + def self.segment_name(segment) + geozones[segment.to_s]&.name || I18n.t("admin.segment_recipient.#{segment}") if valid_segment?(segment) + end def self.all_users User.active.where.not(confirmed_at: nil) @@ -51,8 +57,20 @@ class UserSegments ) end - def self.user_segment_emails(users_segment) - UserSegments.send(users_segment).newsletter.order(:created_at).pluck(:email).compact + def self.valid_segment?(segment) + segments.include?(segment.to_s) + end + + def self.recipients(segment) + if geozones[segment.to_s] + all_users.where(geozone: geozones[segment.to_s]) + else + send(segment) + end + end + + def self.user_segment_emails(segment) + recipients(segment).newsletter.order(:created_at).pluck(:email).compact end private @@ -64,4 +82,14 @@ class UserSegments def self.author_ids(author_ids) all_users.where(id: author_ids) end + + def self.geozones + Geozone.order(:name).map do |geozone| + [geozone.name.gsub(/./) { |char| character_approximation(char) }.underscore.tr(" ", "_"), geozone] + end.to_h + end + + def self.character_approximation(char) + I18n::Backend::Transliterator::HashTransliterator::DEFAULT_APPROXIMATIONS[char] || char + end end diff --git a/spec/factories/emails.rb b/spec/factories/emails.rb index 3ed748737..9ddb6c9bb 100644 --- a/spec/factories/emails.rb +++ b/spec/factories/emails.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :newsletter do sequence(:subject) { |n| "Subject #{n}" } - segment_recipient { UserSegments::SEGMENTS.sample } + segment_recipient { UserSegments.segments.sample } sequence(:from) { |n| "noreply#{n}@consul.dev" } sequence(:body) { |n| "Body #{n}" } end diff --git a/spec/factories/notifications.rb b/spec/factories/notifications.rb index 8b300f85c..b0d8eaecc 100644 --- a/spec/factories/notifications.rb +++ b/spec/factories/notifications.rb @@ -24,7 +24,7 @@ FactoryBot.define do sequence(:title) { |n| "Admin Notification title #{n}" } sequence(:body) { |n| "Admin Notification body #{n}" } link { nil } - segment_recipient { UserSegments::SEGMENTS.sample } + segment_recipient { UserSegments.segments.sample } recipients_count { nil } sent_at { nil } diff --git a/spec/lib/user_segments_spec.rb b/spec/lib/user_segments_spec.rb index f59f55968..79b6d176f 100644 --- a/spec/lib/user_segments_spec.rb +++ b/spec/lib/user_segments_spec.rb @@ -5,7 +5,99 @@ describe UserSegments do let(:user2) { create(:user) } let(:user3) { create(:user) } - describe "#all_users" do + describe ".segment_name" do + it "returns a readable name of the segment" do + expect(UserSegments.segment_name("all_users")).to eq "All users" + expect(UserSegments.segment_name("administrators")).to eq "Administrators" + expect(UserSegments.segment_name("proposal_authors")).to eq "Proposal authors" + end + + it "accepts symbols as parameters" do + expect(UserSegments.segment_name(:all_users)).to eq "All users" + end + + it "returns nil for invalid segments" do + expect(UserSegments.segment_name("invalid")).to be nil + end + + context "with geozones in the database" do + before do + create(:geozone, name: "Lands and Borderlands") + create(:geozone, name: "Lowlands and Highlands") + end + + it "returns geozone names when the geozone exists" do + expect(UserSegments.segment_name("lands_and_borderlands")).to eq "Lands and Borderlands" + expect(UserSegments.segment_name("lowlands_and_highlands")).to eq "Lowlands and Highlands" + end + + it "supports international alphabets" do + create(:geozone, name: "Česká republika") + create(:geozone, name: "България") + create(:geozone, name: "日本") + + expect(UserSegments.segment_name("ceska_republika")).to eq "Česká republika" + expect(UserSegments.segment_name("България")).to eq "България" + expect(UserSegments.segment_name("日本")).to eq "日本" + end + + it "returns regular segments when the geozone doesn't exist" do + expect(UserSegments.segment_name("all_users")).to eq "All users" + end + + it "returns nil for invalid segments" do + expect(UserSegments.segment_name("invalid")).to be nil + end + end + end + + describe ".valid_segment?" do + it "returns true when the segment exists" do + expect(UserSegments.valid_segment?("all_proposal_authors")).to be true + expect(UserSegments.valid_segment?("investment_authors")).to be true + expect(UserSegments.valid_segment?("feasible_and_undecided_investment_authors")).to be true + end + + it "accepts symbols as parameters" do + expect(UserSegments.valid_segment?(:selected_investment_authors)).to be true + expect(UserSegments.valid_segment?(:winner_investment_authors)).to be true + expect(UserSegments.valid_segment?(:not_supported_on_current_budget)).to be true + end + + it "is falsey when the segment doesn't exist" do + expect(UserSegments.valid_segment?("imaginary_segment")).to be_falsey + end + + it "is falsey when nil is passed" do + expect(UserSegments.valid_segment?(nil)).to be_falsey + end + + context "with geozones in the database" do + before do + create(:geozone, name: "Lands and Borderlands") + create(:geozone, name: "Lowlands and Highlands") + end + + it "returns true when the geozone exists" do + expect(UserSegments.valid_segment?("lands_and_borderlands")).to be true + expect(UserSegments.valid_segment?("lowlands_and_highlands")).to be true + end + + it "returns true when the segment exists" do + expect(UserSegments.valid_segment?("all_users")).to be true + end + + it "is falsey when the segment doesn't exist" do + expect(UserSegments.valid_segment?("imaginary_segment")).to be_falsey + end + + it "is falsey when nil is passed" do + expect(UserSegments.valid_segment?(nil)).to be_falsey + end + end + end + + describe ".all_users" do it "returns all active users enabled" do active_user = create(:user) erased_user = create(:user, erased_at: Time.current) @@ -15,7 +107,7 @@ describe UserSegments do end end - describe "#administrators" do + describe ".administrators" do it "returns all active administrators users" do active_user = create(:user) active_admin = create(:administrator).user @@ -27,7 +119,7 @@ describe UserSegments do end end - describe "#all_proposal_authors" do + describe ".all_proposal_authors" do it "returns users that have created a proposal even if is archived or retired" do create(:proposal, author: user1) create(:proposal, :archived, author: user2) @@ -49,7 +141,7 @@ describe UserSegments do end end - describe "#proposal_authors" do + describe ".proposal_authors" do it "returns users that have created a proposal" do create(:proposal, author: user1) @@ -67,7 +159,7 @@ describe UserSegments do end end - describe "#investment_authors" do + describe ".investment_authors" do it "returns users that have created a budget investment" do investment = create(:budget_investment, author: user1) budget = create(:budget) @@ -90,7 +182,7 @@ describe UserSegments do end end - describe "#feasible_and_undecided_investment_authors" do + describe ".feasible_and_undecided_investment_authors" do it "returns authors of a feasible or an undecided budget investment" do user4 = create(:user) user5 = create(:user) @@ -128,7 +220,7 @@ describe UserSegments do end end - describe "#selected_investment_authors" do + describe ".selected_investment_authors" do it "returns authors of selected budget investments" do selected_investment = create(:budget_investment, :selected, author: user1) unselected_investment = create(:budget_investment, :unselected, author: user2) @@ -153,7 +245,7 @@ describe UserSegments do end end - describe "#winner_investment_authors" do + describe ".winner_investment_authors" do it "returns authors of winner budget investments" do winner_investment = create(:budget_investment, :winner, author: user1) selected_investment = create(:budget_investment, :selected, author: user2) @@ -178,7 +270,7 @@ describe UserSegments do end end - describe "#current_budget_investments" do + describe ".current_budget_investments" do it "only returns investments from the current budget" do investment1 = create(:budget_investment, author: create(:user)) investment2 = create(:budget_investment, author: create(:user)) @@ -192,7 +284,7 @@ describe UserSegments do end end - describe "#not_supported_on_current_budget" do + describe ".not_supported_on_current_budget" do it "only returns users that haven't supported investments on current budget" do investment1 = create(:budget_investment) investment2 = create(:budget_investment) @@ -209,7 +301,7 @@ describe UserSegments do end end - describe "#user_segment_emails" do + describe ".user_segment_emails" do it "returns list of emails sorted by user creation date" do create(:user, email: "first@email.com", created_at: 1.day.ago) create(:user, email: "last@email.com") @@ -218,4 +310,40 @@ describe UserSegments do expect(emails).to eq ["first@email.com", "last@email.com"] end end + + context "Geozones" do + let!(:new_york) { create(:geozone, name: "New York") } + let!(:california) { create(:geozone, name: "California") } + let!(:user1) { create(:user, geozone: new_york) } + let!(:user2) { create(:user, geozone: new_york) } + let!(:user3) { create(:user, geozone: california) } + + before do + create(:geozone, name: "Mars") + create(:user, geozone: nil) + end + + it "includes geozones in available segments" do + expect(UserSegments.segments).to include("new_york") + expect(UserSegments.segments).to include("california") + expect(UserSegments.segments).to include("mars") + expect(UserSegments.segments).not_to include("jupiter") + end + + it "returns users of a geozone" do + expect(UserSegments.recipients("new_york")).to match_array [user1, user2] + expect(UserSegments.recipients("california")).to eq [user3] + end + + it "accepts symbols as parameters" do + expect(UserSegments.recipients(:new_york)).to match_array [user1, user2] + expect(UserSegments.recipients(:california)).to eq [user3] + end + + it "only returns active users of a geozone" do + user2.update!(erased_at: Time.current) + + expect(UserSegments.recipients("new_york")).to eq [user1] + end + end end diff --git a/spec/system/admin/admin_notifications_spec.rb b/spec/system/admin/admin_notifications_spec.rb index 901342fad..e6c2be180 100644 --- a/spec/system/admin/admin_notifications_spec.rb +++ b/spec/system/admin/admin_notifications_spec.rb @@ -215,15 +215,14 @@ describe "Admin Notifications", :admin do end scenario "Select list of users to send notification" do - UserSegments::SEGMENTS.each do |user_segment| - segment_recipient = I18n.t("admin.segment_recipient.#{user_segment}") + segment = UserSegments.segments.sample + segment_recipient = UserSegments.segment_name(segment) - visit new_admin_admin_notification_path + visit new_admin_admin_notification_path - fill_in_admin_notification_form(segment_recipient: segment_recipient) - click_button "Create notification" + fill_in_admin_notification_form(segment_recipient: segment_recipient) + click_button "Create notification" - expect(page).to have_content(I18n.t("admin.segment_recipient.#{user_segment}")) - end + expect(page).to have_content segment_recipient end end diff --git a/spec/system/admin/emails/newsletters_spec.rb b/spec/system/admin/emails/newsletters_spec.rb index 91f391c33..ad09ed149 100644 --- a/spec/system/admin/emails/newsletters_spec.rb +++ b/spec/system/admin/emails/newsletters_spec.rb @@ -16,7 +16,7 @@ describe "Admin newsletter emails", :admin do expect(page).to have_link "Go back", href: admin_newsletters_path expect(page).to have_content "This is a subject" - expect(page).to have_content I18n.t("admin.segment_recipient.#{newsletter.segment_recipient}") + expect(page).to have_content "All users" expect(page).to have_content "no-reply@consul.dev" expect(page).to have_content "This is a body" end @@ -40,10 +40,9 @@ describe "Admin newsletter emails", :admin do expect(page).to have_css(".newsletter", count: 3) newsletters.each do |newsletter| - segment_recipient = I18n.t("admin.segment_recipient.#{newsletter.segment_recipient}") within("#newsletter_#{newsletter.id}") do expect(page).to have_content newsletter.subject - expect(page).to have_content segment_recipient + expect(page).to have_content UserSegments.segment_name(newsletter.segment_recipient) end end end @@ -161,14 +160,28 @@ describe "Admin newsletter emails", :admin do end end - scenario "Select list of users to send newsletter" do - UserSegments::SEGMENTS.each do |user_segment| + describe "Select list of users to send newsletter" do + scenario "Custom user segments" do + segment = UserSegments.segments.sample + segment_recipient = UserSegments.segment_name(segment) + visit new_admin_newsletter_path - fill_in_newsletter_form(segment_recipient: I18n.t("admin.segment_recipient.#{user_segment}")) + fill_in_newsletter_form(segment_recipient: segment_recipient) click_button "Create Newsletter" - expect(page).to have_content(I18n.t("admin.segment_recipient.#{user_segment}")) + expect(page).to have_content segment_recipient + end + + scenario "Geozone segments" do + create(:geozone, name: "Queens and Brooklyn") + + visit new_admin_newsletter_path + + fill_in_newsletter_form(segment_recipient: "Queens and Brooklyn") + click_button "Create Newsletter" + + expect(page).to have_content "Queens and Brooklyn" end end end