WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 8 months ago

Last modified 8 months ago

#17928 closed defect (bug) (fixed)

When a comment is spammed/trashed, change the parent of it's children

Reported by: Viper007Bond Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.2
Component: Comments Keywords:
Focuses: Cc:

Description

When removing (spam/trash) a comment that has children, everything goes to hell in terms of hierarchy and you're left with a bunch of orphaned comments.

See two attached images for what currently happens after removing a comment that has children.

When a comment is removed, all immediate children of the comment to be removed should have their parent changed that of the the parent of the comment being removed (or top level if the comment to be removed is top level). Each comment that has it's parent changed then gets that action logged to it's post meta so that it can be moved back in the case of it's previous comment being restored (on post un-delete/spam, find all matching comment metas).

Attachments (3)

before.png (45.0 KB) - added by Viper007Bond 5 years ago.
A bunch of nested comments
after.png (37.8 KB) - added by Viper007Bond 5 years ago.
Note how "Depth = 3, comment #1" is no longer even a child of "Depth = 2, comment #1"
17928.patch (2.7 KB) - added by jakub.tyrcha 5 years ago.

Download all attachments as: .zip

Change History (28)

@Viper007Bond
5 years ago

A bunch of nested comments

@Viper007Bond
5 years ago

Note how "Depth = 3, comment #1" is no longer even a child of "Depth = 2, comment #1"

#1 @Viper007Bond
5 years ago

In the screenshotted example, "Depth = 1, comment #1" is the comment that is deleted.

#2 @scribu
5 years ago

Hey, at least they still show up. :)

#3 @jakub.tyrcha
5 years ago

  • Owner set to jakub.tyrcha
  • Status changed from new to assigned

#4 @azaozz
5 years ago

Thinking it would be best if all children (and grandchildren, etc.) of a deleted, spammed or unapproved comment go back to the moderation queue. Tested this a bit and it looks quite weird/meaningless if the main comment is missing but the answers to it (the children) are shown.

#5 follow-up: @jakub.tyrcha
5 years ago

Dammit, I was just finishing writing a fancy graph traversal :P

And what if the parent's moderation is ruled back? All children stay in the queue?

Maybe it would be a good idea to keep the moderated comment blacked out and shown with a moderation notice?

#6 in reply to: ↑ 5 @azaozz
5 years ago

Replying to jakub.tyrcha:

And what if the parent's moderation is ruled back? All children stay in the queue?

IMO they need to get approved again automatically. All this would apply to approved children only of course. Children in the moderation or spam queues should stay there. And here's the next difficulty:

  • parent is approved, has some approved children and some children in moderation
  • then parent gets spammed, all approved children go to moderation (preferably with a note that the parent is unavailable/spam)
  • the user runs into a child comment that was previously in moderation and clicks Approve...

So to avoid confusion this can only be done while moderation comments in hierarchical mode, the flat list we currently have won't work.

#7 @jakub.tyrcha
5 years ago

So I'll submit the first version of the patch for now because it's still better than what we have now and leave the real deal for later ;)

#8 @scribu
5 years ago

Why not move all the children to the trash also? It's the most likely behaviour anyway.

Then, if the user deliberately tries to rescue a child from the trash, it's parent can be set to 0.

#9 @scribu
5 years ago

Obviously, you'd still need to bring back all the children, if the parent is un-trashed.

#10 @jakub.tyrcha
5 years ago

If the user tries to rescue a child from the trash and the parent is set to 0, the problem remains

#11 @nacin
5 years ago

Why not move all the children to the trash also?

Not sure I agree with that paradigm.

#12 @azaozz
5 years ago

Perhaps it would be best to put all approved children in moderation and disable the action links on them. Workflow:

  • a comment is spammed/trashed/unapproved, if it has children the user is prompted what to do with them. Choices: treat as separate comments, spam/trash/unapprove all, do nothing.
  • if "do nothing" is selected, all children are left in moderation until a decision on the parent is made. There is a message "Parent is Spam/In the trash/Unapproved" and the action links on the children are disabled.
  • if the status of the parent is reverted, the status on all children is reverted to the previous.

#13 @jakub.tyrcha
5 years ago

I'm just wondering if "treat as separate comments" vs "do nothing " would be clear for some users

#14 @azaozz
5 years ago

Well, we can word it better. Perhaps "Decide later"?

Also the warning that the parent is in the trash/spam/unapproved can be a link to editing the parent. Of course there are other enhancements to this workflow but for now think we need to set the main part.

Last edited 5 years ago by azaozz (previous) (diff)

@jakub.tyrcha
5 years ago

#15 @jakub.tyrcha
5 years ago

just in case anyone want's to play with Viper007Bond's solution ;) I had it done anyway

Version 0, edited 5 years ago by jakub.tyrcha (next)

#16 @Viper007Bond
5 years ago

I'd like to note that this also affects paging. On some of the CNN blogs on WordPress.com for example, a page that would normally have about 15 comments can end up having many hundreds. It makes for some slow page generation times due to the CPU work required to loop through them all.

#17 @SergeyBiryukov
5 years ago

  • Keywords has-patch added; needs-patch removed

#18 @jane
4 years ago

  • Owner jakub.tyrcha deleted

I see everyone's points about the various possible behaviors, but why not take a page from threaded forums who've been doing this way longer than us -- if parent trashed, spammed, deleted, replace it on the page with a placeholder that says it has been removed by an administrator/the site owner. Then the children still show appropriate hierarchy and if there are references to missing parent it will be clear there's something missing and you're not an idiot for not understanding a child comment, and the admin doesn't have to go through the bother of re-approving everything.

#19 @thatrobyn
20 months ago

  • Keywords has-patch removed
  • Type changed from enhancement to defect (bug)

Continuing the discussion three years later...

I've noticed a handful of users who are still fretting that when they delete a comment from a post, the child comments appear as out-of-order parent comments.

For instance, this one on the public WordPress.com forum:

http://en.forums.wordpress.com/topic/comment-sort-order-appears-to-be-broken?replies=2

The user links this Post as an example:
http://nuxref.com/2014/04/22/configuring-a-secure-mail-server-on-centos-6/

Also, an older example from 2013:
http://hro001.wordpress.com/2013/01/19/breaking-minamata-mercury-finally-a-unep-success-story/

#20 @rachelbaker
9 months ago

  • Keywords needs-patch added

Related #33710

#21 follow-up: @knutsp
9 months ago

Moderators that see an unacceptable comment that already has replies, tend to replace the comment content with something like "Deleted", to keep the thread.

When comment is marked as spam or deleted, the user should get the option to set the comment status to placeholder (a new status). Such comments should be displayed with a default text ("This comment was deleted by a moderator") in replacement for the actual comment content.

If this choice is not selected, all replies down the thread should be trashed, or (better) set to to inherited (new status). The inherited status must then also be handled properly by examining the parent comment(s). The inherited status makes it possible to either unspam or untrash the parent comment, reviving the thread.

#22 in reply to: ↑ 21 @rachelbaker
9 months ago

Replying to knutsp:

When comment is marked as spam or deleted, the user should get the option to set the comment status to placeholder (a new status). Such comments should be displayed with a default text ("This comment was deleted by a moderator") in replacement for the actual comment content.

If this choice is not selected, all replies down the thread should be trashed, or (better) set to to inherited (new status). The inherited status must then also be handled properly by examining the parent comment(s). The inherited status makes it possible to either unspam or untrash the parent comment, reviving the thread.

In addition to introducing two new comment "statuses", comments themselves don't have a status (there is only comment_approved).
I don't have an alternative idea that is a lighter lift, but I wonder if there is another way.

#23 @wonderboymusic
9 months ago

  • Milestone changed from Future Release to 4.4
  • Owner set to wonderboymusic

I think we need a Purgatory state.

  • Comment gets modded (trash or spam)
  • Everything below it in the hierarchy gets comment_approved set to inherit-{modded_ID} and its current value for comment_approved saved in meta as something
  • If modded gets restored or whatever, all comments with inherit-{modded_ID} return to their status which is saved in meta

I may rig something up to demonstrate this. As long as queries are bound by some sort of value for comment_approved, this might work. Without the ID in the value, this would require a meta query, which I want nothing to do with.

#24 @wonderboymusic
8 months ago

  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from assigned to closed

You're welcome for [34546]

#25 @wonderboymusic
8 months ago

For the record - we didn't have to do anything weird here. We just fixed the way that comments are queried - it now happens in a way that isn't destructively insane.

Note: See TracTickets for help on using tickets.