silverstripe-comments icon indicating copy to clipboard operation
silverstripe-comments copied to clipboard

Deleting a Comment With Child Comments Removes SubTree

Open gordonbanderson opened this issue 9 years ago • 5 comments

Change to add a flag 'MarkedAsDeleted' and render a message likes of 'this comment was deleted by an administrator'.

A spam comment with children should probably be treated similarly though I guess the message will be different for an admin (who can see the comment to ham it) and someone without admin rights who would see likes of 'this comment was marked as spam by an administrator'

gordonbanderson avatar Feb 14 '16 10:02 gordonbanderson

We'll need to build in a new status, but it's probably fine; Maybe a "ShowDeleted" status flag?

And also a config option to toggle between "real" delete and "show deleted".

tractorcow avatar Feb 14 '16 22:02 tractorcow

I went with 'MarkedAsDeleted' but easy enough to change. When writing tests I realize there are some interesting interface implications. For example if a comment which is marked as deleted has a valid child comment, and the child is deleted, the child and the parent should both be removed. And this of course could cascade up the hierarchy if there is a long ancestry of marked as deleted comments with a valid child comment. Edge case but one to consider. And then of course there is the admin interface...

gordonbanderson avatar Feb 15 '16 01:02 gordonbanderson

Just testing on master with the admin section, with a parent and child comment, deleting the parent deletes both parent and child.

gordonbanderson avatar Feb 15 '16 01:02 gordonbanderson

Given the number of configuration options for this module I'm reluctant to add more unless we have to. I'm not sure that what you refer to as 'real delete' is a good idea, at the very least a confirmation step for non JS users is required, otherwise a whole subtree of comments will disappear.

One other possibility is to present the admin with both options, namely 'Mark this as deleted' and 'Delete this comment and descendants' (that needs better phrasing). For a comment that is a leaf on a tree, just present a delete button as is now.

gordonbanderson avatar Feb 15 '16 01:02 gordonbanderson

I had a chance to think about this over a 5km jog earlier. My thinking is that the there should be no deletion of the subtree unless explicitly stated in the interface. This suggests the following:

  • In the admin cms interface only one comment is deleted. This makes perfect sense as the comments are represented linearly.
  • In the front end a Delete button is presented for both leaves and non leaves. On a leaf this deletes a comment. When the comment is a non leaf in the tree show the delete button but also an optional tick box to delete descendant comments.

In the case of comments that are not nested things are as they were before. Best solution I can think of

gordonbanderson avatar Feb 15 '16 14:02 gordonbanderson