[wp-trac] [WordPress Trac] #20419: get_sample_permalink() passes the wrong post_status to wp_unique_post_slug()

[WordPress Trac]

#20419: get_sample_permalink() passes the wrong post_status to
wp_unique_post_slug()
--------------------------+------------------
 Reporter:  mintindeed    |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  3.5
Component:  Permalinks    |     Version:  3.3
 Severity:  minor         |  Resolution:
 Keywords:  has-patch     |
--------------------------+------------------

Comment (by mintindeed):

 Not sure it makes sense to do that.  It would be more efficient, but it
 would kinda deprecate post_status and introduce a bunch of (unnecessary?)
 changes.

 In order to not break backwards compatibility *too* much it would look
 something like:
 {{{
 function wp_unique_post_slug( $slug, $post_ID, $post_status, $post_type,
 $post_parent ) {
         if ( is_object($post_status) )
                 $post_status = $post->post_status;
         elseif ( in_array( $post_status, array( 'draft', 'pending', 'auto-
 draft' ) ) )
                 return $slug;
 }}}

 but that elseif really should be an "else" in order for it to work as
 expected when passing an object -- otherwise you would be changing the API
 where "version 1" is a string and only allows certain statuses, and
 "version 2" is an object and allows any status.  I guess you would put in
 a piece of branching logic to determing whether you're using "version 1
 (string)" or "version 2 (object)" and if your'e using an obj, allow any
 string as value assuming that the user is overriding the behaviour via
 plugin?  That seems a little dangerous.

 Maybe turn it on its head, and just make sure the status isn't "publish":
 {{{
 function wp_unique_post_slug( $slug, $post_ID, $post_status, $post_type,
 $post_parent ) {
         if ( isset($post_ID->post_status) )
                 $post_status = $post_ID->post_status;

         if ( 'publish' === $post_status )
                 return $slug;
 }}}

 20419-post-obj.patch -- This changes the behaviour a little bit, because
 instead of allowing a whitelist of post_status it's allowing anything
 that's not "publish".  That's OK IMO because really the only status I know
 about that matters w/r/t post_status is "publish" because that's the "it's
 public, therefore it's permanent" status.  Including a patch with that
 scenario: 20419-russian-reversal.patch -- I don't like this nearly as
 much, and if you see the diff you'll understand why.  Unless I'm being
 dense and missing something, there are a lot of maybe-edge-cases it
 doesn't account for -- namely if it's falling back to faking a $post
 object when an ID-only is passed to it, it could break things.  Also it
 has to add an extra param to the "wp_unique_post_slug" filter in order to
 not break things, and allow plugins to take advantage of the new
 functionality.

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