WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#17109 closed enhancement (fixed)

Add post-success action hooks to edit/create/delete calls.

Reported by: ericmann Owned by: westi
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: XML-RPC Keywords: has-patch needs-testing commit
Focuses: Cc:

Description

It was suggested in #14283 that we add call_success hooks for post creation using the Blogger API. The ticket was later expanded to add hooks for all relevant post/page methods.

We should extend this workflow concept and add hooks to all edit/create/delete calls for different data types (comments, media, etc) to make the XML-RPC system more pluggable.

These hooks should use the following format:

do_action( 'xmlrpc_call_success_{method name}', {object id}, $args );

These hooks should be called before returning data to the client, but should not be called before any processing is done or before error messages are created.

Attachments (4)

17109.diff (2.1 KB) - added by ericmann 10 years ago.
17109-2.diff (2.0 KB) - added by ericmann 10 years ago.
Uses filters where appropriate.
17109-3.diff (2.0 KB) - added by ericmann 10 years ago.
Switch new action hooks to filters.
17109-refresh.diff (2.1 KB) - added by ericmann 10 years ago.
Refresh patch to match latest version of trunk.

Download all attachments as: .zip

Change History (16)

@ericmann
10 years ago

#1 @ericmann
10 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Add action hooks to the following calls:

  • wp_newCategory
  • wp_deleteCategory
  • wp_deleteComment
  • wp_editComment
  • wp_newComment
  • mw_newMediaObject

Post-related hooks are added by #14283.

#2 follow-ups: @josephscott
10 years ago

In order to potentially modify the data before returning it shouldn't we be filtering instead and returning the filter variable?

#3 in reply to: ↑ 2 @ericmann
10 years ago

Replying to josephscott:

In order to potentially modify the data before returning it shouldn't we be filtering instead and returning the filter variable?

That's a good question. But I would say no.

For many of these calls, the only thing returned at this stage is the ID or a boolean representing whether or not the call succeeded. Adding a filter doesn't add any value, but adding an action hook provides the user with a way to further manipulate data or act on the content of a successful call after it's completed. But there's no further output at this stage to be filtered.

#4 in reply to: ↑ 2 @ericmann
10 years ago

Replying to josephscott:

In order to potentially modify the data before returning it shouldn't we be filtering instead and returning the filter variable?

Actually, I've changed my mind. With the methods that actually return data, we should be using filters rather than actions. But not everything returns data. Some only return Boolean values that don't make sense being passed through a filter.

The new patch uses filters where they make sense, but sticks to actions where they don't.

@ericmann
10 years ago

Uses filters where appropriate.

#5 @daniloercoli
10 years ago

  • Cc ercoli@… added

#6 follow-up: @josephscott
10 years ago

Any reason not to make these all filters? Seems strange to allow for filtering the return value of some methods, but not others.

#7 in reply to: ↑ 6 @ericmann
10 years ago

Replying to josephscott:

Any reason not to make these all filters? Seems strange to allow for filtering the return value of some methods, but not others.

It was based on return values. For example, the functions that return data (i.e. IDs) are using filters on the IDs. The functions that return bools (i.e. success/failure) fire an action instead.

My intention was to only use filters when the result of the filter would be returned by the function and use actions elsewhere to still allow devs to hook in to the function.

@ericmann
10 years ago

Switch new action hooks to filters.

#8 follow-up: @markoheijnen
10 years ago

  • Cc marko@… added

To be honest I don't get the patch. Why use filters over actions and not use the data that get returned. I guess that is a mistake.

I think it is better to put in actions like #14283. Changing data that should be returned feels weird to me. Maybe that I can't think of any situation where this would be necessary.

#9 in reply to: ↑ 8 @ericmann
10 years ago

Replying to markoheijnen:

Why use filters over actions and not use the data that get returned. I guess that is a mistake.

If you look at the patch history, originally I was using filters only when data was being returned and actions elsewhere. But @josephscott recommended using filters for all methods for the sake of consistency in the API. Functionally, they serve the same purpose.

@ericmann
10 years ago

Refresh patch to match latest version of trunk.

#10 @ericmann
10 years ago

  • Keywords commit added

Since actions are more appropriate hooks here, all filters have been replaced in the latest patch refresh.

#11 @westi
10 years ago

  • Milestone changed from Awaiting Review to 3.4
  • Owner set to westi
  • Status changed from new to accepted
  • Version set to 3.4

This looks good.

I agree that actions are more appropriate here to allow a plugin to action on the change but not to affect the returned data which is already a success value.

#12 @westi
10 years ago

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

In [20157]:

XMLRPC: Add some xmlrpc_call_success_* actions to some of the credit/edit/delete calls. Fixes #17109 props ericmann.

Note: See TracTickets for help on using tickets.