Skip to:

Opened 9 months ago

Last modified 3 weeks ago

#8415 accepted defect (bug)

Deleting a comment doesn't delete comment's descendants

Reported by: yesbutmaybeno Owned by: 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 weeks ago.
8415.patch (10.1 KB) - added by imath 3 weeks ago.

Download all attachments as: .zip

Change History (7)

#1 @yesbutmaybeno
9 months 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
4 weeks ago

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

#3 @imath
3 weeks 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 weeks 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 weeks ago

#5 @imath
3 weeks 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?

Note: See TracTickets for help on using tickets.