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.

Sign up for GitHub

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

Jump to bottom

Fixes AC with more then 4 publishers #6164

Merged
merged 1 commit into from
Jul 22, 2020
Merged

Fixes AC with more then 4 publishers #6164

merged 1 commit into from
Jul 22, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Jul 22, 2020 •
edited
Loading

Resolves brave/brave-browser#10834

Submitter Checklist:

Test Plan:

  • covered with browser tests

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 1.13.x - Nightly milestone Jul 22, 2020
@NejcZdovc NejcZdovc requested a review from a team July 22, 2020 08:53
@NejcZdovc NejcZdovc self-assigned this Jul 22, 2020
@NejcZdovc NejcZdovc requested review from emerick and zenparsing and removed request for a team July 22, 2020 08:53
NejcZdovc
NejcZdovc commented Jul 22, 2020
View reviewed changes
vendor/bat-native-ledger/src/bat/ledger/internal/contribution/contribution.cc
@@ -708,7 +708,8 @@ void Contribution::SetRetryCounter(ledger::ContributionInfoPtr contribution) {
return;
}

if (contribution->retry_count == 3) {
if (contribution->retry_count == 3 &&
contribution->step != ledger::ContributionStep::STEP_PREPARE) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the fix, so we should ignore limit of 3 retries for step STEP_PREPARE

@NejcZdovc
Copy link
Contributor Author

CI failed on SpeedReaderBrowserTest.SmokeTest (known thing), all good

emerick
emerick approved these changes Jul 22, 2020
View reviewed changes
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@NejcZdovc NejcZdovc merged commit fed6ef9 into master Jul 22, 2020
@NejcZdovc NejcZdovc deleted the ac-retries branch July 22, 2020 16:03
brave-builds pushed a commit that referenced this pull request Jul 22, 2020
@brave-browser-releases
Uplift of #6164 (squashed) to beta
2c448f7
@brave-builds brave-builds mentioned this pull request Jul 22, 2020
Merged
4 tasks
@kjozwiak kjozwiak mentioned this pull request Jul 23, 2020
Closed
@LaurenWags
Copy link
Member

Verified using

Brave | 1.13.40 Chromium: 84.0.4147.89 (Official Build) nightly (64-bit)
-- | --
Revision | 19abfe7bcba9318a0b2a6bc6634a67fc834aa592-refs/branch-heads/4147@{#852}
OS | macOS Version 10.14.6 (Build 18G3020)

Verified STR from brave/brave-browser#10834 using various wallet types:

  • VG funded wallet:

VG

  • Anon user deposited wallet:

deposited BAT

  • Uphold wallet:

Uphold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@zenparsing zenparsing zenparsing approved these changes

Assignees

@NejcZdovc NejcZdovc

Labels
feature/rewards
Projects
None yet
Milestone
1.13.x - Release #1
Development

Successfully merging this pull request may close these issues.

[Desktop] Auto-contribute doesn't process if more than four publishers are on the list
4 participants
@NejcZdovc @LaurenWags @emerick @zenparsing