Extract the Account Session logic from Embedded components onboarding into a reusable utility hook #10273
+413
−113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: -8.25 kB (-1%) Total Size: 1.36 MB
ℹ️ View Unchanged
|
dmallory42
reviewed
Jan 30, 2025
oaratovskyi
requested changes
Jan 30, 2025
There was a problem hiding this comment.
@mordeth for some readon those requests are in the endless loop. I used the same code as you for the KYC and it does not work as expected - see the video
const stripeConnectInstance = useAccountSession( {
isOnboarding: false,
data: [],
continueKyc: false,
setLoadErrorMessage: () => {},
appearance,
} );
gNFzB9.mp4
in the embedded KYC it works better - only two duplicated requests. Could you please take a look?
@oaratovskyi I was able to reproduce the loop using the provided code. However, replacing |
|
Yes, works for me, thanks! |
oaratovskyi
approved these changes
Jan 30, 2025
dmallory42
approved these changes
Jan 31, 2025
There was a problem hiding this comment.
Looks good to me! I tested everything expect the Progressive flow (I wasn't able to get a slot) - if possible it would be good to test that locally. But since we have a few globalstep cycles prior to the 9.0 release, I think it is okay to go ahead with this, we just need to ensure the critical flows are tested ahead of that release.
A small note that I didn't see the Congratulations component on onboarding completion, but I think it was because I onboarded a test account, so instead I saw the test account notification component, I think it is expected.
dmallory42
reviewed
Feb 3, 2025
There was a problem hiding this comment.
Nitpick - can we use Kyc instead of KYC to stay consistent with other usages on the client (example)?
e.g. KycAccountSession instead of KYCAccountSession.
There was a problem hiding this comment.
Great spot! I’ve renamed all occurrences accordingly.
oaratovskyi
reviewed
Feb 3, 2025
oaratovskyi
reviewed
Feb 3, 2025
oaratovskyi
reviewed
Feb 3, 2025
| * | ||
| * @return WP_Error|WP_REST_Response | ||
| */ | ||
| public function get_embedded_session( WP_REST_Request $request ) { |
There was a problem hiding this comment.
Suggested change
| public function get_embedded_session( WP_REST_Request $request ) { | |
| public function create_embedded_account_session( WP_REST_Request $request ) { |
There was a problem hiding this comment.
That change makes sense as we are sending a POST request. Updated.
oaratovskyi
reviewed
Feb 3, 2025
| fetchClientSecret, | ||
| locale, | ||
| ] ); | ||
| const stripeConnectInstance = useKYCAccountSession( { |
There was a problem hiding this comment.
Suggested change
| const stripeConnectInstance = useKYCAccountSession( { | |
| const stripeConnectInstance = useKycAccountSession( { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #10256
Changes proposed in this Pull Request
To reduce code duplication and improve maintainability, this PR extracts the shared JavaScript logic for retrieving the account session from the embedded components onboarding flow into a reusable utility hook. This refactoring ensures consistency across components, making the logic easier to manage, reuse, and update in the future.
Testing instructions
Before testing
npm installto install the Stripe libraries.npm run watchBefore each test (prerequisites)
Non-progressive onboarding
Progressive onboarding
United Statesas the countryOnboarding isresumed
Localisation
Tracking
wcadmin_wcpay_onboarding_flow_started- upon redirect to the MOXwcadmin_wcpay_onboarding_flow_step_completed- after completion of a step in the MOXwcadmin_wcpay_onboarding_flow_redirected- after loading the embedded component (note that the name of this event is slightly inaccurate to maintain backwards compatibility)wcadmin_wcpay_stripe_connected- after completion of the embedded onboarding.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge