The release ZIP for this PR is accessible via:

https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-10348.zip

Script Dependencies Report

The compare-assets action has detected some changed script dependencies between this branch and trunk. Please review and confirm the following are correct before merging.

This comment was automatically generated by the ./github/compare-assets action.

TypeScript Errors Report

• Files with errors: 470
• Total errors: 2246

🎉 🎉 This PR does not introduce new TS errors.

_{comments-aggregator}

Size Change: -18.2 kB (-1%)

Total Size: 1.34 MB

_{compressed-size-action}

Related PR review

The previous Store API PRs made sense to me (ability to pay for orders from 3rd party app), but this is where things start to become unclear for me because this overlaps with core functionality.

1. Where does this login form/pay page fit in to the flow WooPay are building?
2. How would a user hit this page if they are not coming from the "My Account" page in WooCommerce (where they are already logged in), where the "pay" links are found?
3. Would it make sense to protect the legacy shortcode with the same login form? If so, does this change belong in blocks or core?

It's more difficult to maintain core changes from blocks because we have to rely on filters to adjust content, so I want to make sure this change makes sense in the context of blocks rather than core.

cc @ralucaStan who should be aware of these changes.

1. Where does this login form/pay page fit in to the flow WooPay are building?

The login form doesn't affect WooPay but for Blocks Pay-for-order, which will be used in WooPay. WooPay users are already logged in, so they will only see billing email verification or a notice that they can't access the order if it does not belong to them.

I think we should solve the flow in blocks first and then we can adjust if needed in WooPay. What do you expect the flow to look like for blocks checkout with pay-for-orders?

2. How would a user hit this page if they are not coming from the "My Account" page in WooCommerce (where they are already logged in), where the "pay" links are found?

Here are the two flows and how you'd see them. These are flows included in WooCommerce core, the only reason we don't see them with the blocks checkout is because they are hidden with the is_wc_endpoint_url( 'order-pay' ) check. This PR removes that, but I should probably hide it behind a development flag.

Guest

Logged-in user

3. Would it make sense to protect the legacy shortcode with the same login form? If so, does this change belong in blocks or core?

This login form is included already in the legacy shortcode checkout.

This PR just adds the login form to the blocks checkout.

The login form doesn't affect WooPay but for Blocks Pay-for-order, which will be used in WooPay. WooPay users are already logged in, so they will only see billing email verification or a notice that they can't access the order if it does not belong to them.

Ok so to summarize:

• This PR makes it so that the order-pay endpoint no longer falls back to the shortcode based checkout
• This PR filters post content to inject a login form before blocks/shortcodes are rendered, if the current request matches the order-pay endpoint.

And for reference this is the code in place currently to use the legacy experience for checkout endpoints, so that the block is only used for the main checkout view:

if ( $this->is_checkout_endpoint() ) {
	// Note: Currently the block only takes care of the main checkout form -- if an endpoint is set, refer to the
	// legacy shortcode instead and do not render block.
	return wc_current_theme_is_fse_theme() ? do_shortcode( '[woocommerce_checkout]' ) : '[woocommerce_checkout]';
}

I think we should solve the flow in blocks first and then we can adjust if needed in WooPay. What do you expect the flow to look like for blocks checkout with pay-for-orders?

My concern is this is widening the scope of responsibility for Blocks substantially. With this change in place, block based checkout will be used for the pay page. This is untested, and hasn't be validated as a viable solution. We also have the concerns about changing address fields. Checkout addresses can not currently be locked down.

Due to this I am not comfortable approving this work without feedback from @ralucaStan and @pmcpinto first.

As for the login form injection, I am not a fan of the approach taken in this PR (filtering the post content and inserting the form). For a quick fix, it could be handled in the Checkout Block render method, or we could just redirect to an existing login page. The Checkout Block does not facilitate logins at present, it just shows a login link.

Handling store login feels like it could use a deeper dive because there would be opportunities to:

• Create a login form block/component that could be used in multiple places
• Create a template for Block themes, so the user could customise the login page

Again, this is something that could use feedback from @ralucaStan / @pmcpinto because I am uncertain where this fits in with the roadmap.

My concern is this is widening the scope of responsibility for Blocks substantially. With this change in place, block based checkout will be used for the pay page.

I thought this was the original plan behind this project. pdpOw2-2sP-p2#comment-2950 The goal is to get the pay-for-order flow working in blocks and WooPay will just proxy the endpoints. Are you saying that we should try to make WooCommerce Blocks allow payment for pay-for-orders?

We could try to figure out how to make this work just for WooPay, but this seems like a wasted opportunity to make this also work in blocks.

This is untested, and hasn't be validated as a viable solution.

This PR puts this change behind the development flag. I thought we had agreed on this solution and are slowly implementing the changes to make this happen, but please let me know if you were thinking something else.

We also have the concerns about changing address fields. Checkout addresses can not currently be locked down.

That will happen in a future PR. The endpoints and client changes still need to be added. Since this is behind a development flag, it should be okay, right?

I thought this comment meant this authentication flow would be okay as long as it was behind a feature flag.

For a quick fix, it could be handled in the Checkout Block render method, or we could just redirect to an existing login page.

I thought the fix in this PR was the quickest solution. Could you share why replacing the content is not a good solution?

Adding new blocks would be a better solution long-term, but it seems like it would increase the scope of this project a lot. I think this is something we could always add later.

I don't think a redirect works. There is a login page, but there's no email verification page right now in core. We need to render that form somewhere.

@hsingyuc I want to get some feedback from the rest of the team before proceeding—they are on a meetup this week.

The Store API changes were safe behind a flag, but we're starting to touch Checkout Block logic now and I want to avoid regressions. I'm also slightly worried we'll stuff all the new payment page logic into the existing checkout block and make it more difficult to maintain.

There are also other ways we could approach offering a payment page, for example:

• there could be a template for the endpoint (like we've just implemented for the Order Confirmation page)
• there could be a new block, similar to checkout, on a different page that is managed separately to the main checkout block to keep the logic separated

We should make sure both teams are in alignment before proceeding, especially if Rubik will maintain this once complete.

I thought the fix in this PR was the quickest solution. Could you share why replacing the content is not a good solution?

• the_content hook runs for all rendered content, site wide, so there are performance concerns
• this would break any content outside of the block thats within post_content
• it's unclear to me if this would work with block templates, or just on pages. Would need to test that.

Login should be kept within the confines of the block requiring it IMO.

@hsingyuc I want to get some feedback from the rest of the team before proceeding—they are on a meetup this week.

Got it, let's wait for feedback from @ralucaStan and @pmcpinto.

the_content hook runs for all rendered content, site wide, so there are performance concerns

Isn't this only run once per page?

this would break any content outside of the block thats within post_content

Could you expand on this? I'm not quite sure I understand.

it's unclear to me if this would work with block templates, or just on pages. Would need to test that.

That's a good point. I think this is something that needs to be tested. Currently, pay-for-order routes are limited to the "checkout" page pay for order route checkout/order-pay/ORDER_ID so I don't think we would hit this issue.

But if we wanted this to work anywhere the block is used, it might be good to move to use a block instead of filtering the_content. This is not how core works right now, but I think it would be a good addition.

there could be a new block, similar to checkout

I think this could be a good idea. My only concern is that it greatly increases the scope of this project. I'm not sure it's absolutely necessary for this MVP, but if we decide that's the best solution I'm okay with doing that as long as we understand it will take longer and cannot meet GA deadlines.

Thanks for the ping.

My concern is this is widening the scope of responsibility for Blocks substantially. With this change in place, block based checkout will be used for the pay page. This is untested, and hasn't be validated as a viable solution. We also have the concerns about changing address fields. Checkout addresses can not currently be locked down.

What are we really trying to achieve with the changes? Is it having WooPay as an option in the Pay for order the end goal?

Handling store login feels like it could use a deeper dive because there would be opportunities to:
Create a login form block/component that could be used in multiple places
Create a template for Block themes, so the user could customise the login page

Yes. With the changes to strengthen the order confirmation flow, we'll need a login page for the order confirmation template.

Is it having WooPay as an option in the Pay for order the end goal?

Yes. I think a secondary goal is to set this up so that WooCommerce Blocks also can turn on the pay-for-order flow in blocks checkout when ready.

Right now this is all behind a development flag so it will not be turned on for blocks by default.

Yes. With the changes to strengthen the order confirmation flow, we'll need a login page for the order confirmation template.

My current solution adds a login and email confirmation using the forms in core. I think @mikejolley wants to create those as blocks in the current experience instead of reusing the core ones.

I believe this is a good idea long-term, but I wanted to use the existing flows to save time for GA. If we are not worried about including this logic in blocks right now, we can skip the forms here since it's not needed for WooPay.

@hsingyuc could you be more specific about how to Create a pay-for-order order as a guest?

My concern is this is widening the scope of responsibility for Blocks substantially. With this change in place, block based checkout will be used for the pay page. This is untested, and hasn't be validated as a viable solution. We also have the concerns about changing address fields. Checkout addresses can not currently be locked down.

It appears @mikejolley has some concerns. Where was the decision taken? For some background WC also has email address check on the order confirmation page woocommerce/woocommerce#39191

As for the login form injection, I am not a fan of the approach taken in this PR (filtering the post content and inserting the form). For a quick fix, it could be handled in the Checkout Block render method, or we could just redirect to an existing login page. The Checkout Block does not facilitate logins at present, it just shows a login link. - @mikejolley

I think we should solve the flow in blocks first and then we can adjust if needed in WooPay. What do you expect the flow to look like for blocks checkout with pay-for-orders? - @hsingyuc

@hsingyuc we are skeptical of this approach because we have never included login in the checkout blocks. If there is a need to login, we redirect users to the shortcode login page. This is an approach we are using and we are sticking with it until we have an explicit request to implement authentication at the block level; we need to have first an overview of adding login here, what is impacted, how it works with the template experience etc. before proceeding to implementation. @pmcpinto can confirm that this is on our roadmap, but not a priority at the moment.

This login form is included already in the legacy shortcode checkout.
This PR just adds the login form to the blocks checkout.

This PR puts this change behind the development flag. I thought we had agreed on this solution and are slowly implementing the changes to make this happen, but please let me know if you were thinking something else.- @hsingyuc

Just to highlight that we were not aware of the implications of having login at this level. Can't we work with using the shortcode login, and then redirecting to the pay for order? - @hsingyuc

I thought the fix in this PR was the quickest solution. Could you share why replacing the content is not a good solution?
Adding new blocks would be a better solution long-term, but it seems like it would increase the scope of this project a lot. I think this is something we could always add later. - @hsingyuc

I am going to let @pmcpinto answer this one. I don't think we should pick solutions for the sake of speed. We clearly need to align, especially since, as Mike pointed out, the responsibility for this change will land on Team Rubik and we can't give a confidence vote on this approach; and it's not aligned with the roadmap for blocks. @hsingyuc I understand where you are coming from, but we might need to reassess the expected result.

@hsingyuc could you be more specific about how to Create a pay-for-order order as a guest?

For example, when you have a bookable product and need the merchant's approval on the booking. Merchants will receive an email to approve the booking. Customers will receive the confirmation and the pay-for-order link to the checkout page.

@hsingyuc we are skeptical of this approach because we have never included login in the checkout blocks. If there is a need to login, we redirect users to the shortcode login page.

The approach I took above is also using the shortcode login and not including a login directly in blocks. I think Mike's concern is that it uses the_content filter, and he's worried that this will not work if the block is used on a non-checkout page. I understand the concern for this, but the current pay-for-order page can't be used outside of the checkout page assigned in settings.

@hsingyuc we are skeptical of this approach because we have never included login in the checkout blocks. If there is a need to login, we redirect users to the shortcode login page. This is an approach we are using and we are sticking with it until we have an explicit request to implement authentication at the block level; we need to have first an overview of adding login here, what is impacted, how it works with the template experience etc. before proceeding to implementation. @pmcpinto can confirm that this is on our roadmap, but not a priority at the moment.

It's okay if it's not a priority because we can add the block login page later when you are ready. We can add this on the WooPay side separately right now.

Just to highlight that we were not aware of the implications of having login at this level. Can't we work with using the shortcode login, and then redirecting to the pay for order? - @hsingyuc

Just to be clear, we are using the shortcode login embedded in the current page (not within blocks) instead of a redirect. The problem is that Core does not have an email form verification page, so switching to redirect for that does not work. They do have an email verification form, which is what we use here.

I am going to let @pmcpinto answer this one. I don't think we should pick solutions for the sake of speed. We clearly need to align, especially since, as Mike pointed out, the responsibility for this change will land on Team Rubik and we can't give a confidence vote on this approach; and it's not aligned with the roadmap for blocks. @hsingyuc I understand where you are coming from, but we might need to reassess the expected result.

Sounds good. @pmcpinto, any inputs?

Thanks for the clarification. This niche use case isn’t a priority for us. The team is hesitant to offer support for it at this time due to its untested nature, which could potentially result in unforeseen repercussions. Moreover, this could increase our maintenance workload. Is there a feasible approach to integrate this into WooPay without impacting the blocks and slowing down the path for GA?

This niche use case isn’t a priority for us.

@pmcpinto CMIIAW. Pay-for-order is a Core function, so eventually, Blocks will add pay-for-order block like Block checkout/Shortcode checkout. By niche use case, do you mean we don't have enough pay-for-order orders? It seems like we might have enough use cases between Subscriptions, WooCommerce Bookings, and Pre-Orders.

The team is hesitant to offer support for it at this time due to its untested nature, which could potentially result in unforeseen repercussions.

Currently, we put them behind the feature flag for testing the whole flow. Any other suggestions on a thorough testing plan so we can prevent any regressions?

It seems like eventually we'll have to do this unless we plan to remove this feature from Core entirely and not support those products.

Moreover, this could increase our maintenance workload. Is there a feasible approach to integrate this into WooPay without impacting the blocks and slowing down the path for GA?

Yes, we can move the logic to WooPay. I'm not sure how complex it would be right now.

Thanks for the clarification. This niche use case isn’t a priority for us.

@pmcpinto what specifically are you referring to as a niche use case? Pay-for-order flow is core order payment functionality. It enables multiple Woo extensions including WooCommerce Bookings (Over 20K+ installs, ~2900 active merchants $249/year) and pre-orders canonical extension (~1100 active merchants at $159/year). It was a key configuration option in the core onboarding flow when we started, and which was recently replaced with a non-industry specific site builder wizard (TBD if the new generic approach is justified. I can create a service store with bookings in < 2 minutes on multiple competing platforms). Not having any accelerated checkouts compatible with this order placement flow is a problem.

Regarding embedding pay-for-order flow functionality into WooPay, I think this is where systems design guidance and perspective from @marcinbot would be valuable. My opinion is that the BE contributing to core/blocks is more appropriate and reliable than shifting core functionality to a non-core feature of a plugin.

My opinion is that the BE contributing to core/blocks is more appropriate and reliable than shifting core functionality to a non-core feature of a plugin.

After reading the discussion here my understanding is that:

• We're trying to add functionality for guests to pay for orders using a link provided in, for example, emails. They have to confirm their identity before payment and are presented with a login form.
• The feature is put behind a flag
• The team feels this falls outside of the existing area covered by Blocks (checkout vs pay-for-order vs login)
• The team has some doubts regarding the implementation detail (login form embedded rather than redirected to)
• The team feels that this addresses only an edge case and as such the cost/benefit ratio of having to maintain it is not justifiable

I think historically similar scenarios resulted in the change being added to the non-core plugin. This has often resulted in questions being asked about why some work is being duplicated, so I realise it's far from an ideal solution.

If this is a blocker for WooPay, then I think we might have to implement it there, at least for the time being. The politics of how the code can be contributed to on a larger scale, and who should maintain it, should probably be discussed separately. It does sound like the changes proposed here open the door for a larger expansion of the Blocks plugin, so some more planning might be required.

Thank you all for your time and the discussion! I believe we have a conclusion to not implement the checks into Blocks now but maybe later in the future. I'm going to close this PR and next conversation is happening here.

Thanks @marcinbot.

Thank you all!