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 sku contribution flow for unblinded tokens #5671

Merged
merged 1 commit into from
May 28, 2020
Merged

Fixes sku contribution flow for unblinded tokens #5671

merged 1 commit into from
May 28, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented May 27, 2020 •
edited
Loading

Resolves brave/brave-browser#9962

Submitter Checklist:

Test Plan:

plan 1:

  • enable rewards
  • add verified publisher to AC list
  • connect KYC uphold wallet
  • wait for AC to trigger
  • make sure that contribution goes through

plan 2:

  • enable rewards
  • claim grant
  • tip one verified publisher
  • make sure that tip goes 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 1.11.x - Nightly milestone May 27, 2020
@NejcZdovc NejcZdovc requested a review from a team May 27, 2020 14:29
@NejcZdovc NejcZdovc self-assigned this May 27, 2020
NejcZdovc
NejcZdovc commented May 27, 2020
View reviewed changes
vendor/bat-native-ledger/src/bat/ledger/internal/contribution/contribution_unblinded.cc
@@ -393,8 +396,13 @@ void Unblinded::OnProcessTokens(
redeem.token_list = token_list;
redeem.contribution_id = contribution->contribution_id;

credentials_->RedeemTokens(redeem, redeem_callback);
return;
if (redeem.processor == ledger::ContributionProcessor::UPHOLD ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for this two processors we need to go through different credentials flow as for brave funds

emerick
emerick approved these changes May 27, 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 force-pushed the sku-tokens-contribution branch from 793f876 to 9e7a60b Compare May 27, 2020 19:42
@NejcZdovc NejcZdovc added CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels May 28, 2020
@NejcZdovc NejcZdovc merged commit 8bf0692 into master May 28, 2020
@NejcZdovc NejcZdovc deleted the sku-tokens-contribution branch May 28, 2020 06:22
@brave-builds brave-builds mentioned this pull request May 28, 2020
Merged
4 tasks
brave-builds pushed a commit that referenced this pull request May 28, 2020
@brave-browser-releases
Uplift of #5671 (squashed) to beta
5c256f8
@LaurenWags
Copy link
Member

LaurenWags commented May 29, 2020 •
edited
Loading

@NejcZdovc I'm still seeing the issue @GeetaSarvadnya described in brave/brave-browser#9962. Here are the STR I used:

plan 1 from above description:

Attempt 1:
enable rewards
add verified publisher to AC list (used DDG on staging which is a connected publisher)
connect KYC uphold wallet
wait for AC to trigger
--> AC does not go through

Attempt 2:
after above AC failed (on same profile), added a new verified publisher to AC list (used 3zsistemi.si on staging which is a KYC'd publisher)
wait for AC to trigger
--> AC does not go through

In both attempt 1 & 2 I get this:
Screen Shot 2020-05-29 at 3 11 22 PM

Seems as though AC is not working for me on Nightly with an Uphold wallet.

Brave | 1.11.37 Chromium: 83.0.4103.61 (Official Build) nightly (64-bit)
-- | --
Revision | 94f915a8d7c408b09cc7352161ad592299f384d2-refs/branch-heads/4103@{#561}
OS | macOS Version 10.14.6 (Build 18G3020)

Update - upon further inspection there seems to be a server side issue with staging env.

@LaurenWags
Copy link
Member

Verified passed with

Brave | 1.11.37 Chromium: 83.0.4103.61 (Official Build) nightly (64-bit)
-- | --
Revision | 94f915a8d7c408b09cc7352161ad592299f384d2-refs/branch-heads/4103@{#561}
OS | macOS Version 10.14.6 (Build 18G3020)

Plan 1:

  1. enable rewards (dev env)
  2. add verified publisher to AC list
  3. connect KYC uphold wallet
  4. wait for AC to trigger
  5. make sure that contribution goes through

Screen Shot 2020-05-29 at 4 02 18 PM

Screen Shot 2020-05-29 at 4 02 29 PM

Screen Shot 2020-05-29 at 4 04 45 PM

Plan 2:

  1. enable rewards (dev env)
  2. claim grant
  3. tip one verified publisher
  4. make sure that tip goes through

Screen Shot 2020-05-29 at 4 06 34 PM

Screen Shot 2020-05-29 at 4 06 48 PM

@GeetaSarvadnya GeetaSarvadnya mentioned this pull request Jun 2, 2020
Closed
petemill pushed a commit that referenced this pull request Jul 27, 2020
@bsclifton
Merge pull request #5671 from brave/incognito_marker_in_downloads_page
7351a29 
Replace incognito marker svg file
petemill pushed a commit that referenced this pull request Jul 28, 2020
@bsclifton
Merge pull request #5671 from brave/incognito_marker_in_downloads_page
6f39d3e 
Replace incognito marker svg file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zenparsing zenparsing zenparsing approved these changes

@emerick emerick emerick approved these changes

Assignees

@NejcZdovc NejcZdovc

Labels
CI/skip-ios Do not run CI builds for iOS feature/rewards
Projects
None yet
Milestone
1.11.x - Release #1
Development

Successfully merging this pull request may close these issues.

AC isn't going through for the connected publisher from KYC'd uphold account
4 participants
@NejcZdovc @LaurenWags @emerick @zenparsing