Use terms text from the blocks/shortcode page as a fallback #9887
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
…as a fallback when the WooPay specific field is empty
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: 0 B Total Size: 1.39 MB ℹ️ View Unchanged
|
leonardola
changed the title
a-danae
requested changes
Dec 10, 2024
There was a problem hiding this comment.
Thanks for continuing the implementation from #9752, @leonardola !
I went through the following flows. It all looked good. I just noticed a few behaviors we might want to check. I left the details about them below.
- Blocks as the checkout page
- Filled WooPay policies
- Checkout page
- Required checkbox
- Optional checkbox
- Cart page
- Product page
- + no T&C field
- Checkout page
- Empty WooPay policies
- Checkout page
- Required checkbox
- Optional checkbox
- Cart page
- Product page
- + no T&C field
- Checkout page
- Filled WooPay policies
- Shortcode as the checkout page
- Filled WooPay policies
- Checkout page
- Cart page
- Product page
- + empty store policies
- Empty WooPay policies
- Checkout page
- Cart page
- Product page
- + empty store policies
- Filled WooPay policies
Also, let's please remember to create an issue for this PR for when we create the "What's new" post for WooPayments!
I'll go through the missing flows when reviewing the PR again. Thanks again for you work here!
| @@ -953,13 +955,15 @@ private static function get_option_fields_status() { | |||
| $fields_block = self::get_inner_block( $checkout_page_blocks[ $checkout_block_index ], 'woocommerce/checkout-fields-block' ); | |||
| $terms_block = self::get_inner_block( $fields_block, 'woocommerce/checkout-terms-block' ); | |||
| $terms_checkbox = isset( $terms_block['attrs']['checkbox'] ) && $terms_block['attrs']['checkbox']; | |||
| $terms_text = $terms_block['attrs']['text'] ?? null; | |||
There was a problem hiding this comment.
How about adding a fallback from where this is empty? This can happen when deleting the text of the T&C field.
- Use the blocks checkout page as the store checkout
- Ensure there's a T&C field in it
- Make the T&C checkbox required
- Delete its text and save
- On WooPayments settings, make the WooPay checkout conditions custom text empty and save
- As a shopper, go to the checkout page. Notice that the T&C field is present and required to check out
- Go to WooPay
- Notice that there's no T&C checkbox and you can place an order, even though it's required on the merchant store
Yes, edgy but simple to fix.
Video: https://d.pr/v/H5hiyF
There was a problem hiding this comment.
Good catch @a-danae this shouldn't be possible. Either the merchant's T&Cs are pulled in, or they don't have any on their site and they've deleted the default settings in WooPay, there won't be anything to show and the merchant does so at their own risk
Yep. I though of that but the above message from @pierorocca made it seem like we shouldn't set a default text. I'm glad to update if necessary though
| @@ -902,6 +902,7 @@ private static function get_option_fields_status() { | |||
| $address_2 = get_option( 'woocommerce_checkout_address_2_field', 'optional' ); | |||
| $phone = get_option( 'woocommerce_checkout_phone_field', 'required' ); | |||
| $terms_checkbox = ! empty( get_option( 'woocommerce_terms_page_id', null ) ); | |||
| $terms_text = get_option( 'woocommerce_checkout_privacy_policy_text' ); | |||
There was a problem hiding this comment.
Should we use the woocommerce_checkout_terms_and_conditions_checkbox_text option instead?
That seems to be what it's retrieved by WooCommerce for the shortcode Terms and conditions checkbox.
WC has the wrapper wc_get_privacy_policy_text() to retrieve that option. We could mimic wc_terms_and_conditions_checkbox_text() without the echo to replace the placeholder and escape. However, we can discuss escaping closer to output on WooPay's end.
There was a problem hiding this comment.
Fixed with a52190b
| @@ -902,6 +902,7 @@ private static function get_option_fields_status() { | |||
| $address_2 = get_option( 'woocommerce_checkout_address_2_field', 'optional' ); | |||
| $phone = get_option( 'woocommerce_checkout_phone_field', 'required' ); | |||
| $terms_checkbox = ! empty( get_option( 'woocommerce_terms_page_id', null ) ); | |||
There was a problem hiding this comment.
Would it be better to use wc_terms_and_conditions_checkbox_enabled()?
This is what WC uses for rendering the terms section.
We updated this in the previous PR but I'm noticing it now.
Another reason to use that function now is that it checks for both whether the page exists and whether the T&C text isn't empty, instead of the page existence alone.
We have a discrepancy rendering of the T&C checkbox between the merchant store and WooPay with the current approach:
- Under WC settings, set a shortcode checkout page as the store checkout
- Under the customizer options, set a Terms and conditions page
- Under the customizer options, remove the Terms and conditions text. That, assuming we update the option we pass to WooPay, from my other comment
- Under WooPayments settings, add a custom checkout policy text
- As a shopper, go to the merchant checkout page. Notice no checkbox is displayed for T&C
- Go to WooPay. Notice the checkbox is required
There was a problem hiding this comment.
Would it be better to use wc_terms_and_conditions_checkbox_enabled()?
We have a discrepancy rendering of the T&C checkbox between the merchant store and WooPay with the current approach:
@pierorocca How should WooPay behave in case there is a custom Terms and conditions text on /wp-admin > Payments > WooPay > Terms and Conditions, there is a terms and conditions page set on the shortcode checkout settings but there is no shortcode terms and conditions?
| @@ -902,6 +902,7 @@ private static function get_option_fields_status() { | |||
| $address_2 = get_option( 'woocommerce_checkout_address_2_field', 'optional' ); | |||
| $phone = get_option( 'woocommerce_checkout_phone_field', 'required' ); | |||
| $terms_checkbox = ! empty( get_option( 'woocommerce_terms_page_id', null ) ); | |||
| $terms_text = get_option( 'woocommerce_checkout_privacy_policy_text' ); | |||
There was a problem hiding this comment.
Different issue, although mentioned in the other comment for this line:
The placeholders aren't being replaced with the current approach. We could use WC's wc_replace_policy_page_link_placeholders() for this.
There was a problem hiding this comment.
Fixed with a52190b
…licy_text to woocommerce_get_terms_and_conditions_checkbox_text
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.
Related to #3012
Changes proposed in this Pull Request
This PR adds code to send the terms text from the shortcode checkout page settings or the blocks checkout page to be used by WooPay as a fallback when the WooPay specific field is empty.
Testing instructions
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