Skip to content
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

Sync: do not sync comments made via Action Scheduler #13355

Merged
merged 1 commit into from
Sep 19, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Aug 28, 2019

Follow-up of #12830.

Fixes #13354

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:

image

In #12830, I stopped posts from that Post Type from being synchronized with WordPress.com, but that wasn't enough. This PR aims to stop the comments from being synchronized as well.

Here is an example of a comment we are trying to catch with this:

'WP_Comment Object
(
    [comment_ID] => 3468
    [comment_post_ID] => 25402
    [comment_author] => ActionScheduler
    [comment_author_email] =>
    [comment_author_url] =>
    [comment_author_IP] =>
    [comment_date] => 2019-08-28 18:33:41
    [comment_date_gmt] => 2019-08-28 09:33:41
    [comment_content] => action created
    [comment_karma] => 0
    [comment_approved] => 1
    [comment_agent] => ActionScheduler
    [comment_type] => action_log
    [comment_parent] => 0
    [user_id] => 0
    [children:protected] =>
    [populated_children:protected] =>
    [post_fields:protected] => Array
        (
            [0] => post_author
            [1] => post_date
            [2] => post_date_gmt
            [3] => post_content
            [4] => post_title
            [5] => post_excerpt
            [6] => post_status
            [7] => comment_status
            [8] => ping_status
            [9] => post_name
            [10] => to_ping
            [11] => pinged
            [12] => post_modified
            [13] => post_modified_gmt
            [14] => post_content_filtered
            [15] => post_parent
            [16] => guid
            [17] => menu_order
            [18] => post_type
            [19] => post_mime_type
            [20] => comment_count
        )

)
'

(matching blog ID is 114940227)

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

  • If you're an Automattician, include a shortlink to the p2 discussion with Jetpack Product here.

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:

  • Activity Log: avoid displaying events from the Action Scheduler.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Package] Sync [Pri] Normal labels Aug 28, 2019
@jeherve jeherve added this to the 7.8 milestone Aug 28, 2019
@jeherve jeherve requested review from roccotripaldi and a team August 28, 2019 10:36
@jeherve jeherve self-assigned this Aug 28, 2019
@jeherve
Copy link
Member Author

jeherve commented Aug 28, 2019

@roccotripaldi If you have the chance I'd love to have your review on this since you worked on a related PR a little while ago.

Thank you!

@jetpackbot
Copy link

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: September 3, 2019.
Scheduled code freeze: August 27, 2019

Generated by 🚫 dangerJS against b560ba7

@kraftbj kraftbj self-requested a review September 18, 2019 17:10
@kraftbj
Copy link
Contributor

kraftbj commented Sep 18, 2019

Adding this to a live WC site to confirm.

Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised.

@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 18, 2019
@jeherve jeherve merged commit a41b084 into master Sep 19, 2019
@jeherve jeherve deleted the sync/ignore-action-scheduler-comments branch September 19, 2019 07:40
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 19, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Sync [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Activity Log: ActionScheduler actions on the Activity page
4 participants