fix(modal-checkout): optimize iframe load and refactor anonymous cart generation #1896

miguelpeixe · 2024-10-08T15:34:00Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR addresses a few different issues:

Introduces a new strategy to detect iframe load so it can be ready sooner. The load event dispatch waits for render-blocking assets to load before firing. The new strategy watches for the iframe content document to trigger when the DOM is ready.

The dequeuing strategy is now reversed so that only whitelisted assets can be enqueued. This prevents unnecessary assets from loading.

Cart generation via ajax

To handle an edge case in which carts are getting wiped between an account registration and the modal checkout render, this PR changes the strategy for anonymous readers.

Carts should now be generated in parallel to the auth/account creation flow. This allows for the cart session to be created before the authentication, which, once authenticated, Woo should handle the transition without issues.

Because of this new strategy, readers signing in via link should also be able to restore the checkout flow with the existing cart. The pending checkout logic has been simplified here and in Automattic/newspack-plugin#3471.

How to test the changes in this Pull Request:

Checkout this branch and fix(auth): simplify pending checkout newspack-plugin#3471
In a fresh session, start the checkout button flow with a subscription product
Register a new reader and confirm you are able to purchase without issues
In another fresh session, start the checkout button flow with the same product
This time, enter an existing email and click to sign in via link
Paste the link in your session and confirm you are authenticated and able to proceed with the checkout
Repeat steps 2-6 but through the Donate block

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

chickenn00dle · 2024-10-08T15:42:47Z

includes/class-modal-checkout.php

+			'wc-',
+			'wc_',


Its not uncommon for woo extensions to utilize these prefixes for scripts, so we probably wouldn't be fully covered. That said this is definitely a more thorough approach and better than what we have now.

I think we talked about making this filterable so that publishers can have other payment gateways if they'd like.

Right! Thank you for reminding me, updated in 893a9c4

src/modal-checkout/modal.js

chickenn00dle

Nice! Very cool way to decouple the iframe logic from the load event. I like this approach, although it doesn't seem to take into account the transition between completing checkout and loading the thank you view.

I think we will need to unflag the ready state during this transition so the handler doesn't return early. Or maybe implement a new flag that triggers the complete state separately?

miguelpeixe · 2024-10-08T18:22:05Z

doesn't seem to take into account the transition between completing checkout and loading the thank you view.

Oops! Thank you for catching that. That was actually because the newsletter subscription script wasn't whitelisted to be enqueued. Fix in f0972b7

Also, the iframe resize observer failed to resize the modal after the checkout because of the new iframe._ready checks. I tweaked it a bit in f55c4bb and should work now.

miguelpeixe · 2024-10-08T21:51:41Z

25f6118 removes the init_checkout listener and runs the init() directly. The Woo trigger is called on the main thread, whereas our listener is attached asynchronously (both async script tag and after domReady). This causes a race condition and the modal to eventually never leave the loading state.

It's safe to run the logic outside the trigger. To keep the diff reasonable, I'm maintaining the code inside an init() function, but we might want to remove that.

You can reproduce the race condition by throttling your network to 3G and disabling the cache before starting the modal while on the epic branch.

chickenn00dle

Thanks for the changes @miguelpeixe!

I am running into a new issue where stripe throws the following exception when removing the init logic from the init_checkout event, which causes modal checkout to be stuck in a blocked state on the payment gateway step:

{
  "message": "We could not retrieve data from the specified Element.\n              Please make sure the Element you are attempting to use is still mounted."
}

This only happens when I've throttled my network speed to 3G and when going through the auth + checkout flow as a new reader, so maybe another race condition between Woo payment gateway logic and our init method? Not sure.

miguelpeixe · 2024-10-09T18:35:18Z

I found another race condition issue with the domReady callback. It's an immediately invoked function that will not run as a domReady callback. Updated in 7dcfc12

miguelpeixe · 2024-10-11T18:40:58Z

includes/class-modal-checkout.php

 			return;
 		}

-		$is_newspack_checkout       = filter_input( INPUT_GET, 'newspack_checkout', FILTER_SANITIZE_NUMBER_INT );
-		$is_newspack_donate         = filter_input( INPUT_GET, 'newspack_donate', FILTER_SANITIZE_NUMBER_INT );


I removed this because donations are not handled here. Please let me know if I'm missing something.

IIRC, this method will set the registration checkout flag for donations, which is why this was added. This was to set some order meta so we know whether to send a regular thank you email, or a welcome + thank you email. Maybe there is a better way to handle this now with the async cart.

Is it working? Because donation checkout requests are processed in a different method. We can replicate the flag logic there.

Ah I see it works because it's before the is_newspack_product check. WDYT we replicate it to the donate request method?

Hmm... Given the entire logic for the checkout registration flag lives in the blocks plugin, I see why it's better to have it there. I'll restore it.

Updated in 35b6d59. I've also changed the is_newspack_checkout to be a guard clause for consistency.

I just now realized that because of the new strategy, the registration flag has to be handled differently. The anonymous cart generation doesn't know if it's a registration or not. Moved in 50f0e68 so this can set the WC session var.

chickenn00dle

Nice work @miguelpeixe!

This is working very nicely for the most part. It looks like I am no longer able to receive the welcome + thankyou emails for new registration checkout though. These should look like this:

We use that checkout registration flag to set session data, and when the order is created set some order meta that our email logic can use to determine which thank you email to send:

newspack-blocks/includes/class-modal-checkout.php

Line 1703 in 6a68ca6

public static function maybe_add_checkout_registration_order_meta( $order ) {

chickenn00dle · 2024-10-11T20:11:13Z

src/modal-checkout/modal.js

+	iframe.addEventListener( 'load', handleIframeReady );
+
+	/**
+	 * Generate cart.


Non-blocking suggestion: Just so it's documented somewhere in the code, maybe its worth adding some further details about the need to trigger this asynchronously to avoid potential session buginess?

Updated in bffe929, let me know if it's clear enough

miguelpeixe · 2024-10-11T20:13:42Z

Thank you for reviewing, @chickenn00dle!

I am no longer able to receive the welcome + thankyou emails for new registration checkout though. These should look like this

Yes! I noticed that too, I've recently pushed some changes that should address that. I commented further here

chickenn00dle

Thanks for all the changes @miguelpeixe! This is looking good now! I left one non-blocking comment but feel free to ignore that.

chickenn00dle · 2024-10-11T20:28:26Z

src/modal-checkout/modal.js

-			triggerCheckout( form );
-		}
-	}
-
 	/**
 	 * Handle modal checkout url param triggers.
 	 */
 	const handleModalCheckoutUrlParams = () => {


Non-blocking nit: Even though we are no longer using url params for the magic link, I think it might be worth keeping this checkoutURLParam logic so publishers can do things like craft urls in newsletters so readers can be taken directly to checkout for xyz product.

Good call! Restored in 21f542c

chickenn00dle

Great. Thanks for the quick turnaround @miguelpeixe! 🙌

miguelpeixe · 2024-10-11T22:02:59Z

Thank you for the reviews!

miguelpeixe added 2 commits October 8, 2024 12:27

fix(modal-checkout): optimize iframe load

e48e12c

fix: prevent multiple runners

f3f6992

chickenn00dle reviewed Oct 8, 2024

View reviewed changes

src/modal-checkout/modal.js Outdated Show resolved Hide resolved

chickenn00dle reviewed Oct 8, 2024

View reviewed changes

miguelpeixe added 4 commits October 8, 2024 14:54

fix: allow newspack-newsletters- scripts

f0972b7

fix: call cb on early return and log missing jquery

abb28ac

chore: log when unable to block form

07f5df3

fix: fix iframe ready check and remove ie8 support

f55c4bb

fix: remove init_checkout listener

25f6118

chickenn00dle reviewed Oct 9, 2024

View reviewed changes

miguelpeixe and others added 2 commits October 9, 2024 15:32

fix: immediately invoked function

7dcfc12

Merge branch 'epic/ras-acc' into fix/optimize-modal-checkout-load

7929abe

miguelpeixe and others added 4 commits October 10, 2024 16:57

Merge branch 'epic/ras-acc' into fix/optimize-modal-checkout-load

7b3ccd9

feat: add to cart async with the auth flow

27d3bc4

chore: lint

12b8f26

fix: refactor anonymous cart handling

3cfbf80

miguelpeixe changed the title ~~fix(modal-checkout): optimize iframe load~~ fix(modal-checkout): optimize iframe load and refactor anonymous cart generation Oct 11, 2024

miguelpeixe commented Oct 11, 2024

View reviewed changes

fix: update method names

047850d

miguelpeixe mentioned this pull request Oct 11, 2024

fix(auth): simplify pending checkout Automattic/newspack-plugin#3471

Merged

6 tasks

miguelpeixe marked this pull request as ready for review October 11, 2024 18:52

miguelpeixe requested a review from a team as a code owner October 11, 2024 18:52

miguelpeixe self-assigned this Oct 11, 2024

miguelpeixe added ras-acc testing [Status] Needs Review labels Oct 11, 2024

miguelpeixe requested a review from chickenn00dle October 11, 2024 19:05

miguelpeixe added 6 commits October 11, 2024 16:11

feat: filter allowed assets

893a9c4

fix: restore is_newspack_donate and use guard clause

35b6d59

fix: variable position

6a68ca6

fix: isolate checkout registration flag

50f0e68

chore: remove extra line

68c787c

fix: use url query param for checkout registration flag

af02c11

chickenn00dle requested changes Oct 11, 2024

View reviewed changes

chore: improve docblock

bffe929

chickenn00dle approved these changes Oct 11, 2024

View reviewed changes

github-actions bot added [Status] Approved and removed [Status] Needs Review labels Oct 11, 2024

fix: restore checkout url params functionality

21f542c

miguelpeixe requested a review from chickenn00dle October 11, 2024 20:40

chickenn00dle approved these changes Oct 11, 2024

View reviewed changes

miguelpeixe merged commit 3968033 into epic/ras-acc Oct 11, 2024
9 checks passed

miguelpeixe deleted the fix/optimize-modal-checkout-load branch October 11, 2024 22:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(modal-checkout): optimize iframe load and refactor anonymous cart generation #1896

fix(modal-checkout): optimize iframe load and refactor anonymous cart generation #1896

miguelpeixe commented Oct 8, 2024 •

edited

Loading

chickenn00dle Oct 8, 2024

chickenn00dle Oct 11, 2024

miguelpeixe Oct 11, 2024

chickenn00dle left a comment

miguelpeixe commented Oct 8, 2024

miguelpeixe commented Oct 8, 2024 •

edited

Loading

chickenn00dle left a comment

miguelpeixe commented Oct 9, 2024

miguelpeixe Oct 11, 2024

chickenn00dle Oct 11, 2024

miguelpeixe Oct 11, 2024

miguelpeixe Oct 11, 2024

miguelpeixe Oct 11, 2024

miguelpeixe Oct 11, 2024

miguelpeixe Oct 11, 2024 •

edited

Loading

chickenn00dle left a comment

chickenn00dle Oct 11, 2024

miguelpeixe Oct 11, 2024

miguelpeixe Oct 11, 2024

miguelpeixe commented Oct 11, 2024

chickenn00dle left a comment

chickenn00dle Oct 11, 2024

miguelpeixe Oct 11, 2024

chickenn00dle left a comment

miguelpeixe commented Oct 11, 2024

fix(modal-checkout): optimize iframe load and refactor anonymous cart generation #1896

fix(modal-checkout): optimize iframe load and refactor anonymous cart generation #1896

Conversation

miguelpeixe commented Oct 8, 2024 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

Cart generation via ajax

How to test the changes in this Pull Request:

Other information:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chickenn00dle left a comment

Choose a reason for hiding this comment

miguelpeixe commented Oct 8, 2024

miguelpeixe commented Oct 8, 2024 • edited Loading

chickenn00dle left a comment

Choose a reason for hiding this comment

miguelpeixe commented Oct 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelpeixe Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

chickenn00dle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguelpeixe commented Oct 11, 2024

chickenn00dle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chickenn00dle left a comment

Choose a reason for hiding this comment

miguelpeixe commented Oct 11, 2024

miguelpeixe commented Oct 8, 2024 •

edited

Loading

miguelpeixe commented Oct 8, 2024 •

edited

Loading

miguelpeixe Oct 11, 2024 •

edited

Loading