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 when rewards is reset ads service should shut down #6284

Merged
merged 1 commit into from
Aug 3, 2020

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jul 31, 2020

Resolves brave/brave-browser#11035

Submitter Checklist:

Test Plan:

  • Confirm Ads state is deleted when reseting Brave Rewards
  • Confirm Confirmations state is deleted when reseting Brave Rewards
  • Confirm Ads state is not deleted when toggling Ads on/off switch
  • Confirm Ads is started and ads can be shown when disabling and then re-enabling ads

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.

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

@tmancey
Copy link
Collaborator Author

tmancey commented Aug 1, 2020

Linux and Windows failed CI, restarting.

CI failed for unrelated gn check and browser tests on other platforms

@tmancey tmancey added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Aug 1, 2020
@tmancey tmancey force-pushed the issues/11035 branch 2 times, most recently from f774ea3 to 0f89213 Compare August 3, 2020 07:49
@NejcZdovc NejcZdovc added this to the 1.14.x - Nightly milestone Aug 3, 2020
@tmancey tmancey requested a review from NejcZdovc August 3, 2020 10:00
@tmancey
Copy link
Collaborator Author

tmancey commented Aug 3, 2020

CI failed for unrelated gn_check and rewards browser tests

@LaurenWags
Copy link
Member

Discussed with @NejcZdovc and instead of testing on Nightly (which is blocked by #6248), this is being tested with PR build from https://bravesoftware.slack.com/archives/C5DT3GPCH/p1596464740328400

Brave | 1.12.106 Chromium: 84.0.4147.105 (Official Build) nightly (64-bit)
-- | --
Revision | a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS | macOS Version 10.14.6 (Build 18G3020)
  • Verified test plan from description.
  • Confirmed if rewards are reset, then Brave-Browser-Nightly/Default/ads_service folder is deleted.
  • Confirmed if rewards are reset, then Brave-Browser-Nightly/Default/rewards_service folder is deleted.
  • Confirmed re-enabling rewards creates these two folders again.
  • Confirmed toggling Ads off did not remove Brave-Browser-Nightly/Default/ads_service or Brave-Browser-Nightly/Default/rewards_service folders.
  • Confirmed no ads messages seen in logs when Ads were toggled off.
  • Confirmed if Ads are toggled off and then the browser is restarted, ads are still off after restart.
  • Confirmed Ads can be toggled on and ads messages start appearing in the logs.
  • Confirmed when Ads were toggled back on the ads panel information was not lost.
  • Confirmed when re-enabling Ads that ads were shown based on max per hour setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64 feature/ads
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When Rewards is reset ads service should shut down
4 participants