fix: complete referral trigger flow and payout ownership enforcement#349
Merged
OlufunbiIK merged 1 commit intoOlufunbiIK:mainfrom Mar 29, 2026
Merged
Conversation
|
@Kaylahray is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR mounts the ReferralModule and PayoutsModule in the application, enforces JWT authentication on both modules' controllers, adds artist/user ownership validation throughout the payout service, refactors the referral controller to use full CurrentUserData injection, and connects referral reward claiming to tip-verified events via an event listener. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Authenticated Client
participant ReferralCtrl as Referral Controller
participant ReferralSvc as Referral Service
participant TipsEventBus as Event Bus (Tip Service)
participant ReferralListnr as Referral Event Listener
Client->>ReferralCtrl: applyCode(currentUser, code)
ReferralCtrl->>ReferralSvc: applyCode(user.userId, code)
ReferralSvc->>ReferralSvc: Create referral record in transaction
Note over ReferralSvc: Returns ApplyReferralResponseDto
rect rgba(100, 200, 100, 0.5)
Note over TipsEventBus,ReferralListnr: Separate Flow: Tip Verification Triggers Reward
Client->>TipsEventBus: Verify tip
TipsEventBus->>TipsEventBus: Emit tip.verified event
TipsEventBus->>ReferralListnr: handleTipVerified(event)
ReferralListnr->>ReferralSvc: claimReward(event.senderUserId, event.tipId)
ReferralSvc->>ReferralSvc: Update referral reward claim status
Note over ReferralSvc: Returns boolean (true if claimed, false if already claimed)
end
sequenceDiagram
participant Client as Authenticated Client
participant PayoutCtrl as Payout Controller
participant JwtGuard as JWT Auth Guard
participant PayoutSvc as Payout Service
participant ArtistRepo as Artist Repository
Client->>PayoutCtrl: requestPayout(currentUser, dto)
PayoutCtrl->>JwtGuard: Validate JWT & extract user
JwtGuard->>PayoutCtrl: user.userId
PayoutCtrl->>PayoutSvc: requestPayout(user.userId, dto)
rect rgba(100, 150, 200, 0.5)
Note over PayoutSvc,ArtistRepo: Ownership Validation
PayoutSvc->>ArtistRepo: Load artist by artistId
PayoutSvc->>PayoutSvc: assertArtistOwnership(user.userId, artist)
alt Artist belongs to user
PayoutSvc->>PayoutSvc: verifyArtistAddress(artist, destinationAddress)
PayoutSvc->>PayoutSvc: Reserve balance & create pending payout
else User does not own artist
PayoutSvc->>PayoutCtrl: ForbiddenException
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
🧹 Nitpick comments (4)
backend/src/tips/events/tip-verified.event.ts (1)
8-15: Consider consolidating sender identity fields.You now expose both
senderIdandsenderUserIdwith identical values. Optional cleanup: keep one canonical field and deprecate the alias to reduce downstream ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/tips/events/tip-verified.event.ts` around lines 8 - 15, The class exposes duplicate sender identity fields (senderId and senderUserId); pick a single canonical property (e.g., senderUserId) and remove the redundant one, or preserve the old name only as a deprecated alias. Update the constructor signature (constructor(..., senderUserId: string)) and assignments in the constructor (assign to this.senderUserId only), and update any consumers of TipVerifiedEvent, TipVerifiedEvent.constructor, or spreading of the event to use the chosen canonical symbol; if you must keep the old name for compatibility, keep senderId only as a getter or deprecated alias that returns this.senderUserId and add a comment marking it deprecated.backend/src/social-sharing/referral.service.spec.ts (1)
85-85: Consider removingReferralControllerfrom service spec.The controller is imported but not used in any test. This adds unnecessary coupling and could cause test failures if controller dependencies change.
♻️ Proposed fix
const module: TestingModule = await Test.createTestingModule({ - controllers: [ReferralController], providers: [ ReferralService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/social-sharing/referral.service.spec.ts` at line 85, Remove the unused ReferralController from the test module in the referral service unit tests: locate the TestingModule setup in referral.service.spec.ts that includes controllers: [ReferralController] and delete the controllers entry and any unused import of ReferralController so the spec only instantiates ReferralService (and its required providers/mocks); this reduces unnecessary coupling and prevents controller dependency breakage.backend/src/artiste-payout/payouts.service.ts (1)
306-313: Consider returning the updated payout directly from the update result.The current implementation calls
findOneafterupdateto return the updated entity. While functional, this creates a potential race condition if another process modifies the payout between update and read.♻️ Alternative using QueryBuilder with RETURNING clause
- await this.payoutRepo.update(payoutId, { + const result = await this.payoutRepo + .createQueryBuilder() + .update(PayoutRequest) + .set({ - status: PayoutStatus.PENDING, - failureReason: null, - stellarTxHash: null, - processedAt: null, - }); + status: PayoutStatus.PENDING, + failureReason: null, + stellarTxHash: null, + processedAt: null, + }) + .where("id = :payoutId", { payoutId }) + .returning("*") + .execute(); - return this.payoutRepo.findOne({ where: { id: payoutId } }); + return result.raw[0] as PayoutRequest;Note: TypeORM's
RETURNINGsupport varies by database. Verify PostgreSQL is used in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/artiste-payout/payouts.service.ts` around lines 306 - 313, The current flow updates a payout via this.payoutRepo.update(...) then re-reads with this.payoutRepo.findOne(...), risking a race; change to perform the update and return the updated row in one operation (use the repository or QueryBuilder update with .set({...}) .where({ id: payoutId }) and .returning('*') then .execute()) and extract the returned row (from result.raw or result.generatedMaps) to return directly; keep the same fields being set (status: PayoutStatus.PENDING, failureReason: null, stellarTxHash: null, processedAt: null) and ensure you verify PostgreSQL/RETURNING support in production before merging.backend/src/artiste-payout/payouts.service.spec.ts (1)
1-297: Consider adding test coverage for settlement lifecycle methods.The spec covers user-facing operations but omits
finaliseSuccess,finaliseFailure,getPendingPayouts, andmarkProcessing. These are critical for payout settlement integrity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/artiste-payout/payouts.service.spec.ts` around lines 1 - 297, Add unit tests to payouts.service.spec.ts covering the settlement lifecycle methods on PayoutsService: write specs for getPendingPayouts (should return pending payouts via payoutRepo.find with correct filters), markProcessing (should set status to PROCESSING and save/update the payout and create an audit), finaliseSuccess (should set status to COMPLETED, set stellarTxHash and processedAt, update balances/audit and commit transaction), and finaliseFailure (should set status to FAILED, record failureReason, release reserved balance via ArtistBalance update and create audit). In each test reference and mock PayoutsService methods and repo interactions (payoutRepo.find/findOne/update/save, balanceRepo.findOne/update, auditRepo.save) and assert the expected repo calls, status transitions (PayoutStatus.PROCESSING/COMPLETED/FAILED), and permission checks when payout.artistId belongs to another artist (use artistRepo.findOne mocks to simulate ownership).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/src/artiste-payout/payouts.service.spec.ts`:
- Around line 1-297: Add unit tests to payouts.service.spec.ts covering the
settlement lifecycle methods on PayoutsService: write specs for
getPendingPayouts (should return pending payouts via payoutRepo.find with
correct filters), markProcessing (should set status to PROCESSING and
save/update the payout and create an audit), finaliseSuccess (should set status
to COMPLETED, set stellarTxHash and processedAt, update balances/audit and
commit transaction), and finaliseFailure (should set status to FAILED, record
failureReason, release reserved balance via ArtistBalance update and create
audit). In each test reference and mock PayoutsService methods and repo
interactions (payoutRepo.find/findOne/update/save, balanceRepo.findOne/update,
auditRepo.save) and assert the expected repo calls, status transitions
(PayoutStatus.PROCESSING/COMPLETED/FAILED), and permission checks when
payout.artistId belongs to another artist (use artistRepo.findOne mocks to
simulate ownership).
In `@backend/src/artiste-payout/payouts.service.ts`:
- Around line 306-313: The current flow updates a payout via
this.payoutRepo.update(...) then re-reads with this.payoutRepo.findOne(...),
risking a race; change to perform the update and return the updated row in one
operation (use the repository or QueryBuilder update with .set({...}) .where({
id: payoutId }) and .returning('*') then .execute()) and extract the returned
row (from result.raw or result.generatedMaps) to return directly; keep the same
fields being set (status: PayoutStatus.PENDING, failureReason: null,
stellarTxHash: null, processedAt: null) and ensure you verify
PostgreSQL/RETURNING support in production before merging.
In `@backend/src/social-sharing/referral.service.spec.ts`:
- Line 85: Remove the unused ReferralController from the test module in the
referral service unit tests: locate the TestingModule setup in
referral.service.spec.ts that includes controllers: [ReferralController] and
delete the controllers entry and any unused import of ReferralController so the
spec only instantiates ReferralService (and its required providers/mocks); this
reduces unnecessary coupling and prevents controller dependency breakage.
In `@backend/src/tips/events/tip-verified.event.ts`:
- Around line 8-15: The class exposes duplicate sender identity fields (senderId
and senderUserId); pick a single canonical property (e.g., senderUserId) and
remove the redundant one, or preserve the old name only as a deprecated alias.
Update the constructor signature (constructor(..., senderUserId: string)) and
assignments in the constructor (assign to this.senderUserId only), and update
any consumers of TipVerifiedEvent, TipVerifiedEvent.constructor, or spreading of
the event to use the chosen canonical symbol; if you must keep the old name for
compatibility, keep senderId only as a getter or deprecated alias that returns
this.senderUserId and add a comment marking it deprecated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93689931-f0a7-4a35-a8d6-90f83e732cde
📒 Files selected for processing (15)
backend/src/app.module.tsbackend/src/artiste-payout/create-payout.dto.tsbackend/src/artiste-payout/payout-request.entity.tsbackend/src/artiste-payout/payouts.controller.spec.tsbackend/src/artiste-payout/payouts.controller.tsbackend/src/artiste-payout/payouts.integration.spec.tsbackend/src/artiste-payout/payouts.module.tsbackend/src/artiste-payout/payouts.service.spec.tsbackend/src/artiste-payout/payouts.service.tsbackend/src/auth/decorators/current-user.decorator.tsbackend/src/social-sharing/referral.controller.spec.tsbackend/src/social-sharing/referral.controller.tsbackend/src/social-sharing/referral.service.spec.tsbackend/src/social-sharing/referral.service.tsbackend/src/tips/events/tip-verified.event.ts
Owner
|
LGTM |
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.
Closes #326
Closes #327
Summary
This PR completes the referral and payout backend fixes.
#326 Referral flow
tip.verifiedtrigger#327 Payout flow
Summary by CodeRabbit
New Features
Bug Fixes
Security