[wp-trac] [WordPress Trac] #13822: Menu items that get unpublished still appear

[WordPress Trac]

#13822: Menu items that get unpublished still appear
------------------------------------------------+---------------------------
 Reporter:  nacin                               |        Owner:  nacin    
     Type:  defect (bug)                        |       Status:  reviewing
 Priority:  highest omg bbq                     |    Milestone:  3.0      
Component:  Menus                               |      Version:  3.0      
 Severity:  major                               |   Resolution:           
 Keywords:  has-patch dev-feedback i18n-change  |  
------------------------------------------------+---------------------------

Comment(by filosofo):

 Replying to [comment:7 nacin]:
 > Listening for status changes is not very effective, as you can't easily
 account for all options.
 >
 > The commit does not account for pending, private, or scheduled posts,
 only draft posts.

 True, but it seems like the proposal changes a single-point logic failure
 into an extensive code rewrite.  Fix the logic at the point of failure,
 instead.

 Among my concerns:
  * This proposal rewrites lots of code to deal with a niche situation that
 has, at worst, a dead link as its consequence.
  * The problem can be better solved at object-change time (as it currently
 does):
     * When the associated object changes is the right time to change its
 associated menu item.
     * This proposal puts additional burdens on the public generation of
 menus, rather than handling the issue one time when it happens.
  * We should ''not'' be putting query logic into `wp_setup_nav_menu_item`,
 as this proposal does.  `wp_setup_nav_menu_item` simply decorates an
 object to give it the requisite standard menu item properties.
     * This proposal takes `wp_setup_nav_menu_item`, a function that does
 one thing, and one thing well, and gives it additional things to do, which
 means the bug possibilities open up proportionally.
     * By having `wp_setup_nav_menu_item` handle additional arguments, it
 introduces the possibility of returning null having passed in an object,
 which doesn't make sense for something that's only supposed to ''add''
 properties.
  * We should ''not'' be putting view logic into `wp_get_nav_menu_items`
     * It makes `wp_get_nav_menu_items` more complicated: instead of
 getting menu items and passing them to `wp_setup_nav_menu_item` to
 decorate them, the proposal has it determining view state
 ('public_consumption').  This is the wrong place for that logic.
  * r15219 also had logic simplification for auto-page-add.  Those should
 not be reverted.


 To re-iterate:
  * I think this can be handled more simply.
  * The potential problems of the status quo (in other words, dead menu
 links) do not warrant wholesale code change at this point.
  * This proposal involves a number of questionable design decisions with
 long-term implications.

 Therefore I strongly object to these proposed changes.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/13822#comment:12>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software