Merge pull request rubyforgood#4648 from DariusPirvulescu/4486-addres…

…s-pickup-email-issues

4486: address pickup email issues

app/mailers/distribution_mailer.rb

@@ -27,7 +27,11 @@ def partner_mailer(current_organization, distribution, subject, distribution_cha
     pdf = DistributionPdf.new(current_organization, @distribution).compute_and_render
     attachments[format("%s %s.pdf", @partner.name, @distribution.created_at.strftime("%Y-%m-%d"))] = pdf
     cc = [@partner.email]
-    cc.push(@partner.profile&.pick_up_email) if distribution.pick_up?
+    if distribution.pick_up? && @partner.profile&.pick_up_email
+      pick_up_emails = @partner.profile.split_pick_up_emails
+      cc.push(pick_up_emails)
+    end
+    cc.flatten!
     cc.compact!
     cc.uniq!
 

app/models/partners/profile.rb

@@ -98,6 +98,7 @@ class Profile < Base
 
     validate :client_share_is_0_or_100
     validate :has_at_least_one_request_setting
+    validate :pick_up_email_addresses
 
     self.ignored_columns = %w[
       evidence_based_description
@@ -118,6 +119,12 @@ def client_share_total
       served_areas.map(&:client_share).compact.sum
     end
 
+    def split_pick_up_emails
+      return nil if pick_up_email.nil?
+
+      pick_up_email.split(/,|\s+/).compact_blank
+    end
+
     private
 
     def check_social_media
@@ -144,5 +151,22 @@ def has_at_least_one_request_setting
         errors.add(:base, "At least one request type must be set")
       end
     end
+
+    def pick_up_email_addresses
+      # pick_up_email is a string of comma-separated emails, check specs for details
+      return if pick_up_email.nil?
+
+      emails = split_pick_up_emails
+      if emails.size > 3
+        errors.add(:pick_up_email, "There can't be more than three pick up email addresses.")
+        nil
+      end
+      if emails.uniq.size != emails.size
+        errors.add(:pick_up_email, "There should not be repeated email addresses.")
+      end
+      emails.each do |e|
+        errors.add(:pick_up_email, "Invalid email address for '#{e}'.") unless e.match? URI::MailTo::EMAIL_REGEXP
+      end
+    end
   end
 end

app/views/partners/profiles/edit/_pick_up_person.html.erb

@@ -9,7 +9,7 @@
           <div class="card-body">
             <%= form.input :pick_up_name, label: "Pick Up Person Name", class: "form-control", wrapper: :input_group %>
             <%= form.input :pick_up_phone, label: "Pick Up Person's Phone #", class: "form-control", wrapper: :input_group %>
-            <%= form.input :pick_up_email, label: "Pick Up Person's Email", class: "form-control", wrapper: :input_group %>
+            <%= form.input :pick_up_email, label: "Pick Up Person's Email (comma-separated if multiple addresses)", placeholder: "example1@org.com, example2@org.com", class: "form-control", wrapper: :input_group %>
           </div>
         </div>
       </div>

spec/mailers/distribution_mailer_spec.rb

@@ -5,7 +5,8 @@
   let(:distribution) { create(:distribution, organization: user.organization, comment: "Distribution comment", partner: partner) }
   let(:request) { create(:request, distribution: distribution) }
 
-  let(:pick_up_email) { 'pick_up@org.com' }
+  let(:pick_up_email) { 'pick_up@org.com, second_pick_up@org.com' }
+  let(:pick_up_emails) { ['pick_up@org.com', 'second_pick_up@org.com'] }
 
   before do
     organization.default_email_text = "Default email text example\n\n%{delivery_method} %{distribution_date}\n\n%{partner_name}\n\n%{comment}"
@@ -23,7 +24,7 @@
       expect(mail.body.encoded).to match("Default email text example")
       expect(mail.html_part.body).to match(%(From: <a href="mailto:me@org.com">me@org.com</a>))
       expect(mail.to).to eq([distribution.request.user_email])
-      expect(mail.cc).to eq([distribution.partner.email, pick_up_email])
+      expect(mail.cc).to eq([distribution.partner.email, pick_up_emails.first, pick_up_emails.second])
       expect(mail.from).to eq(["no-reply@humanessentials.app"])
       expect(mail.subject).to eq("test subject from TEST ORG")
     end
@@ -43,10 +44,11 @@
         expect(mail.body.encoded).to match("picked up")
       end
 
-      context 'when parners profile pick_up_email is present' do
-        it 'sends email to the primary contact and partner`s pickup person' do
+      context 'when partners profile pick_up_email is present' do
+        it 'sends email to the primary contact and partner`s pickup persons' do
           expect(mail.cc.first).to match(partner.email)
-          expect(mail.cc.second).to match(pick_up_email)
+          expect(mail.cc.second).to match(pick_up_emails.first)
+          expect(mail.cc.third).to match(pick_up_emails.second)
         end
 
         context 'when pickup person happens to be the same as the primary contact' do

spec/models/partners/profile_spec.rb

@@ -167,6 +167,85 @@
     end
   end
 
+  describe "split pick up email" do
+    let(:profile) { build(:partner_profile, pick_up_email: "pick_up@org.com, pick_up2@org.com") }
+    it "should disregard commas at the beginning or end of the string" do
+      profile.update(pick_up_email: ", pick_up@org.com, pick_up2@org.com,")
+      expect(profile.split_pick_up_emails).to match_array(["pick_up@org.com", "pick_up2@org.com"])
+    end
+
+    it "should allow optional whitespace between email addresses" do
+      profile.update(pick_up_email: "pick_up@org.com, pick_up2@org.com")
+      expect(profile.split_pick_up_emails).to match_array(["pick_up@org.com", "pick_up2@org.com"])
+      profile.update(pick_up_email: "pick_up@org.com,pick_up2@org.com")
+      expect(profile.split_pick_up_emails).to match_array(["pick_up@org.com", "pick_up2@org.com"])
+    end
+
+    it "should handle nil value" do
+      profile.update(pick_up_email: nil)
+      expect(profile.split_pick_up_emails).to be_nil
+    end
+
+    it "should return empty array if for when pick_up_email is an empty string" do
+      profile.update(pick_up_email: "")
+      expect(profile.split_pick_up_emails).to match_array([])
+    end
+
+    it "should correctly split strings" do
+      profile.update(pick_up_email: "test me, pick_up@org.com, pick_up2@org.com, test me")
+      expect(profile.split_pick_up_emails).to match_array(["test", "me", "pick_up@org.com", "pick_up2@org.com", "test", "me"])
+      profile.update(pick_up_email: "test me. pick_up@org.com, pick_up2@org.com. test me")
+      expect(profile.split_pick_up_emails).to match_array(["test", "me.", "pick_up@org.com", "pick_up2@org.com.", "test", "me"])
+    end
+  end
+
+  describe "pick up email address validation" do
+    context "number of email addresses" do
+      let(:profile) { build(:partner_profile, pick_up_email: "pick_up@org.com, pick_up2@org.com, pick_up3@org.com, pick_up4@org.com") }
+      it "should not allow more than three email addresses" do
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com, pick_up2@org.com, pick_up3@org.com")
+        expect(profile).to be_valid
+      end
+
+      it "should not allow repeated email addresses" do
+        profile.update(pick_up_email: "pick_up@org.com, pick_up2@org.com, pick_up@org.com")
+        expect(profile).to_not be_valid
+      end
+    end
+
+    context "invalid emails" do
+      let(:profile) { build(:partner_profile, pick_up_email: "pick_up@org.com, pick_up2@org.com, asdf") }
+      it "should not allow invalid email addresses" do
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "test me, pick_up@org.com, pick_up2@org.com, test me")
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com, pick_up2@org.com")
+        expect(profile).to be_valid
+      end
+
+      it "should not allow input having emails separated by non-word characters" do
+        profile.update(pick_up_email: "test me. pick_up@org.com, pick_up2@org.com. test me")
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com. pick_up2@org.com")
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com.pick_up2@org.com")
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com/ pick_up2@org.com/ pick_up3@org.com")
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com- pick_up2@org.com- pick_up3@org.com")
+        expect(profile).to_not be_valid
+        profile.update(pick_up_email: "pick_up@org.com' pick_up2@org.com' pick_up3@org.com")
+        expect(profile).to_not be_valid
+      end
+
+      it "should handle nil value" do
+        profile.update(pick_up_email: nil)
+        expect(profile).to be_valid
+      end
+    end
+  end
+
   describe "client share behaviour" do
     context "no served areas" do
       let(:profile) { build(:partner_profile) }