[wp-trac] [WordPress Trac] #28172: edit_post() should call {un}stick_post() before calling wp_update_post()

[WordPress Trac]

#28172: edit_post() should call {un}stick_post() before calling wp_update_post()
-------------------------------+-----------------------------
 Reporter:  pbiron             |      Owner:
     Type:  enhancement        |     Status:  new
 Priority:  normal             |  Milestone:  Awaiting Review
Component:  Posts, Post Types  |    Version:  trunk
 Severity:  normal             |   Keywords:
  Focuses:                     |
-------------------------------+-----------------------------
 In a plugin I'm writing I have a need to hook into the `save_post` action
 and decide whether to allow a given post to be sticky based on postmeta
 associated with that post (and possibly other posts).

 However, if the function I attach to the `save_post` action calls
 `{un}stick_post()` it doesn't have the desired effect because of the way
 `edit_post()` is written.

 In particular, lines 320-332 of
 [https://core.trac.wordpress.org/browser/trunk/src/wp-
 admin/includes/post.php#L320] are:

 {{{
 wp_update_post( $post_data );

 // Now that we have an ID we can fix any attachment anchor hrefs
 _fix_attachment_links( $post_ID );

 wp_set_post_lock( $post_ID );

 if ( current_user_can( $ptype->cap->edit_others_posts ) ) {
         if ( ! empty( $post_data['sticky'] ) )
                 stick_post( $post_ID );
         else
                 unstick_post( $post_ID );
 }
 }}}

 The `save_post` action is fired within `wp_update_post()` (technically, it
 is fired by `wp_insert_post()` which is called by `wp_update_post()`).
 Since `edit_post()` calls `{un}stick_post()` '''after''' it calls
 `wp_update_post()`, anything I do in my `save_post` function with regard
 to whether the post should be sticky is undone!

 Hence, I suggest changing the relevant portion of `edit_post()` to
 {{{
 if ( current_user_can( $ptype->cap->edit_others_posts ) ) {
         if ( ! empty( $post_data['sticky'] ) )
                 stick_post( $post_ID );
         else
                 unstick_post( $post_ID );
 }

 wp_update_post( $post_data );

 // Now that we have an ID we can fix any attachment anchor hrefs
 _fix_attachment_links( $post_ID );

 wp_set_post_lock( $post_ID );
 }}}

 Is there a specific reason why `edit_post()` currently calls
 `wp_update_post()` before `{un}stick_post()`? (it isn't apparent to me)

 Another option I investigated was whether it was possible to have my
 plugin change `$post_data` via a filter but I couldn't find one.

 If others agree that this change would be good and wouldn't break anything
 else, I'll submit a patch.

 [Note: I went back and forth on whether to call this a bug or an
 enhancement.  It's a bug in the sense that there is certainly an
 unexpected result ({un}sticking a post in a `save_post` action is
 undone...I lost about an hour trying to figure out why my `save_post`
 function wasn't doing the right thing);  But it is an enhancement in the
 sense that core, itself, is not broken].

--
Ticket URL: <https://core.trac.wordpress.org/ticket/28172>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform