Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transition_post_status runs twice with same old and new status #15094

Open
RavanH opened this issue Apr 21, 2019 · 26 comments
Open

transition_post_status runs twice with same old and new status #15094

RavanH opened this issue Apr 21, 2019 · 26 comments
Labels
REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended
Milestone

Comments

@RavanH
Copy link

RavanH commented Apr 21, 2019

Describe the bug
When submitting a post update or an new post, it appears the transition_post_status hook(s) get called twice. First without the is_admin flag, then with is_admin passing true. The result may be unexpected as actions may behave differently or different actions may be included in both runs.

Please note this behavior is not seen with the classic editor.

Related issue #12897

To reproduce
I'll write a quick test plugin to show what is happening and how it may affect behavior...

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended Needs Testing Needs further testing to be confirmed. labels Apr 22, 2019
@earnjam earnjam self-assigned this Apr 22, 2019
@drov0
Copy link

drov0 commented Apr 23, 2019

I've had this issue for as long as gutenberg rooled out too.

In the end I used a mutex to make sure that only one of the two was taken into account.

@RavanH
Copy link
Author

RavanH commented Apr 24, 2019

To show what is going on behind the scenes, I've made this little plugin:

<?php
/*
Plugin Name: Github Issue #15094
Plugin URI: https://github.com/WordPress/gutenberg/issues/15094
Version: 0.1
Author: RavanH
*/

add_action( 'transition_post_status', 'ghi15094_publish_action', 10, 3 );

function ghi15094_publish_action( $new_status, $old_status, $post ) {
	error_log('Request '.$_SERVER['REQUEST_URI']);
	error_log('transition_post_status '.$old_status.' > '.$new_status);
	error_log(print_r($_POST,true));
}

With WP_DEBUG on, this will reveal the difference between the two passes that occur with Gutenberg and not with Classic Editor (or older WP versions).

With Classic Editor on, when editing and saving an existing published post, the log entries will look like this (as expected) :

[23-Apr-2019 23:51:11 UTC] Request /wp-admin/post.php
[23-Apr-2019 23:51:11 UTC] transition_post_status publish > publish
[23-Apr-2019 23:51:11 UTC] Array
(
... LONG ARRAY HOLDING ALL POST DATA ...
)

But with the block editor it's like this:

[23-Apr-2019 23:55:37 UTC] Request /wp-json/wp/v2/posts/5277?_locale=user
[23-Apr-2019 23:55:37 UTC] transition_post_status publish > publish
[23-Apr-2019 23:55:37 UTC] Array
(
) <<< EMPTY ARRAY !!!

[23-Apr-2019 23:55:38 UTC] Request /wp-admin/post.php?post=5277&action=edit&meta-box-loader=1&_wpnonce=c53452de80&_locale=user
[23-Apr-2019 23:55:38 UTC] transition_post_status publish > publish
[23-Apr-2019 23:55:38 UTC] Array
(
... LONG ARRAY HOLDING ALL POST DATA ...
)

This shows that both passes have the same post old/new status, but the first pass does not carry any $_POST data (empty array).

So simply testing for post status (old or new) does not allow us to distinguish between the two, nor can we rely on up-tot-date post data or (custom) post meta data as the first pass will fetch such data from the database, not from the post submission.

It does show however, that we can work around the issue by polling for an empty $_POST array. Like:

function ghi15094_publish_action( $new_status, $old_status, $post ) {
	if ( empty($_POST) ) return;

	// Do our action on the second pass with the block editor, on the first with the classic editor.
	// Note: updated (custom) post metadata is also available in the $_POST array!
	// Fetching such data with get_post_meta will likely get the old data because meta data is 
	// typically saved at "save_post" which runs after "transition_post_status"...
}

@RavanH RavanH changed the title transition_post_status runs twice transition_post_status runs twice with same old and new status Apr 24, 2019
@RavanH
Copy link
Author

RavanH commented Apr 24, 2019

Hmmm... come to think of it, that won't work for planned posts. A future post transitioning to published status shows these log messages:

[24-Apr-2019 14:33:13 UTC] Request /wp-cron.php?doing_wp_cron=1556116393.1422309875488281250000
[24-Apr-2019 14:33:13 UTC] transition_post_status future > publish
[24-Apr-2019 14:33:13 UTC] Array
(
)

Which means we cannot distinguish between the first Gutenberg API pass and a publication via WP Cron... We'd need something like an is_rest() flag here as suggested three years ago on https://core.trac.wordpress.org/ticket/34373 or manually test for defined( 'REST_REQUEST' ) && REST_REQUEST like:

function ghi15094_publish_action( $new_status, $old_status, $post ) {
	if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) return;

	// Do our action on the second pass with the block editor, on the first with the classic editor.
	// Note: updated (custom) post metadata is available in the $_POST array!
	// Fetching such data with get_post_meta will likely get the old data because meta data is 
	// typically saved at "save_post" which runs after "transition_post_status"...
}

@RavanH
Copy link
Author

RavanH commented Apr 24, 2019

Improved test plugin, shows which flags are present during the different passes, and on different occasions:

<?php
/*
Plugin Name: Github Issue #15094
Plugin URI: https://github.com/WordPress/gutenberg/issues/15094
Version: 0.2
Author: RavanH
*/

add_action( 'transition_post_status', 'ghi15094_transition_action', 10, 3 );

function ghi15094_transition_action( $new_status, $old_status, $post ) {
	error_log('Transitioning post from "'.$old_status.'" to "'.$new_status.'"');
	error_log('Request: '.$_SERVER['REQUEST_URI']);
	if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) error_log(' - Is a REST request');
	if ( is_admin() ) error_log(' - Is an admin request');
	if ( !empty($_POST['action']) && 'editpost' == $_POST['action'] ) error_log(' - Submitted from post edit page');
	if ( !empty($_POST['action']) && 'inline-save' == $_POST['action'] ) error_log(' - Submitted from Quick Edit post list');
	error_log(print_r($_POST,true));
}

@RavanH
Copy link
Author

RavanH commented Apr 24, 2019

Further testing shows that issue #12897 is indeed related: when switching a post from draft to publish, the first REST API pass will say draft > publish without $_POST data while the second pass will say publish > publish but with the $_POST data. So there is actually NO way to both test for draft to publish and have up-to-date post or post meta data available at the same time...

This is completely useless when we need the actual posted data (not what is stored as draft in the DB) on an new publication (not a post edit) :'(

@rgomezp
Copy link

rgomezp commented Aug 13, 2019

I'm also having the same issue

@drov0,
Could you please provide some more info on how exactly you used a mutex to solve this? Would appreciate it

@noisysocks
Copy link
Member

This was also reported here: https://core.trac.wordpress.org/ticket/47548

@noisysocks noisysocks removed the Needs Testing Needs further testing to be confirmed. label Aug 15, 2019
@noisysocks
Copy link
Member

I can confirm that this happens but am not yet clear on why. We'll need to do some more digging. It could be due to the extra HTTP request made by saving meta boxes. Maybe this second request happens so soon after the first that the database hasn't had a chance to update, yet.

@noisysocks noisysocks added this to the WordPress 5.x milestone Aug 15, 2019
@rgomezp
Copy link

rgomezp commented Aug 15, 2019

@noisysocks looking forward to your findings

@RavanH
Copy link
Author

RavanH commented Sep 20, 2019

Editing a post with the test plugin above on WordPress 5.3-alpha-46194, I notice something has changed. Now, only two REST requests are logged. One publish>publish with the full post object as $post (not passed through $_POST) and a second new>inherit which is a revision...

Tested updating from Quick Edit and cron passing a future post as well and in all cases, the logged data seems coherent. My preliminary conclusion : this can be marked as solved for 5.3

Can anyone confirm this?

Note: when using transition actions, do NOT rely on $_POST data because that only gets populated when submitting from the Quick Edit post list. Instead, get the third function parameter $post.

@RavanH
Copy link
Author

RavanH commented Sep 20, 2019

Oh, hang on... I'm mistaken. As soon as there is a plugin active that adds meta boxes to the post edit page, the double passes start appearing again. One REST request without $_POST data (as above) followed by a second pass with $_POST data. Only on the second, the submitted meta box data is passed. Apparently, this can only be done with a regular $_POST request outside of Gutenberg hence the need for the second action run...

So I guess the issue remains unaddressed.

Or do I need to look into how meta boxes should be hooking into the new editor REST request to avoid the need for this second $_POST request?

@RavanH
Copy link
Author

RavanH commented Sep 21, 2019

In fact, even the WordPress internal Custom Fields option will create a traditional meta box which in turn causes these double transition_post_status calls...

So @noisysocks and @rgomezp it is indeed the extra request to save meta box data. And to distinguish between the two, I only see the REST_REQUEST test...

@luigitek
Copy link

In fact, even the WordPress internal Custom Fields option will create a traditional meta box which in turn causes these double transition_post_status calls...

So @noisysocks and @rgomezp it is indeed the extra request to save meta box data. And to distinguish between the two, I only see the REST_REQUEST test...

In my case, it's still happening on 5.3

@RavanH
Copy link
Author

RavanH commented Sep 25, 2019

@luigitek

In my case, it's still happening on 5.3

Do you see any non-core metaboxes on your post edit screen? Either at the bottom of the center/content area or at the bottom of the right side under the Document tab? Or maybe you have the Custom Fields meta box activated? In those cases, there will be two post submissions (one via REST and one traditional post request) and therefore two transition_post_status passes. Without the additional (second!) pass, WordPress would not be able to save the post meta data.

Currently, as far as I can figure, the only way to distinguish between the two passes is to either test for the REST_REQUEST constant (if it's set and true, it's a REST request) or the $_POST global (if it's empty, it's probably a REST request) or a combination of these... Whatever you need to make it work :)

Just know: the first pass with REST_REQUEST set, has an empty $_POST and no meta data, the second pass, that normally happens about one second later (I suppose only after successful REST respons), has $_POST holding the new meta box data.

@kevinfodness
Copy link

This is related to #12903. I'm having a similar issue, where my code may need to run twice if there are postmeta updates coming in from metabox save, but it's not possible to tell from the REST request whether there will be a second request or not. There is no way to tell when a post is "done" saving.

@RavanH
Copy link
Author

RavanH commented Sep 27, 2019

@kevinfodness indeed, that's entirely unknown :/

@rgomezp
Copy link

rgomezp commented Nov 26, 2019

Any update?

@RavanH
Copy link
Author

RavanH commented Nov 27, 2019

Hi @rgomezp et all,

Why this happens?
The second pass of transition_post_status happens only if there is a plugin that adds meta boxes to the post editor. A second pass is needed to save the custom meta box data because those can not use the new AJAX call that the new Gutenberg editor uses. So this is by design, for backward compatibility with plugins that use meta box fields. It cannot be avoided.

What can you do as plugin or theme dev?
You can distinguish between the two passes by testing for the REST_REQUEST constant:

if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) { 
  // this goes when it's the first pass by the new block editor, does NOT occur when Classic Editor is activated
  // $_POST is not available here
} else {
  // this happens as second pass when there are custom meta boxes in the new block editor OR as a first and ONLY pass when Classic Editor is activated OR when running on older WP version
  // $_POST is available
}

Now if you have your own custom meta box and want to save its data on transition_post_status or save_post, then you can simply test for the $_POST data availability like this:

    if ( !empty($_POST) && array_key_exists('my_custom_field', $_POST)) {
        update_post_meta(
            $post_id,
            '_my_custom_field_key',
            $_POST['my_custom_field']
        );
    }

But if you are doing something else on transition post status, and you cannot be sure there is a second pass going to happen (because that depends on other plugins adding meta boxes or not), then I suggest something more complex like this:

function ghi15094_my_updater( $new_status, $old_status, $post ) {
	// run your code but do NOT count on $_POST data being available here!
}

function ghi15094_transition_action( $new_status, $old_status, $post ) {
	if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) { 
		ghi15094_my_updater( $new_status, $old_status, $post );
		set_transient( 'my_updater_flag', 'done', 10 );
	} else {
		if ( false === get_transient( 'my_updater_flag' ) ) {
			ghi15094_my_updater( $new_status, $old_status, $post );
		}
	}
}
add_action( 'transition_post_status', 'ghi15094_transition_action', 10, 3 );

This should make sure your code runs on every case, and only once.

Only thing is that $_POST will not be always available. If you depend on $_POST data then you'll have to find another way to fetch your data, like extract it from $post data or fetch it from the database. $_POST It simply is not there anymore in the new block editor except when there is a custom meta box... (Hint: you could always force the new editor to use a second pass by adding your own custom meta box... but that's not a very elegant solution.)

@rgomezp
Copy link

rgomezp commented Nov 27, 2019

@RavanH thanks so much for such a detailed response!

@christincooper
Copy link

This is very nice RavanH.

@mihdan
Copy link

mihdan commented Nov 3, 2021

Add this code in callback function:

if ( ! empty( $_REQUEST['meta-box-loader'] ) ) { // phpcs:ignore
    return;
}

@RavanH
Copy link
Author

RavanH commented Nov 8, 2021

@mihdan Nice! I was not aware of 'meta-box-loader' (available since WP 5.0 it seems?)... How does that request parameter behave when someone is using Classic Editor for example?

@scofennell
Copy link

scofennell commented Nov 10, 2021

		set_transient( 'my_updater_flag', 'done', 10 );
	} else {
		if ( false === get_transient( 'my_updater_flag' ) ) {
			ghi15094_my_updater( $new_status, $old_status, $post );
		}
	}

Could you clarify what the purpose of the transient is, and how you arrived at 10 as a sensible value for lifespan?

This is very nice, it seems to work perfectly, but I want to understand it more.

@sebastiantheobald
Copy link

Add this code in callback function:

if ( ! empty( $_REQUEST['meta-box-loader'] ) ) { // phpcs:ignore
    return;
}

Great! Thanks for that :)!!

@RavanH
Copy link
Author

RavanH commented Jan 25, 2022

@scofennell

Could you clarify what the purpose of the transient is, and how you arrived at 10 as a sensible value for lifespan?

This is very nice, it seems to work perfectly, but I want to understand it more.

The transient is used to know if the ghi15094_my_updater was run before or not. This would happen if ghi15094_transition_action is called twice, frist by the REST request (with the REST_REQUEST constant) and then shortly after that by a possible legacy request (to accommodate plugins or theme). I just assume the time between these passes should be no longer than 10 seconds. I'm also assuming that there would not be a second update post action within 10 seconds. Hence that transient timeout. But that is based on 2 assumptions, I admit...

@itsmeit268
Copy link

Add this code in callback function:

if ( ! empty( $_REQUEST['meta-box-loader'] ) ) { // phpcs:ignore
    return;
}

thank you, you saved me time, it took me 2 days to process it :(

@ellatrix ellatrix added the REST API Interaction Related to REST API label Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests