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

AC contribution change #3564

Merged
merged 1 commit into from
Oct 10, 2019
Merged

AC contribution change #3564

merged 1 commit into from
Oct 10, 2019

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Sep 30, 2019

Resolves brave/brave-browser#6228

Submitter Checklist:

Test Plan:

Plan 1

  • start with short contribution interval and staging
  • claim grant
  • add one verified publisher to monthly tip and set AC to 50 BAT
  • restart browser
  • you shouldn't receive notification about not enough funds

Plan 2

  • start with short contribution interval and staging
  • claim grant
  • add multiple verified publishers to monthly tip so that you are over balance
  • restart browser
  • you should receive notification about not enough funds

Plan 3

  • start with short contribution interval and staging
  • claim grant
  • add one verified publisher and multiple unverified publishers to that total is over balance
  • restart browser
  • you shouldn't receive notification about not enough funds

Plan 4

  • start with short contribution interval and staging
  • claim grant (30 BAT)
  • set AC budget to 50 BAT
  • add verified publisher to AC table
  • add verified monthly tips publishers so that total is 20 BAT
  • wait for contribution to happen
  • monthly tips and AC should go through, where AC amount would be 10

Plan 5

  • start with short contribution interval and staging
  • claim grant (30 BAT)
  • set AC budget to 20 BAT
  • populate AC table with verified and unverified publishers
  • wait for contribution to happen
  • make sure that 20 BAT went through and that only verified publishers received BAT

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 Sep 30, 2019
@NejcZdovc NejcZdovc added this to the 0.72.x - Nightly milestone Sep 30, 2019
@NejcZdovc NejcZdovc force-pushed the ac-change branch 2 times, most recently from 532b9af to 9036f88 Compare October 3, 2019 00:29
@NejcZdovc NejcZdovc marked this pull request as ready for review October 3, 2019 16:43
@NejcZdovc NejcZdovc force-pushed the ac-change branch 5 times, most recently from 98f0a01 to 1fd9f41 Compare October 7, 2019 14:57
@NejcZdovc NejcZdovc requested a review from a team October 7, 2019 15:01
@NejcZdovc NejcZdovc changed the title AC change AC contribution change Oct 8, 2019
ryanml
ryanml previously requested changes Oct 8, 2019
Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

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

Looks like there's a browser test failure

08:23:55  ../../brave/components/brave_rewards/browser/rewards_service_browsertest.cc:913: Failure
08:23:55  Expected equality of these values:
08:23:55    tip_reconcile_status_
08:23:55      Which is: Result::LEDGER_ERROR
08:23:55    result
08:23:55      Which is: Result::RECURRING_TABLE_EMPTY

@masparrow
Copy link
Contributor

masparrow commented Oct 9, 2019

Code-wise - LGTM
@NejcZdovc
Plan 1 & 3 seem ok for me - no notification.
Plan 2 - I didn't see any notification (tried twice - but could be me/user error)
Plan 4 (both of them :D) my monthly contribution date is set to Nov 7th... is there a way to advance this for testing?

@NejcZdovc
Copy link
Contributor Author

@masparrow you need to set short interval for plans to work npm start -- --rewards=reconcile-interval=3 --vmodule=*rewards*=6

Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

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

++

@NejcZdovc NejcZdovc merged commit 236bd5d into master Oct 10, 2019
@NejcZdovc NejcZdovc deleted the ac-change branch October 10, 2019 06:47
@LaurenWags
Copy link
Member

LaurenWags commented Oct 16, 2019

Verified passed with

Brave 0.72.102 Chromium: 78.0.3904.50 (Official Build) nightly (64-bit)
Revision 2accdc52c79976e264cd2694df6db31d1fccd8e8-refs/branch-heads/3904@{#658}
OS macOS Version 10.13.6 (Build 17G5019)
  • Verified test plan 1 worked as expected. Verified no message about not enough funds was displayed as BAT logo notification. Tip for verified, KYC'd publisher was contributed (confirmed tip on publishers dashboard).

  • Verified test plan 2 worked as expected. BAT logo notification about not enough funds was displayed after browser restart.

  • Verified test plan 3 worked as expected. Recurring monthly tips to unverified publishers were added to Pending Contributions list. Tip for verified, KYC'd publisher was contributed (confirmed tip on publishers dashboard). Verified no message about not enough funds was displayed as BAT logo notification.

  • Verified test plan 4 worked as expected. Recurring monthly tips for 20 BAT were deducted from balance first. Remaining balance (10 BAT) was contributed to the single verified, KYC'd publisher in the list after. Confirmed tip and AC amounts on publishers dashboard.

TP4-ss

  • Verified test plan 5 worked as expected. Auto Contribute went thru for 20 BAT. Only Verified publishers received BAT. Confirmed 20 BAT (less the 5% fee) AC amount on publisher dashboard. AC for unverified publishers were not listed in the Pending Contributions table.

TP5-ss

Also:

  • Verified recurring monthly tips showed in Monthly Contributions panel on clean profile.
  • Verified recurring monthly tips from previous version showed in Monthly Contributions panel on upgrade profile, not in Tips panel. Verified recurring monthly tips set up in previous profile were contributed after upgrade.
  • Verified Text/Label changes as outlined in the spec. Logged label changes for Auto Contribute - follow up to 6228 brave-browser#6483 for label changes that were missed/need to be updated. See slack for discussions.

label1

label2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Auto-Contribute to use "up to" budgeting, and to only contribute to verified content creators
4 participants