Store creation using a webview #7602

0nko · 2022-10-23T20:02:58Z

This PR adds the store creation feature using a webview. Like on iOS, it's currently hidden behind a feature flag (debug build only).

Only a logged-in user use case is supported because we can't get the login credentials from a webview.

After a purchase is made, the store creation takes a long time and it's expected because a site must first be created, then the WooCommerce plugin is installed and then it takes a while to register as a new site for the user. A user's sites keep getting re-fetched until the new site is found.

store_creation_final.mp4

To test: (make sure you have free credits for testing)

Log in to a site and wait for a site picker to load the sites
Tap on the Create new store button
Approve the WooCommerce.com access request, if asked
Answer the survey questions
Tap on the plan purchase button
Select a domain (doesn't matter if it's TLD or *.wordpress.com)
Finish the purchase
Wait for the store creation process to finish
After the webview is closed, wait for the store to be loaded (could take a while)
Notice the new store is automatically selected

…aded

…fragment

peril-woocommerce · 2022-10-23T20:03:06Z

	Warnings
⚠️	PR is not assigned to a milestone.

	Messages
📖	This PR contains changes to Tracks-related logic. Please ensure the following are completed: PR Author The PR must be assigned the Tracks label PR Reviewer The tracks events must be validated in the Tracks system. Verify the internal tracks spreadsheet has also been updated.

Generated by 🚫 dangerJS

codecov-commenter · 2022-10-23T20:05:34Z

Codecov Report

Base: 42.98% // Head: 42.94% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (61f6895) compared to base (e90894a).
Patch coverage: 13.15% of modified lines in pull request are covered.

❗ Current head 61f6895 differs from pull request most recent head eda1ab1. Consider uploading reports for the commit eda1ab1 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk    #7602      +/-   ##
============================================
- Coverage     42.98%   42.94%   -0.04%     
+ Complexity     3301     3299       -2     
============================================
  Files           607      607              
  Lines         34105    34141      +36     
  Branches       4421     4430       +9     
============================================
+ Hits          14659    14663       +4     
- Misses        18105    18137      +32     
  Partials       1341     1341

Impacted Files	Coverage Δ
...merce/android/ui/sitepicker/SitePickerViewModel.kt	`76.71% <5.88%> (-6.27%)`	⬇️
...kotlin/com/woocommerce/android/util/FeatureFlag.kt	`56.52% <50.00%> (-0.63%)`	⬇️
...om/woocommerce/android/analytics/AnalyticsEvent.kt	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

0nko · 2022-10-23T20:05:56Z

...merce/src/main/kotlin/com/woocommerce/android/ui/common/wpcomwebview/WPComWebViewFragment.kt

-            navigateBackWithNotice(WEBVIEW_RESULT)
+        extractSiteUrl(url)
+
+        lifecycleScope.launchWhenResumed {


This was necessary because the app kept crashing for me after a result was returned after site creation. Turns out the onLoadUrl() callback was still getting called even after that, which would the trigger the navigation again and crash.

This will have an unwanted side effect, let's say the user started loading the page that we want to detect to close the screen after, then they navigated to another app, the listener then will ignore the URL that's being loaded, and we'll end up showing a page that the user is not expected to navigate to.

We have the isAdded check below to avoid this crash, I think since the app crashed for you, then it's not enough (maybe because fragment transactions are async), so as an alternative, we can simply use a boolean to track whether the navigation was triggered by the listener, and then use it to ignore all subsequent calls.

Good point! It's now fixed.

0nko · 2022-10-23T20:07:54Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/SitePickerViewModel.kt

+            var newSite: SiteModel? = null
+            while (newSite?.hasWooCommerce != true) {
+                newSite = urls.firstNotNullOfOrNull { url -> repository.getSiteBySiteUrl(url) }
+                if (newSite != null && newSite.hasWooCommerce) {


When a new site is first fetched, the Woo plugin is still not ready and the store can't be selected. So we need to wait for the hasWooCommerce to be true.

0nko · 2022-10-23T20:11:54Z

...merce/src/main/kotlin/com/woocommerce/android/ui/common/wpcomwebview/WPComWebViewFragment.kt


    private val webViewClient by lazy { WPComWebViewClient(this) }
    private val navArgs: WPComWebViewFragmentArgs by navArgs()
+    private var siteUrls = ArrayList<String>()


There are multiple address aliases associated with a new site. That's why a list of addresses is returned so that the site can be loaded and selected.

wpmobilebot · 2022-10-23T20:18:43Z

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

hichamboushaba · 2022-10-24T08:53:56Z

...merce/src/main/kotlin/com/woocommerce/android/ui/common/wpcomwebview/WPComWebViewFragment.kt

+        const val WEBVIEW_STORE_CHECKOUT_STRING = "checkout/thank-you/"
+        const val WEBVIEW_STORE_URL_KEY = "store-url-key"


Before starting a full review for the PR @0nko, I wanted to raise the point below 🙂, if you feel like what I'm raising below doesn't make sense, or if the time doesn't allow it, please let me know, and I'll review the PR as is.

I feel like these constants and the logic below to extract the site URL don't belong in this class, this fragment was supposed to be a reusable screen for the authenticated WPCom logic, with the sole responsibility of loading a URL and then informing the parent screens when a specific URL was loaded as a termination of a specific flow, but with the current logic, it's responsible for more than this, and it has logic that's not reusable or part of other flows.
If the siteUrl extraction logic needed only the last URL of the flow, then I would've suggested to update the logic of the screen to return the last URL, and make the parsing of this URL a responsibility of the calling Fragment.
But given how it has to be done, I suggest we create a separate screen for this, I believe we can use the Composable component WCWebView, it supports WPCom authentication, and it's flexible enough, so that we can have the URL parsing and extraction easily separated. This will have some other advantages:

Remove the responsibility of waiting from the site picker, it already handles too much, and by being implemented in the new screen, we can use a better ui/text for the loading if needed (something other than "verifying your site" which sounds weird for a site that we were responsible for creating 😅).

Then in the site picker, the onStoreCreated can be very simplified, basically the same implementation as onSiteAddressReceived.

Thanks for the great suggestions, @hichamboushaba! I've been so focused on making the store creation flow load the new site automatically that I didn't stop and think about how to properly separate the new functionality from the existing components, thinking this is all just a temporary solution anyway...

As you suggested, we can use the WCWebView in its own Composable screen, with a ViewModel, which can then be used as a base for Iteration 2. I started working on it but I can already see it will be a bit more work.

Considering the circumstances, could we go ahead with the current PR (since it's already providing the desired functionality), and in the meantime, I'll refactor the code, as I mentioned above?

Sounds good @0nko 🙂, thanks for this.
For the review, I added a comment about a potential issue that we want to fix before releasing this, and for the rest, I will review the code and functionality today or tomorrow morning.

hichamboushaba

Nice work @0nko, I left one np comment, but it can be ignored for this PR.

hichamboushaba · 2022-10-25T12:35:05Z

WooCommerce/src/main/kotlin/com/woocommerce/android/util/FeatureFlag.kt

            STORE_CREATION_FLOW -> PackageUtils.isDebugBuild()
+            STORE_CREATION_WEBVIEW_FLOW -> PackageUtils.isDebugBuild()


np, I think these two feature flags might be confusing, especially with how we are going to interact with them for the simplified login project.
I think we should updateSTORE_CREATION to be called ACCOUNT_CREATION, as this is what's really controls right now.

# Conflicts: # WooCommerce/src/main/res/values/strings.xml

0nko added 12 commits October 18, 2022 14:22

Add logic to extract a new store URL from a webview

430f4e5

Add new store creation buttons

fdcabaf

Select the newly created site and display the store

a558a5b

Try multiple different URLs returned from a site creation flow

48cb8ea

Fix the site reload logic to make sure the new site is ready to be lo…

6040d21

…aded

Prevent the navigation logic from being executed in a paused webview …

43eb48c

…fragment

Remove unused code

2f4a98b

Fix ktlint issues

1b61062

Add a feature flag

fd11b02

Merge branch 'trunk' into feature/webview-store-creation

6f9f490

Fix the feature flag

28d47e2

Update the fragment title for store creation

a1d5f6c

0nko added the feature: store creation Store creation flow label Oct 23, 2022

0nko requested review from JorgeMucientes and hichamboushaba October 23, 2022 20:02

0nko commented Oct 23, 2022

View reviewed changes

Fix the lint issues

61f6895

hichamboushaba self-assigned this Oct 24, 2022

hichamboushaba reviewed Oct 24, 2022

View reviewed changes

Fix the way a repeated result returning is handled

eda1ab1

0nko requested a review from hichamboushaba October 24, 2022 21:10

hichamboushaba approved these changes Oct 25, 2022

View reviewed changes

0nko added 2 commits October 25, 2022 15:10

Rename the STORE_CREATION_FLOW feature flag to ACCOUNT_CREATION_FLOW

71c6a95

Merge branch 'trunk' into feature/webview-store-creation

6627d58

# Conflicts: # WooCommerce/src/main/res/values/strings.xml

0nko merged commit a3367ba into trunk Oct 25, 2022

0nko deleted the feature/webview-store-creation branch October 25, 2022 13:47

		const val WEBVIEW_STORE_CHECKOUT_STRING = "checkout/thank-you/"
		const val WEBVIEW_STORE_URL_KEY = "store-url-key"

		STORE_CREATION_FLOW -> PackageUtils.isDebugBuild()
		STORE_CREATION_WEBVIEW_FLOW -> PackageUtils.isDebugBuild()

Store creation using a webview #7602

Store creation using a webview #7602

Uh oh!

Conversation

0nko commented Oct 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-woocommerce bot commented Oct 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Oct 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hichamboushaba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

0nko commented Oct 23, 2022 •

edited

Loading

peril-woocommerce bot commented Oct 23, 2022 •

edited

Loading

codecov-commenter commented Oct 23, 2022 •

edited

Loading

wpmobilebot commented Oct 23, 2022 •

edited

Loading