Make WordPress Core

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#16358 closed defect (bug) (fixed)

Menu items get deleted with the user who created them

Reported by: nacin Owned by: ryan
Milestone: 3.4 Priority: high
Severity: major Version: 3.0
Component: Menus Keywords: has-patch commit needs-codex
Focuses: Cc:


Say a user edits a nav menu or two, adding some menu items. Then, said user gets deleted. Rut roh:

SELECT ID FROM $wpdb->posts WHERE post_author = %d

The menu items were tied to them, therefore they are deleted.

We need to ensure that wp_delete_user() doesn't touch nav menu items.

This has repercussions for any post type that doesn't care about authors. Tempted to actually check, post_type_supports(), which is typically reserved for UI-level actions. Otherwise, post_author always gets stored, and that seems to be just a bit of a problem. (It could be argued this is a UI-level check, in terms of confirming things to delete.)

Unsure how this didn't come up previously. Reported by flashingcursor over Twitter.

Attachments (8)

16358-static.diff (634 bytes) - added by nacin 10 years ago.
Restrict to posts, pages, attachments.
16358-post_type_supports.diff (909 bytes) - added by nacin 10 years ago.
16358-no-nav_menu_item.diff (860 bytes) - added by nacin 10 years ago.
16358.diff (1.5 KB) - added by nacin 9 years ago.
16358.2.diff (2.6 KB) - added by ryan 9 years ago.
delete_with_user flag. Props nacin
16358-unit-tests.diff (1.6 KB) - added by ryan 9 years ago.
16358.3.diff (3.8 KB) - added by ryan 9 years ago.
Use trash instead of force delete. Add phpdoc.
16358-unit-tests.2.diff (1.8 KB) - added by ryan 9 years ago.

Download all attachments as: .zip

Change History (28)

#1 @nacin
10 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.1

To be honest, it says 'posts and links'. We do fire an action there that custom post types could use.

Tempted to hard restrict it to posts, pages, and attachments.

Moving to 3.1. This one seems bad.

10 years ago

Restrict to posts, pages, attachments.

#2 @nacin
10 years ago

In patch 1, I forgot revision. You get the idea.

Second patch leverages post_type_supports.

#3 @nacin
10 years ago

Option 3 is to avoid deleting nav menu items only, and leave CPT handling to a filter, with the default of delete. (Default of not delete would be option 1.)

#4 @nacin
10 years ago

Patch might overlap with #16359.

Note, probably goes without saying, but none of these were actually tested. We need a plan of action on which to choose, more importantly.

#5 @nacin
10 years ago

  • Keywords dev-feedback added

#6 @westi
10 years ago

  • Keywords 3.2-early added; dev-feedback removed
  • Milestone changed from 3.1 to Future Release
  • Severity changed from critical to major

This is not a regression in 3.1 so can wait till 3.2 to be fixed.

#7 @nacin
10 years ago

Would not mind a `AND post_type <> 'nav_menu_item' in 3.1. This is a nasty dumb bug.

#8 @filosofo
10 years ago

I like the idea of doing this based on whether the post type supports authors, as that generalizes well.

Is there any reason the core post type definitions don't declare author support for revisions and attachments?

#9 @flashingcursor
10 years ago

  • Cc flashingcursor added

#10 @WraithKenny
10 years ago

I also like your second patch (post type supports authors)

#11 @aaroncampbell
10 years ago

  • Cc aaroncampbell added

#12 @ocean90
10 years ago

  • Keywords 3.3-early added; 3.2-early removed

#13 @jane
9 years ago

  • Keywords 3.3-early removed
  • Milestone changed from Future Release to 3.4

Bummed this keeps getting punted. I think it definitely falls into 'making your site look the way you want it to look' plan, for 3.4, so let's see if someone can work on this for 3.4 early.

#14 @husobj
9 years ago

  • Cc ben@… added

9 years ago

#15 @nacin
9 years ago

  • Keywords commit added

9 years ago

delete_with_user flag. Props nacin

#16 @ryan
9 years ago

That patch also changes to a force delete instead of putting posts in trash.

9 years ago

Use trash instead of force delete. Add phpdoc.

#17 @nacin
9 years ago

Looks good.

#18 @ryan
9 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [20739]:

Don't delete nav menu items when the user that owns them is deleted.

  • Introduce delete_with_user flag to register_post_type
  • Set delete_with_user to false for the nav_menu_item post type
  • Set it to true for all other core post types
  • If delete_with_user is not set, fallback to post_type_supports('author')

Props nacin
Fixes #16358

#20 @johnbillion
9 years ago

  • Keywords needs-codex added
Note: See TracTickets for help on using tickets.