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

WIP: Rewards data can now be cleared #1187

Closed
wants to merge 1 commit into from
Closed

WIP: Rewards data can now be cleared #1187

wants to merge 1 commit into from

Conversation

jasonrsadler
Copy link
Contributor

@jasonrsadler jasonrsadler commented Dec 24, 2018 •
edited
Loading

Fixes brave/brave-browser#926

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan 1:

  • Start Brave - Clean profile - Do not enable rewards
  • Go to settings, then clear rewards data
  • Check "Clear All Rewards" and make sure "Clear Rewards Auto-Contribute Sites" is grayed out.
  • Uncheck "Clear All Rewards" and make sure "Clear Rewards Auto-Contribute Sites" is available.
  • Clear Auto-Contribute Sites and make sure the browser does not crash.
  • Reopen Clear Rewards Data and check Clear All Rewards and press clear data.
  • Make sure you get a warning about Rewards data loss.
  • Press clear data and make sure browser does not crash

Test Plan 2:

  • Start Brave - Clean profile - Enable rewards
  • Visit a site for the required amount of time to place in auto-contribute list
  • Go to settings, then clear rewards data
  • Observe that Auto-Contribute Sites option shows "1 site"
  • Clear Auto-Contribute Sites
  • Open Rewards panel and make sure that auto-contribute list is empty

Test Plan 3:

  • Start Brave - Clean profile - Enable rewards
  • Visit a few sites for the required amount of time to place in auto-contribute list
  • Go to settings, then clear rewards data
  • Observe that Auto-Contribute Sites option shows "(#) sites"
  • Clear Auto-Contribute Sites
  • Open Rewards panel and make sure that auto-contribute list is empty

Test Plan 4:

  • Start Brave with a reconcile interval - Clean profile - Enable rewards
  • Visit a few sites for the required amount of time to place in auto-contribute list
  • Go to settings, then clear rewards data
  • When contribution starts, make sure checkboxes are grayed out and that you see a message about not being able to clear during contribution
  • close clear rewards
  • reopen. Make sure text under auto-contribute says 'None since your last contribution on [date of contribution]'

Test Plan 5:

  • Start Brave - Clean profile - Enable rewards
  • Visit a few sites for the required amount of time to place in auto-contribute list
  • Go to settings, then clear rewards data
  • Select "Clear All Rewards Data"
  • When the warning pops up, click "Cancel"
  • Go to Rewards and make sure Rewards data was not cleared.

Test Plan 6:

  • Start Brave - Clean profile - Enable rewards
  • Visit a few sites for the required amount of time to place in auto-contribute list
  • Visit some sites and make some tips
  • Go to settings, then clear rewards data
  • Select "Clear All Rewards Data" and clear the data
  • Go to Rewards and make sure all data in auto-contribute and tips is cleared
  • Make sure the correct amount of BAT is still retained

————————

Additional Info

  • When clearing only Auto-Contribute data, only the sites listed in the Auto-Contribute table (including the ones listed under "See All XX Sites" will be cleared. Auto-Contribute data from previous months is not affected and no other data that is visible to users is affected.

  • Auto-Contribute data for previous months is untouched if a user selects to only "Clear Auto-Contribute Data". Auto-Contribute data for previous months is discarded if a user selects to "Clear All Rewards Data".

  • BAT designated for unverified publishers through tipping is not affected by "Clear Auto-Contribute Data". BAT designated for unverified publishers through tipping WILL be discarded if "Clear All Rewards Data" is selected.

  • Tips will be discarded whether recurring or one time if "Clear All Rewards" data is selected. Tips are not affected by "Clear Auto-Contribute"

  • Past Auto-Contribute sites are currently not visible in the UI and past Auto-Contribute sites that didn't make vote count would also not be visible in the UI and would be discarded when "Clear All Rewards Data" is selected.

  • Clearing any Rewards data only affects data available in publisher_info_db. The amount of BAT in a user's wallet is not affected in any way

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

@jasonrsadler jasonrsadler self-assigned this Dec 24, 2018
@jasonrsadler jasonrsadler reopened this Jan 19, 2019
@jasonrsadler
Copy link
Contributor Author

@mandar-brave @davidtemkin Looking for feedback for text for clearing rewards data boxes:

Rewards auto-contribute shows number of sites currently in auto-contribute list (current reconciliation period):
screen shot 2019-02-04 at 1 18 18 pm

All rewards:
screen shot 2019-02-04 at 1 19 24 pm

If all rewards is checked and clear data button is pushed:
screen shot 2019-02-04 at 1 20 08 pm

@jasonrsadler jasonrsadler reopened this Feb 4, 2019
@davidtemkin
Copy link

@jasonrsadler how would a user back up their transaction history?

I'd go with less text: "This action cannot be undone. All transaction history and reports will no longer be available. Please note that Brave does not store a copy of your data on our servers, so deleting this data from your browser is final."

Also I'd recommend we default to "Cancel" rather than "Clear data" (default button)

@jasonrsadler
Copy link
Contributor Author

@davidtemkin backup will be implemented in brave/brave-browser#930

Will use your text and default to 'Cancel'

Thanks 🙂

@jasonrsadler jasonrsadler reopened this Feb 9, 2019
@jasonrsadler jasonrsadler reopened this Feb 9, 2019
@jasonrsadler jasonrsadler requested review from NejcZdovc, bbondy, bridiver and ryanml and removed request for NejcZdovc, bbondy, bridiver and ryanml February 10, 2019 21:38
@jasonrsadler jasonrsadler requested a review from bridiver March 25, 2019 19:50
@jasonrsadler jasonrsadler requested a review from bridiver March 25, 2019 22:17
bridiver
bridiver reviewed Mar 26, 2019
View reviewed changes
browser/ui/brave_pages.cc Outdated Show resolved Hide resolved
bridiver
bridiver requested changes Mar 26, 2019
View reviewed changes
Copy link
Collaborator

@bridiver bridiver left a comment

Choose a reason for hiding this comment

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

see comments

components/brave_rewards/browser/rewards_service_impl.cc Show resolved Hide resolved
components/brave_rewards/browser/rewards_service_impl.cc Outdated Show resolved Hide resolved
components/brave_rewards/browser/rewards_service_impl.cc Outdated Show resolved Hide resolved
components/brave_rewards/browser/rewards_service_impl.cc Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/ledger_impl.cc Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/ledger_impl.cc Show resolved Hide resolved
browser/BUILD.gn Outdated Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/ledger_impl.h Show resolved Hide resolved
bridiver
bridiver reviewed Mar 26, 2019
View reviewed changes
components/brave_rewards/browser/brave_rewards_data_remover_delegate.cc Outdated
int origin_type_mask,
base::OnceClosure callback) {

ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is going to call the callback before we've even started to remove the rewards data. base::DoNothing below should instead call ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData

Fixes brave/brave-browser#926
1801275 
linting fixes and generated string fixes

Added actual removal implementation.

Updated return value for counters on rewards not enabled.

Removed unused and added resource for clear all rewards modal.

patch updates

Refreshes Rewards dashboard after clear.

Refactoring how to get current contributions in progress

Rebase to Cr 73

Fix merge

Repurpose clear data to separate Rewards section

Included routing patch

Corrected pref names

Refactored dialog and counters

Disables for contribution in progress

Impemented footer details when a contribution is in progress

Update patches to move basic to advanced (privacy) section

Removed unused callbacks. Corrected checkbox pairing after ac completion

Contribution now checked on counter.

Removed logging

Modified sql parameters to use ledger values

Cleanup header guards, linting, license header

Fix existing tests

Style tests

webui: paper-button gets 'warn' style

minor UI cleanup

Address gh comments

Implemented bitwise operation into RewardsService for clearing rewards data

UI fixes

Fix missing arg list

Removing Rewards specific impl

Remover reimplemnt

Updated ledger db access

Fixing database calls

Moved contribution monitor logic out of native ledger

Corrected naming for contribution in progress method

Method cleanups

Moving db calls to taskrunner

Clean up RewardsCounter

Refactor native ledger and database call

Moved anon functions to anon namespace

patch updates

Function rename and override delegate header

Updated db to go through ledger

minor cleanup

updated tests

Make clear rewards link from single point brave_privacy

remove commented lines

Updated task_runner functions.

Moved components/rewards_data

Moved delegate and rewards items to components/brave_rewards/browser

crlf

Moved pref registration

removed header

Removed unused functions

Removed headers

Addressing PR comments

addressing PR comments

Fix converting to date string

Fix crlf

Remove unused vars

Removed unused function

Removed unused mock
@bbondy bbondy removed their request for review April 18, 2019 17:29
@NejcZdovc NejcZdovc removed this from the 0.65.x - Dev milestone Apr 19, 2019
@NejcZdovc NejcZdovc changed the title Rewards data can now be cleared WIP: Rewards data can now be cleared May 20, 2019
@NejcZdovc NejcZdovc removed their request for review May 20, 2019 10:14
@bsclifton
Copy link
Member

Closing as stale

@bsclifton bsclifton closed this Jul 12, 2019
@NejcZdovc NejcZdovc added this to the Closed milestone Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NejcZdovc NejcZdovc NejcZdovc left review comments

@fmarier fmarier fmarier left review comments

@bridiver bridiver bridiver requested changes

@ryanml ryanml Awaiting requested review from ryanml

@petemill petemill Awaiting requested review from petemill

Assignees

@jasonrsadler jasonrsadler

Labels
feature/rewards
Projects
None yet
Milestone
Closed
Development

Successfully merging this pull request may close these issues.

Add clear rewards data and clear payment data
10 participants
@jasonrsadler @davidtemkin @NejcZdovc @mandar-brave @fmarier @petemill @bsclifton @bridiver @ryanml @bbondy