WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 9 months ago

#19038 closed defect (bug) (fixed)

Menu Items aren't deleting on page being trashed, only when deleted

Reported by: jeremyatignition Owned by: swissspidy
Milestone: 4.7 Priority: normal
Severity: normal Version: 3.2.1
Component: Menus Keywords: has-patch commit has-screenshots has-unit-tests
Focuses: Cc:

Description

The linked menu item for a page isn't removed when the page is trashed.

Attachments (3)

19038.diff (596 bytes) - added by solarissmoke 6 years ago.
19038.2.diff (513 bytes) - added by swissspidy 19 months ago.
19038.3.diff (1.4 KB) - added by swissspidy 9 months ago.
adds unit test

Download all attachments as: .zip

Change History (19)

#1 @solarissmoke
6 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

This is because _wp_delete_post_menu_item is only hooked to the delete_post action.

Patch adds the same hook to the trash action. Of course this means that if you restore the post then the menu item has to be restored manually if you still want it - but this seems better than having to delete the menu item manually when you trash a post.

@solarissmoke
6 years ago

#3 @jpyper
4 years ago

Just wanted to say this works like a charm for me. I can even add that line into a plugin or my functions.php file.

#4 @SergeyBiryukov
3 years ago

#28765 was marked as a duplicate.

#5 @svinqvmraka
2 years ago

I can also confirm that the patch still works perfectly on 4.1.

#6 @swissspidy
19 months ago

  • Keywords needs-refresh added

Patch needs a refresh.

What if you're just temporarily trashing a page to untrash it again a bit later?

I think a menu item of a trashed object should show as invalid in the admin and not show up at all on the front-end.

#7 follow-up: @Jpyper
19 months ago

Honest question: when is it better to trash a page rather than just set it to draft to temporarily remove it? I'd consider trashing a page as intent to never get it back. This may just be my opinion/preference though and not the majority of other user's way.

#8 in reply to: ↑ 7 ; follow-up: @swissspidy
19 months ago

Replying to Jpyper:

Honest question: when is it better to trash a page rather than just set it to draft to temporarily remove it? I'd consider trashing a page as intent to never get it back. This may just be my opinion/preference though and not the majority of other user's way.

True. I was just giving an example. There's a reason why this wasn't implemented for trashed pages. My suggestion is still the same as pages or posts can be untrashed.

#9 in reply to: ↑ 8 @Jpyper
19 months ago

Replying to swissspidy:

Replying to Jpyper:

Honest question: when is it better to trash a page rather than just set it to draft to temporarily remove it? I'd consider trashing a page as intent to never get it back. This may just be my opinion/preference though and not the majority of other user's way.

True. I was just giving an example. There's a reason why this wasn't implemented for trashed pages. My suggestion is still the same as pages or posts can be untrashed.

I don't know if I've ever seen that "invalid" menu item scenario come up before, I just had to look it up. That actually sounds better. The main goal is to have the menu item not show on the front end if it's object was trashed. It would be nice to make it easy to reinstate that menu item just by bringing the object back (as long as it's not removed from the DB, obviously).

@swissspidy
19 months ago

#10 @swissspidy
19 months ago

  • Keywords ux-feedback added; dev-feedback needs-refresh removed

The main goal is to have the menu item not show on the front end if it's object was trashed. It would be nice to make it easy to reinstate that menu item just by bringing the object back (as long as it's not removed from the DB, obviously).

That's how it's currently working. When you trash a page, the menu item isn't shown on the front-end.

19038.diff sets the 'Invalid' flag in the admin to any menu items which have trashed parent objects. The same flag is set when the post type does not exist anymore.

Needs UX review. Perhaps a red warning is too much in this case.

#11 @swissspidy
18 months ago

  • Milestone changed from Awaiting Review to Future Release

#12 @swissspidy
10 months ago

  • Milestone changed from Future Release to 4.7

I think a menu item of a trashed object should show as invalid in the admin and not show up at all on the front-end.

Just ran into this again. When I add a page or post to the menu and trash it, the menu item is still visible on the front-end. The permalink changes to http://src.wordpress-develop.dev/<slug>__trashed/ as expected because the slug changes when trashing. There's no indication when editing the menu that the post/page is trashed either.

#13 @swissspidy
9 months ago

  • Owner set to swissspidy
  • Status changed from new to assigned

#14 @swissspidy
9 months ago

  • Keywords commit has-screenshots added; ux-feedback removed

19038.2.diff still applies cleanly. A trashed page isn't displayed in the nav menu anymore.

Here's how a deleted page is shown in the menu editor with the patch applied.

https://cldup.com/BcqF9fbGSj.png

@swissspidy
9 months ago

adds unit test

#15 @swissspidy
9 months ago

  • Keywords has-unit-tests added

#16 @swissspidy
9 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38744:

Menus: Do not show trashed posts in nav menus.

Trashed posts cannot be accessed by site visitors and thus should not be visible on the front end. By marking menu items of trashed posts as invalid, they are excluded from the output.

Props solarissmoke, swissspidy.
Fixes #19038.

Note: See TracTickets for help on using tickets.