PM-1099 withdraw action #45
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -12,7 +12,9 @@ export class PaymentMethodRepository { | |||
| * @param userId user id | |||
| * @param tx transaction | |||
| */ | |||
| async hasVerifiedPaymentMethod(userId: string): Promise<boolean> { | |||
| async hasVerifiedPaymentMethod( | |||
There was a problem hiding this comment.
Can we please rename this to getConnectedPaymentMethod as it is more explainatory and makes sense after recent changes.
|
|
||
| export class WithdrawRequestDto { | ||
| @ApiProperty({ | ||
| description: 'The ID of the winnings to withdraw', |
There was a problem hiding this comment.
Can we rename winnings to payments?
| }) | ||
| @IsArray() | ||
| @ArrayNotEmpty() | ||
| @IsString({ each: true }) |
There was a problem hiding this comment.
Probably we could refactor this a bit by using @IsUUID decorator as payment IDs are UUIDs?
| const winningIds = (payload.externalId ?? '').split(',').slice(0, -1); | ||
| const externalTransactionId = payload.batch.id; | ||
|
|
||
| if (!winningIds.length) { |
There was a problem hiding this comment.
Code should throw here. Noting of the following logic will or shouldn't work without winningIds.
Agian, why we call those winnings as they are actually payment IDs? Can we update all of the refs and service signatures to reflect that?
| this.logger.error( | ||
| `Failed to process trolley payment batch: ${error.message}`, | ||
| ); | ||
| throw new Error('Failed to process trolley payment batch!'); |
There was a problem hiding this comment.
We are sure that throwing here will revert the transactions for payment & payment_release from above?
There was a problem hiding this comment.
No, it won't... good catch! updating.
src/api/winnings/winnings.service.ts
Outdated
| @@ -74,7 +74,7 @@ export class WinningsService { | |||
| body.winnerId, | |||
| ); | |||
| const hasPaymentMethod = | |||
| await this.paymentMethodRepo.hasVerifiedPaymentMethod(body.winnerId); | |||
| await this.paymentMethodRepo.getConnectedPaymentMethod(body.winnerId); | |||
There was a problem hiding this comment.
The method name getConnectedPaymentMethod suggests it retrieves a payment method, but the variable hasPaymentMethod implies a boolean value. Consider renaming the variable to better reflect the return type or ensure the method returns a boolean if the intention is to check for a connected payment method.
https://topcoder.atlassian.net/browse/PM-1099 - Implement /withdraw endpoint in tc-finance API
https://topcoder.atlassian.net/browse/PM-1160 - Payment processed event handler
https://topcoder.atlassian.net/browse/PM-1167 - Payment failed & returned event handler