Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4486: address pickup email issues #4648

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b15db30
Add validation on pick_up_email
DariusPirvulescu Sep 11, 2024
e4ffe4e
Add specs for pick_up_email validation
DariusPirvulescu Sep 11, 2024
df4135c
fix typo in specs
DariusPirvulescu Sep 11, 2024
ae32e37
update distribution mailer spec to support multiple pick up emails
DariusPirvulescu Sep 12, 2024
7a78bc1
change pick up person to plural
DariusPirvulescu Sep 12, 2024
3b32502
handle multiple pick up addresses in cc
DariusPirvulescu Sep 12, 2024
ab1df0a
Merge branch 'main' into 4486-address-pickup-email-issues
DariusPirvulescu Sep 12, 2024
202efcc
add another test for pick_up_email validator
DariusPirvulescu Sep 12, 2024
d3dfed5
update pick_up_email validator to disregard commas at beginning, end …
DariusPirvulescu Sep 12, 2024
1389c1d
flatten the cc array after merging with pick_up_emails
DariusPirvulescu Sep 12, 2024
5be76e1
update test with unique pick up emails
DariusPirvulescu Sep 12, 2024
70777ab
not allow repeated pick_up_email addresses
DariusPirvulescu Sep 12, 2024
9d3ed3c
add tests for profile#split_pick_up_emails
DariusPirvulescu Sep 13, 2024
53e98af
add split_pick_up_emails method on Profile model
DariusPirvulescu Sep 13, 2024
d21aed3
update distribution mailer with split_pick_up_emails
DariusPirvulescu Sep 13, 2024
bd72060
update test to use be_valid
DariusPirvulescu Sep 13, 2024
92ac8fb
Merge branch 'main' into 4486-address-pickup-email-issues
DariusPirvulescu Sep 13, 2024
1fb21ab
remove context wrapper
DariusPirvulescu Sep 15, 2024
f9db471
add more tests for profile pick_up_emails
DariusPirvulescu Sep 15, 2024
02fb105
fix split_pick_up_emails to handle edge cases
DariusPirvulescu Sep 15, 2024
463a1f1
Merge branch 'main' into 4486-address-pickup-email-issues
DariusPirvulescu Sep 15, 2024
4e4300a
add instructions for comma-separated emails
DariusPirvulescu Sep 16, 2024
91d4db1
add placeholder to pick_up_email input
DariusPirvulescu Sep 16, 2024
ab7eba7
Merge branch 'main' into 4486-address-pickup-email-issues
DariusPirvulescu Sep 16, 2024
5bafd76
change emails label to addresses
DariusPirvulescu Sep 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/mailers/distribution_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ 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.pick_up_email.delete(" ").split(",")
(cc << pick_up_emails).flatten! if pick_up_emails
DariusPirvulescu marked this conversation as resolved.
Show resolved Hide resolved
end
cc.compact!
cc.uniq!

Expand Down
15 changes: 15 additions & 0 deletions app/models/partners/profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -144,5 +145,19 @@ 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 details
return if pick_up_email.nil?

emails = pick_up_email.delete(" ").split(",")
if emails.size > 3
errors.add(:pick_up_email, "There can't be more than three pick up email addresses")
nil
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
12 changes: 7 additions & 5 deletions spec/mailers/distribution_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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
Expand All @@ -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
Expand Down
30 changes: 30 additions & 0 deletions spec/models/partners/profile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,36 @@
end
end

describe "pick up email address validation" do
context "number of email addresses" do
let(:profile) { build(:partner_profile, pick_up_email: "a@b.c, a@b.c, a@b.c, a@b.c") }
DariusPirvulescu marked this conversation as resolved.
Show resolved Hide resolved
it "should not allow more than three email addresses" do
expect(profile.valid?).to eq(false)
profile.update(pick_up_email: "a@b.c, a@b.c, a@b.c")
expect(profile.valid?).to eq(true)
end

it "should allow optional whitespace between email addresses" do
profile.update(pick_up_email: "a@b.c,a@b.c")
expect(profile.valid?).to eq(true)
end
end

context "invalid emails" do
let(:profile) { build(:partner_profile, pick_up_email: "a@b.c, a@b.c, asdf") }
it "should not allow invalid email addresses" do
expect(profile.valid?).to eq(false)
profile.update(pick_up_email: "a@b.c")
expect(profile.valid?).to eq(true)
end

it "should handle nil value" do
profile.update(pick_up_email: nil)
expect(profile.valid?).to eq(true)
end
end
end

describe "client share behaviour" do
context "no served areas" do
let(:profile) { build(:partner_profile) }
Expand Down