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

Update BAP UI #7698

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Update BAP UI #7698

merged 1 commit into from
Feb 3, 2021

Conversation

zenparsing
Copy link
Collaborator

@zenparsing zenparsing commented Jan 25, 2021

Resolves brave/brave-browser#13701

Submitter Checklist:

  • There is a ticket for my issue.
  • Used Github auto-closing keywords in the commit message.
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed).
  • Requested a security/privacy review as needed.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Text and design for popup:

Screen Shot 2021-02-01 at 3 35 41 PM

Note: due to technical limitations we were unable to render rounded borders and the bubble arrow:

Screen Shot 2021-02-01 at 15 56 06

Text and design for alert:

Screen Shot 2021-02-01 at 3 34 57 PM

Scenario 1: Non-JP Regions

  • Open the browser in rewards staging environment.
  • Open the rewards panel and claim the promotion.
  • Wait for the promotion value to appear in the wallet balance on the panel.
  • Close the rewards panel and open a new tab.
    • Verify that the BAP popup does not appear on the new tab page.
  • Close the new tab page.
  • Advance the system time to 2021-03-06.
  • Open a new tab page.
    • Verify that the BAP alert does not appear on the new tab page.
  • Close the new tab page.
  • Advance the system time to 2021-03-13.
  • Open a new tab page and open the rewards panel.
    • Verify that the wallet balance is not zero.

Scenario 2: JP Regions

  • Set the system region to JP.
  • Open the browser in rewards staging environment.
  • Open the new tab page.
    • Verify that the BAP popup does not appear (because the user's balance is zero).
  • Close the new tab page.
  • Open the rewards panel and claim the promotion.
  • Wait for the promotion value to appear in the wallet balance on the panel.
  • Close the rewards panel and open a new tab.
    • Verify that the BAP popup appears.
  • Click the "Learn more" button on the popup.
    • Verify that the BAP alert appears in the new tab page.
  • Close the BAP alert and the new tab page.
  • Open a new tab.
    • Verify that the BAP popup does not appear.
  • Close the new tab page.
  • Advance the system time by 3 days.
  • Open a new tab page.
    • Verify that the BAP popup is displayed.
  • Close the new tab page.
  • Advance the system time to 2021-03-06.
  • Open the new tab page.
    • Verify that the BAP alert is displayed.
  • Dismiss the BAP alert by clicking the "X" button and close the new tab page.
  • Open a tab.
    • Verify that the BAP alert is not displayed.
  • Close the tab.
  • Advance the system time by 3 days.
  • Open a tab.
    • Verify that the BAP alert is displayed.
  • Click the "OK" button on the BAP alert.
  • Close the tab.
  • Open a tab.
    • Verify that the BAP alert is not displayed.
  • Close the tab.
  • Advance the system time to 2021-03-13.
  • Open a new tab.
    • Verify that the BAP popup and the BAP alert are not displayed.
  • Open the rewards panel.
    • Verify that zero is shown for the user's balance.

Scenario 3: JP Regions (Promotions)

  • Set the system region to JP.
  • Advance the system time to 2021-03-13.
  • Open the browser in rewards staging environment.
  • Open the rewards panel.
  • Verify that no grant promotions are displayed to the user.

@zenparsing zenparsing force-pushed the ksmith-bap-deprecation-ui branch 10 times, most recently from b0cba21 to d69445f Compare January 28, 2021 16:54
@zenparsing zenparsing marked this pull request as ready for review January 28, 2021 17:21
@zenparsing zenparsing requested a review from a team as a code owner January 28, 2021 17:21
@zenparsing zenparsing force-pushed the ksmith-bap-deprecation-ui branch 3 times, most recently from f01d9c0 to becff2b Compare January 29, 2021 16:13
@zenparsing
Copy link
Collaborator Author

Added two browsertests to cover code in RewardsServiceImpl that determines whether BAP is enabled - I modeled them on some of the non-flaky rewards browsertests, so I'm hoping that they will be reliable.

@zenparsing zenparsing force-pushed the ksmith-bap-deprecation-ui branch 2 times, most recently from d75e4fb to 48559c5 Compare January 29, 2021 18:58
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

@zenparsing zenparsing self-assigned this Jan 29, 2021
@zenparsing zenparsing added feature/rewards CI/skip Do not run CI builds (except noplatform) CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS labels Jan 29, 2021
@zenparsing zenparsing force-pushed the ksmith-bap-deprecation-ui branch 4 times, most recently from a032a38 to d5ba97d Compare February 1, 2021 20:21
Copy link
Collaborator

@tmancey tmancey left a comment

Choose a reason for hiding this comment

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

LGTM++

@ghost
Copy link

ghost commented Feb 1, 2021

looking in from a time expediency lens, this tooltip notice should suffice. One quick thought though @zenparsing is perhaps to use our existing blue tooltip for the phased welcome tour for new users but that might be out of reach for this. In any case this will work fine, great job!

@zenparsing zenparsing merged commit 4a268d3 into master Feb 3, 2021
@zenparsing zenparsing deleted the ksmith-bap-deprecation-ui branch February 3, 2021 16:30
@zenparsing zenparsing added this to the 1.21.x - Nightly milestone Feb 3, 2021
brave-builds pushed a commit that referenced this pull request Feb 3, 2021
@LaurenWags
Copy link
Member

Testing of this on Nightly is blocked by brave/brave-browser#14092. Discussed with @zenparsing so he is aware. Unable to get BAP so I can't check scenarios 2 and 3. cc @Miyayes @kjozwiak

@LaurenWags
Copy link
Member

LaurenWags commented Feb 11, 2021

Verified using

Brave	1.22.19 Chromium: 89.0.4389.40 (Official Build) nightly (x86_64)
Revision	2c2ed83cd507b23e4845edd09a7d1dfc727daf4b-refs/branch-heads/4389@{#602}
OS	macOS Version 10.15.7 (Build 19H512)

Verified test plan from #7698
Verified brave/brave-browser#14092 (comment) also.

Scenario 1: Non-JP Regions

Checked with US/English and France/French

Confirmed able to claim promotion:

US/English France/French
US1 FR1
US2 FR2

Confirmed BAP popup does not appear on NTP:

US/English France/French
US3 FR3

Confirmed BAP alert does not display on NTP when system date is set to 2021-03-06:

US/English France/French
US4 FR4

Confirmed wallet balance is not zero when system date is set to 2021-03-13:

US/English France/French
US5 FR5
Scenario 2: JP Region

Checked with Japan/English and Japan/Japanese. Note, issue encountered where Japanese translations are missing, logged with brave/brave-browser#14117.

Open NTP with zero BAP balance. Confirmed no BAP popup.

Japan/English Japan/Japanese
E1 J1

Confirmed able to claim UGP grant, balance is now non-zero (30 BAP)

Japan/English Japan/Japanese
E2 J2
E3 J3

Confirmed BAP popup displays when there is a non-zero BAP balance:

Japan/English Japan/Japanese
E4 J4

Confirmed clicking outside of the BAP popup closes the popup (there is no "x" to close).
Confirmed clicking on "Learn More" shows the BAP alert:

Japan/English Japan/Japanese
E5 J5

Confirmed able to close the BAP alert via "OK".
Confirmed able to close the BAP alert via "x".
Confirmed opening a NTP did not show the BAP popup again.
Confirmed in 3 days the BAP popup shows on NTP again (advanced computer clock per test plan).

Confirmed when computer date is 2021-03-06 and first NTP is opened, the BAP alert is displayed and the BAP popup is not:

Japan/English Japan/Japanese
E6 J6

Confirmed able to close the BAP alert via "OK".
Confirmed able to close the BAP alert via "x".
Confirmed opening a NTP did not show the BAP popup again if it had been dismissed via "OK" or "x".
Confirmed in 3 days the BAP popup shows on NTP again (advanced computer clock per test plan).
Note, also confirmed if BAP alert was not acknowledged (by either clicking "OK" or "x") it remained on the NTP (each NTP opened, and after browser restart) until acknowledged.

Confirmed when computer date is 2021-03-13 and first NTP is opened, the BAP popup and alert are not displayed.
Confirmed that the BAP balance is now zero on the panel and brave://rewards:

Japan/English Japan/Japanese
E7 J7
E8 J8

Encountered the following while testing Scenario 2

Scenario 3: JP Region (Promotions)

Checked with Japan/English. Confirmed when date = 2021-03-13, no promotions are fetched.

Example Example
1 2

Saw the below item in my logs:

[15944:775:0313/165710.902955:VERBOSE1:promotion.cc(113)] Fetch promotions disabled for BAP migration

Checked with US/English. Confirmed when date = 2021-03-13, promotions are fetched.

Example Example
3 4

Saw the below in my logs:

[ REQUEST ]
> URL: https://grant.rewards.bravesoftware.com/v1/promotions?migrate=true&platform=osx
> Method: UrlMethod::GET
[16090:775:0313/170114.688089:VERBOSE6:logging_util.cc(136)] 
[ RESPONSE - OnRequest ]
> Url: https://grant.rewards.bravesoftware.com/v1/promotions?migrate=true&platform=osx
> Result: Success
> HTTP Code: 200
> Body: {"promotions":[{"id":"2afc219e-4e94-4d15-8758-b457b30fb50c","createdAt":"2020-12-09T18:59:26.30816Z","expiresAt":"2021-04-09T18:59:26.30816Z","version":5,"suggestionsPerGrant":120,"approximateValue":"30","type":"ugp","available":true,"platform":"desktop","publicKeys":["FkAp9KoURjqvziKPs8xXwSnBI89e+rAY9Lik8aETcBU="],"legacyClaimed":false}]}

@kjozwiak
Copy link
Member

Moving this into 1.22.x as the BAP work is being reverted via #8088. Once we have more information next week, we'll re-revert #8088 and move everything back into 1.21.x and release 1.21.x - Release #2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Desktop] Update BAP UI
6 participants