JITM: Show ToS update notice

[mmtr]

Changes proposed in this Pull Request:

• Allows JITMs to trigger AJAX actions when the CTA button is clicked.
• Adds an AJAX action for the ToS JITM to mark the ToS as accepted.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Enhances the JITMs so they can display legal updates that need to be accepted such as the ToS 2020 update notice. See p58i-8v7-p2

Testing instructions:

• Apply D38880-code to your WP.com sandbox.
• Make sure you don't have ToS marked as accepted. See D38880-code for instructions on how to reset ToS acceptance.
• Spin up a JN site running this branch.
• Set up Jetpack and connect the JN site to WP.com.
• Open the Blog RC and add the tos-test blog sticker to the JN site.
• Go to WP Admin > Settings > Jetpack Constants.
• Set JETPACK__SANDBOX_DOMAIN to the URL of your WP.com sandbox.
• Access any WP Admin page of your Jetpack test site.
• You should see a ToS update notice.
• Click on Accept.
• Make sure the JITM is hidden and the ToS are accepted.
• Reload the WP Amin page.
• Make sure the ToS update notice doesn't show up.
• Reset ToS acceptance and repeat steps with a JN site running the master branch.
• Sandbox jetpack.com and wordpress.com.
• Since the AJAX handler is not available, clicking on the CTA button should perform a fallback redirect:
  • CTA links to jetpack.com/redirect?source=jitm-tos
  • That link redirects to wordpress.com/tos/accept which marks ToS as accepted
• Make sure you're redirected back to the same WP Admin where you saw the JITM.
• Make sure the ToS update notice doesn't show up.

Proposed changelog entry for your changes:

JITM: Show ToS update notice

[jetpackbot]

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.

Scheduled Jetpack release: March 3, 2020.
Scheduled code freeze: February 25, 2020

Generated by 🚫 dangerJS against b49281c

[kwight]

Oh nice, I like this addition. Using D38262-code as a test, I was able to dismiss the banner and see an AJAX call fired.

[gwwar]

@mmtr in the summary for:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

We can note that this allows us to create banners for policy or terms of service updates, and anything else that might need an accept button. It's nice to paraphrase reasoning/context to folks if we link to additional internal context.

[kwight]

This is all perfecto, I <3 it! I do find the slow fadeOut to be a little... 2004, but I can see that it's the same as other JITM fades.

I was also curious what happens if we click on a CTA button with an invalid action. It's a little confusing for the user, since the button disables, then becomes active again with no real feedback (other than a 400 error for the ajax call in the console), but I don't think we want to go any further handling odd situations like that.

Works great!

[dereksmart]

Just a note to make sure that from the WPCOM side of things, we're only displaying this JITM to sites that are running Jetpack 8.3+, as older versions will not include this ajax handler and will get errors anytime they try to click.

[kwight]

Just a note to make sure that from the WPCOM side of things, we're only displaying this JITM to sites that are running Jetpack 8.3+, as older versions will not include this ajax handler and will get errors anytime they try to click.

Ooooh, that... brings up an interesting point: if we need to show the JITM to users with old JP versions, and can't rely on a client click handler needed for the user to accept, then... are we full-circle and back to the hacky redirect @mmtr ?

[kwight]

Noting that in the future for other JITMs, we can do version compares in the JITM engine with with_option_matching:

->with_CTA_ajax_action( 'jetpack_accept_tos' )
->with_option_matching(
	'jetpack_version',
	function( $version ) {
		return version_compare( $version, '8.3', '>=' );
	}
),

[mmtr]

Fallback for sites running older versions of Jetpack available at D38880-code.

[kbrown9]

I’m having some trouble with testing these changes. My test site is a JN site running this branch. The TOS JITM displays as expected. However, when I click the CTA, I’m redirected to a plans page. Also, the JITM still displays when I navigate back to a wp-admin page.

Is anyone else running into problems when testing this branch?

[mmtr]

I’m having some trouble with testing these changes. My test site is a JN site running this branch. The TOS JITM displays as expected. However, when I click the CTA, I’m redirected to a plans page.

That's weird. The click handler should intercept any click on the button and trigger an AJAX action rather than following the CTA link (which ends up redirecting to the plans pages if D38880-code it's not in place). I'll take a look at this to see if I can reproduce your issue.

Also, the JITM still displays when I navigate back to a wp-admin page.

Yup, this one has been identified by @kwight (D38262-code#771154) since we changed the signature of a function but didn't update its usage in the JITMs. Will prepare a fix for this today.

Edit: He beat me to it D38890-code

[mmtr]

when I click the CTA, I’m redirected to a plans page

Confirmed and fixed the issue in b49281c

[kwight]

Edit: He beat me to it D38890-code

Which has been deployed 👍

[kwight]

Fantastic ❤️

[kbrown9]

I tested this using the provided testing instructions, and everything was successful!

When using this branch, clicking the CTA button triggered the ajax action. When using the master branch, clicking the CTA button caused a redirect to wordpress.com/tos/accept. In both cases, the ToS was marked as accepted, and the ToS JITM no longer displayed.