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

Fixes ac being processed when ac is off #3915

Merged
merged 1 commit into from Nov 8, 2019
Merged

Fixes ac being processed when ac is off #3915

merged 1 commit into from Nov 8, 2019

Conversation

@NejcZdovc
Copy link
Member

NejcZdovc commented Nov 7, 2019

Resolves brave/brave-browser#6716

Submitter Checklist:

Test Plan:

  • start browser with short interval
  • enable rewards
  • claim grant
  • add verified publisher to ac table
  • disable ac
  • wait for interval to be over
  • make sure that ac didn't go through

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 added this to the 0.74.x - Nightly milestone Nov 7, 2019
@NejcZdovc NejcZdovc requested a review from brave/rewards Nov 7, 2019
@NejcZdovc NejcZdovc self-assigned this Nov 7, 2019
@@ -251,6 +251,11 @@ bool Contribution::ShouldStartAutoContribute() {
}

void Contribution::StartAutoContribute(uint64_t reconcile_stamp) {
if (!ShouldStartAutoContribute()) {

This comment has been minimized.

Copy link
@NejcZdovc

NejcZdovc Nov 7, 2019

Author Member

this is the fix as we didn't checked if AC is enabled before we started it

@tmancey
tmancey approved these changes Nov 7, 2019
Copy link
Collaborator

tmancey left a comment

LGTM

" .getAttribute(\"data-toggled\") === 'false');"
" }"
" }"
"}, 500);});",

This comment has been minimized.

Copy link
@jhoneycutt

jhoneycutt Nov 7, 2019

Contributor

There's a WaitForSelector function that will handle some of this work for you.

@NejcZdovc NejcZdovc force-pushed the ac-check branch from 78c086a to d2546a2 Nov 7, 2019
@NejcZdovc NejcZdovc merged commit 1e3ae13 into master Nov 8, 2019
2 checks passed
2 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@NejcZdovc NejcZdovc deleted the ac-check branch Nov 8, 2019
brave-builds pushed a commit that referenced this pull request Nov 8, 2019
brave-builds pushed a commit that referenced this pull request Nov 8, 2019
brave-builds pushed a commit that referenced this pull request Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.