Fix/verify #228
Fix/verify #228
Conversation
|
|
||
| q := db.New() | ||
|
|
||
| email, err := q.GetEmailByTxnId(ctx, tx, req.TxnID) |
There was a problem hiding this comment.
Club booking details fetching and email fetching into single query using Common Table Expression or sub-queries in SQL and return the data together.
|
|
||
| q := db.New() | ||
|
|
||
| email, err := q.GetEmailByTxnId(ctx, tx, req.TxnID) |
There was a problem hiding this comment.
Again, same issue. Club the email and booking details in same query. Reduce DB calls on hot paths.
| return | ||
| } | ||
|
|
||
| // Only reverifying failed transactions |
There was a problem hiding this comment.
Remove the nesting!! Do if booking.TxnStatus != models.PaymentFailed then return back with no action taken and a corresponding HTTP status code.
| // if still failed | ||
| if gatewayStatus == models.PaymentFailed || gatewayStatus == models.PaymentNotFound { | ||
| // Restore released seats and close dispute | ||
| disputeId, err := q.GetDisputeIdFromTxnIdQuery(ctx, tx, req.TxnID) |
There was a problem hiding this comment.
Why to get dispute and then close the dispute. Just mark the dispute given the transaction id which is OPEN as CLOSED ?
| // if payment is success | ||
| if gatewayStatus == models.PaymentSuccess { | ||
| // Closing the dispute as valid payment | ||
| disputeId, err := q.GetDisputeIdFromTxnIdQuery(ctx, tx, req.TxnID) |
There was a problem hiding this comment.
Why can't close as True dispute just work on transaction ID ? Anyways, we are not allowing multiple dispute creation if one dispute for given transaction ID is open right ?
| @@ -0,0 +1,74 @@ | |||
| -- +goose Up | |||
There was a problem hiding this comment.
As you have changed the materialized view here and it's name. Have you informed @Naganathan05 about it ? Please make sure that the materialized view changes are in sync or they are dropped from this PR.
There was a problem hiding this comment.
@Nandgopal-R
Does this change introduce any new field to the view. If so can you just mention it so that I can include that in the upcoming analytics PR which is almost done. This change can be dropped from this PR in my opinion.
| } | ||
| } | ||
| } else { | ||
| teamId, err := q.GetTeamIDByBooking(ctx, tx, booking.ID) |
There was a problem hiding this comment.
You don't need teamId to come back into the controller. You only need teamMembers's studentIDs right ? Use a subquery or CTE and do it.
|
@Nandgopal-R do you want to fix these or just close the PR ? |
3 participants
No description provided.