WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 4 years ago

Last modified 4 years 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 7 years ago.
16176.patch (613 bytes) - added by SergeyBiryukov 6 years ago.
16176.2.patch (611 bytes) - added by SergeyBiryukov 6 years ago.
With double quotes
16176.3.patch (685 bytes) - added by ocean90 4 years ago.
Refresh of .2 and with $update
16176.4.patch (1.6 KB) - added by ocean90 4 years ago.
Updated inline docs
16176.5.patch (1.7 KB) - added by ocean90 4 years ago.
save_post_{post_type} before save_post
16176.6.patch (1.7 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (41)

#1 @mikeschinkel
7 years ago

  • Cc mikeschinkel@… added

#2 follow-up: @mikeschinkel
7 years ago

Use case?

When exactly would you propose it being triggered?

#3 @Denis-de-Bernardy
7 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).

#4 in reply to: ↑ 2 @bmb
7 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})

#5 @mikeschinkel
7 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?

#6 @dd32
7 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.

#7 @markjaquith
7 years ago

  • Milestone changed from Awaiting Review to Future Release

#8 @bmb
7 years ago

  • Keywords custom post type removed

@bmb
7 years ago

#9 @bmb
7 years ago

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

#10 @westi
7 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

#11 @nacin
7 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.

#12 @nacin
7 years ago

  • Milestone changed from 3.2 to Future Release

#13 @SergeyBiryukov
6 years ago

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

@SergeyBiryukov
6 years ago

With double quotes

#14 @rmccue
6 years ago

  • Cc me@… added

#15 @jeremyfelt
6 years ago

  • Cc jeremy.felt@… added

#16 @stephenh1988
6 years ago

  • Cc stephen@… added

#17 @SergeyBiryukov
5 years ago

#24299 was marked as a duplicate.

#18 @SergeyBiryukov
5 years ago

  • Severity changed from trivial to normal

#19 @SergeyBiryukov
5 years ago

  • Keywords 3.7-early added

#20 @ethitter
5 years ago

  • Cc erick@… added

#21 @toscho
5 years ago

  • Cc info@… added

#22 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#23 @mordauk
4 years ago

  • Cc pippin@… added

#24 @desrosj
4 years ago

  • Cc desrosj@… added

@ocean90
4 years ago

Refresh of .2 and with $update

#25 @ocean90
4 years ago

Looks good, nacin?

@ocean90
4 years ago

Updated inline docs

#26 follow-up: @nacin
4 years 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.

#27 in reply to: ↑ 26 @ocean90
4 years 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']);

@ocean90
4 years ago

save_post_{post_type} before save_post

#28 @ocean90
4 years ago

  • Keywords commit added

#29 @nacin
4 years ago

Go for it.

#30 @nacin
4 years ago

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

#31 follow-up: @SergeyBiryukov
4 years ago

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

#32 @ocean90
4 years 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.

#33 in reply to: ↑ 31 @ocean90
4 years ago

Replying to SergeyBiryukov:

Thanks Sergey!

#34 @buffler
4 years ago

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