Blogging prompts: Refinements #27557

creativecoder · 2022-11-22T18:32:21Z

Addresses feedback from #26680 (review)

Changes proposed in this Pull Request:

Adds nonce exception comments
Use proper functions for time and date generation
Remove locale conversion function (and tests) in favor of wpcom API change
Remove unneeded escaping work-around for JSON data
Better escaping/error handling in a couple of places

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?

Jetpack product discussion

See #26680 for context

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

No.

Testing instructions:

No user facing changes from #26680. Testing instructions from that PR
Jetpack Site

Be sure the site is connected to WP.com to load Jetpack blocks in the editor
Enable experimental blocks by adding define( 'JETPACK_EXPERIMENTAL_BLOCKS', true ) (or Settings > Jetpack Constants if you're using jurassic.ninja)
Set the site as "having a blog" by doing at least one of the following
- Set the site_intent option to write
- Set the front page to show posts in Settings > Reading
- Set a static page for the posts page in Settings > Reading
Create a new post, you should see a writing prompt in the placeholder for the first paragraph
Create a new post and add the answer_prompt query param with the prompt id for today as the value (e.g. /wp-admin/post-new.php?answer_prompt=2130), and you should see a writing prompt as a pullquote block, the tag dailyprompt added to the post, and the _jetpack_blogging_prompt_key post meta set to the prompt id.
Create a new page or custom post type, and you should not see the writing prompt
Open an existing post, and you should not see the writing prompt

Simple Site

Sandbox a Simple site domain
Apply this PR to your sandbox: bin/jetpack-downloader test jetpack try/paragraph-block-writing-prompts
Continue with the testing instructions, as above.

github-actions · 2022-11-22T18:33:16Z

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run bin/jetpack-downloader test jetpack update/blogging-prompt-refinements to get started. More details: p9dueE-5Nn-p2

github-actions · 2022-11-22T18:33:37Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: January 3, 2023.
Scheduled code freeze: December 26, 2022.

anomiex

Comment on line 75 is the only one preventing me giving an approval. The others are optional as far as I'm concerned or are just for discussion.

anomiex · 2022-11-22T19:05:41Z

projects/plugins/jetpack/_inc/blogging-prompts.php


 	$blog_id = \Jetpack_Options::get_option( 'id' );
-	$path    = '/sites/' . $blog_id . '/blogging-prompts?from=' . $day_before . '&number=10&_locale=' . $locale;
+	$path    = rawurldecode( '/sites/' . $blog_id . '/blogging-prompts?from=' . $day_before . '&number=10&_locale=' . $locale );


Wrong function, and the right function won't do the right thing as you're doing it here.

Suggested change

$path = rawurldecode( '/sites/' . $blog_id . '/blogging-prompts?from=' . $day_before . '&number=10&_locale=' . $locale );

$path = '/sites/' . rawurlencode( $blog_id ) . '/blogging-prompts?from=' . rawurlencode( $day_before ) . '&number=10&_locale=' . rawurlencode( $locale );

🤦🏼 Working too quickly on that one--thanks for the correction.

anomiex · 2022-11-22T19:06:35Z

projects/plugins/jetpack/extensions/blocks/blogging-prompts/blogging-prompts.php

-			wp_add_inline_script( 'jetpack-blocks-editor', 'var Jetpack_BloggingPrompts = JSON.parse( decodeURIComponent( "' . rawurlencode( $daily_prompts ) . '" ) );', 'before' );
+			wp_add_inline_script(
+				'jetpack-blocks-editor',
+				'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG ) . ';',


If you want to be extra safe

Suggested change

'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG ) . ';',

'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG | JSON_HEX_AMP ) . ';',

anomiex · 2022-11-22T19:07:24Z

projects/plugins/jetpack/_inc/blogging-prompts.php

 */
 function jetpack_setup_blogging_prompt_response( $post_id ) {
-	// phpcs:ignore WordPress.Security.NonceVerification.Recommended
+	// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Clicking a prompt response link can happen from notifications, Calypso, wp-admin, etc and only setups up a response post (tag, meta, prompt text); the user must take action to actually publish the post.


Should we worry about the list of "draft" posts getting filled up by an attacker?

It's a fair question! In practice, I'm personally not worried--it seems like a lot of effort (or patience) to get a user to open the answer_prompt link enough times to be concerning. I think there are probably easier and lower effort attacks.

For me the desire to make answering a writing prompt as seamless as possible from multiple contexts outweighs the (arguably) minimal security benefit of a nonce in this particular case.

Fair enough. I don't know enough about this to second-guess the decision, I just wanted to confirm that the decision was made with relevant factors having been considered.

creativecoder · 2022-11-23T00:37:04Z

FYI, I'm attempting to move the mag16 locale logic to wpcom here: D93225-code

creativecoder added 3 commits November 22, 2022 12:23

Blogging Prompts: address feedback

c71c70b

changelog

44ece72

Removes unused test file

135499a

creativecoder self-assigned this Nov 22, 2022

creativecoder added [Status] Needs Review This PR is ready for review. [Block] Blogging Prompts labels Nov 22, 2022

github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] Needs Test Review labels Nov 22, 2022

creativecoder mentioned this pull request Nov 22, 2022

Editor: add blogging prompts to posts #26680

Merged

2 tasks

creativecoder requested review from a team and anomiex November 22, 2022 18:33

anomiex reviewed Nov 22, 2022

View reviewed changes

creativecoder added 2 commits November 22, 2022 18:27

Commit suggestions

85404f4

Fix nonce comments

d850896

anomiex approved these changes Nov 23, 2022

View reviewed changes

coder-karen added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Nov 24, 2022

creativecoder enabled auto-merge (squash) November 30, 2022 22:39

Merge branch 'trunk' into update/blogging-prompt-refinements

5edd6c9

creativecoder merged commit 13eccf5 into trunk Dec 1, 2022

creativecoder deleted the update/blogging-prompt-refinements branch December 1, 2022 17:05

github-actions bot added this to the jetpack/11.6 milestone Dec 1, 2022

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Blogging prompts: Refinements #27557

Blogging prompts: Refinements #27557

Uh oh!

creativecoder commented Nov 22, 2022

Uh oh!

github-actions bot commented Nov 22, 2022 •

edited

Loading

Uh oh!

github-actions bot commented Nov 22, 2022 •

edited

Loading

Uh oh!

anomiex left a comment •

edited

Loading

Uh oh!

anomiex Nov 22, 2022

Uh oh!

creativecoder Nov 23, 2022

Uh oh!

anomiex Nov 22, 2022

Uh oh!

creativecoder Nov 23, 2022

Uh oh!

anomiex Nov 22, 2022

Uh oh!

creativecoder Nov 23, 2022

Uh oh!

anomiex Nov 23, 2022

Uh oh!

creativecoder commented Nov 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	$path = rawurldecode( '/sites/' . $blog_id . '/blogging-prompts?from=' . $day_before . '&number=10&_locale=' . $locale );
	$path = '/sites/' . rawurlencode( $blog_id ) . '/blogging-prompts?from=' . rawurlencode( $day_before ) . '&number=10&_locale=' . rawurlencode( $locale );

	'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG ) . ';',
	'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG \| JSON_HEX_AMP ) . ';',

Blogging prompts: Refinements #27557

Blogging prompts: Refinements #27557

Uh oh!

Conversation

creativecoder commented Nov 22, 2022

Changes proposed in this Pull Request:

Other information:

Jetpack product discussion

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

Testing instructions:

Uh oh!

github-actions bot commented Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anomiex left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anomiex Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

creativecoder Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

anomiex Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

creativecoder Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

anomiex Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

creativecoder Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

anomiex Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

creativecoder commented Nov 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Nov 22, 2022 •

edited

Loading

github-actions bot commented Nov 22, 2022 •

edited

Loading

anomiex left a comment •

edited

Loading