Sync: add Action Scheduler to the list of blacklisted post types

[jeherve]

Fixes #12714

Changes proposed in this Pull Request:

• Action Scheduler is used to schedule and run background jobs on WordPress, and is used extensively by WooCommerce and WooCommerce extensions.

The data is currently stored in a custom post type, and as such it appears in WordPress.com' activity log even though it does not provide a lot of value to site owners:

This PR stops this post type from being synchronized with WordPress.com, so the events should then stop appearing in the activity log.

Edit

I've now removed the additonal blacklisting of the Action Scheduler comments, as those will not be synced once #13079 gets merged.

Testing instructions:

• Switch your Woo site to run that branch for a few days.
• You should spot no events from ActionScheduler in your activity log.
• You should still be able to use things like the Woo mobile app, or the Woo Store in Calypso.

Proposed changelog entry for your changes:

• Sync: add Action Scheduler to the list of blacklisted post types

[lezama]

re running tests

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 9129c5a

[timmyc]

@jeherve thanks for running with the fix here. So looks like this is working somewhat - I'm not seeing the posts synched anymore in the activity log, but I am seeing comments on the Action Scheduler post showing up in Calypso:

I ran this test by creating a new jurassic.ninja instance with this branch... so I think I did it right, but just in case here is my plugin setup on the site:

Jetpack Free plan.

Wondering if we could also filter these off in the calypso view too. That would effectively hide all past actions too. I could look into that if that would help.

[timmyc]

So did some looking to see if there was a clean way to filter off these events in Calypso. While we could add in some checks there in the components to see if the log events are related to ActionScheduler, and then not display them, that will obviously making pagination wonky in the UI.

Next stop was the endpoint logic https://public-api.wordpress.com/wpcom/v2/sites/<SITE ID>/activity?_envelope=1&aggregate=1&number=1000 which doesn't support a query arg to filter off by a comment type, or post type.

I suppose some logic could be added to the endpoint itself to filter off activity log results prior to sending the API response. I didn't dig further but that seems do-able.

But ultimately looks like using this filter should allow preventing a sync of a comment made by ActionScheduler and clear up the Activity log on wpcom completely.

I think I may have mentioned this already, but the plan is for ActionScheduler to move away from being a CPT ( /cc @rrennick ) in the next major release of 3.0. So really this problem is kind of a temporary issue until that happens.

Going to comment on the original issue to that affect to. I guess my .02 - this has been reported as a bug by one person. We do have a way to fix this it seems, but if we do we should also circle back and clean this code up once a CPT is no longer used.

[rrennick]

I think I may have mentioned this already, but the plan is for ActionScheduler to move away from being a CPT ( /cc @rrennick ) in the next major release of 3.0.

Migrating off the CPT will depend on the site running PHP 5.6 or higher. So, this could be coordinated with when JP also requires 5.6.

[jeherve]

I am seeing comments on the Action Scheduler post

Ah, that's a good point. I like the idea of filtering off comments belonging to a specific post type, or comments of a specific comment type.

@lezama What's your take on this?

this could be coordinated with when JP also requires 5.6.

That sounds like a good idea. Jetpack aims to support the last 2 versions of WordPress, and match their requirements. That means that when WordPress 5.3 is released, we'll support WP 5.3 and WP 5.2, and our PHP requirements will then match WP 5.2 (PHP 5.6).

[lezama]

But ultimately looks like using this filter should allow preventing a sync of a comment made by ActionScheduler and clear up the Activity log on wpcom completely.

that should do it yes 👍

Are these actions completely useless in order to debug what's going on on the site? shouldn't these leave Rewind "checkpoints"?

[timmyc]

Are these actions completely useless in order to debug what's going on on the site? shouldn't these leave Rewind "checkpoints"?

ActionScheduler jobs can do any variety of logic behind the scenes. So theoretically I could see someone wanting to rewind to a point in time before a specific job executed. But it seems like the actions spawned by the jobs ( updating a post, product inventory whatever it might be ) will also create a new restore point/action that could be restored against too.... so kind of redundant.

[jeherve]

@lezama Would something like this work, or should this live within packages/sync/src/modules/Comments.php directly?

diff --git a/packages/sync/src/modules/WooCommerce.php b/packages/sync/src/modules/WooCommerce.php
index 84d77a037..f4b5a9f2d 100644
--- a/packages/sync/src/modules/WooCommerce.php
+++ b/packages/sync/src/modules/WooCommerce.php
@@ -48,6 +48,9 @@ class WooCommerce extends Module {
 
 		add_filter( 'jetpack_sync_before_enqueue_woocommerce_new_order_item', array( $this, 'filter_order_item' ) );
 		add_filter( 'jetpack_sync_before_enqueue_woocommerce_update_order_item', array( $this, 'filter_order_item' ) );
+
+		// Blacklist Action Scheduler comment types.
+		add_filter( 'jetpack_sync_prevent_sending_comment_data', array( $this, 'filter_action_scheduler_comments' ), 10, 2 );
 	}
 
 	function name() {
@@ -168,6 +171,24 @@ class WooCommerce extends Module {
 		return array_merge( $list, self::$wc_comment_meta_whitelist );
 	}
 
+	/**
+	 * Stop comments from the Action Scheduler from being synced.
+	 * https://github.com/woocommerce/woocommerce/tree/e7762627c37ec1f7590e6cac4218ba0c6a20024d/includes/libraries/action-scheduler
+	 *
+	 * @since 7.6.0
+	 *
+	 * @param boolean $can_sync Should we prevent comment data from bing synced to WordPress.com.
+	 * @param mixed   $comment  WP_COMMENT object.
+	 *
+	 * @return bool
+	 */
+	public function filter_action_scheduler_comments( $can_sync, $comment ) {
+		if ( isset( $comment->comment_agent ) && 'ActionScheduler' === $comment->comment_agent ) {
+			return true;
+		}
+		return $can_sync;
+	}
+
 	private static $wc_options_whitelist = array(
 		'woocommerce_currency',
 		'woocommerce_db_version',

[lezama]

@jeherve perfect, I think it needs to go in the woo module 👍