Skip to content
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

Move data from ledger_state into preferences #5647

Merged
merged 6 commits into from
Jun 3, 2020
Merged

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented May 25, 2020

Resolves brave/brave-browser#9869
Resolves brave/brave-browser#9750
Resolves brave/brave-browser#7165

Submitter Checklist:

Test Plan:

plan 1:

  • start browser from mater
  • enable rewards
  • set values in AC settings
  • close the browser
  • switch to this PR
  • start the browser
  • go to AC settings and make sure that all are the same
  • go rewards internals and make sure that wallet info matches as well
  • (try different upgrade paths as well)

plan 2:

  • enable rewards
  • claim grant
  • make sure that you see total balance with rates correctly

plan 3:

  • enable rewards
  • go to unverified publisher and open tip and monthly banner. Make sure that you see default values (currently 1, 10, 100) in there

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@NejcZdovc NejcZdovc self-assigned this May 25, 2020
@NejcZdovc NejcZdovc changed the title Removes un-needed saves Move data from ledger_state into preferences May 25, 2020
@NejcZdovc NejcZdovc force-pushed the ledger-state-move branch 2 times, most recently from 6d428b1 to abdb64d Compare May 25, 2020 11:34
@@ -746,8 +744,6 @@ void RewardsServiceImpl::OnWalletInitialized(ledger::Result result) {
ready_.Signal();

if (result == ledger::Result::WALLET_CREATED) {
SetRewardsMainEnabled(true);
SetAutoContribute(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NejcZdovc NejcZdovc force-pushed the ledger-state-move branch 7 times, most recently from 69c4697 to 7f205ab Compare May 27, 2020 11:23
@NejcZdovc NejcZdovc mentioned this pull request May 27, 2020
32 tasks
@NejcZdovc NejcZdovc force-pushed the ledger-state-move branch 9 times, most recently from f585505 to 288ab76 Compare June 1, 2020 17:02
@NejcZdovc NejcZdovc marked this pull request as ready for review June 1, 2020 19:30
Copy link
Collaborator

@kylehickinson kylehickinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iOS looks good to me. will require a few changes on client side

Copy link
Collaborator

@bridiver bridiver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the chromium_src changes are fine, but I think we're missing most of the benefit of using prefs here by keeping async methods going back and forth to access every value

@NejcZdovc NejcZdovc added CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Jun 2, 2020
@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Jun 2, 2020

CI passed on window, linux and ios. Restarting macos and android

@NejcZdovc NejcZdovc added CI/skip Do not run CI builds (except noplatform) and removed CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Jun 2, 2020
@NejcZdovc NejcZdovc removed the CI/skip Do not run CI builds (except noplatform) label Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants