Skip to content

fix: deliver monthly attendance reminder emails#2622

Merged
mroderick merged 1 commit into
codebar:masterfrom
mroderick:fix/monthly-attendance-reminder-delivery
May 26, 2026
Merged

fix: deliver monthly attendance reminder emails#2622
mroderick merged 1 commit into
codebar:masterfrom
mroderick:fix/monthly-attendance-reminder-delivery

Conversation

@mroderick
Copy link
Copy Markdown
Collaborator

Summary

send_monthly_attendance_reminder_emails has been broken since the feature was introduced in 2016. It instantiates a mailer message but never calls .deliver_now or .deliver_later, so the message is created in memory and immediately discarded.

The Bug

# Before (broken — message is instantiated and discarded)
MeetingInvitationMailer.attendance_reminder(monthly, member)

# After (fixed — message is actually sent)
MeetingInvitationMailer.attendance_reminder(monthly, member).deliver_now

Since the method is already wrapped in handle_asynchronously, the correct fix is .deliver_now (send synchronously within the already-async job). Using .deliver_later would double-queue emails.

History

See discussion #2593 for the full analysis. In short:

  • May 2016 — Feature introduced by Denise Yu (PR Change mailer copy #445) without .deliver_now
  • May 2018 — Refactored by Despo Pentara, but the bug persisted
  • ~9 years — Mock-based tests masked the bug by verifying the mailer was called, not that emails were sent

Changes

  • app/models/invitation_manager.rb: add .deliver_now to the mailer call
  • spec/models/invitation_manager_spec.rb: convert :wip mock-based test to a delivery assertion

Testing

All 39 examples in spec/models/invitation_manager_spec.rb pass, including the newly-converted delivery assertion. The rake task spec also passes.

Risk

Very low. This only affects upcoming monthly meetings. There is no backlog of unsent emails (the mailer objects were never persisted). Fixing this will simply cause future monthly attendance reminders to actually be delivered.

send_monthly_attendance_reminder_emails has been broken since the feature
was introduced in 2016. It instantiates a mailer message but never calls
deliver_now or deliver_later, so the message is discarded without ever
being sent.

Since the method is already wrapped in handle_asynchronously, the correct
fix is deliver_now (send synchronously within the already-async job).

Also converts the existing :wip test from a mock-based assertion to a
delivery assertion, verifying that emails are actually sent.

Fixes discussion codebar#2593
@mroderick mroderick requested a review from olleolleolle May 26, 2026 19:37
@mroderick mroderick marked this pull request as ready for review May 26, 2026 19:37
@mroderick mroderick requested a review from till May 26, 2026 19:38
Copy link
Copy Markdown
Collaborator

@till till left a comment

Choose a reason for hiding this comment

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

Nice find

Comment thread app/models/invitation_manager.rb
@mroderick mroderick merged commit 1600f08 into codebar:master May 26, 2026
8 checks passed
@mroderick mroderick deleted the fix/monthly-attendance-reminder-delivery branch May 26, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants