verbb / comments

A Craft CMS plugin for managing comments directly within the CMS.
Other
137 stars 33 forks source link

Add soft delete for comments that have children #215

Closed jsunsawyer closed 3 years ago

jsunsawyer commented 3 years ago

Currently, a user can delete their own comment. This comment and all its children get removed from a thread. This essentially deletes other users' comments. (I'm unsure if this would have any effect on those users' notifications statuses, but something worth checking out. Could be a separate issue)

It might make more sense for deleted comments that have children (that haven't been deleted) to persist, but show as Deleted in the nested comments.

engram-design commented 3 years ago

Yeah, I'd considered how best to handle this generally at the time, as it's not exactly ideal. Should replies on a deleted comment also be deleted, or should remain? If the comment has gone, replies kind of loose their context.

This was the idea around whenever front-end users "delete" a comment, they're "trashing" it - which is just a status, so the comment remains in Craft, and the hierarchy isn't messed up.

Deleting a comment permanently is only possible through the control panel - at least last time I checked. The issue with even if we did introduce soft deletes would be that Craft's garbage collection kicks in at some random point. So it doesn't really solve the problem, but just delays it.

As such, control panel users should be careful about deleting comments.

jsunsawyer commented 3 years ago

I see. I think I got confused about how the trash section works. I didn't really mean soft deletes like the term used in Craft – whoops.

Now I see that I can add .anyStatus() to the query using a custom template. This is great! Unfortunately, when a user deletes their message, it immediately removes the thread and its children from the dom. For other users, they could see an edited version where I check each comment's status and display alternate messages.

Might be nice if there was a toggle for this. I think this make more sense. Other conversations could be happening in nested comments.

engram-design commented 3 years ago

Yep, so the default templates don't include deleted comments at all (https://github.com/verbb/comments/blob/craft-3/src/services/CommentsService.php#L102-L111). I suppose you're right in maybe this behaviour should be changed by default. You can certainly override the query and how comments are rendered through your own templates. But as you say, children shouldn't need to suffer if a parent was deleted.

engram-design commented 3 years ago

There's no plan to include trashed comments in the default templates, so you'll be left to do this yourself through custom means.

So the only real way around this is to either write your own JS to take over the plugin's built-in JS (which removed the comment node), or to make use of some new settings I've added. I'd suggest the latter, otherwise you'll end up with a whole load of JS debt for your project - unless you can live with that!

You can provide a collection of settings using the below:

{{ craft.comments.render(entry.id, {}, {
    trashAction: 'message',
    trashActionMessage: '[Deleted]',
}) }}

These are additional JS settings for our provided JS class. You can set trashAction to be either remove, message, refresh or none. I imagine what you're after is message - where you can also provide trashActionMessage to have a message shown.

To get this early, change your verbb/comments requirement in composer.json to:

"require": {
  "verbb/comments": "dev-craft-3 as 1.7.5",
  "...": "..."
}

Then run composer update.

engram-design commented 3 years ago

Added in 1.8.0

jsunsawyer commented 3 years ago

I was finally able to test these updates.

When testing with the default template, it seems that deleted comments get shown upon a refresh, so the trashActionMessage is only showing upon delete.

I'm working through this now, but it seems that this messaging should be updated like so:

{% if comment.isFlagged() %}
  <p class="comments-notice">[{{ 'Comment marked as inappropriate' | t('comments') }}]</p>
{% elseif comment.isPoorlyRated() %}
  <p class="comments-notice">[{{ 'Comment hidden due to low rating' | t('comments') }}]</p>
{% elseif comment.status == 'trashed' %}
  <p class="comments-notice">[{{ 'Comment deleted' | t('comments') }}]</p>
{% else %}
  <p>{{ comment.comment | nl2br }}</p>
{% endif %}

Also, in my custom template, I added some logic to not show a trashed message when it has no children. Could be worth considering here.

{% for nestedComment in comment.children if nestedComment.status != 'trashed' or (nestedComment.status == 'trashed' and nestedComment.children|length) %}

^ These were both from _includes/comment.html.

engram-design commented 3 years ago

Comments by default won't show trashed comments. As you're doing that yourself, custom, you'll probably need to maintain these sort of custom changes to templates.

But yes, if you're showing trashed comments, you'll need to handle that case in your templates. I might add this case, as it doesn't do appear with the default templates.

jsunsawyer commented 3 years ago

Right, so this is my render tag:

{{ craft.comments.render(product.id, {
  status: null,
  with: [
    ['children', { status: null }],
  ],
}, {
  trashAction: 'message',
  trashActionMessage: '[Comment deleted]',
}) }}

In the default template, when a message is deleted, its children persist, but its own comment content is replaced with the deleted message.

Upon refresh, the (now deleted -> "trashed") comment still gets rendered.

I'm currently working on updating my custom template to work with the new functionality, so some clarity as to what should be happening here would be appreciated!