-
Notifications
You must be signed in to change notification settings - Fork 121
Store creation MVP: site picker entry point behind a feature flag #7886
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
Conversation
…w by implementing `WKUIDelegate`.
You can test the changes from this Pull Request by:
|
selanthiraiyan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing
-
I was able to successfully create a store. ✅
-
Getting stuck after selecting the WPCOM plan, which happens to me half of the time. In the success case, it navigates to the domain selection page
I faced this problem as well.
-
Thanks for covering the subtasks here. 👍
Code
I left a non-blocking nit. ✅
LGTM ![]()
| throw StoreCreationError.invalidCompletionPath | ||
| } | ||
|
|
||
| // Extracts the site URL substring matching the named capture group `siteURL` in the regex. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Would it make sense to move the site URL extraction code to a private method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 9139d17
| /// | ||
| /// - Parameter transform: a function that transforms the upstream publisher asynchronously with the option to throw an error. | ||
| /// - Returns: a new publisher that is transformed by the given operator asynchronously. | ||
| func asyncMap<T>(_ transform: @escaping (Output) async throws -> T) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL 🙇
* trunk: Conditionally connect InAppPurchaseStore to remote Sync StoreKit configuration with App Store Connect Add product purchase to IAP debug Revert "Include purchasing and transactions in IAP debug" Include purchasing and transactions in IAP debug Add product loading to IAP debug view Use local StoreKit configuration Remove commented out networking code until needed InAppPurchaseStore implementation Hide IAP debug behind experimental feature toggle Add IAP debug view to Hub menu Move release notes to 10.9 Bump deployment version in all projects/targets Bump deployment target for Pods Add mention in release notes about dropping iOS 14 Update usage of UTType identifiers Update use of SwiftUI animation Bump deployment target to 15.0
Part of #7879
Description
Previously in pe5sF9-Jv-p2, we decided to release the first iteration of store creation in the specified implementation. This PR is the very first part of the MVP implementation, after many rounds of testing (which is time-consuming having to go through 4 profiler questions and a 50% failure rate on my end). Please watch out for the issues in pe5sF9-JD-p2#comment-1124, and let me know if you also see the first two issues during your testing:
My suspicion is that because the WPCOM plan CTA opens a new window and requires a workaround f323544, I thought I should look into this issue separately if it's also happening to anyone else in iOS. I can try opening the URL in a new view instead of loading it in the same view.
Please also refer to the subtasks in #7879 for any followup tasks, and suggest any improvements that aren't in the list.
Delays & retries
As you can see in
StorePickerViewController, many retries with a delay in between is there because it takes an indefinite amount of time for the newly created site to be available in the WPCOM/me/sitesAPI response also as a Jetpack site. The new site first appears withjetpackandjetpack_connectionasfalse, then they gradually becometrue😅 Please feel free to suggest a better way to handle this.Unit testing
I decided to leave unit testing for now, because we're implementing the second iteration in pe5sF9-Ky-p2 right after MVP and honestly I find this iteration quite hacky and potentially unreliable.
Testing instructions
Feature flag on
Feature flag off
DefaultFeatureFlagServiceScreenshots
RELEASE-NOTES.txtif necessary.