-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
There is no ads enable alert, ads are enabled by default when upgrading build from 0.62.51 to 0.67.119 #5434
Comments
Reproduces on macOS.
Reproduces for US, UK (probably also Canada, France, Germany). |
Using US and UK regions:
Note: My expectation is that Canada, France, Germany all behave as US/UK do. Using US region only:
Using Italy region:
Testing for Australia, NZ, Ireland is outlined here: #5153 (comment) |
Additional tests done - all working as expected:
|
Thanks for all the hard work on this @LaurenWags & @GeetaSarvadnya. I'm going to remove the CCing @rebron @bsclifton & @jsecretan as this will need to get reprioritized for the next release/HF. |
Is this something we should even fix? Since we've already released something which introduces this problem (requiring manual intervention), I don't think there would be enough users on the old 0.62.x version |
@bsclifton I would say we should fix as the changes that were required to fix this PR will make migration easier for future versions too. |
@tmancey perfect - let's do it then 😄 |
…build from 0.62.51 to 0.67.119 Fixes brave/brave-browser#5434
@LaurenWags Will be in nightly 0.70.30 or above, if we could test asap that would be great |
@tmancey I can't test on Nightly, we didn't have Nightly versions for 0.62.x. I believe the first official Nightly version was 0.64.5 and Ads were at least in Dev by that point. |
@LaurenWags Could we copy a 0.62.x profile from Dev/Release? |
@tmancey I can try that. I don't think it's quite the same, but it might be the closest testing that can be done in Nightly. Will check and post results. |
@tmancey Seems as though there are some issues. Specifically, some cases which were working before are now not working. Please see results below: A. Using US and UK regions:
B. Using Australia region:
C. Using Italy region:
D. Using US region only:
|
@LaurenWags I have created #5531 to resolve the above issues. I now have profiles for 0.62.x and 0.63.x if that helps with testing. |
@tmancey please look at this scenario which failed from #5434 (comment) as well, I don't think #5531 covers it. As I said above/below, I think this will affect multiple versions not just 0.66.x: D. Using US region only:
|
@LaurenWags I have tested the above with #5531 and now working as expected. I am awaiting approval so I can merge with nightly |
@tmancey Verified the issue against version 0.69.128. Now ads enable alert is shown in 0.69.128 after upgrading build from 0.62.51 to 0.69.128 but Ads are enabled by default in brave://rewards page which is not expected. |
Blocked due to #6148 |
Verified passed with
B. Ads Off before upgrade
B. Ads Off before upgrade
B. Ads Off before upgrade
B. Ads Off before upgrade
Verification passed on
B. Ads Off before upgrade
B. Ads Off before upgrade
B. Ads Off before upgrade
B. Ads Off before upgrade
Verified passed with
A. Ads On before upgrade
B. Ads Off before upgrade
A. Ads On before upgrade
B. Ads Off before upgrade
A. Ads On before upgrade
B. Ads Off before upgrade
A. Ads On before upgrade
B. Ads Off before upgrade
|
Description
There is no ads enable alert, ads are enabled by default when upgrading build from 0.62.51 to 0.67.119
Steps to Reproduce
Pre-requisites: VPN is connected to US region and language is set to English(US)
Actual result:
There is no ads enable alert, ads are enabled by default when upgrading build from 0.62.51 to 0.67.119
Expected result:
Ads enable alert should be shown on BAT logo and ads should not be enabled by default
Reproduces how often:
Always
Brave version (brave://version info)
Version/Channel Information:
Other Additional Information:
Miscellaneous Information:
@brave/legacy_qa @tmancey
The text was updated successfully, but these errors were encountered: