Merge pull request #4648 from DariusPirvulescu/4486-address-pickup-em…

…ail-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) }