Adds delays for suggestion api #4372

NejcZdovc · 2020-01-14T19:08:46Z

Resolves brave/brave-browser#7709
Resolves brave/brave-browser#6940

Submitter Checklist:

Test Plan:

Plan 1:

enable rewards (with short contribution)
claim promotion
add couple of monthly contributions
add couple of verified publishers to ac table
wait for contribution to be triggered
make sure that everything goes through and that reports and balance is reflected correctly

Plan 2:

enable rewards
claim promotion
do one time tips
make sure that everything is ok (summary and balance)

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.

kylehickinson

iOS specifics look good.

@NejcZdovc Are these all are going to be removed with DB migration? I assume we can't do much to implement them in its current state?

kylehickinson · 2020-01-21T17:29:01Z

vendor/brave-ios/Ledger/Generated/NativeLedgerClientBridge.h

+- (void)updateContributionInfoStepAndCount:(const std::string&)contribution_id step:(const ledger::ContributionStep)step retry_count:(const int32_t)retry_count callback:(ledger::ResultCallback)callback;
+- (void)updateContributionInfoContributedAmount:(const std::string&)contribution_id publisher_key:(const std::string&)publisher_key callback:(ledger::ResultCallback)callback;


I'd say we should stick with Obj-C naming convention here (i.e. retry_count → retryCount), however, if these methods are just going to be deleted with DB migration at a later date its fine to keep them how they are.

yup all of it will be removed with db migration

NejcZdovc · 2020-01-21T17:45:50Z

components/brave_rewards/browser/database/database_contribution_info_publishers.cc

@@ -157,8 +157,12 @@ bool DatabaseContributionInfoPublishers::InsertOrUpdate(
    return false;
  }

-  const std::string query = base::StringPrintf(
-    "INSERT OR REPLACE INTO %s "
+  const std::string query_delete = base::StringPrintf(


temp fix as we can't do db changes in 1.3. Issue that will resolve it in a right way brave/brave-browser#7838

NejcZdovc · 2020-01-21T19:12:34Z

CI failed on windows for test-scripts (https://ci.brave.com/job/brave-browser-build-pr/job/ac-delay/2/execution/node/225/log/), everything else is ok. Restarting windows

vendor/bat-native-ledger/include/bat/ledger/public/interfaces/ledger.mojom

vendor/bat-native-ledger/src/bat/ledger/internal/contribution/contribution_unblinded.cc

vendor/bat-native-ledger/src/bat/ledger/internal/contribution/contribution_unblinded.h

tmancey

LGTM++

Resolves brave/brave-browser#7709 Resolves brave/brave-browser#6940

SergeyZhukovsky

++

bsclifton · 2020-01-22T20:30:54Z

Merging as this is all approved and CI is passing 😄

LaurenWags · 2020-01-23T22:28:39Z

Verified using

Brave	1.5.53 Chromium: 79.0.3945.130 (Official Build) nightly (64-bit)
Revision	e22de67c28798d98833a7137c0e22876237fc40a-refs/branch-heads/3945@{#1047}
OS	macOS Version 10.14.6 (Build 18G103)

Verified Test Plan from Adds delays for suggestion api #4372
- Test plan 1 worked as expected. Note, AC processed slowly and the entire budget amount (in my case 10 BAT) was not removed from my wallet all at once. As the AC was processed the wallet balance went down 10.5 --> 9.3 --> 8 --> 7.5 --> etc until finally the entire amount was deducted from my balance. At this point, then Auto-Contribute was displayed on brave://rewards and panel wallet summary screens.

Test plan 2 worked as expected

Verified STR from entire a-c not going through due to missing delays brave-browser#7709
- Case 1 worked as expected

Case 2 is the same as test plan 1 from PR description, worked as expected.

Unable to verify STR from entire a-c not going through due to missing delays brave-browser#7709 (comment) - due to caching problem on staging balance server (see thread in #ledger)

Additionally, since PR test plan used UGP grants (virtual grants) only as a means of funding the wallet for testing, I ran the following additional scenarios:
A. Using a self-funded anon wallet did:

1 time tip
recurring tip
auto contribute
Then, for each of the above,
verified it was recorded on brave://rewards
verified it was recorded on the panel
verified wallet balance decreased as expected
verified on browser relaunch, wallet balance was not reduced
contribution_info and contribution_info_publishers tables were populated as expected

B. Using a KYC'd user wallet did:

1 time tip
recurring tip
auto contribute
Then, for each of the above,
verified it was recorded on brave://rewards
verified it was recorded on the panel
verified wallet balance decreased as expected
verified on browser relaunch, wallet balance was not reduced
contribution_info and contribution_info_publishers tables were populated as expected

Adds delays for suggestion api

NejcZdovc added the feature/rewards label Jan 14, 2020

NejcZdovc self-assigned this Jan 14, 2020

NejcZdovc force-pushed the ac-delay branch 5 times, most recently from d0e07b9 to 111c264 Compare January 20, 2020 13:19

NejcZdovc mentioned this pull request Jan 21, 2020

Add unique index to contribution_info_publishers brave/brave-browser#7838

Closed

NejcZdovc added this to the 1.5.x - Nightly milestone Jan 21, 2020

NejcZdovc marked this pull request as ready for review January 21, 2020 17:21

NejcZdovc requested a review from gdregalo as a code owner January 21, 2020 17:21

NejcZdovc force-pushed the ac-delay branch from 111c264 to f565d24 Compare January 21, 2020 17:24

NejcZdovc requested review from kylehickinson and tmancey as code owners January 21, 2020 17:24

kylehickinson approved these changes Jan 21, 2020

View reviewed changes

NejcZdovc commented Jan 21, 2020

View reviewed changes

NejcZdovc force-pushed the ac-delay branch from f565d24 to ec7a262 Compare January 21, 2020 17:57

NejcZdovc added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Jan 21, 2020

NejcZdovc requested a review from a team January 21, 2020 19:49

SergeyZhukovsky requested changes Jan 21, 2020

View reviewed changes

tmancey approved these changes Jan 22, 2020

View reviewed changes

NejcZdovc requested a review from SergeyZhukovsky January 22, 2020 10:17

NejcZdovc force-pushed the ac-delay branch from ec7a262 to 2da77ca Compare January 22, 2020 10:17

NejcZdovc removed CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Jan 22, 2020

NejcZdovc requested a review from SergeyZhukovsky January 22, 2020 16:47

NejcZdovc force-pushed the ac-delay branch 2 times, most recently from 26eaefb to 319bb49 Compare January 22, 2020 16:48

Adds delays for suggestion api

2ee5944

Resolves brave/brave-browser#7709 Resolves brave/brave-browser#6940

NejcZdovc force-pushed the ac-delay branch from 319bb49 to 2ee5944 Compare January 22, 2020 16:54

SergeyZhukovsky approved these changes Jan 22, 2020

View reviewed changes

bsclifton merged commit acbcf4f into master Jan 22, 2020

bsclifton deleted the ac-delay branch January 22, 2020 20:31

This was referenced Jan 23, 2020

no AC notifications in certain cases brave/brave-browser#7836

Closed

Adds cc scope for uphold #4379

Merged

This was referenced Jan 24, 2020

After excluding a site from A-C table Show All link is not shown in ac table brave/brave-browser#7840

Closed

Reserved BAT text isn't displayed in Rewards summary section brave/brave-browser#7635

Closed

bsclifton added a commit that referenced this pull request Jan 27, 2020

Merge pull request #4372 from brave/ac-delay

a4987c3

Adds delays for suggestion api

bsclifton mentioned this pull request Jan 27, 2020

Adds delays for suggestion api (uplift to 1.4) #4436

Merged

4 tasks

bsclifton added a commit that referenced this pull request Jan 27, 2020

Merge pull request #4372 from brave/ac-delay

0fcca2f

Adds delays for suggestion api

bsclifton mentioned this pull request Jan 27, 2020

WIP: Adds delays for suggestion api (uplift to 1.3) #4441

Closed

4 tasks

tmancey pushed a commit that referenced this pull request Jan 27, 2020

Merge pull request #4372 from brave/ac-delay

f7e3d4f

Adds delays for suggestion api

bsclifton added a commit that referenced this pull request Jan 28, 2020

Merge pull request #4372 from brave/ac-delay

28d731e

Adds delays for suggestion api

bsclifton mentioned this pull request Jan 28, 2020

Adds delays for suggestion api (uplift to 1.3) #4448

Merged

4 tasks

This was referenced Jan 28, 2020

Changing scope for Uphold brave/brave-browser#7713

Closed

wallet balance decreasing by AC amount each time browser is relaunched brave/brave-browser#7844

Closed

bsclifton modified the milestones: 1.5.x - Nightly, 1.4.x - Dev Jan 29, 2020

LaurenWags mentioned this pull request Feb 3, 2020

entire a-c not going through due to missing delays brave/brave-browser#7709

Closed

NejcZdovc mentioned this pull request Feb 11, 2020

Grant virtualization - Phase 2 brave/brave-browser#6899

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds delays for suggestion api #4372

Adds delays for suggestion api #4372

NejcZdovc commented Jan 14, 2020 •

edited

Loading

kylehickinson left a comment

kylehickinson Jan 21, 2020

NejcZdovc Jan 21, 2020

NejcZdovc Jan 21, 2020

NejcZdovc commented Jan 21, 2020

tmancey left a comment

SergeyZhukovsky left a comment

bsclifton commented Jan 22, 2020

LaurenWags commented Jan 23, 2020

		- (void)updateContributionInfoStepAndCount:(const std::string&)contribution_id step:(const ledger::ContributionStep)step retry_count:(const int32_t)retry_count callback:(ledger::ResultCallback)callback;
		- (void)updateContributionInfoContributedAmount:(const std::string&)contribution_id publisher_key:(const std::string&)publisher_key callback:(ledger::ResultCallback)callback;

Adds delays for suggestion api #4372

Adds delays for suggestion api #4372

Conversation

NejcZdovc commented Jan 14, 2020 • edited Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

After-merge Checklist:

kylehickinson left a comment

Choose a reason for hiding this comment

kylehickinson Jan 21, 2020

Choose a reason for hiding this comment

NejcZdovc Jan 21, 2020

Choose a reason for hiding this comment

NejcZdovc Jan 21, 2020

Choose a reason for hiding this comment

NejcZdovc commented Jan 21, 2020

tmancey left a comment

Choose a reason for hiding this comment

SergeyZhukovsky left a comment

Choose a reason for hiding this comment

bsclifton commented Jan 22, 2020

LaurenWags commented Jan 23, 2020

NejcZdovc commented Jan 14, 2020 •

edited

Loading