Make WordPress Core

Opened 13 years ago

Closed 8 years ago

#15644 closed defect (bug) (wontfix)

When deleting a user, corresponding comment user IDs aren't reset

Reported by: nacin's profile nacin Owned by:
Milestone: Priority: lowest
Severity: minor Version:
Component: Comments Keywords:
Focuses: Cc:

Description

Delete a user with a comment.

The user ID stays.

Potential for clashes later.

We should run this: $wpdb->update( $wpdb->comments, array( 'user_id' => 0 ), array( 'user_id' => $user_id ) ); in wp_delete_user.

Alternative is to do a full query then manually run wp_insert_comment() on each, so all hooks get fired, which is fine too and is more in line with what we do elsewhere.

Attachments (2)

15644.diff (877 bytes) - added by logiclord 13 years ago.
user_id is set to 0 when a user is deleted
15644.patch (1.0 KB) - added by jakub.tyrcha 13 years ago.

Download all attachments as: .zip

Change History (16)

#1 @nacin
13 years ago

  • Keywords needs-patch added; has-patch removed

@todo submit this for gci.

#2 @westi
13 years ago

That query could get ouchy on a large site though?

The comment table is more of a snapshot of data at the comment time.

#3 follow-up: @nacin
13 years ago

  • Keywords 2nd-opinion added; gci removed

I don't have a problem with the rest of the data as a snapshot, but this has the potential for a future user to then clash.

You're right though, this query could possibly hurt. Not my area of expertise.

#4 in reply to: ↑ 3 @westi
13 years ago

Replying to nacin:

I don't have a problem with the rest of the data as a snapshot, but this has the potential for a future user to then clash.

User id is auto increment which should deal with this AFAIK

ID bigint(20) unsigned NOT NULL auto_increment,

#5 @nacin
13 years ago

  • Priority changed from normal to lowest
  • Severity changed from normal to minor

Ah ha, I thought the increment gets reset on an optimize/repair, but it actually requires an ALTER TABLE AUTO_INCREMENT = 1.

@logiclord
13 years ago

user_id is set to 0 when a user is deleted

#6 @solarissmoke
13 years ago

@logiclord I think you diffed the wrong way :), those lines should be added not deleted. Also please make a diff from the root wp folder, not just the file in question.

@jakub.tyrcha
13 years ago

#7 @SergeyBiryukov
13 years ago

  • Keywords has-patch added; needs-patch removed

#8 @kovshenin
13 years ago

  • Cc kovshenin@… added

Hey there!

I was getting rid of the admin user on an existing website who had existing blog posts and comments. I created a new admin user and when I went to remove the admin user it asked me whether I want to move all posts and links to a different one where I can pick from the list. I think the same approach should go with comments. I had to manually run an update statement on the comments table to replace all comments with user_id = 1 to user_id = 9 (the ID of the newly created user) for my old gravatars to show up again. Thoughts?

#10 @Mamaduka
12 years ago

  • Cc georgemamadashvili@… added

#11 @chriscct7
9 years ago

  • Keywords 2nd-opinion has-patch removed
  • Resolution set to invalid
  • Status changed from new to closed

Given the user_id column is an autoincrementing index, the user would manually (as pointed out by nacin) need to run an ALTER TABLE. As such, the collision possibility is only limited to where the auto incrementing index is intentionally set below the highest user id's user_id, and therefore there isn't a chance for collision during normal operation

#12 @SergeyBiryukov
9 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Implementing the idea from comment:8 still makes sense to me.

#13 @chriscct7
9 years ago

  • Keywords close added

So my thoughts on comment:8.

So when you associate a post with another admin, you're preserving content in terms of if you make pages for a website, generally speaking those pages could be non-personal associated. For example, if I make an about page, and delete the user who made that, I might want to associate that with another user because the About Page is something that doesn't really depend on who wrote it.

However, comments are different. They're inherently reflective of the individual who wrote them.

For that reason, associating comments with another user doesn't make a lot of sense.

However if that's something we want to add, it should be handled on a new ticket, because it gets away from the bug that this ticket is reporting. It could also be easily handled by a plugin

#14 @chriscct7
8 years ago

  • Keywords close removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

No further interest in 5 months. Closing as wontfix. If there's interest in it, the proposal in comment:8 should be open in a new ticket, baring in mind comment:13

Note: See TracTickets for help on using tickets.