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

Deprecate CONNECTED publisher status #15167

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Conversation

zenparsing
Copy link
Collaborator

@zenparsing zenparsing commented Sep 21, 2022

Resolves brave/brave-browser#25085

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  • Enable Rewards
  • Visit a publisher that is "connected, but not verified" (e.g. duckduckgo.com on staging)
  • Verify that publisher appears "not verified":
    • There is no blue checkmark on the Rewards icon
    • In the Rewards panel, the status is displayed as "Unverified Creator"
    • In the tipping banner, the "creator is not signed up yet" message is displayed
  • Send a tip using vBAT.
  • Verify that a pending tip has been created.

@@ -144,6 +145,12 @@ void DatabaseServerPublisherInfo::OnGetRecord(
info->updated_at = GetInt64Column(record, 2);
info->banner = banner.Clone();

// The `CONNECTED` status is deprecated. If this value appears in the database
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we are going to uplift I chose not to implement a migration. We can add a migration when we remove the CONNECTED state entirely.

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

@zenparsing zenparsing merged commit 930adc0 into master Sep 22, 2022
@zenparsing zenparsing deleted the ksmith-verified-publishers branch September 22, 2022 13:37
@github-actions github-actions bot added this to the 1.46.x - Nightly milestone Sep 22, 2022
brave-builds pushed a commit that referenced this pull request Sep 22, 2022
@LaurenWags
Copy link
Member

LaurenWags commented Sep 28, 2022

Verified with

Brave | 1.46.18 Chromium: 106.0.5249.65 (Official Build) nightly (x86_64)
-- | --
Revision | 3269dc3633cdd2ab94546fdbe54962e45b17a6e0-refs/branch-heads/5249@{#580}
OS | macOS Version 12.6 (Build 21G115)

Verified test plan from #15167 (comment)

Clean Profile Checks - PASSED
  • Confirmed that the "connected" publisher appears "not verified" (no blue checkmark on the Rewards icon)
  • Confirmed that in the Rewards panel, the status is displayed as "Unverified Creator" and appropriate hover text is shown
  • Confirmed that in the tipping banner, the "creator is not signed up yet" message is displayed
  • Confirmed no "verified" checkmark displays on the tipping banner
  • Confirmed that when tipping this "connected" creator, using vBAT, a pending tip is created
Example Example Example Example Example
1 2 3 4 5

Also confirmed that if a connected creator is listed in the Auto Contribution list, it does not get any AC BAT:

Example Example
2 3
Upgrade Profile Checks - Failed, follow up issue logged
  1. Install release version (1.44.x) w/ staging env and reconcile interval
  2. Close and relaunch (pull griffin seed)
  3. Enable Rewards, claim UGP grant
  4. Populate AC list with a mix of creators, including at least one "connected" creator
  5. Do a one time tip for a "connected" creator
  6. Set up a monthly tip for "connected" creator
  7. Close Brave
  8. Rename profile for Nightly
  9. Launch with Nightly version which contains this fix
  10. Look at brave://rewards page --> "connected" creator still displays the verified checkmark, expected?
  11. Wait for reconcile interval to occur so AC & monthly tips are contributed
  12. AC and monthly tips are completed --> however "connected" creator still gets the AC and monthly tip set up prior to upgrade, expected?
Step 4 Step 5 Step 6 Step 10 Step 12 Step 12 Step 12
4 5 6 10 12a 12b 12c

Follow up issue: brave/brave-browser#25699

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

Successfully merging this pull request may close these issues.

Update verified creator criteria
3 participants