fix(billing): preserve trial status for coupon claim invoice top-ups#2780
Open
ygrishajev wants to merge 1 commit intomainfrom
Open
fix(billing): preserve trial status for coupon claim invoice top-ups#2780ygrishajev wants to merge 1 commit intomainfrom
ygrishajev wants to merge 1 commit intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence DiagramsequenceDiagram
participant Webhook as Stripe Webhook Handler
participant SWS as StripeWebhookService
participant RS as RefillService
participant BS as BalancesService
Webhook->>SWS: receive event (invoice / payout / transaction)
SWS->>SWS: determine endTrial = (transaction.type !== "coupon_claim")
SWS->>SWS: updateTransactionAndTopUp(..., endTrial)
SWS->>RS: topUpWallet(amount, userId, { endTrial })
RS->>BS: refreshUserWalletLimits(wallet, { endTrial })
BS-->>RS: limits refreshed / trial state updated
RS-->>SWS: top-up complete
SWS-->>Webhook: acknowledge processing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/billing/services/refill/refill.service.spec.ts (1)
40-51: Test description should not start with "should".Per coding guidelines, test descriptions should use present simple, 3rd person singular without prepending "should".
✏️ Suggested rename
- it("should not end trial when endTrial option is false", async () => { + it("does not end trial when endTrial option is false", async () => {As per coding guidelines: "Use present simple, 3rd person singular for test descriptions without prepending 'should'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/billing/services/refill/refill.service.spec.ts` around lines 40 - 51, Update the test description string in the it(...) block in refill.service.spec.ts to use present simple 3rd person singular without "should": replace "should not end trial when endTrial option is false" with "does not end trial when endTrial option is false" (the test around service.topUpWallet and the expectation on balancesService.refreshUserWalletLimits). Ensure the updated description is used for the it(...) call that references service.topUpWallet, userWalletRepository.findOneBy, managedUserWalletService.authorizeSpending, and balancesService.refreshUserWalletLimits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/billing/services/refill/refill.service.spec.ts`:
- Around line 40-51: Update the test description string in the it(...) block in
refill.service.spec.ts to use present simple 3rd person singular without
"should": replace "should not end trial when endTrial option is false" with
"does not end trial when endTrial option is false" (the test around
service.topUpWallet and the expectation on
balancesService.refreshUserWalletLimits). Ensure the updated description is used
for the it(...) call that references service.topUpWallet,
userWalletRepository.findOneBy, managedUserWalletService.authorizeSpending, and
balancesService.refreshUserWalletLimits.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2780 +/- ##
==========================================
- Coverage 52.08% 51.29% -0.80%
==========================================
Files 1043 1008 -35
Lines 27454 26623 -831
Branches 6359 6266 -93
==========================================
- Hits 14300 13655 -645
+ Misses 12681 12504 -177
+ Partials 473 464 -9
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Coupon claim invoices should not end the user's trial period. This passes endTrial=false for coupon_claim transactions through the webhook-refill pipeline and updates tests accordingly.
ygrishajev
force-pushed
the
fix/billing
branch
from
04b9c61 to
b20a4dc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/billing/services/refill/refill.service.spec.ts`:
- Around line 40-51: Update the test description string to follow the naming
guideline (present simple, 3rd person singular, no "should"); locate the test
case that calls service.topUpWallet(amountUsd, userId, { endTrial: false }) and
asserts balancesService.refreshUserWalletLimits was called with (existingWallet,
{ endTrial: false }), then change the it(...) description from "should not end
trial when endTrial option is false" to a present-simple form such as "does not
end trial when endTrial option is false" (or similar phrasing in 3rd person
singular) so the test name conforms to the project convention while leaving the
test body (including references to UserWalletSeeder.create,
userWalletRepository.findOneBy, managedUserWalletService.authorizeSpending,
balancesService.retrieveDeploymentLimit, and
balancesService.refreshUserWalletLimits) unchanged.
Comment on lines
+40
to
+51
| it("should not end trial when endTrial option is false", async () => { | ||
| const { service, userWalletRepository, managedUserWalletService, balancesService } = setup(); | ||
| const existingWallet = UserWalletSeeder.create({ userId }); | ||
| userWalletRepository.findOneBy.mockResolvedValue(existingWallet); | ||
| managedUserWalletService.authorizeSpending.mockResolvedValue(); | ||
| balancesService.retrieveDeploymentLimit.mockResolvedValue(5000); | ||
| balancesService.refreshUserWalletLimits.mockResolvedValue(); | ||
|
|
||
| await service.topUpWallet(amountUsd, userId, { endTrial: false }); | ||
|
|
||
| expect(balancesService.refreshUserWalletLimits).toHaveBeenCalledWith(existingWallet, { endTrial: false }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Rename test description to meet naming guideline.
The new test name starts with “should”. Please use present simple, 3rd person singular without “should”.
✏️ Proposed fix
-it("should not end trial when endTrial option is false", async () => {
+it("does not end trial when endTrial option is false", async () => {As per coding guidelines: Use present simple, 3rd person singular for test descriptions without prepending 'should'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("should not end trial when endTrial option is false", async () => { | |
| const { service, userWalletRepository, managedUserWalletService, balancesService } = setup(); | |
| const existingWallet = UserWalletSeeder.create({ userId }); | |
| userWalletRepository.findOneBy.mockResolvedValue(existingWallet); | |
| managedUserWalletService.authorizeSpending.mockResolvedValue(); | |
| balancesService.retrieveDeploymentLimit.mockResolvedValue(5000); | |
| balancesService.refreshUserWalletLimits.mockResolvedValue(); | |
| await service.topUpWallet(amountUsd, userId, { endTrial: false }); | |
| expect(balancesService.refreshUserWalletLimits).toHaveBeenCalledWith(existingWallet, { endTrial: false }); | |
| }); | |
| it("does not end trial when endTrial option is false", async () => { | |
| const { service, userWalletRepository, managedUserWalletService, balancesService } = setup(); | |
| const existingWallet = UserWalletSeeder.create({ userId }); | |
| userWalletRepository.findOneBy.mockResolvedValue(existingWallet); | |
| managedUserWalletService.authorizeSpending.mockResolvedValue(); | |
| balancesService.retrieveDeploymentLimit.mockResolvedValue(5000); | |
| balancesService.refreshUserWalletLimits.mockResolvedValue(); | |
| await service.topUpWallet(amountUsd, userId, { endTrial: false }); | |
| expect(balancesService.refreshUserWalletLimits).toHaveBeenCalledWith(existingWallet, { endTrial: false }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/billing/services/refill/refill.service.spec.ts` around lines 40
- 51, Update the test description string to follow the naming guideline (present
simple, 3rd person singular, no "should"); locate the test case that calls
service.topUpWallet(amountUsd, userId, { endTrial: false }) and asserts
balancesService.refreshUserWalletLimits was called with (existingWallet, {
endTrial: false }), then change the it(...) description from "should not end
trial when endTrial option is false" to a present-simple form such as "does not
end trial when endTrial option is false" (or similar phrasing in 3rd person
singular) so the test name conforms to the project convention while leaving the
test body (including references to UserWalletSeeder.create,
userWalletRepository.findOneBy, managedUserWalletService.authorizeSpending,
balancesService.retrieveDeploymentLimit, and
balancesService.refreshUserWalletLimits) unchanged.
baktun14
approved these changes
Feb 19, 2026
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
2 participants
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.
Why
Coupon claim invoices should not end the user trial period. Previously, all invoice-based wallet top-ups would unconditionally end the trial, which incorrectly affected users redeeming coupons.
What
Summary by CodeRabbit
Bug Fixes
Tests