From 7cfa7b18f9b711afb8ade90ff01316f24e83fae2 Mon Sep 17 00:00:00 2001
From: Bertocq <bertocq@gmail.com>
Date: Tue, 20 Feb 2018 22:46:00 +0100
Subject: [PATCH] Validate Newsletter segment_recipient value

Why:

A Newsletter can only be sent if the are available user recipient emails
and that means the `segment_recipient` value actually corresponds to a
function on the UserSegments class.

We could rely on the UserSegments::SEGMENTS constant as the list of
possible user segments functions that a Newsletter can use to gather
emails, so any value not included in that hash would not be valid.

But to be 100% sure the newsletter can get a recipients_list we should
just check if the UserSegments class has a method with same name as the
`segment_recipient` value.

How:

* Adding an validation method that checks if UserSegment has a method
with same name as the `segment_recipient` value.
* Adding an scenario to the Newsletter model spec to check this
---
 app/models/newsletter.rb           | 13 ++++++++++++-
 config/locales/en/activerecord.yml |  4 ++++
 config/locales/es/activerecord.yml |  4 ++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb
index 98b53d295..4ac6d59af 100644
--- a/app/models/newsletter.rb
+++ b/app/models/newsletter.rb
@@ -4,14 +4,25 @@ class Newsletter < ActiveRecord::Base
   validates :segment_recipient, presence: true
   validates :from, presence: true
   validates :body, presence: true
+  validate :validate_segment_recipient
 
   validates_format_of :from, :with => /@/
 
   def list_of_recipients
-    UserSegments.send(segment_recipient).newsletter
+    UserSegments.send(segment_recipient).newsletter if valid_segment_recipient?
+  end
+
+  def valid_segment_recipient?
+    segment_recipient && UserSegments.respond_to?(segment_recipient)
   end
 
   def draft?
     sent_at.nil?
   end
+
+  private
+
+  def validate_segment_recipient
+    errors.add(:segment_recipient, :invalid) unless valid_segment_recipient?
+  end
 end
diff --git a/config/locales/en/activerecord.yml b/config/locales/en/activerecord.yml
index 94b8b3d73..765f92932 100644
--- a/config/locales/en/activerecord.yml
+++ b/config/locales/en/activerecord.yml
@@ -261,6 +261,10 @@ en:
             attachment:
               min_image_width: "Image Width must be at least %{required_min_width}px"
               min_image_height: "Image Height must be at least %{required_min_height}px"
+        newsletter:
+          attributes:
+            segment_recipient:
+              invalid: "The user recipients segment is invalid"
         map_location:
           attributes:
             map:
diff --git a/config/locales/es/activerecord.yml b/config/locales/es/activerecord.yml
index 564676f23..9e195bd25 100644
--- a/config/locales/es/activerecord.yml
+++ b/config/locales/es/activerecord.yml
@@ -257,6 +257,10 @@ es:
             attachment:
               min_image_width: "La imagen debe tener al menos %{required_min_width}px de largo"
               min_image_height: "La imagen debe tener al menos %{required_min_height}px de alto"
+        newsletter:
+          attributes:
+            segment_recipient:
+              invalid: "El segmento de usuarios es inválido"
         map_location:
           attributes:
             map: