Introduces `FetchBalanceResult`. #17480

szilardszaloki · 2023-03-06T05:29:07Z

This PR introduces the base::expected<>-style API for the fetch balance flow, which makes it easier to add new error conditions on the backend (or fine-tune any existing ones), and to reflect them in the UI.

The rest of the external wallet provider flows (connect external wallet, get external wallet) already leverage base::expected<>-style APIs, except for external wallet transactions (BitFlyerTransfer, GeminiTransfer, UpholdTransfer), which go through the contribution engine.

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Before enabling Rewards, there's no balance info displayed on Rewards Internals anymore:

After enabling Rewards but before connecting a custodial wallet service, we only display vBAT balance on Rewards Internals (and the Connect account UI on the Rewards page/Rewards panel/Rewards NTP widget):

After connecting a custodial wallet service, we display both the vBAT/external wallet balance on Rewards Internals (and show the total balance on the Rewards page/Rewards panel/Rewards NTP widget):

In the logged out state, we only show vBAT balance on Rewards Internals (and the Logged out UI on the Rewards page/Rewards panel/Rewards NTP widget, including the tipping banner — :( You are currently logged out.):

On an error coming from a custodial wallet service balance endpoint, we show a spinner for both the vBAT/external wallet balance on Rewards Internals (and a spinner on the Rewards page/Rewards panel/Rewards NTP widget, except for the tipping banner, where we show the Server connection error UI):

components/brave_rewards/resources/tip/lib/interfaces.ts

components/definitions/rewards.d.ts

components/brave_rewards/resources/internals/components/general.tsx

emerick

LGTM

brave-builds · 2023-03-07T19:02:55Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2023-03-07T20:53:23Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2023-03-07T21:53:09Z

A Storybook has been deployed to preview UI for the latest push

deeppandya

Android changes LGTM

brave-builds · 2023-03-09T22:03:10Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2023-03-10T18:59:02Z

A Storybook has been deployed to preview UI for the latest push

GeetaSarvadnya · 2023-03-14T16:04:21Z

Verification PASSED on

Brave | 1.51.30 Chromium: 111.0.5563.64 (Official Build) nightly (64-bit)
-- | --
Revision | c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS | Windows 10 Version 21H2 (Build 19044.2604)

Verified the test plan from Introduces FetchBalanceResult. #17480 (comment)

1. Before enabling rewards

Confirmed that there is no Balance info displayed on the brave://rewards-internals page before enabling the rewards

2. After enabling rewards

Confirmed that after enabling the rewards the vBAT balance is shown on the Rewards Internals page
Confirmed that the Connect account is shown on the brave://rewards settings page, BR panel and rewards NTP widget

3. After connecting a custodial wallet

Confirmed that after connecting custodian wallet, both the vBAT/external wallet balance is displayed on the Rewards Internals page
Confirmed that after connecting custodian wallet, total balance is displayed on the Rewards page, Rewards panel and Rewards NTP widget

brave://settings	BR panel	rewards NTP widget	brave://rewards-internals

4. Logged out state

Confirmed that in the logged out state,the vBAT balanceis shown only on the Rewards Internals page
Confirmed that Logged out UI is shown on the brave://rewards, BR panel, Rewards NTP and Tipping banner
Confirmed that : ( You are currently logged out text message is shown on the tipping banner

brave://settings	BR panel	rewards NTP widget	Tipping banner	brave://rewards-internals

5. Fail the fetch balance request

Uphold wallet (pre-req: have Charles Proxy set up to return 500 error for the endpoint (https://api-sandbox.uphold.com/v0/me/cards/*)
Confirmed that spinner animation is shown for both the Rewards BAT (vBAT) and external wallet balance on brave://rewards-internals page

brave://settings	BR panel	rewards NTP widget	Tipping banner	brave://rewards-internals

Confirmed that the rewards NTP widget Log in to Uphold to see your balance padding issue is been resolved in 1.51.x

1.50.x	1.51.x

6. Reset rewards

Confirmed that after Reset rewards the wallet balance info is not displayed on the brave://rewards-internals page

kylehickinson · 2023-03-15T20:32:38Z

If uplifted also uplift #17628

szilardszaloki · 2023-03-16T13:18:57Z

If uplifted also uplift #17456
cc: @kjozwiak

* Uplift of #17480 (squashed) to beta * [iOS] Add flag to control namespace generation for Obj-C mojom wrappers * Remove note from Reset Rewards modal --------- Co-authored-by: Kyle Hickinson <[email protected]> Co-authored-by: Emerick Rogul <[email protected]>

szilardszaloki requested review from bsclifton, emerick and zenparsing March 6, 2023 05:29

szilardszaloki requested a review from a team as a code owner March 6, 2023 05:29

github-actions bot assigned szilardszaloki Mar 6, 2023

github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Mar 6, 2023

szilardszaloki force-pushed the sszaloki-28793-audit-network-errors-and-their-ui-effects branch 2 times, most recently from 9ba926d to 67339ce Compare March 6, 2023 20:50

zenparsing reviewed Mar 6, 2023

View reviewed changes

components/brave_rewards/resources/tip/lib/interfaces.ts Outdated Show resolved Hide resolved

components/definitions/rewards.d.ts Outdated Show resolved Hide resolved

components/brave_rewards/resources/internals/components/general.tsx Outdated Show resolved Hide resolved

emerick approved these changes Mar 7, 2023

View reviewed changes

szilardszaloki force-pushed the sszaloki-28793-audit-network-errors-and-their-ui-effects branch 6 times, most recently from 018c059 to 8f15a41 Compare March 7, 2023 18:10

szilardszaloki force-pushed the sszaloki-28793-audit-network-errors-and-their-ui-effects branch 2 times, most recently from cf4fbea to 1018d93 Compare March 7, 2023 20:12

kylehickinson requested a review from a team as a code owner March 7, 2023 21:13

szilardszaloki requested a review from deeppandya March 9, 2023 15:09

Brandon-T approved these changes Mar 9, 2023

View reviewed changes

zenparsing approved these changes Mar 9, 2023

View reviewed changes

deeppandya approved these changes Mar 9, 2023

View reviewed changes

szilardszaloki and others added 2 commits March 9, 2023 16:05

Introduces FetchBalanceResult.

a61c5a3

[iOS] Codegen ledger_types.mojom as well and update fetch balance call

59f4e1e

szilardszaloki force-pushed the sszaloki-28793-audit-network-errors-and-their-ui-effects branch from fc00389 to 59f4e1e Compare March 9, 2023 21:06

szilardszaloki changed the title ~~[WIP] Introduces FetchBalanceResult.~~ Introduces FetchBalanceResult. Mar 9, 2023

[iOS] Move mojo template compilation to a separate target

3ffae3c

wknapik merged commit b528d94 into master Mar 10, 2023

wknapik deleted the sszaloki-28793-audit-network-errors-and-their-ui-effects branch March 10, 2023 21:44

github-actions bot added this to the 1.51.x - Nightly milestone Mar 10, 2023

brave-builds added a commit that referenced this pull request Mar 10, 2023

Uplift of #17480 (squashed) to beta

e67051e

brave-builds mentioned this pull request Mar 10, 2023

Introduces FetchBalanceResult. (uplift to 1.50.x) #17567

Merged

7 tasks

GeetaSarvadnya mentioned this pull request Mar 13, 2023

Remove "Backup" tab in Manage section in brave://rewards brave/brave-browser#27547

Closed

4 tasks

szilardszaloki mentioned this pull request Mar 14, 2023

Do not chain calls to fetch balance from custodian brave/brave-browser#28557

Closed

LaurenWags mentioned this pull request Mar 24, 2023

Add base::expected<>-style API to the "fetch balance" flow brave/brave-browser#28793

Closed

szilardszaloki mentioned this pull request Apr 17, 2023

Don't use external_wallet_type to determine external wallet type. #18105

Merged

25 tasks

GeetaSarvadnya mentioned this pull request Apr 19, 2023

0.00 BAT showing in Rewards panel, even though user is logged in with custodian, and rewards-internals shows Loading... brave/brave-browser#29627

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduces `FetchBalanceResult`. #17480

Introduces `FetchBalanceResult`. #17480

szilardszaloki commented Mar 6, 2023 •

edited

Loading

emerick left a comment

brave-builds commented Mar 7, 2023

brave-builds commented Mar 7, 2023

brave-builds commented Mar 7, 2023

deeppandya left a comment

brave-builds commented Mar 9, 2023

brave-builds commented Mar 10, 2023

GeetaSarvadnya commented Mar 14, 2023 •

edited

Loading

kylehickinson commented Mar 15, 2023

szilardszaloki commented Mar 16, 2023 •

edited

Loading

Introduces FetchBalanceResult. #17480

Introduces FetchBalanceResult. #17480

Conversation

szilardszaloki commented Mar 6, 2023 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

emerick left a comment

Choose a reason for hiding this comment

brave-builds commented Mar 7, 2023

brave-builds commented Mar 7, 2023

brave-builds commented Mar 7, 2023

deeppandya left a comment

Choose a reason for hiding this comment

brave-builds commented Mar 9, 2023

brave-builds commented Mar 10, 2023

GeetaSarvadnya commented Mar 14, 2023 • edited Loading

kylehickinson commented Mar 15, 2023

szilardszaloki commented Mar 16, 2023 • edited Loading

Introduces `FetchBalanceResult`. #17480

Introduces `FetchBalanceResult`. #17480

szilardszaloki commented Mar 6, 2023 •

edited

Loading

GeetaSarvadnya commented Mar 14, 2023 •

edited

Loading

szilardszaloki commented Mar 16, 2023 •

edited

Loading