Skip to:

Opened 3 years ago

Closed 3 years ago

#8415 closed defect (bug) (fixed)

Deleting a comment doesn't delete comment's descendants

Reported by: yesbutmaybeno's profile yesbutmaybeno Owned by: imath's profile imath
Milestone: 10.0.0 Priority: normal
Severity: normal Version:
Component: Activity Keywords: has-patch needs-testing reporter-feedback


Given a comment thread

  • comment1
  • - comment2
  • - - comment3

If on a normal activity with threaded comments, and you delete comment2, it also deletes comment3. That's normal.

Now, in my setup, when you go to a member's page activity stream, it only lists their posts/comments in the stream BUT if it's a comment (the entry is a type of activity_comment), it doesn't also list all the comments to that comment (makes the page messy and lots of redundancy).

If I go to that activity stream and delete the same comment2 (now it's listed as an entry in the member's activity stream), it does NOT delete its children comments. So comment3 becomes a first-level comment and can't be replied to (it hangs forever).

Now, I believe this is because of my setup for member page activity streams mentioned above. I think it's Javascript that handles the finding and deleting of all children comments and that's why it's not deleting the children of the comment (they're not on the page)? Not sure if true and kinda sounds exploitable but shouldn't it be handled in the PHP of whatever the delete_activity/comment function is? (PHP: Deleting a comment - find children, delete as well.)

Attachments (2)

8415.bpNouveau.patch (6.1 KB) - added by imath 3 years ago.
8415.patch (10.1 KB) - added by imath 3 years ago.

Download all attachments as: .zip

Change History (8)

#1 @yesbutmaybeno
3 years ago

Actually my idea was wrong, it ends up a simple fix though maybe a bit hacky

I had to edit the buddypress.js file that ships with the legacy theme, on its delete_activity function, change what the ajax action will be:

var the_action = 'delete_activity';
if ( li.hasClass('activity_comment') )
	the_action = 'delete_activity_comment';

Now it properly treats the "activity" as an activity comment. Probably should be added into the default JS file.

#2 @imath
3 years ago

  • Component changed from Core to Activity
  • Keywords needs-testing added
  • Owner set to imath
  • Status changed from new to reviewing

#3 @imath
3 years ago

  • Keywords needs-patch added; needs-testing removed
  • Milestone changed from Awaiting Review to 10.0.0

Thanks for your feedback. I confirm I was able to reproduce the issue. I agree we need to be consistent. I'll look at your suggestion asap.

#4 @imath
3 years ago

  • Keywords has-patch added; needs-patch removed
  • Status changed from reviewing to accepted

The above patch should fix the issue for BP Nouveau. I'll look at BP Legacy later. In the meantime feel free to test the patch making sure the BP Nouveau template pack is active.

3 years ago

#5 @imath
3 years ago

  • Keywords needs-testing reporter-feedback added

8415.patch is fixing the issue for both template packs and also takes care of redirecting the user to the activity permalink when there is no comment form available to reply to a comment. The later happens on the user's activity page when the main activity is an activity comment.

It would be great to have some more testing about it. @yesbutmaybeno do you have time to do this testing?

#6 @imath
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 13114:

Make sure deleting an activity comment also deletes its children

This commit also takes care of redirecting the user to the activity single view when clicking on the comment button and there is no comment form available for the corresponding activity. This case can happen on a single member's activity stream when the main activity is an activity comment.

Props yesbutmaybeno

Fixes #8415

Note: See TracTickets for help on using tickets.