Make WordPress Core

Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#16176 closed enhancement (fixed)

save_post_{$post_type}

Reported by: bmb's profile bmb Owned by: westi's profile 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 13 years ago.
16176.patch (613 bytes) - added by SergeyBiryukov 13 years ago.
16176.2.patch (611 bytes) - added by SergeyBiryukov 13 years ago.
With double quotes
16176.3.patch (685 bytes) - added by ocean90 11 years ago.
Refresh of .2 and with $update
16176.4.patch (1.6 KB) - added by ocean90 11 years ago.
Updated inline docs
16176.5.patch (1.7 KB) - added by ocean90 11 years ago.
save_post_{post_type} before save_post
16176.6.patch (1.7 KB) - added by SergeyBiryukov 11 years ago.

Download all attachments as: .zip

Change History (41)

#1 @mikeschinkel
13 years ago

  • Cc mikeschinkel@… added

#2 follow-up: @mikeschinkel
13 years ago

Use case?

When exactly would you propose it being triggered?

#3 @Denis-de-Bernardy
13 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
13 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
13 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
13 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
13 years ago

  • Milestone changed from Awaiting Review to Future Release

#8 @bmb
13 years ago

  • Keywords custom post type removed

@bmb
13 years ago

#9 @bmb
13 years ago

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

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

  • Milestone changed from 3.2 to Future Release

#13 @SergeyBiryukov
13 years ago

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

@SergeyBiryukov
13 years ago

With double quotes

#14 @rmccue
12 years ago

  • Cc me@… added

#15 @jeremyfelt
12 years ago

  • Cc jeremy.felt@… added

#16 @stephenh1988
12 years ago

  • Cc stephen@… added

#17 @SergeyBiryukov
11 years ago

#24299 was marked as a duplicate.

#18 @SergeyBiryukov
11 years ago

  • Severity changed from trivial to normal

#19 @SergeyBiryukov
11 years ago

  • Keywords 3.7-early added

#20 @ethitter
11 years ago

  • Cc erick@… added

#21 @toscho
11 years ago

  • Cc info@… added

#22 @wonderboymusic
11 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#23 @mordauk
11 years ago

  • Cc pippin@… added

#24 @desrosj
11 years ago

  • Cc desrosj@… added

@ocean90
11 years ago

Refresh of .2 and with $update

#25 @ocean90
11 years ago

Looks good, nacin?

@ocean90
11 years ago

Updated inline docs

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

save_post_{post_type} before save_post

#28 @ocean90
11 years ago

  • Keywords commit added

#29 @nacin
11 years ago

Go for it.

#30 @nacin
11 years ago

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

#31 follow-up: @SergeyBiryukov
11 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
11 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
11 years ago

Replying to SergeyBiryukov:

Thanks Sergey!

#34 @buffler
11 years ago

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