fix(background)!: better payment confirmation & message #694
Conversation
github-actions
bot
added
area: background
Extension builds preview
|
){kind=icon}
){kind=icon}
In case we decide to do the polling separately, we don't need to store entire initial outgoingPayment for reference.
[ci skip]
github-actions
bot
added
the
area: popup
sidvishnoi
changed the title
|
src/_locales/en/messages.json
Outdated
| "pay_error_outgoingPaymentFailed": { | ||
| "message": "We were unable to process this transaction. Please try again!", | ||
| "description": "OutgoingPayment.failed === true" |
There was a problem hiding this comment.
Failed should happen when outgoing_payment.failed = false and outgoing_payment.sent_amount === 0, since the OP can fail during sending and only send a partial amount.
There was a problem hiding this comment.
Just to be clear, OutgoingPayment.failed = true and OutgoingPayment.sent_amount > 0 is possible?
We want to treat any partial amount to be treated as success here?
There was a problem hiding this comment.
Just to be clear, OutgoingPayment.failed = true and OutgoingPayment.sent_amount > 0 is possible?
Yes.
We want to treat any partial amount to be treated as success here?
We cannot treat a sent partial amount as an error (failed = true && sent_amount = 0), since an amount was actually sent. We can use payStatus.type = 'partial' for this.
There was a problem hiding this comment.
Discussed cases with @raducristianpopa on Slack. We need to handle these cases:
failed = T; sent_amount = 0 => definitely error. immediate fail.
failed = F; sent_amount = 0 => we don't know yet. maybe warn user? It could be an error if check for long enough, or sent_amount might become non-zero
failed = T; sent_amount > 0 => ok, but only a partial was sent
failed = F; sent_amount > 0 => ok, but only a partial was sent, can be successfully completed afterwards
failed = T; sent_amount = debit_amount => not possible
failed = F; sent_amount = debit_amount => ok. immediate success.
@RabebOthmani How to best show this to user? For added context, after the payment, we've to repeatedly check payment status (polling). Right now, we check for up to 8 seconds only. The payment might be in any of above 5 states.
Also, note that, we'll be showing user a spinner for those 8s. Might be better to show "verifying payment status" message while we are in verify stage?
There was a problem hiding this comment.
8s is a long time for just a spinner. My unsolicited opinion is that we should show some kind of messaging for sure.
There was a problem hiding this comment.
Per Slack:
- " It looks like this is taking long. Check your wallet later to ensure the payment went through"
- Partial payment- only was successfully sent. to learn more
| const token = await this.rotateToken(); | ||
| const hasReadAccess = token.access_token.access.find( | ||
| (e) => e.type === 'outgoing-payment' && e.actions.includes('read'), | ||
| ); |
There was a problem hiding this comment.
Looks good to me. Lets just add a comment, that this specific part can be removed once we have the RS errors in place.
Yep, we treat it as regular payment, and as success ( |
|
@sidvishnoi all good on here ? Do you need more input from me? |
|
@RabebOthmani Two points actually (can be addressed as follow-up I guess, as this PR is pending since too long) (1)
(2)
|
Context
Changes proposed in this pull request
sentAmount == debitAmount.OutgoingPaymentstate, so even if polling isn't success, we may have partial payment.OutgoingPayment.failed = trueandsentAmount == 0pay({ amount })readpermission on outgoing-payment (breaking change as calls from existing connected extensions will throw on read), but avoided breaking change by catching the error and checkingdebitAmountfrom initial outgoing payment response like before.