-
Notifications
You must be signed in to change notification settings - Fork 846
Writing prompts: modify filter and differentiate from setting #27779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run |
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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. Jetpack plugin:
|
jeherve
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should work. I had an idea on simplifying things a bit, let me know what you think about it:
diff --git a/projects/plugins/jetpack/_inc/blogging-prompts.php b/projects/plugins/jetpack/_inc/blogging-prompts.php
index c49a5bd3bc..588b99c19c 100644
--- a/projects/plugins/jetpack/_inc/blogging-prompts.php
+++ b/projects/plugins/jetpack/_inc/blogging-prompts.php
@@ -57,7 +57,16 @@ add_action( 'wp_insert_post', 'jetpack_setup_blogging_prompt_response' );
* @return boolean
*/
function jetpack_are_blogging_prompts_enabled() {
- return (bool) get_option( 'jetpack_blogging_prompts_enabled', jetpack_has_write_intent() || jetpack_has_posts_page() );
+ $prompts_enabled = (bool) get_option( 'jetpack_blogging_prompts_enabled', jetpack_has_write_intent() || jetpack_has_posts_page() );
+
+ /**
+ * Filters whether blogging prompts are enabled.
+ *
+ * @since $$next-version$$
+ *
+ * @param bool $prompts_enabled Whether blogging prompts are enabled.
+ */
+ return apply_filters( 'jetpack_are_blogging_prompts_enabled', $prompts_enabled );
}
/**
diff --git a/projects/plugins/jetpack/extensions/blocks/blogging-prompts/blogging-prompts.php b/projects/plugins/jetpack/extensions/blocks/blogging-prompts/blogging-prompts.php
index 4b1a98d86f..34603f832c 100644
--- a/projects/plugins/jetpack/extensions/blocks/blogging-prompts/blogging-prompts.php
+++ b/projects/plugins/jetpack/extensions/blocks/blogging-prompts/blogging-prompts.php
@@ -18,42 +18,34 @@ const BLOCK_NAME = 'jetpack/' . FEATURE_NAME;
/**
* Registers the blogging prompt integration for the block editor.
*/
-function register_extension() {
- Blocks::jetpack_register_block( BLOCK_NAME );
-
- // Load the blogging-prompts endpoint here on init so its route will be registered.
- // We can use it with `WPCOM_API_Direct::do_request` to avoid a network request on Simple Sites.
- if ( defined( 'IS_WPCOM' ) && IS_WPCOM && should_load_blogging_prompts() ) {
- wpcom_rest_api_v2_load_plugin_files( 'wp-content/rest-api-plugins/endpoints/blogging-prompts.php' );
- }
-}
-
-/**
- * Adds the blogging prompt settings to wp-admin, if appropriate.
- *
- * @return void
- */
-function load_settings() {
+function register_extension_and_settings() {
// If editor extensions are not loaded, don't show the settings.
if ( ! \Jetpack_Gutenberg::should_load() ) {
return;
}
- // Blogging prompts is an expermental extension: if expermental blocks are not enabled, don't show the settings.
+ // Blogging prompts is an expermental extension: if experimental blocks are not enabled, don't show the settings.
if ( ! Constants::is_true( 'JETPACK_EXPERIMENTAL_BLOCKS' ) && ! Constants::is_true( 'JETPACK_BETA_BLOCKS' ) ) {
return;
}
- /**
- * Filter to hide the blogging prompts settings.
- *
- * @since $$next-version$$
- * @param bool $is_hidden Should the blogging prompts be hidden. Defaults to false.
+ /*
+ * Do not register the block and settings if the site opted out
+ * (either via a filter, or because it is not a blogging site)
*/
- if ( apply_filters( 'jetpack_blogging_prompts_hidden', false ) ) {
+ if ( ! should_load_blogging_prompts() ) {
return;
}
+ Blocks::jetpack_register_block( BLOCK_NAME );
+
+ // Load the blogging-prompts endpoint here on init so its route will be registered.
+ // We can use it with `WPCOM_API_Direct::do_request` to avoid a network request on Simple Sites.
+ if ( defined( 'IS_WPCOM' ) && IS_WPCOM && should_load_blogging_prompts() ) {
+ wpcom_rest_api_v2_load_plugin_files( 'wp-content/rest-api-plugins/endpoints/blogging-prompts.php' );
+ }
+
+ // Add the blogging prompt settings to wp-admin.
require_once __DIR__ . '/settings.php';
}
@@ -71,17 +63,18 @@ function inject_blogging_prompts() {
return;
}
- // And only for blogging sites or those explicitly responding to the prompt.
- if ( should_load_blogging_prompts() ) {
- $daily_prompts = jetpack_get_daily_blogging_prompts();
+ // Or if the blogging prompts are not enabled.
+ if ( ! should_load_blogging_prompts() ) {
+ return;
+ }
- if ( $daily_prompts ) {
- wp_add_inline_script(
- 'jetpack-blocks-editor',
- 'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG | JSON_HEX_AMP ) . ';',
- 'before'
- );
- }
+ $daily_prompts = jetpack_get_daily_blogging_prompts();
+ if ( $daily_prompts ) {
+ wp_add_inline_script(
+ 'jetpack-blocks-editor',
+ 'var Jetpack_BloggingPrompts = ' . wp_json_encode( $daily_prompts, JSON_HEX_TAG | JSON_HEX_AMP ) . ';',
+ 'before'
+ );
}
}
@@ -91,11 +84,9 @@ function inject_blogging_prompts() {
* @return bool
*/
function should_load_blogging_prompts() {
- $show_placeholder_prompt = ! apply_filters( 'jetpack_blogging_prompts_hidden', false ) && jetpack_are_blogging_prompts_enabled();
// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Clicking a prompt response link can happen from notifications, Calypso, wp-admin, email, etc and only sets up a response post (tag, meta, prompt text); the user must take action to actually publish the post.
- return $show_placeholder_prompt || ( isset( $_GET['answer_prompt'] ) && absint( $_GET['answer_prompt'] ) );
+ return jetpack_are_blogging_prompts_enabled() || ( isset( $_GET['answer_prompt'] ) && absint( $_GET['answer_prompt'] ) );
}
-add_action( 'init', __NAMESPACE__ . '\register_extension' );
-add_action( 'init', __NAMESPACE__ . '\load_settings' );
+add_action( 'init', __NAMESPACE__ . '\register_extension_and_settings' );
add_action( 'enqueue_block_assets', __NAMESPACE__ . '\inject_blogging_prompts' );
|
Is this PR obsolete now that the setting is removed in #28387? |
|
Yes, thanks--meant to close this. |
Changes proposed in this Pull Request:
Modifies the filter added in #27778 to differentiate it from the
jetpack_are_blogging_prompts_enabledsetting.With this change, the writing prompt loading logic now looks like this
jetpack_blogging_prompts_hiddenfilter is truejetpack_are_blogging_prompts_enabledsetting field loads in wp-adminanswer_promptquery param is set when loading the editor, it bypasses the filter and setting, allowing answering a prompt from an external link (email, notifications, etc)Other information:
Jetpack product discussion
See previous PRs (e.g. #26680, #27746, and #27778)
Does this pull request change what data or activity we track or use?
No.
Testing instructions: