WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 9 months ago

Last modified 9 months ago

#42864 closed defect (bug) (fixed)

REST API: Fire an action after items are completely inserted/updated

Reported by: pento Owned by: timothyblynjacobs
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch fixed-5.0
Focuses: rest-api Cc:

Description

In wp_insert_post() and wp_insert_comment(), the corresponding wp_insert_post and wp_insert_comment actions are run at the very end, after all meta and additional fields are inserted or update.

The equivalent REST API rest_insert_* actions are fired before the meta and additional fields are updated - there isn't an action to hook into, which is triggered after.

I expect this to become more of an issue with Gutenberg, as the behaviour of the existing post editor is that the post and related data have been completely updated by the time wp_insert_post fires.

Related: Gutenberg#3898, #38905.

Attachments (2)

42864.diff (7.3 KB) - added by TimothyBlynJacobs 11 months ago.
42864.2.diff (7.5 KB) - added by TimothyBlynJacobs 11 months ago.

Download all attachments as: .zip

Change History (22)

#1 @joemcgill
21 months ago

One side affect of this behavior is that if you're trying to support post meta in revisions, there is no hook that fires at the end of the process, meaning meta gets out of sync by one revision each time a new post + meta is saved.

Relevant code is in WP_REST_Posts_Controller::update_item() and it may be relevant that the rest_insert_* hooks were moved to higher up in the method in r39288.

#2 @rmccue
21 months ago

I thought these were also inconsistent, but as it turns out, I think r39288 helped make them consistent, although in the process we lost a crucial ability. We should introduce common hooks across all of these for a final action:

  • rest_insert_attachment is after attachment creation/update, but before meta + additional fields
  • rest_insert_comment is after comment creation/update, but before meta + additional fields
  • rest_insert_{post} is after post creation/update, but before meta + additional fields + sticky + terms + featured media ({post} is the post type).
  • rest_insert_{term} is after term creation/update, but before meta + additional fields ({term} is the taxonomy)
  • rest_insert_user is after user creation/update, but before roles + meta + additional fields

These actions are still useful if you want to do something when the object is updated, but not as useful as they could be.

I'm thinking a rest_did_insert_{type} for all of these across the board, called with the same parameters, but fired at the end of the relevant method rather than after the object change.

#3 @danielbachhuber
17 months ago

  • Owner set to danielbachhuber
  • Status changed from new to assigned

#4 @danielbachhuber
14 months ago

  • Owner danielbachhuber deleted

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


11 months ago

#6 @danielbachhuber
11 months ago

  • Owner set to timothyblynjacobs

@timothyblynjacobs said he'd pick this up at today's #core-restapi chat.

#7 @TimothyBlynJacobs
11 months ago

  • Keywords has-patch added

Added a patch. I went with rest_after_* because it seemed more inline with other hooks like wp_delete_post() which fires after_delete_post at the end. The other style I saw some examples of was something like rest_inserted_* Thoughts?

When working on the attachments hook, I saw that the rest_insert_attachment hook passes the request data as an array for the attachment instead of the actual WP_Post object. Is it too late to fix that? Should it be a separate ticket?

#8 @danielbachhuber
11 months ago

Patch looks pretty good, thanks @TimothyBlynJacobs.

As far as naming goes, I think rest_after_* is fine.

For the sake of precision, should the hooks be called after $request->set_param( 'context', 'edit' ); instead of before? This would ensure that the context value is always consistent.

When working on the attachments hook, I saw that the rest_insert_attachment hook passes the request data as an array for the attachment instead of the actual WP_Post object. Is it too late to fix that? Should it be a separate ticket?

Probably too late :/ It would be a separate ticket though.

This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.


11 months ago

#10 @TimothyBlynJacobs
11 months ago

Added a patch so the context param is always set properly when firing the hook.

#11 @danielbachhuber
11 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 43737:

REST API: Fire actions after items are completely updated/inserted.

The existing rest_insert_* actions are fired before meta and additional fields are updated. These new rest_after_* actions fire after all write operations have completed.

Props timothyblynjacobs.
Fixes #42864.

#12 @danielbachhuber
11 months ago

  • Keywords fixed-5.0 added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merge to trunk.

#13 @TimothyBlynJacobs
11 months ago

#39345 was marked as a duplicate.

This ticket was mentioned in Slack in #core-editor by danielbachhuber. View the logs.


11 months ago

#15 @TimothyBlynJacobs
11 months ago

Attachments have been the death of me...

#16 @peterwilsoncc
11 months ago

@TimothyBlynJacobs sorry, ended up creating a new ticket #45269

#18 @danielbachhuber
11 months ago

In 43862:

REST API: Prevent duplicate firing of rest(_after)?_insert_attachment actions.

Props danielbachhuber, peterwilsoncc.
Fixes #45269.
See #42864.

#19 @desrosj
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43987:

REST API: Fire actions after items are completely updated/inserted.

The existing rest_insert_* actions are fired before meta and additional fields are updated. These new rest_after_* actions fire after all write operations have completed.

Props timothyblynjacobs, danielbachhuber.

Merges [43737] to trunk.

Fixes #42864.

#20 @jeremyfelt
9 months ago

In 44225:

REST API: Prevent duplicate firing of rest(_after)?_insert_attachment actions.

Merges [43862] from the 5.0 branch to trunk.

Props danielbachhuber, peterwilsoncc.
Fixes #45269.
See #42864.

Note: See TracTickets for help on using tickets.