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

Ads per hour frequency cap incorrectly limits based on adsPerHour not adsPerDay #13215

Closed
moritzhaller opened this issue Dec 16, 2020 · 2 comments · Fixed by brave/brave-core#7454

Comments

@moritzhaller
Copy link

moritzhaller commented Dec 16, 2020

Description

ads frequency cap per hour is using the ads per hour value (5) instead of the ads per day value (20). This bug might result in under delivery of ads.

Steps to Reproduce

  1. change ads per day in the UI to 2
  2. view more than 2 ads in a given day
  3. try and trigger an ad in the 2nd hour

Actual result:

ad notification is not delivered due to an incorrect cap which is using ads per hour UI setting

Expected result:

ad notification should be delivered up to the correct cap of 20

Reproduces how often:

easily reproduced

Brave version (info found on brave://version)

introduced in 1.18.x

Version/Channel Information:

Nightly, Beta and Release

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

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

@moritzhaller moritzhaller changed the title Change daycap Change adsPerHour to adsPerDay in frequency cap Dec 16, 2020
@moritzhaller moritzhaller added this to New issues in Ads via automation Dec 16, 2020
@tmancey tmancey changed the title Change adsPerHour to adsPerDay in frequency cap Fix issue with ads per day incorrectly limited to ads per hour Dec 16, 2020
@tmancey tmancey added bug feature/ads priority/P1 A very extremely bad problem. We might push a hotfix for it. labels Dec 16, 2020
@tmancey tmancey moved this from New issues to Review in Ads Dec 16, 2020
@moritzhaller moritzhaller added OS/Android Fixes related to Android browser functionality OS/Desktop OS/iOS Fixes related to iOS browser functionality and removed release-notes/include labels Dec 16, 2020
@tmancey tmancey added OS/Android Fixes related to Android browser functionality and removed OS/Android Fixes related to Android browser functionality OS/iOS Fixes related to iOS browser functionality labels Dec 16, 2020
@moritzhaller moritzhaller changed the title Fix issue with ads per day incorrectly limited to ads per hour Fix issue with adPerHour frequency cap incorrectly limiting based on adsPerHour vs. adsPerDay Dec 16, 2020
@moritzhaller moritzhaller changed the title Fix issue with adPerHour frequency cap incorrectly limiting based on adsPerHour vs. adsPerDay Fix issue with adsPerHour frequency cap incorrectly limiting based on adsPerHour vs. adsPerDay Dec 16, 2020
@moritzhaller moritzhaller changed the title Fix issue with adsPerHour frequency cap incorrectly limiting based on adsPerHour vs. adsPerDay Fix issue with ads per hour frequency cap incorrectly limiting based on adsPerHour vs. adsPerDay Dec 16, 2020
@moritzhaller moritzhaller changed the title Fix issue with ads per hour frequency cap incorrectly limiting based on adsPerHour vs. adsPerDay Ads per hour frequency cap incorrectly limits based on adsPerHour not adsPerDay Dec 16, 2020
@LaurenWags LaurenWags added this to the 1.18.x - Release #2 milestone Dec 16, 2020
Ads automation moved this from Review to Done Dec 16, 2020
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Dec 17, 2020

Verification passed on


Brave | 1.18.74 Chromium: 87.0.4280.101 (Official Build) (64-bit)
-- | --
Revision | 9407c80213cda69c2b7abcb4fa8e3f74488f4956-refs/branch-heads/4280@{#1807}
OS | Windows 10 OS Version 2004 (Build 19041.685)

  • Verified the STR from the description
    Set Maximum number of ads displayed to 2 in ads panel.
  • Ensured that 1st AD is received immediately, waited for 30 mins and ensured the second AD is received, waited for another 30 mins and ensured 3rd AD is received in the second hour.
Ads panel Ads History
image image

Verification passed on Samsung Galaxy J3 version 5.1.1 running Bravearm 1.18.74

  • Verified the STR from the description
    Set Maximum number of ads displayed to 2 in ads panel.
  • Ensured that 1st AD is received immediately, waited for 30 mins and ensured the second AD is received, waited for another 30 mins and ensured 3rd AD is received in the second hour.
12-17 17:56:18.144 3307-3307/? V/chromium: [VERBOSE3:ad_notification_event_viewed.cc(34)] Viewed ad notification with uuid 4fa31759-e76c-4cbe-a9c1-f81805c5fc09 and creative instance id 74b76ca4-c445-4d93-94db-6c53306f10b6
12-17 18:35:58.926 3307-3307/? V/chromium: [VERBOSE3:ad_notification_event_viewed.cc(34)] Viewed ad notification with uuid a9a9db79-59a1-4245-a605-3c98451bfbbd and creative instance id 64deff55-7128-4024-b428-8005f49880c6
12-17 19:07:41.412 3307-3307/? V/chromium: [VERBOSE3:ad_notification_event_viewed.cc(34)] Viewed ad notification with uuid e6ad30c8-7910-4d26-978b-3496ec32dce6 and creative instance id 1253174d-a221-4c56-b5c7-22788e7cd948
Ads settings Ads panel
image image

Verified passed on macOS with

Brave | 1.18.74 Chromium: 87.0.4280.101 (Official Build) (x86_64)
-- | --
Revision | 9407c80213cda69c2b7abcb4fa8e3f74488f4956-refs/branch-heads/4280@{#1807}
OS | macOS Version 10.15.7 (Build 19H15)
  • Verified the STR from the description, confirmed Maximum number of ads displayed is set to 2 in ads panel.
  • Ensured that 1st AD is received immediately, waited for 30 mins and ensured the second AD is received, waited for another 30 mins and ensured 3rd AD is received in the second hour.
Ads settings Ads panel Ads History
Screen Shot 2020-12-17 at 9 42 56 AM Screen Shot 2020-12-17 at 10 44 54 AM Screen Shot 2020-12-17 at 10 45 02 AM
[12107:775:1217/094319.028362:VERBOSE3:ad_notification_event_viewed.cc(34)] Viewed ad notification with uuid 7ff40aad-c6f9-490c-86fc-6cf94ec2290c and creative instance id 30ecd70f-f56d-4745-a457-4035fcab54b6
[12107:775:1217/101401.498491:VERBOSE3:ad_notification_event_viewed.cc(34)] Viewed ad notification with uuid 63ebce4f-47df-46e5-b369-0fda809dbdde and creative instance id ed6c467a-8643-42eb-a7cd-32e878fa3cd4
[12107:775:1217/104438.751842:VERBOSE3:ad_notification_event_viewed.cc(34)] Viewed ad notification with uuid f1a8731b-11a7-41a4-89ef-2d4297e9471e and creative instance id 4983dc92-09fd-4e5c-94c3-616a4728c1d9

Verification passed on

Brave 1.18.74 Chromium: 87.0.4280.101 (Official Build) (64-bit)
Revision 9407c80213cda69c2b7abcb4fa8e3f74488f4956-refs/branch-heads/4280@{#1807}
OS Ubuntu 18.04 LTS
  • Verified the STR from the description
  • Verified that more that 2 ads are shown per day when limit per day is set to 2.
    image

Verification passed on Brave v1.18.74 on Samsung Galaxy Tab S5e (Android 9.0)

  • Verified the STR from the description
    Set Maximum number of ads displayed to 1 in ads panel.
Ads per hour Ads shown
Screenshot_20201217-192152_Brave Screenshot_20201217-192235_Brave

@andrewmclagan
Copy link

i mean... id be happy if this remained unresolved.

:-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Ads
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants