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

WordPress Trac wp-trac at lists.automattic.com
Sun Jun 13 20:21:50 UTC 2010


#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:14 nacin]:
 > The patch you posted to get us going is missing the exclusion of
 $old_status == $new_status

 How would that affect the end result?

 > and also would probably duplicate efforts with trashing and untrashing
 of posts.

 OK, so add a check for when the new status is "trash."  No big deal.

 > The current implementation is not ideal UX, it overlapped with the no-JS
 pending, and triggered a bunch of bugs related to unpublished handling,
 starting with menu items associated with trashed posts.

 Please list the bugs.  I still haven't seen any specifics. Again, I fail
 to see why we can't handle trashed posts with a couple lines in the
 observer.

 > I thought [15219] was way too many changes so deep into an RC phase, and
 additional bandaids on it now is probably the wrong direction.

 That seems like a non-sequitur: there were too many changes, so in
 response we should  make even more substantial ones?

 > Handling this on generation is exactly how wp_list_pages/wp_page_menu
 functions, in that its get_pages() call only grabs published pages.

 The point is that it's not necessary in the first place: no matter how
 minimal, we're doing logic checks for a niche situation on every menu
 generation, logic checks that don't need to be done there at all.

 And other functions like `wp_list_pages` are not a helpful analogy,
 because here we're querying objects (menu items) that are not the same as
 the objects causing the problem (posts, pages, etc.).

 > I fail to see any simplicity in generalize-object-change-logic.1.diff
 that is not present in 13822-on-r15218.diff

 In `generalize-object-change-logic.1.diff` I see about 3 lines changed, as
 opposed to the dozens of new lines, several significant API changes and
 logic branches added in `13822-on-r15218.diff`.

 > > This proposal puts additional burdens on the public generation of
 menus, rather than handling the issue one time when it happens.
 >
 > Absolutely not. It's extraordinarily light code.

 It may be feather-light, but it's a feather that doesn't need to be there
 in the first place.

 > wp_setup_nav_menu_item() decorates a menu object based on the properties
 of post object it is associated with. If the properties of the post object
 invalidate the menu item for public display, that is clearly within the
 definition of setting up the menu item.

 Actually, that's mixing up "view" with "model."  The proposed changes
 attempt to change an object's ''data'' to respond to an issue of its
 ''presentation''.

 > It opens up no additional "bug possibilities".

 Instead of passing one object as the argument, now the proposal wants to
 pass an `$args` variable that is meant to branch on presentation logic.
 Imagine trying to create unit tests each for the current and the proposed,
 and you'll get what I mean.

 > I think most of the concerns outlined here are exaggerated. Your
 suggestions of where logic needs to go is largesly based on opinion, and I
 largely disagree with the assumptions made here.

 Although what's considered good programming practice has a large element
 of the subjective (perhaps judgment and experience) mixed in it, that
 doesn't mean that it's simply a matter of opinion.  I'm not clever enough
 to have come up with MVC, Uncle Bob's "Single Responsibility Principle,"
 or Unix's "do one thing and do it well."

 > Calling it "questionable design decisions with long-term implications"
 is laughable and reactionary, and I think we both know that.

 I've already raised questions against the design decisions; the long-term
 implications are that we'll be stuck with more-complicated, logically-
 convoluted code.

 I am reacting to what seems like a disproportionately large change, ''in
 RC3'', for a trivial problem.

 Nothing about this makes me want to laugh.

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


More information about the wp-trac mailing list