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

Simplify Purchased step on desktop #29520

Closed
simonhong opened this issue Apr 5, 2023 · 4 comments · Fixed by brave/brave-core#18009
Closed

Simplify Purchased step on desktop #29520

simonhong opened this issue Apr 5, 2023 · 4 comments · Fixed by brave/brave-core#18009
Assignees
Labels

Comments

@simonhong
Copy link
Member

simonhong commented Apr 5, 2023

Test plan

Basically make sure happy path is good to go

  1. Fresh profile
  2. Click VPN button
  3. You should see sell panel
  4. Visit account.brave.com and do a purchase
  5. Purchase should work and load credentials
  6. Click VPN button
  7. You should see regions and toggle for VPN on/off

Description

On desktop, we're waiting to enter Purchased status when we have valid region data after getting subscriber credential.
We do this because region data is needed to show vpn main panel.
However, region data is not related with purchasing and it makes purchased status handling complex.
Also, because of that we have many ifdef android because android doesn't need region data from BraveVpnService.

To make it more simpler, we could enter Purchased state after getting subscriber credential.
and region data handling is done by BraveVPNOSConnectionAPI.
As region data is used for connecting, BraveVPNOSConnectionAPI would be good place to manage region data.

VPN panel can show sell view when it's not purchased state yet.
and VPN panel can show main view when BraveVPNOSConnectionAPI have valid region data.

@simonhong simonhong self-assigned this Apr 5, 2023
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Apr 6, 2023
simonhong added a commit to brave/brave-core that referenced this issue Apr 12, 2023
fix brave/brave-browser#29520

RegionData handling will be moved from BraveVpnService to
BraveVPNOSConnectionAPI because region data is used when connecting and
it's global data cached in local state. As BraveVpnService is
per-profile service, it's not fit to handle region data.
simonhong added a commit to brave/brave-core that referenced this issue Apr 18, 2023
fix brave/brave-browser#29520

RegionData handling will be moved from BraveVpnService to
BraveVPNOSConnectionAPI because region data is used when connecting and
it's global data cached in local state. As BraveVpnService is
per-profile service, it's not fit to handle region data.
@brave-builds brave-builds added this to the 1.52.x - Nightly milestone Apr 20, 2023
@kjozwiak
Copy link
Member

kjozwiak commented May 2, 2023

The above requires 1.51.110 or higher for 1.51.x verification 👍

@stephendonner
Copy link

@simonhong mind giving us a unique testplan for this one? What's written so far in the PR for brave/brave-core#18009 (comment) seems to apply more to #29728.

And/or if we already verified #29728 (comment), can that verification apply? Thanks!

@bsclifton
Copy link
Member

@stephendonner added test plan! 😄

And yes! #29728 (comment) should be perfect

@stephendonner
Copy link

Verified PASSED using

Brave 1.51.110 Chromium: 113.0.5672.77 (Official Build) (x86_64)
Revision c4236862955e005c2187105415ac4a2ecf86dff1-refs/branch-heads/5672_62@{#3}
OS macOS Version 13.4 (Build 22F5059b)

Followed the testplan in #29520 (comment) and covered it via #29728 (comment)

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

Successfully merging a pull request may close this issue.

6 participants