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

Enforcing max-ads-per-hour at System Level #8368

Closed
jonathansampson opened this issue Feb 23, 2020 · 5 comments
Closed

Enforcing max-ads-per-hour at System Level #8368

jonathansampson opened this issue Feb 23, 2020 · 5 comments
Assignees
Labels
bug closed/duplicate Issue has already been reported feature/ads priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes

Comments

@jonathansampson
Copy link
Contributor

jonathansampson commented Feb 23, 2020

Description

Users can generate more than 5 ads per hour and 20 per day.

Steps to Reproduce

  1. Create multiple profiles in Brave (I made 4)
  2. Opt all of them into Brave Rewards and Ads, and set max-per-hour to 5
  3. Close all secondary profiles (I did this only after all had shown their first ad)

Actual result:

When ads appear, they appear in multiples.

Expected result:

Users should only see ads for active profiles.
The maximum number of ads per hour should potentially be enforced beyond the profile.

Reproduces how often:

Easily

Brave version (brave://version info)

1.3.118 Chromium: 80.0.3987.116 (Official Build) (64-bit)

Version/Channel Information:

  • Can you reproduce this issue with the current release? Yes
  • Can you reproduce this issue with the beta channel? TBD
  • Can you reproduce this issue with the dev channel? TBD
  • Can you reproduce this issue with the nightly channel? TBD

Miscellaneous Information:

I continued to see ads delivered in sets even when only one profile instance was opened.

@jonathansampson
Copy link
Contributor Author

Restarting the browser will return to showing only 1 ad at a time. Opening (and then closing) windows for the additional profiles will return to the buggy behavior.

@bsclifton
Copy link
Member

Would be good to use local_state for this, versus profile settings

@tmancey
Copy link
Contributor

tmancey commented Feb 23, 2020

@bsclifton @jonathansampson Related to #7441 (however deciding whether to show an ad needs much more business logic based upon state stored in Default/ads_service/client.json. We are transitioning away from storing state in JSON to database(s) in #5150 and #5151 and can investigate how best to support multiple profiles.

@tmancey tmancey self-assigned this May 16, 2020
@tmancey tmancey added QA/Yes priority/P3 The next thing for us to work on. It'll ride the trains. labels May 16, 2020
@tmancey
Copy link
Contributor

tmancey commented May 16, 2020

Duplicate of #7441 and fixed by brave/brave-core#5574

@tmancey
Copy link
Contributor

tmancey commented May 16, 2020

@jonathansampson I have raised #9805 for "The maximum number of ads per hour should potentially be enforced beyond the profile."

@tmancey tmancey added the closed/duplicate Issue has already been reported label May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug closed/duplicate Issue has already been reported feature/ads priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes
Projects
Archived in project
Development

No branches or pull requests

5 participants