-
Notifications
You must be signed in to change notification settings - Fork 922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes duplicate/bad confirmations retry logic #5260
Conversation
a3e91af
to
960bd27
Compare
6a8e942
to
d949299
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just had one comment
vendor/bat-native-confirmations/src/bat/confirmations/internal/redeem_token.cc
Show resolved
Hide resolved
3817e52
to
406227a
Compare
Restarting
Restarting |
LGTM Verification passed on
Verified Test Case 1 (Create confirmation fails with 400 BAD REQUEST):
Test Case 2 (Create confirmation fails with 500 INTERNAL SERVER ERROR):
Test Case 3 (Create confirmation succeeds with 201 CREATED):
Test Case 4 (Fetch payment token fails with 404 NOT FOUND):
confirmations.json with failed confirmations
After disabling Charles overwrite. Confirmation was successfully redeemed
confirmations.json after successful confirmation
Test Case 5 (Fetch payment token fails with 500 INTERNAL SERVER ERROR):
confirmations.json after successful confirmation
Test Case 6
Test Case 7 (Fetch payment token succeeds with 200 OK):
|
Resolves brave/brave-browser#9242
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
@LaurenWags @btlechowski @kjozwiak If you could please check the below test plan, thanks
Pre-requisites:
Test Case 1 (Create confirmation fails with 400 BAD REQUEST):
EXPECTED RESULT: "Duplicate/bad confirmation" and "GET /v1/confirmation/{confirmation_id}/paymentToken" should appear in the console log
Test Case 2 (Create confirmation fails with 500 INTERNAL SERVER ERROR):
EXPECTED RESULT: "GET /v1/confirmation/{confirmation_id}/paymentToken" should appear in the console log
Test Case 3 (Create confirmation succeeds with 201 CREATED):
EXPECTED RESULT: "GET /v1/confirmation/{confirmation_id}/paymentToken" should appear in the console log
Test Case 4 (Fetch payment token fails with 404 NOT FOUND):
EXPECTED RESULT: Compare confirmations > failed_confirmations in
Default/rewards_service/confirmations.json
against the copy you made of this file to confirm a new confirmation is created and added to the retry queueEXPECTED RESULT: Failed confirmation should retry and "Successfully redeemed" should appear in the console log
Test Case 5 (Fetch payment token fails with 500 INTERNAL SERVER ERROR):
EXPECTED RESULT: Compare confirmations > failed_confirmations in
Default/rewards_service/confirmations.json
against the copy you made of this file to confirm the failed confirmation is re-added to the retry queueTest Case 6
Test Case 7 (Fetch payment token succeeds with 200 OK):
EXPECTED RESULT: "Successfully redeemed" should appear in the console log
Reviewer Checklist:
After-merge Checklist:
changes has landed on.