Skip to content

Email volunteers when their reimbursement request is marked complete#7056

Open
arielsalcepk wants to merge 6 commits into
rubyforgood:mainfrom
arielsalcepk:7041-reimbursement-email-notification
Open

Email volunteers when their reimbursement request is marked complete#7056
arielsalcepk wants to merge 6 commits into
rubyforgood:mainfrom
arielsalcepk:7041-reimbursement-email-notification

Conversation

@arielsalcepk

Copy link
Copy Markdown

What github issue is this PR for, if any?

Addresses #7041 — this covers the email-notification half of the ticket only. The status-visibility half is #7055, opened separately by @Jberma23. Not marking this Resolves since #7041 shouldn't auto-close until both land.

What changed, and why?

Wires up email delivery on ReimbursementCompleteNotifier. Volunteers now get an email when a supervisor marks their reimbursement request complete, previously this notification was in-app only (the deliver_by :email block existed but was commented-out scaffolding).

  • Email delivery is scoped to the volunteer recipient only (recipient.volunteer? && recipient.receive_email_notifications?), not the supervisor who triggered the change, since they already know they just did it. The existing in-app notification still goes to both; only the new email channel is scoped.
  • VolunteerMailer#reimbursement_complete_email takes plain positional args (user, case_contact) to match this mailer's existing methods, rather than Noticed's default .with(params) calling convention.
  • Added DebugPreviewMailer#no_data, separate from the existing invalid_user, for the case where a looked-up preview user is valid but has no related record to render (a volunteer with no case contact wanting reimbursement). Keeps that fallback message accurate instead of reusing invalid_user's "ID didn't resolve" wording for a different situation.

How is this tested? (please write rspec and jest tests!) 💖💪

  • bundle exec rspec: 3625 passing, 1 pre-existing failure unrelated to this change (spec/system/case_contacts/case_contacts_new_design_spec.rb, a columns-panel system spec; consistent with the flakiness already tracked in Bug: CI vulnerable to flaking when Github's servers are under high load #6265). Will post in #casa per the note below if this recurs in the Actions build on this PR.
  • bundle exec standardrb: clean
  • New coverage: spec/mailers/volunteer_mailer_spec.rb, spec/notifications/reimbursement_complete_notifier_spec.rb, spec/mailers/previews/volunteer_mailer_preview_spec.rb
  • No new system/Capybara spec, this PR is backend mailer/notifier plumbing with no new UI, the existing reimbursements-page system specs already cover the mark_as_complete interaction this hooks into.
  • Verified manually end-to-end locally: created a reimbursement-eligible case contact as a volunteer, marked it complete as a supervisor, confirmed the real email fires (not a preview) with the correct recipient, subject, and content via Letter Opener.

Note: if you see a flake in your test build in github actions, please post in slack #casa "Flaky test: " :) 💪

Screenshots please :)

Run your local server and take a screenshot of your work! Try to include the URL of the page as well as the contents of the page.

Case before getting marked as complete:
image

Right after, this file was generated to my system with the HTML for the resulting email:
Screenshot 2026-07-15 at 3 01 22 PM

Feelings gif (optional)

@github-actions github-actions Bot added 🧪 Tests Tests ruby Touches Ruby code erb Touches ERB templates labels Jul 15, 2026
@arielsalcepk arielsalcepk marked this pull request as ready for review July 15, 2026 19:59
Comment thread spec/mailers/volunteer_mailer_spec.rb Outdated
let!(:mileage_rate) { create(:mileage_rate, casa_org: case_contact.casa_case.casa_org, amount: 6.50, effective_date: 3.days.ago) }

it "includes the reimbursement amount" do
expect(mail.body.encoded).to match("$")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is going to catch any string consider $ is a regex end-anchor. Think you may need to use something like match(/$[\d,]+.\d{2}/)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jberma23 Fixed both, thanks for catching these, and here's what changed for a brief summary (although please do check this further):
Almost used your regex as is, just had to escape $ and . as well, otherwise those are the end-of-line anchor and any-character wildcard, not literal characters. And the test now uses: /$[\d,]+.\d{2}/. 👍

Controller now guards the notifier call with ActiveModel::Type::Boolean.new.cast(reimbursement_complete), so unchecking no longer fires it. Worth noting: that bug predates this PR, was harmless before as an in-app-only notification, but our email delivery is what made it consequential. This PR successfully both exposed and fixed it, specially thanks to your call out!

Added tests covering both directions and value formats, plus the no-mileage-rate case.

@Jberma23 Jberma23 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So looking at the logic I think it would send this email out even if they uncheck the reimbursement checkbox. So we probably need to check for that as well

@Jberma23

Jberma23 commented Jul 16, 2026 via email

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

erb Touches ERB templates ruby Touches Ruby code 🧪 Tests Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants