WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 8 months ago

Last modified 8 months ago

#16176 closed enhancement (fixed)

save_post_{$post_type}

Reported by: bmb Owned by: westi
Milestone: 3.7 Priority: low
Severity: normal Version: 3.0.4
Component: Posts, Post Types Keywords: has-patch 3.7-early commit
Focuses: Cc:

Description

a save_{$post_type} hook would be convenient.

Attachments (7)

post.php.diff (595 bytes) - added by bmb 3 years ago.
16176.patch (613 bytes) - added by SergeyBiryukov 3 years ago.
16176.2.patch (611 bytes) - added by SergeyBiryukov 3 years ago.
With double quotes
16176.3.patch (685 bytes) - added by ocean90 8 months ago.
Refresh of .2 and with $update
16176.4.patch (1.6 KB) - added by ocean90 8 months ago.
Updated inline docs
16176.5.patch (1.7 KB) - added by ocean90 8 months ago.
save_post_{post_type} before save_post
16176.6.patch (1.7 KB) - added by SergeyBiryukov 8 months ago.

Download all attachments as: .zip

Change History (41)

comment:1 mikeschinkel3 years ago

  • Cc mikeschinkel@… added

comment:2 follow-up: mikeschinkel3 years ago

Use case?

When exactly would you propose it being triggered?

comment:3 Denis-de-Bernardy3 years ago

Convenient, but I'm unsure about the suggested hook name... Plugins that currently rely on save_post would be seriously broken if we don't maintain BC; or plugins that hook into save_post with the assumption that it's explicitly for posts would also affect pages, attachments, etc. (not to mention that it would then get called twice for actual posts unless we check for it).

comment:4 in reply to: ↑ 2 bmb3 years ago

Replying to mikeschinkel:

Use case?

When exactly would you propose it being triggered?

right after save_post, same as add_meta_boxes and add_meta_boxes_{$post_type}, since this would sort of complement those hooks (e.g. if you added a metabox with add_meta_boxes, save with save_post. If you added with add_meta_boxes_{$post_type}, save with save_{$post_type})

comment:5 mikeschinkel3 years ago

Agree with Denis, it would have to be named different (maybe "save_post_type_{$post_type}") but I'll also ask if it is really needed? I worry about the overhead of running more hooks when you can already do what the proposed hook does in another hook by adding an if statement?

comment:6 dd323 years ago

The overhead of running a extra action on a post save seems rather low, Yes, It's another hook fire, but saving a post is not a time critical operation, such as the front end load, or loading an admin page.

If extra hooks are needed to help plugins avoid multiple if statements, that gets a +1 from me, but it should be fully reviewed to check to see what area's have hook, and what area's still need a hook.

comment:7 markjaquith3 years ago

  • Milestone changed from Awaiting Review to Future Release

comment:8 bmb3 years ago

  • Keywords custom post type removed

bmb3 years ago

comment:9 bmb3 years ago

  • Keywords has-patch added
  • Owner set to bmb
  • Status changed from new to accepted

comment:10 westi3 years ago

  • Milestone changed from Future Release to 3.2
  • Owner changed from bmb to westi
  • Priority changed from normal to low
  • Status changed from accepted to assigned

comment:11 nacin3 years ago

This would be a nice hook to add, but it should be immediately before or after save_post. Should punt at this point.

comment:12 nacin3 years ago

  • Milestone changed from 3.2 to Future Release

SergeyBiryukov3 years ago

comment:13 SergeyBiryukov3 years ago

16176.patch adds the hook right after save_post, as per nacin's suggestion. Also added post ID for consistency.

SergeyBiryukov3 years ago

With double quotes

comment:14 rmccue3 years ago

  • Cc me@… added

comment:15 jeremyfelt2 years ago

  • Cc jeremy.felt@… added

comment:16 stephenh19882 years ago

  • Cc stephen@… added

comment:17 SergeyBiryukov11 months ago

#24299 was marked as a duplicate.

comment:18 SergeyBiryukov11 months ago

  • Severity changed from trivial to normal

comment:19 SergeyBiryukov11 months ago

  • Keywords 3.7-early added

comment:20 ethitter11 months ago

  • Cc erick@… added

comment:21 toscho11 months ago

  • Cc info@… added

comment:22 wonderboymusic9 months ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

comment:23 mordauk8 months ago

  • Cc pippin@… added

comment:24 desrosj8 months ago

  • Cc desrosj@… added

ocean908 months ago

Refresh of .2 and with $update

comment:25 ocean908 months ago

Looks good, nacin?

ocean908 months ago

Updated inline docs

comment:26 follow-up: nacin8 months ago

It would be nice to pass $post as the first argument, but I understand why it shouldn't — so as to remain consistent with the existing save_post hook.

I think the docs could be simplified to 'save_post_{post_type}' just so it looks less crufty.

Is there a rhyme or reason (as determined elsewhere in core) to have this filter specifically before or after save_post? The only other situation I can think of is admin_head-$hook_suffix firing before admin_head. Other than looking into that, I'm +1 commit.

comment:27 in reply to: ↑ 26 ocean908 months ago

Replying to nacin:

Is there a rhyme or reason (as determined elsewhere in core) to have this filter specifically before or after save_post?

Seems like we should move it before save_post because the specific filter is mostly before the general. Examples:

update_option():

do_action( "update_option_{$option}", $oldvalue, $_newvalue );
do_action( 'updated_option', $option, $oldvalue, $_newvalue );

add_option():

do_action( "add_option_{$option}", $option, $_value );
do_action( 'added_option', $option, $_value );

set_transient():

do_action( 'set_transient_' . $transient, $value, $expiration );
do_action( 'setted_transient', $transient, $value, $expiration );

admin-header.php:

do_action("admin_print_styles-$hook_suffix");
do_action('admin_print_styles');
do_action("admin_print_scripts-$hook_suffix");
do_action('admin_print_scripts');
do_action("admin_head-$hook_suffix");
do_action('admin_head');

admin-footer.php:

do_action('admin_footer', '');
do_action('admin_print_footer_scripts');
do_action("admin_footer-" . $GLOBALS['hook_suffix']);

ocean908 months ago

save_post_{post_type} before save_post

comment:28 ocean908 months ago

  • Keywords commit added

comment:29 nacin8 months ago

Go for it.

comment:30 nacin8 months ago

  • Summary changed from save_{$post_type} to save_post_{$post_type}

SergeyBiryukov8 months ago

comment:31 follow-up: SergeyBiryukov8 months ago

16176.6.patch fixes a copy/paste error in the refreshed patches: $post_ID in line 2974 should be $post->ID.

comment:32 ocean908 months ago

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

In 25050:

Introduce save_post_{$post_type} hook.

The hook is fired before the general save_post hook and has the same args as save_post.

props bmb, SergeyBiryukov, ocean90, fixes #16176.

comment:33 in reply to: ↑ 31 ocean908 months ago

Replying to SergeyBiryukov:

Thanks Sergey!

comment:34 buffler8 months ago

  • Cc jeremy.buller@… added
Note: See TracTickets for help on using tickets.