Synchronizing activity comments to main component

[r-a-y]

For some historical context, see #1419.

There are a couple of hurdles that need to be accomplished for this to work.

1) Main component callback registration

When extending the BP_Component class, we need to give plugin devs a place to register some callback methods.

2) Two-way sync of activity comments

For example, let's look at blog comments.

1. If an activity comment is made, a corresponding item should be posted as a blog comment. For comments, this should handle nested levels as well.

2. Likewise if a blog comment is made, a corresponding item should be made as an activity comment under the parent 'new_blog_post' activity item. Currently, when a blog comment is made, it is made as a separate activity item.

3. Deletion should be handled in a similar manner. In the case of blog comments, in WP, when a parent comment is deleted, the child comments are moved up a level, while BP simply deletes all activity children. Something to keep in mind!

3) Moderation considerations

With blog comments, we'll have to check WP's commenting settings such as checking if blog comments are closed after a certain period( 'close_comments_for_old_posts' / 'close_comments_days_old' )

---

Attached patch covers a few of these points that I'll elaborate on in a follow-up reply.

[r-a-y]

01.patch addresses several points from the ticket description and is considered a work-in-progress.

1) Main component callback registration

In the BP_Component class, I have added a new method that will check to see if an extended component has added the activity_comment_post_callback property. If it does, then BP_Component will automatically set up some hooks that the extended component will use.

An example of how this is used can be found in the BP_Blogs_Component class.

I felt this was the best way for plugin devs to register their callbacks in the current API. Let me know what you think.

2) Two-way One-way sync of activity comments :)

So far, I have completed 2-1 and a portion of 2-3.

To test, make sure in "Settings > BuddyPress > Settings", that "Allow activity stream commenting on blog and forum posts" is checked.

For 2-1, once the patch is applied, in the activity directory, navigate to an activity item with the new_blog_post type. Next, add an activity comment. A corresponding blog comment should be posted on the blog post permalink page. Nested levels should work as well!

For 2-3, this is a little tricky and is not quite complete yet. As noted in the ticket description, in WP, when a parent comment is deleted, the child comments are moved up a level, while BP simply deletes all activity children.

---

In the patch, you'll also notice that I'm using a lot of this:

$cannot_comment = isset( buddypress()->site_options['bp-disable-blogforum-comments'] ) ? buddypress()->site_options['bp-disable-blogforum-comments'] : false;

Instead of bp_disable_blogforum_comments() because bp_disable_blogforum_comments() uses bp_get_option() and that will kinda suck on multisite installs.

Anyway, I just wanted to get something up before the start of the week. Hope you dig what I've got so far. Looking forward to some feedback.

[boonebgorges]

Looks like an excellent start, r-a-y! Thanks for the detailed description of the issues, and for the initial patch. Initial thoughts:

should this use display name?

Yeah, I think that's better than the username.

'comment_type' => , could be interesting to add 'buddypress' here...

Needs a bit of research. I think WP uses this to distinguish between trackbacks and regular comments. Could be that other plugins use this field for other things. I don't want to mess with any established conventions. But yeah, it would be neat to track which blog comments come from the activity stream!

if cannot comment, stop now! [ in bp_blogs_sync_delete_from_activity_comment()]

It's possible that this setting would have been toggled in the interim. I'd suggest removing the check - it adds minimal overhead (comments are deleted very rarely), and could catch legit activity deletion in some cases where disable_blogforum_comments has been toggled.

In the patch, you'll also notice that I'm using a lot of this:

$cannot_comment = isset( buddypress()->site_optionsbp-disable-blogforum-comments? ) ? buddypress()->site_optionsbp-disable-blogforum-comments? : false;

Maybe that's what we should be doing in bp_disable_blogforum_comments() instead of the bp_get_option() call. (Same goes for other similar settings functions.)

Main component callback registration

I really like this technique, where plugin authors register their callbacks. It mirrors the WP action/filter model in a nice way.

if a blog comment is made, a corresponding item should be made as an activity comment under the parent 'new_blog_post' activity item. Currently, when a blog comment is made, it is made as a separate activity item.

I know this isn't implemented yet, but just a thought: if we do it this way, will the parent items get bumped to the top of the activity stream? Should they?

I'll try to get a chance to do more thorough testing in the next week or so.

[DJPaul]

I like the callback function in the component loader that handles activity comments. We should pull that into a separate patch and get that committed early.

I dislike the approach that bp_blogs_sync_add_from_activity_comment() is taking. To summarise the patch, after an activity comment is posted on a blog post, we're creating a WP Comment on that post. And it handles removing activity comments after a WP Comment is deleted.

But, we already track new blog comments with the existing bp_blogs_record_comment() function.

I would suggest a more logical approach is that if someone leaves an activity reply to a blog post activity item, we only create the blog comment. The existing code will take care of creating the activity item.

We'll definitely need to handle comment deletion sync that you've highlighted.

As noted in the ticket description, in WP, when a parent comment is deleted, the child comments are moved up a level, while BP simply deletes all activity children.

Can't we just do the same?

[boonebgorges]

I would suggest a more logical approach is that if someone leaves an activity reply to a blog post activity item, we only create the blog comment.

I'm pretty sure that this is what the patch does. There are two scenarios:

1) A WP comment is left on a blog. bp_blogs_record_comment() is then responsible for creating the new_blog_comment activity item. This is unchanged by r-a-y's patch.
2) Someone leaves an activity comment on a new_blog_post or new_blog_comment item in the activity stream. Currently, BP does nothing with this. r-a-y's patch adds the enhancement that the activity comment is added as a WP Comment in the appropiate place in the comment thread of the blog post belonging to the parent activity id (oy, that's complicated).

So, I don't see the redundancy. r-a-y's solution looks pretty direct to me. I might be missing something though.

[DJPaul]

Gotcha. Looks like I misread. So basically what the patch does is adds a comment to a blog post when an appropriate activity comment is added.

What about instead changing bp_activity_new_comment() -- if the comment is being added to a new_blog_post activity parent, we add the new WP Comment *here*, and then bail out; bp_blogs_record_comment() will then take care of the activity comment.

Any benefit to doing it this way vs. the patch?

[boonebgorges]

Any benefit to doing it this way vs. the patch?

One reason for doing it r-a-y's way is to keep the components separate. bp_activity_new_comment() shouldn't make any reference to the Blogs component. The way r-a-y has it set up, Blogs (and Forums, and whatever other component might want to do something similar) will just hook to the generic 'bp_activity_comment_posted' action, keeping things independent.

Either way, there's some "bailing out" required. As r-a-y has it configured, we have to bail from running bp_blogs_record_comment() for WP Comments created this way. If we went with your model, we'd need to put some sort of bailout into bp_activity_new_comment() - maybe something like:

$pre = apply_filters( 'bp_activity_new_comment_pre', false );
if ( $pre !== false ) {
  return $pre;
}

(Sorta like update_metadata() etc work.) But this seems to be more confusing than keeping everything on the same hook at the end of the function, at least IMO.

[r-a-y]

Thanks for the feedback so far, guys. Most of your feedback kind of cancels each other out! :)

Anyway, here's my long-overdue update for this ticket.

Note: You'll probably want to test this with a new blog post. Remember to go to "Settings > BuddyPress > Settings" and check "Allow activity stream commenting on blog and forum posts".

---

What's new in 01b.patch:

• Synching blog post comments back to the main 'new_blog_post' activity entry as an activity comment is now working (two-way sync is complete)
• Disables activity replying to the 'new_blog_post' activity entry (and its activity comment children) if the WP blog post has closed comments off (either manually or automatically)
• Activity comment permalinks now use the WP comment link instead of the main activity permalink. Some caveats here with AJAX. See comments in bp_blogs_sync_add_from_activity_comment().
• Changing bp_get_option() to check the buddypress()->site_options array first before querying with get_blog_option(). This allows me to use bp_disable_blogforum_comments() again as intended. (Should probably create a new ticket for this.)

What isn't done:

• Handling deletion for nested comments and its corresponding activity items (2-3).
  • Still need to think of a decent strategy. I'm thinking of deleting all WP comment children and all BP activity children instead of trying to emulate WP's "move comments up a level" technique.
• Fixing activity comment permalink when posting via AJAX to use WP comment permalink. This is fixed after AJAX posting (when you refresh the page) though. A slight niggle.

General concerns:

• There is a lot of manual code that needs to be written if a plugin dev wants to sync activity comments with their component.
  • I want to extrapolate some of the code in bp_blogs_disable_activity_commenting(), bp_blogs_disable_activity_replies() and bp_blogs_activity_comment_permalink() into the API to make things a little easier. But this means adding more activity code into the BP_Component class... maybe it's a good time to start thinking about a BP_Activity_Extension class.
  • Plugin devs will still have to probably write code to handle trash / spam / ham routines when working with WP comments in their plugins.
• Synchronizing will only work for new entries. So people expecting their older WP comments to show up under the 'new_blog_post' activity entry will be SOL.
• People will undoubtedly ask about supporting forum threads. I don't want to do this for the legacy forums because 'bbpress_init' is kind of a killer, but we'll probably want to add support to bbPress 2.4.

I'll probably start breaking up the patch into smaller commits once the deletion issue is handled.

[r-a-y]

Updated patch to work with trunk

[boonebgorges]

Awesome, r-a-y! You've thought this out really thoroughly (as usual, it's more complex than I'd originally thought). Some thoughts and questions, in no particular order:

we still have to disable activity commenting for 'new_blog_comment'
commenting should only be done on the parent 'new_blog_post' item

How does this work with WP comment threading, where you can reply to a comment (comment_parent)? Should we be checking the 'thread_comments' setting, and recording the thread position appropriately in WP (comment_parent) and BP (item_id + secondary_item_id)? Not a showstopper for 1.9, but just throwing it out there.

handle timestamps for the WP comment after we've switched to the blog

Maybe we should use the actual activity item timestamp rather than the current time? We're already loading the activity item later in the function; could just move it up. (Very minor, obviously.)

what if a site already has some comment email notification plugin setup?
this is why I decided to go with bp_activity_add() to avoid any
with existing comment email notification plugins

Ugh, yeah, we're going to get complaints either way :) Your method seems good for now.

Changing bp_get_option() to check the buddypress()->site_options array first before querying with get_blog_option(). This allows me to use bp_disable_blogforum_comments() again as intended. (Should probably create a new ticket for this.)

Probably falls under #4913

Still need to think of a decent strategy. I'm thinking of deleting all WP comment children and all BP activity children instead of trying to emulate WP's "move comments up a level" technique

I'm wary of messing with WP content in this way. How complicated is WP's method? Do they have a sort of walker/recursive function that we could borrow?

Fixing activity comment permalink when posting via AJAX to use WP comment permalink. This is fixed after AJAX posting (when you refresh the page) though. A slight niggle.

Very slight. I'd put this at the bottom of the priority list.

There is a lot of manual code that needs to be written if a plugin dev wants to sync activity comments with their component.

Your ideas here sound good to me, but I would deemphasize this for now. I can think of very few plugins where this would even make sense (maybe BuddyPress Docs). If it's not modular for 1.9, I think it'll be more than fine.

People will undoubtedly ask about supporting forum threads. I don't want to do this for the legacy forums because 'bbpress_init' is kind of a killer, but we'll probably want to add support to bbPress 2.4.

Agreed about not supporting legacy forums. bbPress 2.x support would be great; let's get blogs working the way we want it, and then do forums as a second pass. I want to be sure we're happy with the technique before getting it copied over to bbPress. (Also, if this were not to happen for 1.9, it probably would not be the end of the world. This is probably related to the point about a plugin API above.)

I'll probably start breaking up the patch into smaller commits once the deletion issue is handled.

Awesome, I am excited about this one. Thanks, r-a-y.

[r-a-y]

In 7448:

Pass the parent activity object to the 'bp_activity_comment_posted'
hook.

This will ensure that plugins can reference the parent activity item
when an activity comment is posted.

See #5130.

[r-a-y]

This isn't going to make 1.9 I'm afraid.

Ran out of time to address the deletion and comment threading issues.

[boonebgorges]

Just marked #4247 as a duplicate.

r-a-y, how do you feel about the state of this patch? I seem to remember that it's pretty good. Do you think it's possible to get a good-enough version in for 2.0, with a specific roadmap for making improvements for 2.1? This seems like one of those cases where "mostly working" is better than nothing.

[r-a-y]

02.patch is updated for trunk.

This patch also addresses the lingering problems from the older patch.

Specifically:

1. Blog comment deletion and activity comment deletion.
2. Checking the blog's threaded comment depth setting
3. Fixes activity comment permalinks on AJAX

So basically all the main issues are resolved in some manner.

1) Blog comment deletion and activity comment deletion

I've chosen to delete blog comments only if the blog comment is associated with an activity comment since BP doesn't have a 'status' field to toggle.

I've also had to change a few things:

• Activity deletion callback now uses the 'bp_activity_delete_comment_pre' filter instead of the bp_activity_delete_comment' action because I need access to the activity comment children. 'bp_activity_delete_comment' is too late.

• Added utility function - bp_activity_recurse_comments_activity_ids() - to grab the activity comment IDs for an activity item

• Added utility function - bp_blogs_remove_associated_blog_comments() - When an array of activity IDs are passed, this will try and find the blog comments that have an associated activity entry. If there are matches, the blog comments are removed.

2) Checking the blog's threaded comment depth setting

This might overlap a bit with these tickets:
​https://buddypress.trac.wordpress.org/ticket/1870#comment:2
​https://buddypress.trac.wordpress.org/ticket/2768

There are some other issues at play here:

• If WP's threaded comments are later disabled, WP's comments are displayed flat. However, BP's activity stream does not change. If we wanted to mirror this, we'd need to rebuild the activity comment tree for each corresponding blog post. Not very great.
• If different blogs have different threaded depths, this will not make for a consistent appearance in the activity stream. The question is whether we should even adopt WP's threaded depth setting?

3) Fixes activity comment permalinks on AJAX

My previous patch did not account for setting up the temporary globals when AJAX was used.

This is now remedied in the new bp_blogs_setup_activity_loop_globals() and bp_blogs_setup_comment_loop_globals_on_ajax() functions.

Perhaps we should look into caching the blog's threaded depth and the post's comment status options ourselves instead of using the singleton-stuffing method in this patch.

Feedback welcome.

[imath]

hi r-a-y

First: very nice work. I like a lot this feature.

I've tested your patch and this is my feedback :)

1. If from the WP Admin comments screen i edit a comment, then the corresponding activity is not edited, a new one is created. And it's no more possible to reply to this activity. We can only delete it and so on for the next activity synced from new comments.
2. If i trash a comment, it's still displayed in the activity stream. If i trash a post, the post and the comments disappear. When i untrash the post, the activity of this post is there, the comments no more, they are deleted. In this particular case the comment_metas 'bp_activity_comment_id' are not deleted.
3. In the Activity Admin, the activity of comments are above the activity of post. When replying to the activity of the post from this area, ajax puts the activity under the one of the post. If i refresh, the activity of the comment go on top.
4. I've tested with a custom post type by filtering 'bp_blogs_record_post_post_types' and 'bp_blogs_record_comment_post_types' and it works fine.

i'm very interested as it will have an impact on #3460 :)

[r-a-y]

Hi imath,

Thanks for the testing! Just going to reply to a few of your comments:

1. Sounds like a bug with how the activity component handles editing blog post comments. Will definitely look into it.

2. When you trash a comment, unfortunately, the corresponding activity stream will still be shown. This is because we don't really have a 'status' column in the activity DB. I'm not sure what the best way is. Do you have any ideas? :) As for the second part about untrashing, this sounds like a bug. Will look into this.

3. Totally missed testing in the Activity Admin dashboard. Will take a closer look.

4. CPT support - Cool, glad it works! Can you test what happens when you disable comments when registering your post type? I'm not sure where but we might need to use post_type_supports() to check if the CPT supports comments somewhere in both our patches.

Thanks again for the feedback!

[boonebgorges]

Looking great, r-a-y.

Perhaps we should look into caching the blog's threaded depth and the post's comment status options ourselves instead of using the singleton-stuffing method in this patch.

I'd suggest mirroring it in blogmeta. Then it'll be autoslurped in activity loops with ​https://buddypress.trac.wordpress.org/browser/trunk/bp-blogs/bp-blogs-activity.php#L214

If different blogs have different threaded depths, this will not make for a consistent appearance in the activity stream. The question is whether we should even adopt WP's threaded depth setting?

Groan, this is a lose-lose. I'd suggest looking at the subblog's comment depth setting and the main blog's comment depth settings (assuming #2768; if not, just hardcode a sensible number like 5), and then choosing the lesser of the two when deciding whether to allow activity commenting.

If WP's threaded comments are later disabled, WP's comments are displayed flat. However, BP's activity stream does not change. If we wanted to mirror this, we'd need to rebuild the activity comment tree for each corresponding blog post. Not very great.

This seems like such a harmless edge case that I'd suggest forgetting that you ever thought of it :)

[marcusyong]

so great idea, But I test it on localhost and my website,
when author leave a comment on post, it works;
but when a other user leave comment, it doesn't creat a activity.
How can I solve this?

[DJPaul]

Please don't change the ticket owner/priority/severity/milestone fields. Thanks.

[r-a-y]

03.patch incorporates the following fixes:

• Editing a post comment properly updates the associated activity comment
• Editing an activity comment entry properly updates the associated blog post comment
• Trashing a post comment will mark the associated activity comment as spam. Untrashing the post comment will reinstate the activity comment.

• Activity admin list table: The mouseover actions now supports whether the activity item allows replies. I had to create a new method - BP_Activity_List_Table::can_comment() - that duplicates some functionality here unfortunately.

• Trashing a blog post will *delete* any blog comments that have an associated activity entry. The reason why is bp_blogs_transition_activity_status() doesn't support trash / untrash post statuses. The proper way would be to mirror how bp_blogs_transition_activity_status() handles post comment statuses. The goal is to do this, but I ran out of time to implement this today and wanted to put something up.

• Miscellaneous: Activity admin dashboard now has an "Action" column. I've created a new utility function called bp_activity_admin_get_activity_actions() to grab a list of registered activity actions. The code already existed in the activity dashboard, but was being duplicated.

---

Remaining issues:

• Support trash / untrash post status changes and toggling nested activity comments as ham / spam
• Comment depth handling (see comment:17)

---

Q+A

I'd suggest mirroring it in blogmeta. Then it'll be autoslurped in activity loops with ​https://buddypress.trac.wordpress.org/browser/trunk/bp-blogs/bp-blogs-activity.php#L214

Was looking into this some more. It would be possible to cache the threaded comments depth as that rarely changes, but the comments_open() function needs to be called at runtime to calculate if a post has automatically closed comments after a certain age.

There is not much gain with only mirroring the threaded comments depth in blogmeta, while comments_open() is called at runtime during the activity loop so I've kept things in the patch as-is.

so great idea, But I test it on localhost and my website,
when author leave a comment on post, it works;
but when a other user leave comment, it doesn't creat a activity.
How can I solve this?

Anonymous blog post comments are not recorded in the BuddyPress activity stream.

[boonebgorges]

Many thanks for the update, r-a-y. Looking great. A couple thoughts/concerns:

• No need to define an 'action' in bp_blogs_record_comment(). This is handled by the callback bp_blogs_format_activity_action_new_blog_comment(). (It looks like I actually forgot to clean this up totally and you just kept what was there - so this is more a self-critique :) )
• I agree that, given comments_open(), something like bp_blogs_setup_activity_loop_globals() is probably necessary. I still feel very uneasy about it. It could result in up to 20 switch_to_blog()s on a single page. (Unlikely, but possible.) I think we should still consider using blogmeta for allow_comments and thread_depth at some point in the future. thread_depth will be pretty easy, since, as you note, it won't be changed often. For comments_open, I wonder if we can set up our own parallel comments_open() (which would reproduce the logic of _close_comments_for_old_post(), but wouldn't require the post object - it'd use the activity date instead). I'm probably just being a worrywart, so this is probably something we could think about in 2.1 if there are reports of performance issues in the wild.
• Likewise, you have a note about how we should probably use the object cache for these values. This is a very good idea, especially since the kind of site using an object cache is the most likely to be busy enough to experience performance problems related to this feature. Doing super-precise cache invalidation will be difficult, since _close_comments_for_old_post() is passive (there's no user action that we can hook to at the moment when comments are auto-closed). This might be a spot to use a transient, so that our cached value would be at most (say) an hour or two off. We could even do a realtime check when someone actually attempts to post a comment, and return an error if they happen to be posting in the window between comments being closed and our cache being invalidated. Not the most elegant experience in the world, but it might work. As above, it's likely that I'm overthinking what's necessary for a first iteration.
• I'm concerned about the force-deletion of blog comments when activity items are deleted. I think there may be cases when a user might legitimately want to remove an activity item but not the content itself. We don't necessarily have to account for that in the UI, but at the very least maybe we should refrain from deleting the original content irrevocably. Are there technical reasons why it has to be deleted rather than just trashed?

[rogercoathup]

@r-a-y -- thanks for work on this, so many catches and interactions to worry about.

Seconding @boonebgorges, re: deleting activity items shouldn't perhaps force deletion of blog comments.

From a UX perspective, we have sites where the activity items are deleted simply to clear up front end display, draw focus to other activity items and so on. There's no intention to remove underlying data in this case... so, deleting corresponding blog comments in this case would not be desirable.

I suggest best 'default' UX would be to delete corresponding blog comment when the specific comment is deleted on the activity item, but not to delete all comments when the activity item itself is deleted.

Perhaps behaviour should be a wp-admin switch, so configurable as needed for a given site -- but, understand this doubtless adds considerably to the workload.

[imath]

r-a-y, boonebgorges

i wasn't sure while dev chat, i think there's another problem about deleting or trashing a comment if the corresponding activity has been deleted by the user.

A subscriber does not have the WordPress capability to trash or delete a comment.
But a subscriber can delete an activity he posted or a reply he posted to an activity.
In 5130.03, if a subscriber deletes a reply he made on a post activity, it will delete the comment (or trash it in a near future).

Personally, i think for this particular case which may happen often we shouldn't allow a regular user to delete his activity to avoid the comment from being trashed and somehow avoid "bypassing" WordPress caps..

[boonebgorges]

A subscriber does not have the WordPress capability to trash or delete a comment.

But a subscriber can delete an activity he posted or a reply he posted to an activity.
In 5130.03, if a subscriber deletes a reply he made on a post activity, it will delete the comment (or trash it in a near future).

Ugh. Very good catch, imath. We can't allow for this kind of privilege escalation, even if it's for content that the user created.

r-a-y, can we address this in a fairly cheap way? I'm thinking: in bp_blogs_sync_delete_from_activity_comment(), just after you switch_to_blog(), do a current_user_can() check. If it fails, just bail. The activity item will still be deleted, but the blog comments will remain. Doesn't really give any user feedback, but this seems like a case where it's not really necessary. What do you think?

[r-a-y]

04.patch addresses:

• bp_blogs_remove_associated_blog_comments() now supports a $force_delete argument. If this is false, the comments are trashed instead of deleted. This is used in bp_blogs_sync_delete_from_activity_comment() using current_user_can( 'moderate_comments' ).

• Mirroring of close comments / threaded comment depth options to blogmeta. See bp_blogs_comment_open(), which does the blogmeta mirroring and the actual post age calculation. There's a tiny issue in bp_blogs_comment_open(); I have commented out the part of the code in question, which relies on a check on the actual post type. We currently don't mirror the post type in activity meta at the moment, so we can't do such a check. Let me know what you think about this.

I think this solves the majority of problems. Let me know what else needs to be addressed.

In this patch, I have also removed the BP component activity callback stuff. I decided to do this because after looking at the amount of code that needs to be written to support post comment synchronization, I don't think this is something that needs to be added in the BP_Component class.

[boonebgorges]

Thanks for your quick turnaround on this, r-a-y.

Mirroring of close comments / threaded comment depth options to blogmeta.

Brilliant. This alleviates all my worries about overhead in blog loops. Thanks for this.

I have commented out the part of the code in question, which relies on a check on the actual post type. We currently don't mirror the post type in activity meta at the moment, so we can't do such a check.

Let's not worry about it at the moment. We only natively support 'post' for new_blog_post activity types anyway. When we implement CPT mirroring at some point (#3460) we can deal with it.

bp_blogs_remove_associated_blog_comments() now supports a $force_delete argument. If this is false, the comments are trashed instead of deleted.

To be honest, I'm still not clear on why we'd want this to be true, but I think your solution is fine for 2.0. Shouldn't we also be doing something in bp_blogs_sync_delete_post_comments_on_post_delete()? You have a $force_delete param but I can't see where it'd ever be used (you're hooked to 'bp_blogs_before_remove_post'.

Once this last bit is cleared up, I think we're good to go for 2.0. Thanks again.

[boonebgorges]

One more thing before I forget. It would set my mind at ease to have some automated tests for the relevant chains of events: delete an activity comment when logged in as a super admin, etc. This is something that can wait until after the 2.0 beta or even the release. But this is pretty fragile and complex stuff with a lot of possible cases, so we ought to have integration coverage if possible.

[r-a-y]

To be honest, I'm still not clear on why we'd want this to be true, but I think your solution is fine for 2.0. Shouldn't we also be doing something in bp_blogs_sync_delete_post_comments_on_post_delete()? You have a $force_delete param but I can't see where it'd ever be used (you're hooked to 'bp_blogs_before_remove_post'.

Sorry, forgot to remove that part of the code from the patch. Was experimenting with trash / untrash post status changes in bp_blogs_catch_transition_post_status() and how that handles comments and activity items related to comments.

There are too many things required to get this working for 2.0-beta, so I decided to hold off.

At the moment, it is easier to delete all associated comments when a post is trashed than worrying about re-recording the activity stream items in their rightful slot for post comments when a post is untrashed. Let me know if this is a dealbreaker.

I'll work on some unit tests as well.

[boonebgorges]

At the moment, it is easier to delete all associated comments when a post is trashed than worrying about re-recording the activity stream items in their rightful slot for post comments when a post is untrashed. Let me know if this is a dealbreaker.

If I understand this right, you mean it's easier to *delete-rather-than-trash* because you don't want to worry about rerecording the activity items? If that's correct, I would vastly prefer that you trash-rather-than-delete, and just don't worry about the untrashing scenario for now. That way, no comment content is ever permanently lost, and the worst that will happen is some minor annoyance in the rare case where someone untrashes comments and is confused why the activity items haven't been reinstated. IMO this is perfectly acceptable for v1 of this feature. (Deleting user content irrevocably is not.)

[rogercoathup]

r-a-y:

wanted to confirm the behaviour when an activity item is deleted -- does it delete / trash all the corresponding comments on the associated blog post, or does it leave them in place?

If it deletes, I think that behaviour is wrong (in some cases).

Activity items are deleted for a number of reasons, and not just because associated content is no longer wanted, e.g. items that are deleted to tidy up / declutter an activity list. In this scenario, the comments should clearly remain on the blog post. [we have a client who does this on a regular basis].

I think synchronisation should be done when comments themselves are deleted / added, not when the activity item is deleted.

Perhaps making it something that is easy for developers to filter (or function parameter) to change behaviour would be best solution.

Please ignore above, if this is already the case.

[r-a-y]

rogercoathup - The issue was trashing a post, which in 04.patch, deleted all comments tied to an activity item.

05.patch solves this.

In particular, the changes are:

• bp_blogs_catch_transition_post_status() now only deletes the blog post's activity item (and therefore any activity children automatically). It doesn't use bp_blogs_remove_post() since that function already runs on the 'delete_post' hook. Post comments are left intact.

• bp_blogs_remove_activity_meta_for_trashed_comments() is new and deletes the associated comment meta key since the activity item no longer exists when trashed.

• bp_blogs_sync_delete_post_comments_on_post_delete() is now removed since I've addressed the post status transition change.

Let me know what you think.

[boonebgorges]

Do it.

[r-a-y]

06.patch adds:

• Updating a blog post will update the corresponding activity stream entry. For some reason, we removed the ability to support updating the activity stream on post edits. View the changes to bp_blogs_catch_transition_post_status() and the new bp_blogs_update_post() function.

• Post comment status mirroring to activitymeta - see bp_blogs_update_post(). The comment status is now checked in bp_blogs_comments_open() when it is mirrored.

Hopefully, this is the last patch!

[boonebgorges]

Excellent, looks good. Thanks!

[r-a-y]

In 8189:

Blogs: Mirror a blog's various comment options to BP's blogmeta.

This commit mirrors the following blog options to BP's blogmeta:

• 'close_comments_for_old_posts'
• 'close_comment_days_old'
• 'thread_comments_depth'

These options will be used later in the new bp_blogs_comments_open()
function to determine whether a blog post's activity item should be
closed from commenting. This is designed to prevent multiple calls
to switch_to_blog().

See #5130

[r-a-y]

In 8190:

Blogs: Support post comment synchronization in the activity stream.

Previously, if a reply to a blog post was made in the activity stream,
the reply would only exist in the activity stream and not as a comment
for the blog post.

This commit:

• Adds a blog comment whenever an activity reply is made under a blog post activity entry
• Adds an activity comment to the blog post's activity entry whenever a blog comment is made
• Checks to see if the blog post is open for comments (whether manually closed or due to age) (see r8189) to determine whether the activity item can be replied to
• Handles content edit synchronization to either the post comment or the activity comment
• Handles deletion synchronization to a degree

See #5130

[r-a-y]

In 8191:

Blogs: Tweak what happens when a post status is changed.

r8073 introduced bp_blogs_catch_transition_post_status() to determine
how blog posts are recorded in the activity stream. However, blog
post edits are not accounted for.

This commit:

• Updates a blog post's activity item when a post is edited
• Mirrors a blog post's comment status to activitymeta for use with bp_blogs_comments_open() (see r8189)
• Removes the usage of bp_blogs_record_post() when a post has changed to an unpublished status. bp_blogs_record_post() already fires on the 'delete_post' action, so this isn't necessary. Instead, we remove the corresponding activity entry.

See #5333, #5130

[r-a-y]

In 8194:

Activity dashboard: Check if activity item can be replied to.

If activity commenting is enabled for blog posts, in the activity
dashboard, we have to check whether or not the activity item can be
replied to.

This commit adds the class method - BP_Activity_List_Table::can_comment() -
to check the current activity item in the activity list table.

See #5130.

[r-a-y]

In 8228:

Blogs: When recording a comment and looking for the blog post activity item, do not add a check for the user ID.

Adding a check for the user ID is not needed and would be inaccurate
since a commenter's user ID will not be the same as the post author's.
When the user IDs do not match, the associated activity comment is not
created.

See #5130, #5507

[r-a-y]

In 8229:

Blogs: When a blog comment is unapproved, mark the corresponding activity item as spam.

This mirrors how BuddyPress handles other similar comment statuses
like 'trash' and 'spam'.

See #5130

[boonebgorges]

Looks like this is working well for beta users. Let's mark this ticket fixed and handle additional problems in separate tickets, to clear the milestone. Thanks, r-a-y!

[unknownterritory20]

I'm confused as to how this works. I can't even make a comment on wp post comments that flow into activity stream. The comment button is missing. I only have the option of marking them as favorites or deleting the comment.