Use WP comments' threaded comment depth settings

[DJPaul]

Related to #1870, I think that the Activity Stream should respect WP comments' threaded comment depth settings (thread_comments and thread_comments_depth in the options table).

I think that from a user's point of view, an activity stream item is the same as a comment on a blog post, and #2729 would only serve to blur the lines between these types of replies.

[boonebgorges]

I disagree that users will necessarily equate activity comments with blog comments, though I like your proclivity for parsimony :)

Suggestion:http://trac.buddypress.org/browser/trunk/bp-activity/bp-activity-templatetags.php#L920 bp_activity_can_comment(), before returning true or false, checks to see whether the WP threaded comments box is checked. If so, then it returns true. bp_activity_can_comment_reply(), in turn, will check the value of $comment->mptt_left against the max depth set on the WP discussion panel, and returns true or false accordingly.

The advantage of this method is that activity depth will mirror blog comment depth by default, but will be filterable.

[DJPaul]

Good idea, that's probably the best approach if we were to patch this. I'm interested to hear any feedback from others.

[DJPaul]

This is not as trivial as it seems. We can't simply count() everything at the start of the activity stream item loop, or use bp_activity_get_comment_count(), as those will look down the entire depth of any set of items. We need to count down each level of the tree to see if any particular set has a number of items higher than the threaded comments limit. I took the approach of passing a counter variable through the process.

This needs testing, please. I think it works ok. Some remaining issues:

1) When submitting a reply via AJAX, if that reply would cause you to equal the threaded comments limit, the returned HTML still has the "reply" link. Probably something to do with the AJAX activity stream loop requesting a specific item rather than starting from the root item.

2) RE: 1, if the user hits reply on the AJAX, it will get posted, ignoring the threaded comments limit. I suggest we need to amend the "add activity stream to DB" function and for "activity_comment" items', check the depth. If it is at or has passed the threaded comment limit, we'll bump it back to a root level reply -- I'm sure we can fiddle the ajax return so that it is inserted visually in the correct place, too.

[DJPaul]

Punting

[boonebgorges]

Marked #5321 as a duplicate.

Since we've got a patch, and there's demand for the feature, let's look at it for 2.0.

[Zachary DuBois]

Thanks @boonebgorges

[boonebgorges]

I've reworked DJPaul's patch a bit.

Instead of passing a value around in the templates (unappealing now that our comment markup is generated by a real WP template), I've opted to build the depth index when the comments are fetched. To make the AJAX play nicely, and because of the somewhat odd way we fake out the global when building the markup for AJAX-inserted comment, I had to manually build the depth in the AJAX callback as well.

Ready for testing and feedback.

[DJPaul]

I'll test this patch next week, but it looks good.

[boonebgorges]

In 8201:

Use WP's comment depth setting to limit activity comment depth

Fixes #2768

[r-a-y]

Came across a loop of debug notices when on my own profile (eg. example.com/members/admin/activity/) that was timing out my server.

PHP Notice: Undefined index: in wp-content\plugins\buddypress\bp-activity\bp-activity-classes.php on line 1016

I've attached a patch that I believe fixes this. Could use a onceover.

[boonebgorges]

Good catch, r-a-y. This only happens when the parent is not retrieved in the current loop, which would only happen in certain instances when display_comments=stream and the parent is out of scope. I've managed to reproduce it.

Your fix will essentially short-circuit the depth calculation in these sorts of situations, by forcing the depth to be set to 1. This prevents the infinite recursion, and doesn't really hurt anything in the interface, since we don't allow comments-on-comments when display_comments=thread. But it does result in an incorrect value for 'depth', which may be misleading down the road.

2768.02c.patch modifies your patch to fetch the direct parent when it's not found in the already-queried items. In some cases, this will result in additional query overhead. But it will also guarantee correct depth calculations, which IMO is a fair tradeoff for this fairly unusual circumstance. Could you give it a quick sanity check?

[r-a-y]

I think 02c.patch works. We could open up activity comments-on-comments in the future, so it's better to be future-proof :)

[boonebgorges]

In 8226:

Better checks for existence of parent item when recursing to calculate activity comment depth

The changes introduced in r8201 involve calculating tree depth for each queried
activity comment in a loop. The technique used involves traversing up a tree by
fetching the parent item until the root level is reached. However, the original
technique did not account for the fact that in certain cases - namely, when
display_comments=threaded - it may not be the case that all of the comment
ancestors are available in the current query. This failed logic resulted in
infinite regression in these sorts of cases.

The correction involves performing a separate query for the direct activity
parent when it is not found in the queried items.

Props r-a-y

Fixes #2768

[boonebgorges]

In 8402:

Disable activity reply threading if WP comment threading is disabled

In r8201, BP adopted WP's thread_comments_depth setting for its own activity
reply threading. See #2768. However, the thread_comments toggle was not
respected. The current changeset adds support for thread_comments, so that
disabling WP comment threading will also enable nested activity comments
(though the top level of comments will still be allowed).

Fixes #5562