Comment syncing between activity and post comments for Custom Post Types

[shanebp]

In 2.2.x the new way of registering post types activities is very useful.

But at the moment, devs have to chose between the new functionality or using deprecated filters so that comment syncing works.
See: ​https://buddypress.trac.wordpress.org/ticket/6478#comment:9

Of course, clients want both.
They love the drop-down filters, but don't want to lose comment syncing.

So this is a request for the ability to have both.
Thereby avoiding the requirement to use deprecated filters
And removing the 'Sophie's Choice' that is part of the current equation.

[imath]

Ok i'm going to work on this for next release.

FYI, i've edited the codex page once again. I understand this feature is very important for you and your clients, but i also think we shouldn't discourage users from using the "Post types activities" if they don't need it. I think the note i've added at the end of the page is enough and more global. Finally I've kept the link to this ticket you added into the codex page.

I hope you'll understand my point of view as i understand your eagerness to have this feature in.

[imath]

6482.01.patch is a first pass to "abstract" comments tracking / post comments - activity comments syncing for any post type.

How to use it the shortest way, example for the page post type :

add_post_type_support( 'page', 'buddypress-activity' );

bp_activity_set_post_type_tracking_args( 'page', array(
	'comment_action_id' => 'new_blog_page_comment',
) );

You just need to specify the action id of the comment action. Of course you can add labels to customize the drop down filters or the activity action string. To test it, i've used ​this gist, so be my guest to use it for your tests!

Some thoughts:

• as the option to allow/disallow syncing is not "attached" to the Blogs component, in a future release i guess we could move a lot of the code that is today in bp-blogs-activity.php in a bp-activity file. For now, to avoid deprecating too much function, i think we should keep it this way.
• Filtering activity stream for new_{post_type}_comment actions when comment syncing is on: i had to use a new activity meta containing the parent action (e.g.: bp_parent_action -> new_{post_type} ) to be able to fetch the right activity_comments into the new_{post_type}_comment. This work made me find an issue with the current feature see #6485. This also means we'll need an upgrade routine for the existing activity comments that are linked to post comments.
• Deleting a comment from front end when comment syncing is on. The current feature will systematically display an error although the activity comments / post comments are successfully deleted. To avoid this, i've used an apply_filters_ref_array(). About deleting, we also have this issue with the current feature #6484

I'd feel more comfortable :

• with lots (and even more than a lot!) of testing by contributors who are interested or requested this feature
• with more unit tests about it.
• having #6143 fixed because i was able to see how this is impacting scopes
• And above all, having r-a-y's eyes on the patch.

If we have this in, we complete the second step of #3460. Last step is the UI which could be fixed once we have #6026 in.

[shanebp]

After applying the 6482.01 patch and adding the gist ( thanks for that! ), the results are:

1. Notice: Undefined index: handle in ...\bp-templates\bp-legacy\buddypress-functions.php on line 313

2. SWA comments are showing in the single Book page comments

3. Book page comments are showing in the SWA for the Book entry

4. SWA comments are duplicated in SWA

Screenshot of SWA entry & comments:

Screenshot of Book post comments:

[shanebp]

SQL dump of the bp_activity rows for the duplicated comments 'again in swa'.

(31, 1, 'activity', 'activity_comment', '<a href="http://localhost/wordpress/members/shane/" title="shane">shane</a> posted a new activity comment', 'again in swa', 'http://localhost/wordpress/book/imath-test/#comment-25', 27, 27, '2015-06-10 18:31:01', 0, 8, 9, 0),

(32, 1, 'activity', 'activity_comment', '<a href="http://localhost/wordpress/members/shane/" title="shane">shane</a> posted a new activity comment', 'again in swa', 'http://localhost/wordpress/members/shane/', 27, 27, '2015-06-10 18:31:01', 0, 10, 11, 0);

[imath]

interesting thank you, i guess i'll need to test with javascript disabled. But it should fail for "post" post type too..

this

Notice: Undefined index: handle in ...\bp-templates\bp-legacy\buddypress-functions.php on line 313

might be due to the fact the SCRIPT_DEBUG constant is not set to true in your config.

[shanebp]

After adding the SCRIPT_DEBUG constant, SWA comments are not duplicated for SWA entries re Books or regular posts.

[imath]

6482.02.patch is fixing the duplicate activity comment issue. It's actually something that already exists in the current comments sync feature see #6494

[imath]

I think we should moved this ticket to next release. Working on it made me find some bugs inside the current Activity comment/Post comment sync feature, and i'd feel safer to have them all fixed in 2.4, then check everything is right once 2.4 is released, then work on this ticket during 2.5 cycle.

FYI, here is the list of bugs i'm talking about:

• #6484
• #6485
• #6494

[imath]

Hi all,

I've made some progress on this ticket, 6482.03.patch :

• is refreshed to latest trunk (r10429)
• includes some new unittests, improves existing ones and is potentially fixing #6128
• improves the wp-admin/activity screen (filtering + BP_Activity_Table-List->can_comment)
• is potentially fixing #6793
• now includes the upgrade routine i was talking about in a previous comment.
• and is potentially fixing this ticket :)

Testing

The regular post and comments are now fully using the Post Type activities, so you can test with this post type.

You can also play with the Page post type, here's an example of code to add inside a bp-custom.php file :

function customize_page_tracking_args() {
    // Check if the Activity component is active before using it.
    if ( ! bp_is_active( 'activity' ) ) {
        return;
    }

    // Don't forget to add the 'buddypress-activity' support!
	add_post_type_support( 'page', 'buddypress-activity' );

	/**
	 * Also don't forget to allow comments from the WordPress Edit Page screen
	 * see this screencap https://cldup.com/nsl4TxBV_j.png
	 */

    bp_activity_set_post_type_tracking_args( 'page', array(
        'action_id'                         => 'new_blog_page',
        'bp_activity_admin_filter'          => __( 'Published a new page', 'custom-textdomain' ),
        'bp_activity_front_filter'          => __( 'Page', 'custom-textdomain' ),
        'bp_activity_new_post'              => __( '%1$s posted a new <a href="%2$s">page</a>', 'custom-textdomain' ),
        'bp_activity_new_post_ms'           => __( '%1$s posted a new <a href="%2$s">page</a>, on the site %3$s', 'custom-textdomain' ),
        'contexts'                          => array( 'activity', 'member' ),
        'comment_action_id'                 => 'new_blog_page_comment',
        'bp_activity_comments_admin_filter' => __( 'Commented a page', 'custom-textdomain' ),
        'bp_activity_comments_front_filter' => __( 'Pages Comments', 'custom-textdomain' ),
        'bp_activity_new_comment'           => __( '%1$s commented on the <a href="%2$s">page</a>', 'custom-textdomain' ),
        'bp_activity_new_comment_ms'        => __( '%1$s commented on the <a href="%2$s">page</a>, on the site %3$s', 'custom-textdomain' ),
        'position'                          => 100,
    ) );
}
add_action( 'bp_init', 'customize_page_tracking_args' );

You can test having the "syncing" feature on or not.

I'd be very happy to have your feedbacks on this new patch.

[shanebp]

That's one big patch!

Applying your patch to trunk seems to work.

I created a new post:

• comments made on the post appear in the SWA
• comments made on the SWA appear in the Post comments

I created a new instance of a cpt:

• comments made on the post appear in the SWA
• comments made on the SWA appear in the Post comments

I was unable to test the front-end SWA filters, probably because I am getting this Notice:

Undefined index: handle in .../wp-content/plugins/buddypress-imath/bp-templates/bp-legacy/buddypress-functions.php on line 311

The filters seem to be working correctly here: .../wp-admin/admin.php?page=bp-activity

I used your example function customize_page_tracking_args() from above to test the cpt.
Would it be possible to include this new comment sync functionality within register_post_type() ?

[imath]

Thanks for your feedback @shanebp

to test the patch on front end, you'll need to add this line inside wp-config.php

define( 'SCRIPT_DEBUG', true );

About register_post_type() It's already built in the patch actually. There's a unit test showing how to do : test_bp_activity_format_activity_action_custom_post_type_comment

For the post type tracking arguments, it hasn't changed from what we have in the codex page about Post type activities. To set the comment tracking feature, you can do something like this :

$labels = array(
	'name'                              => 'foos',
	'singular_name'                     => 'foo',
	'bp_activity_comments_admin_filter' => __( 'Comments about foos', 'buddypress' ), // label for the Admin dropdown filter
	'bp_activity_comments_front_filter' => __( 'Foo Comments', 'buddypress' ), // label for the Front dropdown filter
	'bp_activity_new_comment'           => __( '%1$s commented on the <a href="%2$s">foo</a>', 'buddypress' ),
	'bp_activity_new_comment_ms'        => __( '%1$s commented on the <a href="%2$s">foo</a>, on the site %3$s', 'buddypress' )
);

register_post_type( 'foo', array(
	'labels'   => $labels,
	'public'   => true,
	'supports' => array( 'buddypress-activity', 'comments' ), // Adding the comments support
	'bp_activity' => array(
		'action_id'         => 'new_foo', // The activity type for posts
		'comment_action_id' => 'new_foo_comment', // The activity type for comments
	),
) );

[shanebp]

Thanks for the info re register_post_type().
I've made the changes to that function for a cpt in a BP component and everything is still working.
Including group extensions that use the cpt - sweet.

It'd be nice to see some other feedback - but I think your patch has closed the holes created when the 'the new way of registering post types activities' was introduced.

[imath]

Hi core team!
It's been 3 weeks i'm stuck here... Some feedbacks about 6482.03.patch would be greatly appreciated :)

ping @r-a-y & @DJPaul ;)

[r-a-y]

@imath,

Thanks for working on this and apologies for not looking at this ticket sooner!

I spent the last day or so looking at the code and I really appreciate the thought that has gone into making sure that the "Show" dropdown filter will work for post types that have registered comment tracking support.

Here are my notes:

1. action vs. type

This isn't really a problem with 03.patch, but is a problem we've had since BP v1.5.

Since BP v1.2, in bp_has_activities(), we accept a parameter called 'action', which is really the activity 'type'.

However, we also have several functions such as bp_activity_action_name() that are marked as deprecated since BP v1.5 in favor for their activity 'type' equivalent function.

In multiple parts of 03.patch, we reference the activity 'action'. Should we be referring to the the activity 'type' instead?

2. bp_activity_action_is_post_tracking_action( $action_id ) and bp_activity_action_supports_comments_tracking( $action_id ) functions

Instead of introducing these two new functions, I've created one function called bp_activity_type_supports(), which works similar to WP's post_type_supports() function and gives us some flexibility if we decide to add more features to BP activity types in the future.

So instead of bp_activity_action_supports_comments_tracking( $action_id ), we can use bp_activity_type_supports( $activity_type, 'comment-tracking' ) instead.

In bp_activity_type_supports(), you can also see that I also translated bp_activity_action_is_post_tracking_action() over to what I named 'post-comments'.

In 03b.patch (requires 03.patch to be applied first), I've replaced all instances of bp_activity_action_supports_comments_tracking() and bp_activity_action_is_post_tracking_action() with bp_activity_type_supports(). Let me know what you think of the bp_activity_type_supports() function.

Perhaps the function should be renamed to bp_activity_action_supports() instead? :)

3. Performance tweaks when the activity component is not active.

If for some reason, a site admin decides to disable the Activity component, we shouldn't load any of the activity code for the Blogs component.

In 03b.patch:

• I've moved BP_Blogs_Component::post_tracking_args() from bp-blogs-loader.php to bp-blogs-activity.php.
• I'm also conditionally loading bp-blogs-activity.php if the Activity component is active. This does change a few functions that were previously always available such as bp_blogs_record_activity() and bp_blogs_delete_activity(), but those functions always checked to see if the Activity component was active anyway.

4. Creating an activity item for an existing WordPress post.

This isn't really related to comment tracking, but when using the code snippet from comment:15 during testing, I tried to edit an existing WP page and hoped that would create an activity item for the page. It did not.

I think we should go ahead and create the activity item in this case. Patch adds support for this by adding a check in bp_activity_catch_transition_post_type_status() and adds a unit test.

I also removed an older unit test that explicitly checked that a post edit should not create a new activity.

---

Questions:

1. Should we move the majority of the "Post Comment Synchronization" function hooks from bp-blogs-activity.php to the Activity component (under bp-activity-actions.php)?

The reason I ask is 03.patch already deprecates a few of our older blog-specific activity functions such as bp_blogs_record_comment() and bp_blogs_remove_comment().

We probably should move ​most of these functions over to the Activity component as well.

2. Potential problem with wp_new_comment()

If a dev programatically adds a comment using wp_new_comment(), this bypasses checks for the post's comment status. So even if a post's 'comment_status' is set to 'closed', a comment can still be added with that function. Since this is mostly a WordPress thing, I don't think we need to do anything here, but thought I'd mention it.

[imath]

Thanks a lot @r-a-y for your feedback and suggestions. Here's .04.patch :)

Replying to r-a-y:

1. action vs. type

I've removed all references to 'action' in favor of 'type' where the term action was wrongly used ( meaning i've kept all stuff relative to action strings).

2. bp_activity_action_is_post_tracking_action( $action_id ) and bp_activity_action_supports_comments_tracking( $action_id ) functions

At first, because 3 weeks passed, i thought it was a great idea. i even thought about using 'comment-reply' as a support check. But i'm not feeling good about using it as a generic function to check if an activity type supports a feature or not.

These functions are there to check if the Activity type is about a WordPress Post Type supporting comments. So i see no objections having only one function instead of 2, but i think the name should then look like bp_activity_post_type_supports.

For instance, i've tried to use your idea for the activity_comment support for any activity, and i gave up because of a headache. So i suppose this/these are very specific to *Post Type* Activity types :)

3. Performance tweaks when the activity component is not active.

I agree, this is a very nice improvement. Added it to 04.patch :)

4. Creating an activity item for an existing WordPress post.
This isn't really related to comment tracking

^^ Absolutely. I've created #6834 for this.

Questions:

1. Should we move the majority of the "Post Comment Synchronization" function hooks from bp-blogs-activity.php to the Activity component (under bp-activity-actions.php)?

As long as the component is known as the "Site tracking" component, i think we should keep some code relative to site tracking in it. But i'm not strongly opposed about it :)

2. Potential problem with wp_new_comment()

Is this problem introduced by the patch ? Or is it existing since the introduction of synchronization ?

[r-a-y]

Thanks for making these changes, imath.

Here are my thoughts:

So i see no objections having only one function instead of 2, but i think the name should then look like bp_activity_post_type_supports.

The function name change to bp_activity_post_type_supports() sounded good to me at first. But then, the way we are using this function is to check all activity types to see if it has post type functionality on certain activity hooks. (See bp-blogs-activity.php.)

For example, bp_activity_post_type_supports( 'activity_update', 'comment-reply' ) doesn't really make sense as 'activity_update' is not a post type.

Perhaps we can use bp_activity_type_supports(), but namespace the post type support checks.

eg. bp_activity_type_supports( 'activity_update', 'post-type-comment-reply' )

For instance, i've tried to use your idea for the activity_comment support for any activity, and i gave up because of a headache.

Regarding 'activity_comment', would this mean the 'activity_comment' parameter in register_post_type()?

Or more generic activity comment support, which would be similar to how bp_activity_can_comment() works? (Maybe even an alias of that function?)

I think it would be great to expand on the feature set for this function in the future, but I do not think this is a dealbreaker as we already have the bp_activity_can_comment() function.

Do you have any other concerns that I might not be thinking of?

As long as the component is known as the "Site tracking" component, i think we should keep some code relative to site tracking in it. But i'm not strongly opposed about it :)

Yeah, after reading the description for the "Site Tracking" component -- Record activity for new posts and comments from your site -- I'm inclined to agree with you.

I keep thinking that the Blogs component just handles tracking user blogs.

However, we do have a bunch of code in bp-activity-actions.php referencing activity post type tracking. I guess we can make a decision about this at a later point.

2. Potential problem with wp_new_comment()

Is this problem introduced by the patch ? Or is it existing since the introduction of synchronization ?

Definitely not a problem with the patch!

---

My only minor issue is with the bp_activity_post_type_supports() function. I think another dev needs to chime in here.

Other than that, I think the patch is very close to committing!

[imath]

Many thanks for this new feedback and the chat we had on slack about it yesterday night r-a-y :)

I'm feeling a lot better with bp_activity_type_supports() now we're prefixing the post type activities specific features with a 'post-type' term.

I've updated the patch so it now uses this very promising new function :) In .05.patch i'm also including a 'comment-reply' feature check for any activity. We can do this from now on because we have a complete perimeter for post type activities (generate activity on when a post type is published / a Comment about this post type is validated / Comments between the Activity object and the Post Type object are synchronized). As a result the bp_activity_can_comment() function is now looking nicer. But as i've played in this area i also added 2 new unit tests about it.

I've tested it, and everything seems to work fine.

I'm feeling really confident about splitting/committing this .05.patch really soon :)

Does anyone have any objection ?

[r-a-y]

I'm feeling really confident about splitting/committing this .05.patch really soon :)

Me too! Everything looks good.

For me, I would say go ahead and commit as well! :)

[imath]

As discussed during dev-chat, i'll try to build a new patch to avoid the bp_parent_type activity meta that is problematic considering an upgrade routine + if we can avoid it, it's best :)

[imath]

In 6482.06.patch i'm getting rid of the new activity meta, so we don't need the upgrade routine anymore.

As discussed in dev-chat, the activity meta name used to reference the comment ID is now dynamic "bp_blogs_{$post_type}_comment_id". I've added a new property to the activity tracking argument of an activity type to easily get this $post_type value and a new function to get a tracking arg value according to the activity type bp_activity_post_type_get_tracking_arg().

In some places, we use to check if the activity meta "bp_blogs_post_comment_id" to figure out if the activity_comment was synced with a post comment. As this meta name is now dynamic, i had no other choices than to get the parent activity ID.

@r-a-y & @boone : feedbacks very welcome ;)

[imath]

In 10542:

Post Type Activities: allow post types to register comments tracking

When registering a post type, it is now possible to inform BuddyPress the comments about this post type need to be tracked into the activity stream. This can be achieved by making sure the post type supports "comments" and "buddypress-activity" and by adding extra parameters to the "bp_activity" argument of the register_post_type() function one or by using the bp_activity_set_post_type_tracking_args() function.

We are also introducing a new function to inform if an activity type is supporting a specific feature: bp_activity_type_supports(). For now this function is helping to check if:

• a activity supports a an activity comment
• an activity comment supports synchronization with a post type comment

Props shanebp, r-a-y.

See #6482

[imath]

In 10543:

Post Type Activities: introduce new functions to track the post type comments

These functions are generalizing to any post type what was only performed for the "post" post type so far.

We are also editing the bp_activity_new_comment() so that it is possible to post an activity comment without sending notifications.

Props shanebp, r-a-y.

See #6482

[imath]

In 10544:

Post Type Activities: generalize the Blogs component "post" comment synchronization feature to any post type.

As soon as the Blogs component is active and activity comments about Post Type activities are not disallowed, a new comment added to a post type will generate an activity comment about the corresponding Post Type Activity and vice versa.

Deprecate the Blogs component functions previously used to track the "post" post type in favor of the Activity component functions introduced in r10543.

Only load the Blogs component activity functions if the Activity component is active.

Make sure a synchronized activity comment marked as spam is also marking as spam the corresponding post type comment.

NB: some unit tests will temporarly fail after this commit. Next commit will fix the failing tests.

Props shanebp, r-a-y.

See #6482
Fixes #6793

[imath]

In 10545:

Post Type Activities: add unit tests and improve existing ones.

See #6482
Fixes #6128

[imath]

In 10546:

Post Type Activities: make sure it is possible to filter by post type activity comment action within the Activity Administration screen.

Fixes #6482