Fix transaction #104
Fix transaction #104
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }); | ||
| async completedIdentityVerification( | ||
| userId: string, | ||
| tx?: Prisma.TransactionClient, |
There was a problem hiding this comment.
[maintainability]
Consider documenting the tx parameter to clarify its purpose and usage. This will improve the maintainability and readability of the function, especially for developers unfamiliar with transaction handling.
| }, | ||
| }); | ||
| const connectedUserPaymentMethod = await ( | ||
| tx || this.prisma |
There was a problem hiding this comment.
[💡 readability]
The use of (tx || this.prisma) is technically correct, but it could be improved for clarity. Consider explicitly checking if tx is defined and using it, otherwise default to this.prisma. This can help avoid potential confusion about the precedence of the operations.
| userId: string, | ||
| tx?: Prisma.TransactionClient, | ||
| ): Promise<boolean> { | ||
| const count = await (tx || this.prisma).user_tax_form_associations.count({ |
There was a problem hiding this comment.
[💡 readability]
The use of (tx || this.prisma) is correct for supporting transactions, but it could be improved for clarity. Consider explicitly checking if tx is defined and using it, otherwise defaulting to this.prisma. This makes the intention clearer and avoids potential confusion.
| const payrollPaymentMethod = await this.prisma.payment_method.findFirst({ | ||
| private async setPayrollPaymentMethod( | ||
| userId: string, | ||
| tx?: Prisma.TransactionClient, |
There was a problem hiding this comment.
[maintainability]
The addition of the optional tx parameter allows for transaction handling, which is beneficial for atomic operations. However, ensure that all calls to setPayrollPaymentMethod are reviewed to pass the transaction client where necessary, to maintain consistency and avoid potential issues with transaction management.
|
|
||
| if ( | ||
| await this.prisma.user_payment_methods.findFirst({ | ||
| await (tx || this.prisma).user_payment_methods.findFirst({ |
There was a problem hiding this comment.
[❗❗ correctness]
The use of (tx || this.prisma) suggests that a transaction object tx might be passed in some cases. Ensure that tx is always a valid Prisma transaction object when provided, as misuse could lead to unexpected behavior or errors. Consider adding validation or type-checking if this is not guaranteed by the surrounding logic.
|
|
||
| const hasActiveTaxForm = await this.taxFormRepo.hasActiveTaxForm( | ||
| body.winnerId, | ||
| tx, |
There was a problem hiding this comment.
[correctness]
Passing the transaction object tx to hasActiveTaxForm is a good practice for ensuring that all database operations are part of the same transaction. However, ensure that the hasActiveTaxForm method is designed to handle transactions correctly and that it doesn't introduce any side effects or unexpected behavior.
| await this.paymentMethodRepo.getConnectedPaymentMethod(body.winnerId), | ||
| await this.paymentMethodRepo.getConnectedPaymentMethod( | ||
| body.winnerId, | ||
| tx, |
There was a problem hiding this comment.
[correctness]
Ensure that the getConnectedPaymentMethod method is capable of handling the transaction object tx correctly. This is important to maintain transactional integrity and avoid potential issues with database operations.
| const isIdentityVerified = | ||
| await this.identityVerificationRepo.completedIdentityVerification( | ||
| body.winnerId, | ||
| tx, |
There was a problem hiding this comment.
[correctness]
Verify that the completedIdentityVerification method is designed to work with the transaction object tx. This ensures that the identity verification check is consistent with the transactional context.
| ); | ||
| paymentModel.payment_status = PaymentStatus.PAID; | ||
| await this.setPayrollPaymentMethod(body.winnerId); | ||
| await this.setPayrollPaymentMethod(body.winnerId, tx); |
There was a problem hiding this comment.
[❗❗ correctness]
The addition of the tx parameter to setPayrollPaymentMethod suggests that this method now requires a transaction context. Ensure that tx is correctly initialized and managed to prevent potential issues with transaction handling, such as incomplete or failed transactions.
|
|
||
| this.logger.debug('Attempting to create winning with nested payments.'); | ||
| const createdWinning = await this.prisma.winnings.create({ | ||
| const createdWinning = await tx.winnings.create({ |
There was a problem hiding this comment.
[❗❗ correctness]
Switching from this.prisma.winnings.create to tx.winnings.create suggests a transaction context is being used. Ensure that tx is correctly initialized and managed to avoid potential transaction-related issues, such as incomplete or failed transactions.
When creating payments, in the transaction, use the transaction context rather than opening new connections