From 14c47e311d69f79a71203260726dd743582ba48f Mon Sep 17 00:00:00 2001 From: Augusto Xavier Date: Thu, 18 Jun 2026 16:16:31 -0300 Subject: [PATCH 1/2] Send request cancellation email to the request sender as well as the partner (5564) --- app/mailers/request_mailer.rb | 4 ++- spec/mailers/request_mailer_spec.rb | 39 +++++++++++++++++++---------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index 814916a1e8..570b77c08d 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -16,8 +16,10 @@ def request_cancel_partner_notification(request_id:) end @formatted_requested_items.sort_by! { |rt| rt[:name] } + recipients = [@partner.email, @request.requester.email].uniq + mail( - to: @partner.email, + to: recipients, subject: "Your essentials request (##{@request.id}) has been canceled." ) end diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb index 4ea835077e..ffc7813d00 100644 --- a/spec/mailers/request_mailer_spec.rb +++ b/spec/mailers/request_mailer_spec.rb @@ -1,22 +1,35 @@ RSpec.describe RequestMailer, type: :mailer do describe "#request_cancel_partner_notification" do subject { described_class.request_cancel_partner_notification(request_id: request.id) } - let(:request) { create(:request) } - it "renders the body with correct text with partner information" do - html = html_body(subject) - expect(html).to include("Hello there, #{request.partner.name}") - expect(html).to include("One of your essentials requests (##{request.id}) have been canceled.") - text = text_body(subject) - expect(text).to include("Hello there, #{request.partner.name}") - expect(text).to include("One of your essentials requests (##{request.id}) have been canceled.") + let(:partner) { create(:partner, email: "partner@example.com") } + + context "when the request was sent by a partner user" do + let(:partner_user) { create(:partner_user, email: "requester@example.com", partner: partner) } + let(:request) { create(:request, partner: partner, partner_user: partner_user) } + + it "renders the body with correct text with partner information" do + html = html_body(subject) + expect(html).to include("Hello there, #{request.partner.name}") + expect(html).to include("One of your essentials requests (##{request.id}) have been canceled.") + text = text_body(subject) + expect(text).to include("Hello there, #{request.partner.name}") + expect(text).to include("One of your essentials requests (##{request.id}) have been canceled.") + end + + it "is sent to both the partner and the request sender with the correct subject line" do + expect(subject.to).to match_array(["partner@example.com", "requester@example.com"]) + expect(subject.from).to eq(['no-reply@humanessentials.app']) + expect(subject.subject).to eq("Your essentials request (##{request.id}) has been canceled.") + end end - it "should be sent to the partner main email with the correct subject line" do - expect(subject.to).to eq([request.partner.email]) - expect(subject.from).to eq(['no-reply@humanessentials.app']) - expect(subject.subject).to eq("Your essentials request (##{request.id}) has been canceled.") + context "when the request has no partner user" do + let(:request) { create(:request, partner: partner, partner_user: nil) } + + it "is sent only to the partner main email" do + expect(subject.to).to eq(["partner@example.com"]) + end end end end - From 7d6163f27828bda7411ed7839fcf0750deb4e359 Mon Sep 17 00:00:00 2001 From: Brock Wilcox Date: Sat, 29 Aug 2026 19:34:04 -0400 Subject: [PATCH 2/2] Fall back to the partner when a request's sender has been discarded Request#requester keyed off partner_user_id, but User is default-scoped to kept records, so a discarded partner user leaves the id set while partner_user resolves to nil -- and requester returned nil. That nil is dereferenced in five places. Sending the cancellation email to requester.email made the mailer raise, so the request cancelled with no visible error while the notification silently reached nobody, not even the partner. The cancellation confirmation page was already 500ing on the same nil before this change. Keying off the association restores the intended "partner user or partner" fallback for all of them; the mailer's uniq then collapses the recipients to a single email to the partner in that case. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AoUPX63nd1LSkKDPEiQ7jt --- app/models/request.rb | 6 ++++-- spec/mailers/request_mailer_spec.rb | 12 +++++++++++ spec/models/request_spec.rb | 32 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/app/models/request.rb b/app/models/request.rb index 6c4590975b..0e9efa673a 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -59,8 +59,10 @@ def total_items end def requester - # Despite the field being called "partner_user_id", it can refer to both a partner user or an organization admin - partner_user_id ? partner_user : partner + # Despite the field being called "partner_user_id", it can refer to both a partner user or an organization admin. + # Keyed off the association rather than the id: User is default-scoped to kept records, so a discarded + # user leaves partner_user_id set while partner_user resolves to nil. + partner_user || partner end def request_type_label diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb index ffc7813d00..9e203a1bf8 100644 --- a/spec/mailers/request_mailer_spec.rb +++ b/spec/mailers/request_mailer_spec.rb @@ -31,5 +31,17 @@ expect(subject.to).to eq(["partner@example.com"]) end end + + context "when the partner user who sent the request has since been discarded" do + let(:partner_user) { create(:partner_user, email: "requester@example.com", partner: partner) } + let(:request) { create(:request, partner: partner, partner_user: partner_user) } + + it "is still sent to the partner main email" do + request + partner_user.discard + + expect(subject.to).to eq(["partner@example.com"]) + end + end end end diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index e203179deb..247bd7ec57 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -177,6 +177,38 @@ end end + describe "requester" do + let(:partner) { create(:partner) } + + context "when a partner user submitted the request" do + let(:partner_user) { create(:partner_user, partner: partner) } + + it "returns the partner user" do + request = create(:request, partner: partner, partner_user: partner_user) + expect(request.requester).to eq(partner_user) + end + end + + context "when no partner user is recorded" do + it "returns the partner" do + request = create(:request, partner: partner, partner_user: nil) + expect(request.requester).to eq(partner) + end + end + + context "when the partner user has since been discarded" do + let(:partner_user) { create(:partner_user, partner: partner) } + + it "falls back to the partner" do + request = create(:request, partner: partner, partner_user: partner_user) + partner_user.discard + + expect(request.reload.partner_user_id).to eq(partner_user.id) + expect(request.requester).to eq(partner) + end + end + end + describe "versioning" do it { is_expected.to be_versioned } end