-
Notifications
You must be signed in to change notification settings - Fork 1
Feature: reminder email for blocked workspace #484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Introduces a new email template for reminding users about blocked workspaces due to expired or depleted plans. Adds pluralization utility for Russian, updates template names, and includes logic to calculate days since the last payment.
Added logic to send reminder emails to admins of blocked workspaces at specific intervals after payday. Introduced the sendBlockedWorkspaceReminders method and updated tests to cover this new behavior.
Corrects the Content-Type header to include charset in sendHTML and fixes the getWorkspace method to properly return the workspace document. Also removes a redundant line in calculateDaysAfterPayday.
Added explanatory comments and disabled the @typescript-eslint/no-magic-numbers rule for specific lines in emailOverview.ts and paymaster/src/index.ts where magic numbers are used for date calculations and reminder scheduling.
Replaces the hardcoded value for daysAfterPayday with expect.any(Number) in the blocked-workspace-reminder test payload to allow for dynamic number matching.
Removed the utils.twig file and its pluralize_ru macro, replacing it with a TypeScript implementation registered as a Twig function in extensions.ts. Updated all template usages to call pluralize_ru directly, simplifying imports and improving maintainability.
Introduced DAYS_AFTER_PAYDAY_TO_REMIND constant to define the days after payday when admins are reminded about blocked workspaces. Updated the logic to use this constant for better maintainability and clarity.
Updated test dates and moved the blocked-workspace-reminder assertion to the correct test case in index.test.ts. This ensures the tests accurately reflect the intended scenarios and improve test reliability.
Introduces a test to verify that admins are reminded for blocked workspaces with active subscriptions when 3 days have passed since payday. Ensures the reminder includes the correct number of days after payday.
Updated the sendBlockedWorkspaceReminders method to use 'daysAfterPayday' instead of 'daysBlocked' for clarity and consistency in parameter naming and payload structure.
Added eslint-disable-next-line comments to suppress magic number warnings in the pluralize_ru Twig function. This helps maintain code clarity while adhering to linting rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a reminder email system for blocked workspaces that send notifications to administrators at specific intervals (1, 2, 3, 5, 7, and 30 days) after the payment due date. The feature extends the existing PaymasterWorker to check if a workspace is blocked and, on designated reminder days, triggers email notifications through the email worker.
Key Changes:
- Added reminder scheduling logic in PaymasterWorker that checks blocked workspaces against a configured list of reminder days
- Implemented new email templates (HTML, text, and subject) for blocked workspace reminders in Russian with pluralization support
- Added test coverage for the reminder functionality on day 5 after payday
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| workers/paymaster/src/index.ts | Added DAYS_AFTER_PAYDAY_TO_REMIND constant and sendBlockedWorkspaceReminders() method; modified blocked workspace handling to send reminders on specific days |
| workers/paymaster/tests/index.test.ts | Added test for reminder functionality on day 5; updated existing test date from day 4 to day 5 to avoid conflicts |
| workers/email/src/templates/names.ts | Added BlockedWorkspaceReminder enum entry for the new email template type |
| workers/email/src/templates/extensions.ts | Added pluralize_ru() Twig function for Russian word pluralization based on numeric values |
| workers/email/src/templates/emails/blocked-workspace-reminder/text.twig | Created plain text email template for blocked workspace reminders |
| workers/email/src/templates/emails/blocked-workspace-reminder/subject.twig | Created email subject line template with dynamic day count |
| workers/email/src/templates/emails/blocked-workspace-reminder/html.twig | Created HTML email template with workspace information and billing settings link |
| workers/email/scripts/emailOverview.ts | Added daysAfterPayday to template data and calculateDaysAfterPayday() helper method for email preview functionality; updated Content-Type header to include charset |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
workers/email/src/templates/emails/blocked-workspace-reminder/html.twig
Outdated
Show resolved
Hide resolved
| test('Should remind admins for blocked workspace if it has subscription and after payday passed 3 days', async () => { | ||
| /** | ||
| * Arrange | ||
| */ | ||
| const currentDate = new Date('2005-12-27'); | ||
| const plan = createPlanMock({ | ||
| monthlyCharge: 100, | ||
| isDefault: true, | ||
| }); | ||
| const workspace = createWorkspaceMock({ | ||
| plan, | ||
| subscriptionId: 'some-subscription-id', | ||
| lastChargeDate: new Date('2005-11-22'), | ||
| isBlocked: true, | ||
| billingPeriodEventsCount: 10, | ||
| }); | ||
|
|
||
| await fillDatabaseWithMockedData({ | ||
| workspace, | ||
| plan, | ||
| }); | ||
|
|
||
| MockDate.set(currentDate); | ||
|
|
||
| /** | ||
| * Act | ||
| */ | ||
| const worker = new PaymasterWorker(); | ||
| const blockWorkspaceSpy = jest.spyOn(worker, 'addTask'); | ||
|
|
||
| await worker.start(); | ||
| await worker.handle(WORKSPACE_SUBSCRIPTION_CHECK); | ||
| await worker.finish(); | ||
|
|
||
| /** | ||
| * Assert | ||
| */ | ||
| expect(blockWorkspaceSpy).toHaveBeenCalledWith('sender/email', { | ||
| type: 'blocked-workspace-reminder', | ||
| payload: { | ||
| workspaceId: workspace._id.toString(), | ||
| daysAfterPayday: 5, | ||
| }, | ||
| }); | ||
| MockDate.reset(); | ||
| }); |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test only covers the case where daysAfterPayday is 5. According to the PR description and the DAYS_AFTER_PAYDAY_TO_REMIND constant ([1, 2, 3, 5, 7, 30]), reminders should be sent on days 1, 2, 3, 5, 7, and 30 after payday. Consider adding test cases for at least one or two more reminder days (e.g., day 1 and day 30) to ensure the reminder logic works correctly across the full range of configured days. Also add a test case for a day that's NOT in the array (e.g., day 4 or day 6) to verify that no reminder is sent on those days.
| private async calculateDaysAfterPayday( | ||
| workspace: WorkspaceDBScheme | ||
| ): Promise<number> { | ||
| if (!workspace.paidUntil) { | ||
| return 0; | ||
| } | ||
|
|
||
| const now = new Date(); | ||
| const paidUntil = new Date(workspace.paidUntil); | ||
| const diffTime = now.getTime() - paidUntil.getTime(); | ||
|
|
||
|
|
||
| if (diffTime <= 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| // Calculate difference in days | ||
| // eslint-disable-next-line @typescript-eslint/no-magic-numbers | ||
| const diffDays = Math.floor(diffTime / (1000 * 60 * 60 * 24)); | ||
|
|
||
| return diffDays; | ||
| } |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calculateDaysAfterPayday logic is incomplete compared to the PaymasterWorker implementation. The PaymasterWorker calculates the expected payday as either paidUntil OR lastChargeDate + 1 month (see workers/paymaster/src/index.ts lines 144-151), but this function only uses paidUntil. When paidUntil is not set, it returns 0, which may not accurately represent the days after payday for workspaces that use lastChargeDate for calculation. Consider adding fallback logic to calculate from workspace.lastChargeDate when paidUntil is not available, similar to the PaymasterWorker implementation.
| * Arrange | ||
| */ | ||
| const currentDate = new Date('2005-12-26'); | ||
| const currentDate = new Date('2005-12-27'); |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The date was changed from '2005-12-26' to '2005-12-27', changing the test from 4 days after payday to 5 days after payday. While this change likely avoids interference with the new reminder feature (which triggers on specific days including day 5), it makes the test name misleading. The test is named "Should block workspace if it has subscription and after payday passed 3 days" but now tests 5 days after payday. Consider also updating the test name to accurately reflect the 5-day scenario, or use a different date (e.g., '2005-12-26' for 4 days) and ensure the reminder feature doesn't interfere with this blocking test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need, thanks
workers/email/src/templates/emails/blocked-workspace-reminder/text.twig
Outdated
Show resolved
Hide resolved
workers/email/src/templates/emails/blocked-workspace-reminder/subject.twig
Outdated
Show resolved
Hide resolved
Introduces handling for 'blocked-workspace-reminder' tasks in SenderWorker, including new types and template variables. Renames files for consistency and updates type exports to support the new notification event.
Added JSDoc comments to the calculateDaysAfterPayday method in EmailTestServer to clarify its purpose and usage. This improves code readability and maintainability.
Removed redundant ternary check for daysAfterPayday in pluralize_ru calls across HTML, subject, and text templates for blocked workspace reminder emails. This streamlines the template code and ensures consistent pluralization.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,12 @@ | |||
| Вы уже {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) : '' }} не получаете новые ошибки. | |||
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid Twig syntax: : '' after the function call is incorrect. The correct syntax should be {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) }} without the : '' part. This syntax is not valid for Twig and will cause a template error.
| Вы уже {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) : '' }} не получаете новые ошибки. | |
| Вы уже {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) }} не получаете новые ошибки. |
| /** | ||
| * Payload for task for blocked workspace reminder | ||
| */ | ||
| payload: SenderWorkerBlockedWorkspaceReminderPayload |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon at the end of the interface property. This should have a semicolon for consistency with the rest of the codebase (see similar interfaces in payment-failed.ts line 23).
| payload: SenderWorkerBlockedWorkspaceReminderPayload | |
| payload: SenderWorkerBlockedWorkspaceReminderPayload; |
| @@ -0,0 +1 @@ | |||
| Вы уже {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) : '' }} не получаете новые ошибки No newline at end of file | |||
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid Twig syntax: : '' after the function call is incorrect. The correct syntax should be {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) }} without the : '' part. This syntax is not valid for Twig and will cause a template error.
| Вы уже {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) : '' }} не получаете новые ошибки | |
| Вы уже {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) }} не получаете новые ошибки |
| /** | ||
| * Task for blocked workspace reminder | ||
| */ | ||
| type: 'blocked-workspace-reminder', |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon at the end of the interface property. This should have a semicolon for consistency with the rest of the codebase (see similar interfaces in payment-failed.ts line 22 and other task files).
| type: 'blocked-workspace-reminder', | |
| type: 'blocked-workspace-reminder'; |
| <td align="center" style="padding: 15px 0;"> | ||
| <font color="#dbe6ff" style="font-size: 15px; text-align: center; color: #dbe6ff; letter-spacing: 0.4px;"> | ||
| <span style="vertical-align: middle; display: inline-block;"> | ||
| {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) : '' }} без мониторинга |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid Twig syntax: : '' after the function call is incorrect. The correct syntax should be {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) }} without the : '' part. This syntax is not valid for Twig and will cause a template error.
| {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) : '' }} без мониторинга | |
| {{ daysAfterPayday }} {{ pluralize_ru(daysAfterPayday, ['день', 'дня', 'дней']) }} без мониторинга |
add reminder emails for blocked workspaces after 1, 2, 3, 5, 7, 30 after pay day