Pay with PayPal block: Save fallback into post content #17256

mmtr · 2020-09-24T14:58:47Z

Changes proposed in this Pull Request:

As @mtias noted in paYKcK-FI-p2#comment-831, the save function of a block is conceptually the proper fallback while render_callback is instead intended for augmenting the content dynamically.

In other words, the save function should return an output that works well enough in any context (email, notifications, feed, as well as unknown clients getting post content from an API, etc). Then, for cases where we want to provide a richer UI (such in the site frontend), the render_callback function should be used for manipulating the block content.

This PR modifies the Pay with PayPal block implementation so it follows the approach above:

The block saves now a fallback output into the post content. That fallback includes a textual call-to-action linking to the post so folks can purchase from the website. This means that the block no longer saves a [simple-payment] shortcode.
The block also sets a render callback that augments the UI replacing the link to the post with a PayPal button when the block is rendered in the frontend (determined with a new jetpack_is_frontend function).
Changes from Pay with PayPal: Add alternative shortcode output for email notifications #17180 have been reverted so the fallback is only saved when saving posts from now on. Contexts where the fallback output is going to be rendered are usually timely streamers, so we don't need to worry about using the fallback in existing posts.

Jetpack product discussion

paYKcK-FI-p2

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Web

Spin up a JN site running master.
Set up Jetpack and purchase any paid plan.
Go to WP Admin > Posts > Add new.
Insert a Pay with PayPal block.
Fill it with data.
Publish the post.
Go to WP Admin > Jetpack > Jetpack Beta.
Switch to the branch of this PR (update/pay-with-paypal-save-fallback).
Visit the frontend of the post you published before.
Make sure there are no regressions in the Pay with PayPal block.
Go to WP Admin > Posts.
Edit the post you published before.
Make sure there are no regressions in the Pay with PayPal block.
Open the "More" menu and activate the "Code editor" view.
Make sure the HTML markup is the fallback output (the one with a "Visit the site to purchase" link).
Switch back to the "Visual editor" view.
Click on the "Update" button.
Visit the frontend of the post.
Make sure the augmented UI is displayed (the one with a "PayPal" button).

API

Go to the WP.com API console.
Make a GET request to /sites/:site/posts/:post-id (where :post-id is the ID of a post that contains a Pay with PayPal block).
Make sure the post content uses the fallback.

Email, notifications, and reader (only testable in WP.com)

Apply D50134-code to your WP.com sandbox.
Sandbox the API and the URL of a WP.com site under a Premium plan.
Go to https://wordpress.com/following/manage.
Make sure to follow the site under a Premium plan you sandboxed above.
Enable the "Notify me of new posts" option.

Go to https://wordpress.com/block-editor.
Select the site under a Premium plan you sandboxed above.
Publish a post that contains a "Pay with PayPal" block.
Grab the IDs of the post and the site where the post has been published.
In a WP.com sandbox run php ./bin/subscriptions/send.php <BLOG_ID> post <POST_ID> <YOUR_EMAIL>.
Check the email received and make sure it contains a "Visit the site to purchase" link.

Open the masterbar notifications.
Make sure it includes the "Visit the site to purchase" link.

Go to Reader.
Open the post you just published above.
Make sure it includes the "Visit the site to purchase" link.

Proposed changelog entry for your changes:

Pay with PayPal block: Save fallback into post content

matticbot · 2020-09-24T14:58:52Z

Caution: This PR has changes that must be merged to WordPress.com
Hello mmtr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D50134-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

jetpackbot · 2020-09-24T15:04:24Z

Scheduled Jetpack release: October 6, 2020.
Scheduled code freeze: September 29, 2020

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17256

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 514d09f

krymson24 · 2020-09-27T15:07:02Z

@mmtr

I ran through your test steps --

I'm not sure if I was doing this correctly, but I tried sandboxing the store, but I couldn't purchase a Jetpack plan, so I just gave myself free credits on the store admin, removed the sandbox, and then purchased a Jetpack plan.

Furthermore, I'm not sure what's set up incorrectly locally, but in master as well as your feature branch, I see this error message. So I don't think it's caused by your branch.

No regressions that I found:

Before	After

The fallback link contains what you said it would:

Before	After

krymson24 · 2020-09-27T15:47:50Z

@mmtr I ran through the tests for the email section and it worked as expected!

Just want to note that in your description, this step:

In a WP.com sandbox run php ./bin/subscriptions/send.php <BLOG_ID> post <POST_ID> <YOUR_EMAIL>.

Should wrap the php at the beginning in code formatting as well php ./bin/subscriptions/send.php <BLOG_ID> post <POST_ID> <YOUR_EMAIL>

I ran it the first time, not noticing the php part and ran into a permissions error, but then read it carefully the second time, and also remembered that I had run this command before 😄

mmtr · 2020-09-28T09:11:55Z

Thanks for testing, @krymson24!

I'm not sure if I was doing this correctly, but I tried sandboxing the store, but I couldn't purchase a Jetpack plan, so I just gave myself free credits on the store admin, removed the sandbox, and then purchased a Jetpack plan.

You don't need to sandbox anything for testing this one, so using the production store and not connecting your Jetpack site to your sandbox is fine.

Furthermore, I'm not sure what's set up incorrectly locally, but in master as well as your feature branch, I see this error message. So I don't think it's caused by your branch.

I also saw the SP22 error too, but like you I also get it when testing in master so I was assuming something was wrong on my environment. Not sure if we need to set up something else for testing this block. Will dig into this later.

Should wrap the php at the beginning in code formatting as well php ./bin/subscriptions/send.php <BLOG_ID> post <POST_ID> <YOUR_EMAIL>

Good catch! I actually wanted to include the php as part of the code formatting :) Updated.

functions.global.php

extensions/blocks/simple-payments/simple-payments.php

modules/simple-payments/simple-payments.css

extensions/blocks/simple-payments/edit.js

extensions/blocks/simple-payments/save.js

modules/simple-payments/simple-payments.php

…hen switching languages.

mmtr · 2020-09-28T13:48:48Z

I also saw the SP22 error too, but like you I also get it when testing in master so I was assuming something was wrong on my environment. Not sure if we need to set up something else for testing this block. Will dig into this later.

Found it. It's caused by the new Jetpack plans, which have not been added to the list of plans supporting the "Pay with PayPal" functionality. Should be fixed by D50263-code.

jeherve

This tested well for me on Jetpack. I have one comment, but it's not a blocker, it could be addressed in a follow-up PR. It should be good to merge!

jeherve · 2020-09-28T17:51:23Z

extensions/blocks/simple-payments/simple-payments.php

+	}
+
+	// Keep content as-is if rendered in other contexts than frontend (i.e. feed, emails, API, etc.).
+	if ( ! jetpack_is_frontend() ) {


I wonder if we should add a Blocks::is_amp_request() check here as well, since this may be another context where we'd want to display a simplified block.

cc @westonruter What are your thoughts on this?

Yes, this makes sense to me. I assume the button just doesn't work in AMP currently, so serving the fallback static content which we know is AMP-compatible seems correct.

Suggested change

if ( ! jetpack_is_frontend() ) {

if ( ! jetpack_is_frontend() || Blocks::is_amp_request() ) {

I'm not set up to test this block, but what does this result in on AMP pages?

functions.global.php

kwight · 2020-09-28T23:17:06Z

Wonderful! 🎉 I went through all the cases, and only had one glitch: I'm not able to complete the purchase on the front-end view. The PayPal modal pops up, but immediately dies with the following (I don't see this at all on prod, so maybe it's something sandbox-related?):

Console	Front-end

I'm approving, assuming we can determine that the above error isn't a problem related to these changes 👍

westonruter · 2020-09-29T01:52:02Z

extensions/blocks/simple-payments/simple-payments.php

+	}
+
+	// Keep content as-is if rendered in other contexts than frontend (i.e. feed, emails, API, etc.).
+	if ( ! jetpack_is_frontend() ) {


Suggested change

if ( ! jetpack_is_frontend() ) {

if ( ! jetpack_is_frontend() || Blocks::is_amp_request() ) {

I'm not set up to test this block, but what does this result in on AMP pages?

jeherve · 2020-09-29T08:42:11Z

I'll merge this for now, so I can get started with the release; @mmtr I'll let you handle the WordPress.com counterpart and the feedback above in a follow-up PR?

Let's address this in a follow-up PR

mmtr · 2020-09-29T09:28:48Z

I'm not able to complete the purchase on the front-end view. The PayPal modal pops up, but immediately dies with the following (I don't see this at all on prod, so maybe it's something sandbox-related?)

Yup, seems something caused by our sandboxes, I can see it as long as I'm sandboxed (even when D50134-code is not applied), so definitely unrelated to these changes.

mmtr · 2020-09-29T09:47:53Z

Deployed to WP.com in r214381-wpcom.

mtias · 2020-10-01T07:53:19Z

@mmtr does it make sense to serialize so much markup as a fallback? I was thinking it would only include the button, which is where this block should be heading.

mmtr · 2020-10-01T11:56:19Z

@mtias I think that ideally a block should aim for a markup that works anywhere, so we don't need to augment it. If that is not doable, then the more markup is shared between the fallback and the augmented UI, the easier is going to be the code maintenance.

I agree that it'd be great if this block would be only a button, so users can use other blocks (paragraphs, headings, images) for describing the product they are selling. But in the meantime, I think it's important that the fallback offers enough context to visitors so they can understand the purpose of the block.

Imagine that the fallback of this block would be only a link (it cannot be a button because it does not work on emails) that redirects visitors to the full post in the frontend so they can complete the purchase. These visitors will lack some info such as "what product is the site selling?" or "how much does is cost?".

Pay with PayPal block: Save fallback into post content

0ce72fb

mmtr added [Status] In Progress Pay with Paypal labels Sep 24, 2020

mmtr self-assigned this Sep 24, 2020

matticbot added the Touches WP.com Files label Sep 24, 2020

mmtr added 4 commits September 24, 2020 17:16

Fix postLink attribute type

4eb26c6

Remove fallback from shortcode

c51b1b2

Augment block UI when rendered on frontend

a6d1fd1

Add deprecation

74675d1

mmtr requested a review from a team September 25, 2020 14:47

Handle shortcode in render callbacks

a905aa0

mmtr added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Sep 25, 2020

mmtr requested a review from mtias September 25, 2020 16:09

jeherve reviewed Sep 28, 2020

View reviewed changes

mmtr added 5 commits September 28, 2020 14:49

Cast to boolean

33ccfb0

Do not enqueue frontend assets if other contexts than frontend

6831ace

Switch to wp_unique_id

d398969

Fix typo

5120135

Store post link text in attribute so block does not get invalidated w…

1dfc48f

…hen switching languages.

jeherve added this to the 9.0 milestone Sep 28, 2020

jeherve approved these changes Sep 28, 2020

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Sep 28, 2020

kwight reviewed Sep 28, 2020

View reviewed changes

functions.global.php Show resolved Hide resolved

kwight approved these changes Sep 28, 2020

View reviewed changes

westonruter previously requested changes Sep 29, 2020

View reviewed changes

jeherve merged commit a8b9e2e into master Sep 29, 2020

jeherve deleted the update/pay-with-paypal-save-fallback branch September 29, 2020 08:43

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 29, 2020

mmtr mentioned this pull request Sep 29, 2020

Subscriptions: Remove the shortcode #15227

Closed

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Sep 29, 2020

mmtr mentioned this pull request Sep 29, 2020

Pay with PayPal block: Handle AMP requests #17298

Merged

mmtr mentioned this pull request Oct 5, 2020

Reader: Pay with PayPal Block, PayPal button does not render Automattic/wp-calypso#45104

Closed

jeherve mentioned this pull request Oct 19, 2020

Translatable aria-label for image compare block slider #17501

Merged

mmtr mentioned this pull request Oct 20, 2020

Donations: Save fallback for non-frontend contexts #17381

Merged

htdat mentioned this pull request Oct 22, 2020

Pay with PayPal Block: Jetpack 9.0.x Stops Rendering HTML Tags in Description Fields. #17576

Open

mmtr mentioned this pull request Oct 26, 2020

Reader: some Gutenberg blocks don't display correctly Automattic/wp-calypso#43595

Closed

8 tasks

jeherve added the [Block] Pay With Paypal aka Simple Payments label Feb 12, 2021

jeherve mentioned this pull request Jan 19, 2022

Blocks: potential conflict with WP Super Cache #22410

Closed

jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pay with PayPal block: Save fallback into post content #17256

Pay with PayPal block: Save fallback into post content #17256

mmtr commented Sep 24, 2020 •

edited

Loading

matticbot commented Sep 24, 2020

jetpackbot commented Sep 24, 2020 •

edited

Loading

krymson24 commented Sep 27, 2020 •

edited

Loading

krymson24 commented Sep 27, 2020

mmtr commented Sep 28, 2020

mmtr commented Sep 28, 2020

jeherve left a comment

jeherve Sep 28, 2020

westonruter Sep 28, 2020

westonruter Sep 29, 2020

mmtr Sep 29, 2020

kwight commented Sep 28, 2020 •

edited

Loading

westonruter Sep 29, 2020

jeherve commented Sep 29, 2020

mmtr commented Sep 29, 2020

mmtr commented Sep 29, 2020

mtias commented Oct 1, 2020

mmtr commented Oct 1, 2020

	if ( ! jetpack_is_frontend() ) {
	if ( ! jetpack_is_frontend() \|\| Blocks::is_amp_request() ) {

Pay with PayPal block: Save fallback into post content #17256

Pay with PayPal block: Save fallback into post content #17256

Conversation

mmtr commented Sep 24, 2020 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Proposed changelog entry for your changes:

matticbot commented Sep 24, 2020

jetpackbot commented Sep 24, 2020 • edited Loading

krymson24 commented Sep 27, 2020 • edited Loading

krymson24 commented Sep 27, 2020

mmtr commented Sep 28, 2020

mmtr commented Sep 28, 2020

jeherve left a comment

Choose a reason for hiding this comment

jeherve Sep 28, 2020

Choose a reason for hiding this comment

westonruter Sep 28, 2020

Choose a reason for hiding this comment

westonruter Sep 29, 2020

Choose a reason for hiding this comment

mmtr Sep 29, 2020

Choose a reason for hiding this comment

kwight commented Sep 28, 2020 • edited Loading

westonruter Sep 29, 2020

Choose a reason for hiding this comment

jeherve commented Sep 29, 2020

mmtr commented Sep 29, 2020

mmtr commented Sep 29, 2020

mtias commented Oct 1, 2020

mmtr commented Oct 1, 2020

mmtr commented Sep 24, 2020 •

edited

Loading

jetpackbot commented Sep 24, 2020 •

edited

Loading

krymson24 commented Sep 27, 2020 •

edited

Loading

kwight commented Sep 28, 2020 •

edited

Loading