From 7a028411ab4efd41edb94922ff0920deb464a455 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 10 Nov 2021 20:45:00 +0100
Subject: [PATCH] Extract methods to get recipients and valid segments

This way we don't have to use the `send` method in other places, like
the AdminNotification class, and we can change the internal
implementation at any point.
---
 app/models/admin_notification.rb |  4 ++--
 app/models/newsletter.rb         |  2 +-
 lib/user_segments.rb             | 10 +++++++++-
 spec/lib/user_segments_spec.rb   | 22 ++++++++++++++++++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

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/lib/user_segments.rb b/lib/user_segments.rb
index b125f42c2..47cd36d13 100644
--- a/lib/user_segments.rb
+++ b/lib/user_segments.rb
@@ -57,8 +57,16 @@ class UserSegments
     )
   end
 
+  def self.valid_segment?(segment)
+    segment && respond_to?(segment)
+  end
+
+  def self.recipients(segment)
+    send(segment)
+  end
+
   def self.user_segment_emails(segment)
-    UserSegments.send(segment).newsletter.order(:created_at).pluck(:email).compact
+    recipients(segment).newsletter.order(:created_at).pluck(:email).compact
   end
 
   private
diff --git a/spec/lib/user_segments_spec.rb b/spec/lib/user_segments_spec.rb
index 01194f42e..906827d7e 100644
--- a/spec/lib/user_segments_spec.rb
+++ b/spec/lib/user_segments_spec.rb
@@ -21,6 +21,28 @@ describe UserSegments do
     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
+  end
+
   describe ".all_users" do
     it "returns all active users enabled" do
       active_user = create(:user)