Feature: more specific mailing

[lodewiges]

Summary
Refactors credit delivery report emails to send to a single configured email address (TREASURER_EMAIL) instead of every user with the treasurer role.

Motivation
Previously, all users with the treasurer role received credit notification emails. This caused:

Administrative accounts to receive unnecessary emails

Which annoyed me a lot, so it needed to change :)

Changes

• Modified CreditInsufficientNotificationJob to send one email to [Rails.application.config.x.treasurer_email]
• Updated UserCreditMailer#credit_delivery_report_mail to use the configured email directly
• Updated associated tests and mailer previews

This fixes #442 (kinda)

Summary by CodeRabbit

• Refactor
  • Credit delivery reports are now consolidated into a single report delivery instead of individual per-treasurer emails, and the system now uses centralized configuration settings for treasurer contact information.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR removes per-treasurer email iteration and consolidates credit delivery reporting into a single mail sent to the configured treasurer email (Rails.application.config.x.treasurer_email). Mailer signature and caller sites updated to accept (success_count, unnotifyable_users).

Changes

Cohort / File(s)	Summary
Job app/jobs/credit_insufficient_notification_job.rb	Removed loop over User.treasurer; replaced per-treasurer mail deliveries with a single credit_delivery_report_mail(success_count, unnotifyable_users).deliver_later call.
Mailer app/mailers/user_credit_mailer.rb	Changed credit_delivery_report_mail signature to remove treasurer param; recipient switched to Rails.application.config.x.treasurer_email; constructs a user-like object for name/title. RuboCop disable/enable comments added around method.
Tests & Previews spec/jobs/credit_insufficient_notification_job_spec.rb, spec/mailers/previews/user_credit_mailer_preview.rb	Removed treasurer creation/usages; updated expectations and preview calls to new mailer signature and to use Rails.application.config.x.treasurer_email as recipient.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through lines to tidy mail,
One address now hears the credit tale,
No more loops across each treasurer's name,
A single hop delivers the same,
Carrots and code — neat and hale!

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is vague and does not clearly convey the main purpose of the pull request; 'more specific mailing' is too generic and lacks specificity about what changed.	Use a more descriptive title such as 'Consolidate treasurer emails to single configured address' to clearly indicate the main change.
Linked Issues check	❓ Inconclusive	The PR addresses issue #442 by removing role-based email delivery, but does not implement the proposed periodic role synchronization, only partial mitigation.	Clarify whether this is a complete fix or partial mitigation, or implement the periodic role synchronization mentioned in issue #442.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description lacks the required checklist items from the template (database migrations, testing), though it provides clear summary, motivation, and changes.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the objective of consolidating treasurer emails to a single configured address; no out-of-scope modifications detected.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into staging

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/spefic-mails

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

app/mailers/user_credit_mailer.rb (1)

10-23: Clean refactor — consider simplifying the Struct into a plain ivar.

The Struct.new(:name).new(...) pattern exists solely to give the email template an @user.name accessor. If the template only uses the name, you could assign @name directly (or @user_name) and simplify the template reference, removing the need for the anonymous Struct and the RuboCop disable. That said, if the template is shared and expects an @user object with .name, this is a reasonable duck-type approach.

♻️ Optional: drop the Struct if the template can be updated

-  # rubocop:disable Metrics/AbcSize
   def credit_delivery_report_mail(success_count, unnotifyable_users)
-    `@user` = Struct.new(:name).new(
-      Rails.application.config.x.treasurer_name || Rails.application.config.x.treasurer_title.capitalize
-    )
+    `@user_name` = Rails.application.config.x.treasurer_name || Rails.application.config.x.treasurer_title.capitalize
     `@unnotifyable_users` = unnotifyable_users
     `@success_count` = success_count
     `@title` = 'Notificatie over de saldomail'
 
     subject = "Er is #{`@success_count.positive`? ? 'een' : 'geen'} saldomail verstuurd"
 
     mail to: Rails.application.config.x.treasurer_email, subject:
   end
-  # rubocop:enable Metrics/AbcSize

Then update the email template to use @user_name instead of @user.name.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR refactors the credit delivery report email system to send notifications to a single configured email address (TREASURER_EMAIL) instead of iterating through all users with the treasurer role. This change reduces email duplication, prevents administrative accounts from receiving unnecessary notifications, and decouples notification recipients from user roles.

Changes:

• Removed the loop over User.treasurer in favor of a single email to the configured treasurer address
• Updated the credit_delivery_report_mail method signature to remove the treasurer parameter
• Updated tests and mailer previews to reflect the new method signature

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
app/jobs/credit_insufficient_notification_job.rb	Simplified notification sending by removing the loop over treasurer users and sending a single email to the configured address
app/mailers/user_credit_mailer.rb	Removed the treasurer parameter and updated the email recipient to use Rails.application.config.x.treasurer_email
spec/jobs/credit_insufficient_notification_job_spec.rb	Removed the treasurer user fixture and updated the test to verify email is sent to the configured address
spec/mailers/previews/user_credit_mailer_preview.rb	Updated the preview to match the new method signature without the treasurer parameter

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.59%. Comparing base (b6a0b0e) to head (c8071ad).

Additional details and impacted files

@@             Coverage Diff             @@
##           staging    #1231      +/-   ##
===========================================
- Coverage    77.61%   77.59%   -0.02%     
===========================================
  Files           54       54              
  Lines         1407     1406       -1     
===========================================
- Hits          1092     1091       -1     
  Misses         315      315              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.