Merged
Conversation
WalkthroughUpdated Team5000SubscriptionReminderService: reminder windows changed from [7,3,1] to [30,7,2]; email subject mapping rewritten to label 30/7/2; HTML email generation simplified and no longer includes donor name, daysBefore, or amount; idempotency subject suffixes/filters updated accordingly. (49 words) Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php (1)
10-20: Stale class documentation.The docblock at lines 13-14 still references the old reminder schedule ("7, 3, and 1 day(s)") but
REMINDER_DAYSis now[30, 7, 2]. Additionally, the cron file atwp-content/civi-extensions/goonjcustom/api/v3/Goonjcustom/Team5000SubscriptionReminderCron.php(lines 6-7) contains the same outdated comment.📝 Proposed fix for class docblock
/** * Handles subscription expiry reminder emails for Team 5000 donors. * - * Sends reminders at 7, 3, and 1 day(s) before the calculated subscription + * Sends reminders at 30, 7, and 2 day(s) before the calculated subscription * end date. Logs a CiviCRM activity after each send to prevent duplicates. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php` around lines 10 - 20, Update the stale docblocks to match the current reminder schedule: edit the class docblock for Team5000SubscriptionReminderService and the header comment in Team5000SubscriptionReminderCron (the cron file referenced) so the textual reminder days reflect REMINDER_DAYS (30, 7, 2) instead of the old "7, 3, and 1 day(s)". Keep the wording concise and consistent with the constant REMINDER_DAYS and existing descriptions (e.g., "Sends reminders at 30, 7, and 2 day(s) before the calculated subscription end date").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php`:
- Around line 287-299: The getReminderEmailHtml function currently accepts
$donorName and $daysBefore but doesn't use them; either personalize the template
by interpolating $donorName into the greeting and include $daysBefore in the
expiration sentence (e.g., "set to expire in {$daysBefore} days") inside
getReminderEmailHtml, or remove those parameters from getReminderEmailHtml and
update all callers to stop passing them; ensure you modify the function
signature and every invocation consistently (references: getReminderEmailHtml).
---
Nitpick comments:
In
`@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php`:
- Around line 10-20: Update the stale docblocks to match the current reminder
schedule: edit the class docblock for Team5000SubscriptionReminderService and
the header comment in Team5000SubscriptionReminderCron (the cron file
referenced) so the textual reminder days reflect REMINDER_DAYS (30, 7, 2)
instead of the old "7, 3, and 1 day(s)". Keep the wording concise and consistent
with the constant REMINDER_DAYS and existing descriptions (e.g., "Sends
reminders at 30, 7, and 2 day(s) before the calculated subscription end date").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6b4f7750-abc4-4506-a207-4cbe4f779d5c
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php (1)
10-15:⚠️ Potential issue | 🟡 MinorStale docblock: reminder windows documented as 7/3/1 but code uses 30/7/2.
The class-level comment references the old reminder schedule. Update it to reflect the new windows
[30, 7, 2]to avoid confusing future maintainers.📝 Suggested fix
/** * Handles subscription expiry reminder emails for Team 5000 donors. * - * Sends reminders at 7, 3, and 1 day(s) before the calculated subscription + * Sends reminders at 30, 7, and 2 day(s) before the calculated subscription * end date. Logs a CiviCRM activity after each send to prevent duplicates. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php` around lines 10 - 15, Update the stale class-level docblock on Team5000SubscriptionReminderService to reflect the current reminder schedule of 30, 7, and 2 days (instead of 7, 3, and 1); edit the comment block above the class declaration so the description and any listed reminder windows match the actual values used by the class (e.g., the reminder windows array or logic that schedules sends) to avoid future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php`:
- Around line 290-299: The email body in Team5000SubscriptionReminderService
(the HTML-returning method that builds the reminder body) says "expire in a few
days" which conflicts with the 30-day "1 Month" subject; update the body text to
be consistent by either using neutral phrasing like "is approaching" or accept
and interpolate the expiry label (e.g., pass an $expiryLabel into the method
that builds the HTML and replace "in a few days" with "in {$expiryLabel}" or
similar) so getReminderEmailHtml (or the method that returns the HTML) matches
the subject across all reminder windows.
---
Outside diff comments:
In
`@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php`:
- Around line 10-15: Update the stale class-level docblock on
Team5000SubscriptionReminderService to reflect the current reminder schedule of
30, 7, and 2 days (instead of 7, 3, and 1); edit the comment block above the
class declaration so the description and any listed reminder windows match the
actual values used by the class (e.g., the reminder windows array or logic that
schedules sends) to avoid future confusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 92cb9b1e-8a8d-47ac-88cb-b698c2e526d4
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php
Comment on lines
290
to
299
| return " | ||
| <p>Dear <strong>{$donorName}</strong>,</p> | ||
| <p>Greetings from Goonj!</p> | ||
| <p>{$urgencyLine}</p> | ||
| <p>Your Team 5000 membership is set to expire on <strong>{$formattedDate}</strong>.</p> | ||
| {$amountLine} | ||
| <p>Your generosity has been a cornerstone of our work — empowering communities, restoring dignity, and creating lasting change. We truly value your commitment to this journey.</p> | ||
| <p>To continue your support and ensure your membership stays active, we warmly invite you to renew your Team 5000 membership at your earliest convenience.</p> | ||
| <p>For any questions or assistance, please feel free to write to us at <a href='mailto:accounts@goonj.org'>accounts@goonj.org</a>.</p> | ||
| <p>Thank you for being a vital part of Team 5000 — together, we are making a difference!</p> | ||
| <p>Warm regards,<br>Team Goonj</p> | ||
| <p>Just a gentle reminder that your Team 5000 membership is set to expire in a few days (on <strong>{$formattedDate}</strong>).</p> | ||
| <p>We truly hope you'll continue this journey with us.</p> | ||
| <p>Your contribution has been making a meaningful difference, helping us reach communities that need it most.</p> | ||
| <p>We warmly invite you to renew your Team 5000 membership at your earliest convenience by clicking this link <a href='https://goonj.org/donate/campaign/team-5000-new'>https://goonj.org/donate/campaign/team-5000-new</a></p> | ||
| <p>Kindly ignore if you have already renewed your contribution. If you have any questions or need assistance, please feel free to write to us at <a href='mailto:priyanka@goonj.org'>priyanka@goonj.org</a>.</p> | ||
| <p>Thank you for being such an important part of Team 5000. Together, we are creating real impact.</p> | ||
| <p>Warm regards<br>Team Goonj</p> | ||
| "; |
There was a problem hiding this comment.
Email body says "a few days" but the 30-day reminder contradicts this.
The subject correctly states "1 Month" for the 30-day window, but the body still says "expire in a few days." This inconsistency could confuse recipients.
Consider making the body text match the subject's specificity, or use neutral phrasing like "is approaching" that works for all windows.
📝 Option A: Generic phrasing that works for all windows
return "
<p>Greetings from Goonj!</p>
- <p>Just a gentle reminder that your Team 5000 membership is set to expire in a few days (on <strong>{$formattedDate}</strong>).</p>
+ <p>Just a gentle reminder that your Team 5000 membership is set to expire on <strong>{$formattedDate}</strong>.</p>
<p>We truly hope you'll continue this journey with us.</p>📝 Option B: Pass the label to the template for consistency
Update signature and call site to pass the label:
// In getEmailSubject, extract label lookup to a separate method or pass it
private static function getReminderEmailHtml(string $endDate, string $expiryLabel): string {
// ...
return "
<p>Just a gentle reminder that your Team 5000 membership is set to expire in <strong>{$expiryLabel}</strong> (on <strong>{$formattedDate}</strong>).</p>
// ...
";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@wp-content/civi-extensions/goonjcustom/Civi/Team5000SubscriptionReminderService.php`
around lines 290 - 299, The email body in Team5000SubscriptionReminderService
(the HTML-returning method that builds the reminder body) says "expire in a few
days" which conflicts with the 30-day "1 Month" subject; update the body text to
be consistent by either using neutral phrasing like "is approaching" or accept
and interpolate the expiry label (e.g., pass an $expiryLabel into the method
that builds the HTML and replace "in a few days" with "in {$expiryLabel}" or
similar) so getReminderEmailHtml (or the method that returns the HTML) matches
the subject across all reminder windows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update email template for team 5000
Summary by CodeRabbit