BuddyPress activity comment recording for Blog posts does not honour the WordPress post type visibility

[sbrajesh]

I was trying to use custom post type in one of the components I am writing at the moment.

I made the post type private(hidden) by setting the public=>false while registering post type. This post type allows commenting(Using custom functions for the purpose).

Just found that even if the post type was private, buddypress recorded comments for it. On clicking the link to post will lead page not found as the post type is private.

here is a small patch attached to fix it.

[boonebgorges]

Strictly speaking, what we *should* be doing is not publish comments from non-posts at all. In bp_blogs_record_post(), we do this: http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/buddypress-1-5-theme-updated-themes-list/edit?_wpnonce=9c2e10c299

This forces custom post types to create their own activity-posting function for new items, which puts the onus on plugin authors for this kind of thing.

Since 'post' will always have ->public == true, we wouldn't need the extra overhead of get_post_type_object(). (not that it's much overhead, of course - it just grabs the object out of the global)

In the future, I would like to have nicer API functions for custom post type activity posting (in my own plugins, I usually just copy over bp_blogs_record_comment() and bp_blogs_record_post(), which is silly). Such an API would definitely do the check that sbrajesh has suggested. But it is too late for 1.5.

Thoughts?

[sbrajesh]

Hi Boone,
Thanks for taking a quick look at this ticket. I completely agree with you. We should not record the comments for non posts. It should be ideally left to the developer/designer who create custom post types to handle such thing.

I will love to hear some more on the API for custom post type activity posting. It sounds something which could be very much usable. Just need some thought on that.

As of now, Can we just avoid recording for the non posts or put a conditional to check if the post is a custom post type, we should check for public visibility otherwise just record it as normal.

A side note, as I believe you are best person to tell about it. Is there any plan for using custom post type for activity stream(and the comments as activity comments) in bp 1.6. I may have to work on one such project soon and was wondering to know if that will be in the feature list of 1.6?

[enderandrew]

I'm more of a user than a developer, but I'd prefer for Buddypress to support comments for custom post types rather than force plugin developers to reinvent the wheel, and perhaps do so in a manner that is inconsistent with core, especially if core is updated and the plugin is not.

I know that bbPress 2.x uses custom post types for forum threads. Are the responses considered comments? If so, and Buddypress didn't recognize the comments on thost custom post types, it would break activity of those responses.

[boonebgorges]

I'd prefer for Buddypress to support comments for custom post types rather than force plugin developers to reinvent the wheel

Yes, that's what I'm talking about. Right now, BP appears to handle them sort of accidentally, in a buggy way. That will be disabled for 1.5, as it causes privacy problems like the one that sbrajesh brings up. The future enhancement will have full support for all CPTs.

I will love to hear some more on the API for custom post type activity posting.

I've opened a separate ticket for discussion: #3460

Is there any plan for using custom post type for activity stream(and the comments as activity comments) in bp 1.6.

Yes. The plan, at the moment, is to move all data that we currently keep in custom tables into WP core tables, via CPTs.

[djpaul]

(In [4959]) When recording activity items for comments, check that the post object is a standard WordPress Post. Fixes #3455, props sbrajesh for initial patch

[anointed]

PLEASE don't remove comments from post-types. This would be a huge regression for every one of my sites.

example site:

1. post-type 'sermons' which are custom video posts. I definitely use comments on these just like a regular post. Sbrajesh provided some sample code to work with last year that integrated my post-types into the activity stream, so they are now an integral part of bp for me.

2. post-type 'devotionals' relies 100% on having the comments working, (they are useless without user comments as that is how they track their progress) and being part of bp is quite critical.

[boonebgorges]

anointed - Thanks for jumping in here.

The behavior in 1.2.9, where CPT posts are *not* recorded, while CPT comments *are*, is inconsistent and incorrect. It should be changed. However, anointed's comment shows that some people's BP installations are currently relying on BP's inconsistency for important site functionality.

The "correct" solution is for people like anointed to create custom functions, like the one he's got for "sermons", that integrate CPT comments into the activity stream. In other words, copy the entire function bp_blogs_record_comment() (along with the hook to comment_post), change the name, and then change the newly added CPT check lines ​https://buddypress.trac.wordpress.org/browser/trunk/bp-blogs/bp-blogs-functions.php#L249 to

// Don't record activity if the comment's associated post isn't one of my custom post types
if ( 'sermons' != $recorded_comment->post->post_type && 'devotionals' != $recorded_comment->post->post_type )
    return false;

I'm reopening this ticket to get feedback on a few questions:
1) Do we have a sense of how many users r4959 will negatively affect? anointed is one; do you think there will be many, many more? Are there, for example, common BP plugins that use CPTs and thus are probably exploiting the error that this ticket was opened to address?
2) How many users will r4959 *positively* affect? That is, how many people use CPTs in a way that allows comments, but does not make sense in the activity stream?
3) How do we want to proceed? Regarding #3460, it would be trivially easy to implement a new function that is a catcher for all other custom post type comments, with a filter at the top of blacklisted CPTs; meanwhile, we could leave r4959 as is. This would be the beginning of the API proposed in #3460, and would be 100% non-retrograde. However, given that we are approaching a final beta, is it too late to add something substantial like this?

Feedback welcome. We shouldn't make a large architectural decision based merely on anointed (though I'm sure your site is cool, there is a straightforward enough workaround, as described above!!), his report may be indicative of more widespread complaints after release.

[cnorris23]

I don't think it's too late for no. 3. If we're already making a change like this, we might as well do what we can to ease the burden. I don't know exactly what you're thinking boone, but here's what I have in my head as a possible solution (sorry, I'm not at me dev setup so I can't write a patch):

bp_blogs_record_comment_for_post_type( $post_type = '' ) {
     $retval = true;

     if ( 'post' != $post_type )
          retval = false;

     return apply_filters( 'bp_blogs_record_comment_for_post_type', $retval, $post_type );
}

The post_type check would then change to:

if ( !bp_blogs_record_comment_for_post_type( $recorded_comment->post->post_type ) )
     return false;
}

For annointed, you could then do this:

my_record_comments_for_post_type( $retval, $post_type ) {
     if ( 'sermons' == $post_type || 'devotionals' == $post_type )
          $retval = true;

     return $retval;
}

Something similar could be done for bp_blogs_record_post().

[cnorris23]

I forgot an important line of code in this, currently, hypothetical scenario ;) You would need this after the my_record_comments_for_post_type() code:

add_filter( 'bp_blogs_record_comment_for_post_type', 'my_record_comments_for_post_type', 10, 2 );

[DJPaul]

As always, no problems with adding new actions or filters where they could be useful.

I don't think filtering of the post type needs to have its own function; patch 3455-2.patch does it simply, and inline. I also think we should *not* do something similar for bp_blogs_record_post(), because the issue in this ticket is really more about how do we make it as easy as possible for plugins that may have been using this bug as a feature to continue doing so.

In-depth architecture/API decisions, like Boone refers to, should wait until our custom post type release.

[boonebgorges]

This issue doesn't really have anything to do with BP's custom post type release. But it's too late in our dev cycle to do anything more substantial than what DJPaul is proposing. In fact, I started working on something meatier, but I found that it was quickly getting out of hand, and really needs to wait until the next feature release.

3455-3.patch puts a similar whitelist filter in bp_blogs_record_post(). DJPaul, you say we don't need one, and you're right that in the case of blog posts we are not risking a retrograde movement, but I don't see any reason why not to do it. Old methods for preventing bp_blogs_record_post() from firing (namely, unhooking the function) will continue to work.

I also changed the filter names to be plural, since it's an array.

The fact remains that 3455-3.patch will create problems for anointed, but it makes the solution much easier:

bbg_publish_my_post_type_comments( $types ) {
   $types[] = 'sermons';
   $types[] = 'devotionals';
   return $types;
}
add_filter( 'bp_blogs_record_comment_post_types', 'bbg_publish_my_post_type_comments' );}}}

[DJPaul]

Looks good

[anointed]

@boonebgorges
If that little snippet above is all I have to do in order to maintain the functionality that I currently have, then I am a happy camper. I just wanted to make sure the ability was not 'prohibited' until the api comes out.

[boonebgorges]

anointed - Great. I will commit this change and write a bpdevel post about the issue.

[boonebgorges]

(In [4971]) Adds post_type whitelist filters to bp_blogs_record_post() and bp_blogs_record_comment(). Fixes #3455. Props DJPaul