Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#18713 closed defect (bug) (fixed)

No hooks available to "calculate" a post_title when only custom metaboxes in use

Reported by: mikeschinkel's profile mikeschinkel Owned by: nacin's profile nacin
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:

Description

When building fully custom post data entry screens for custom post types where the default post_title, post_content and post_excerpt fields are not displayed in their standard data entry format the wp_insert_post() function bails without any hooks available to correct the problem.

The code that bails in on line # ~2430 in /wp-includes/post.php within wp_insert_post() when $post_title and $post_content and $post_excerpt are not set and $post_type is not 'attachment':

if ( ('' == $post_content) && ('' == $post_title) && ('' == $post_excerpt) && ('attachment' != $post_type) ) {
  if ( $wp_error )
    return new WP_Error('empty_content', __('Content, title, and excerpt are empty.'));
  else
    return 0;
}

An example where this is an issue is when creating a "Person" custom post type where there are meta fields for 'first_name', 'middle_initial' and 'last_name' and we want to calculate the post_title field based ON (something) like the following code:

extract( $_POST );
if ( empty( $middle_initial ) ) 
  $_POST['post_title'] = "{$first_name} {$last_name}"; 
else
  $_POST['post_title'] = "{$first_name} {$middle_initial} {$last_name}"; 

One solution would be to add a 'wp_translate_postdata' hook on the return statement of the __wp_translate_postdata() function found in /wp-admin/includes/post.php:

return apply_filters( 'wp_translate_postdata', $post_data, $udpate );

This filter would allow someone building a custom post type to update $post_data with values for $post_content, $post_title and $post_excerpt, or really any other field's value, for that matter.

Such a hook would also allow for critical error handling for unit, acceptance and regression tests. A user could write code in this hook for use with WP_DEBUG to ensure that data is always POSTed in the correct format. For example it would be helpful to test for when Javascript is used to create fields with complex values that need to be in the correct format to be properly saved to the database.

I have attached a patch that can add the above pondered 'wp_translate_postdata' hook to core, and as it is very simple hopefully it could make it in without much (any?) testing. However, suggestions for alternate solutions that address these two needs are welcome with the hope that something will make it into WordPress core to address these issues in the near future. Thanks in advance.

Attachments (4)

wp_translate_postdata-hook.diff (404 bytes) - added by mikeschinkel 13 years ago.
Adds a wp_translate_postdata hook
post.php.diff (429 bytes) - added by sirzooro 13 years ago.
New pre_wp_insert_post_data filter
18713.diff (663 bytes) - added by nacin 13 years ago.
18713.2.diff (873 bytes) - added by nacin 12 years ago.

Download all attachments as: .zip

Change History (23)

@mikeschinkel
13 years ago

Adds a wp_translate_postdata hook

#1 @mikeschinkel
13 years ago

  • Cc mikeschinkel@… added

FYI, here is a short term solution that I am using to bypass the code that causes wp_insert_post() to bail when `$_POSTpost_title? is empty:

add_action( 'admin_init', 'myplugin_admin_init' );
function myplugin_admin_init() {
  global $pagenow;
  if ( count( $_POST ) && empty( $_POST['post_title'] ) ) {
    if ( 'post.php' == $pagenow ) {
      $_POST['post_title'] = 'To be replaced';
    } else {
      // This just to let me know if I need to add other tests.   	
      wp_die( "ERROR: Unexpected value for \$pagenow when \$_POST['post_title'] is empty: {$pagenow} " );
    }
  }
}

I then update the 'post_title' field within the 'wp_insert_post_data' hook. Of course this is a real hack, and it doesn't let a plugin ensure that the custom post type can be saved correctly when wp_insert_post() is called directly from code, but the attached patch would allow a plugin to ensure it.

#2 @tollmanz
13 years ago

  • Cc tollmanz added

#3 @sirzooro
13 years ago

What about adding new filter at the beginning of wp_insert_post()? It would be more general.

Related: #18692

#4 @sirzooro
13 years ago

  • Cc sirzooro added

#5 @anointed
13 years ago

  • Cc anointed added

#6 @naomicbush
13 years ago

  • Cc naomicbush added

@sirzooro
13 years ago

New pre_wp_insert_post_data filter

#7 @mikeschinkel
13 years ago

@sirzooro - Either your patch or my patch could resolve the aforementioned use-cases so I would be happy with either. If you and others prefer a 'pre_wp_insert_post_data' filter instead of a 'wp_translate_postdata' filter, it works for me.

Hoping this could make 3.3?

#9 in reply to: ↑ 8 @sirzooro
13 years ago

Replying to mikeschinkel:

Argghh! 3.4?

Unfortunately :( Looks that "Awaiting Review" milestone ended like "Future Release" - endless bucket for tickets which definitely needs some love...

#10 follow-up: @prettyboymp
13 years ago

Is there something that keeps the filters in sanitize_post_field(), ie 'pre_post_title', from being used to fill in an empty title?

#11 in reply to: ↑ 10 @sirzooro
13 years ago

Replying to prettyboymp:

Is there something that keeps the filters in sanitize_post_field(), ie 'pre_post_title', from being used to fill in an empty title?

I am not sure - I use the submitpost_box filter to set post title and content if they are empty. You can check my plugin WyPiekacz for details.

@nacin
13 years ago

#12 @nacin
13 years ago

This is a pretty glaring defect for custom post types. I usually stuff something into post_title via a hidden field, that way it bypasses the check, then I reset post_title to my own value when I'm done.

Attached is a patch that adds a filter around the check as an alternative solution.

#13 @mikeschinkel
13 years ago

@nacin: Nice patch.

Version 1, edited 13 years ago by mikeschinkel (previous) (next) (diff)

#14 @mikeschinkel
13 years ago

@prettyboymp - The santize_post_field() is an interesting suggestion. Here's what I found after trying it:

Yes it is possible to provide a value for post_title using the 'pre_post_title' hook before WordPress punts on an empty value. However when used in the needed'db' context that hook only gets passed the $value and not $post_id or any other needed info. This is ironic because the alternate hook 'edit_post_field' used in the 'edit' context is passed both $value and $post_id. So for it to be a solution we'd minimally need to add $post_id as a 2nd parameter to the 'pre_post_title' hook. Here's the sanitize_post_fields() function so you can see what I'm talking about:

function sanitize_post_field($field, $value, $post_id, $context) {
	$int_fields = array('ID', 'post_parent', 'menu_order');
	if ( in_array($field, $int_fields) )
		$value = (int) $value;

	// Fields which contain arrays of ints.
	$array_int_fields = array( 'ancestors' );
	if ( in_array($field, $array_int_fields) ) {
		$value = array_map( 'absint', $value);
		return $value;
	}

	if ( 'raw' == $context )
		return $value;

	$prefixed = false;
	if ( false !== strpos($field, 'post_') ) {
		$prefixed = true;
		$field_no_prefix = str_replace('post_', '', $field);
	}

	if ( 'edit' == $context ) {
		$format_to_edit = array('post_content', 'post_excerpt', 'post_title', 'post_password');

		if ( $prefixed ) {
			$value = apply_filters("edit_{$field}", $value, $post_id);
			// Old school
			$value = apply_filters("{$field_no_prefix}_edit_pre", $value, $post_id);
		} else {
			$value = apply_filters("edit_post_{$field}", $value, $post_id);
		}

		if ( in_array($field, $format_to_edit) ) {
			if ( 'post_content' == $field )
				$value = format_to_edit($value, user_can_richedit());
			else
				$value = format_to_edit($value);
		} else {
			$value = esc_attr($value);
		}
	} else if ( 'db' == $context ) {
		if ( $prefixed ) {
			$value = apply_filters("pre_{$field}", $value);
			$value = apply_filters("{$field_no_prefix}_save_pre", $value);
		} else {
			$value = apply_filters("pre_post_{$field}", $value);
			$value = apply_filters("{$field}_pre", $value);
		}
	} else {
		// Use display filters by default.
		if ( $prefixed )
			$value = apply_filters($field, $value, $post_id, $context);
		else
			$value = apply_filters("post_{$field}", $value, $post_id, $context);
	}

	if ( 'attribute' == $context )
		$value = esc_attr($value);
	else if ( 'js' == $context )
		$value = esc_js($value);

	return $value;
}

However sanitize_post_field() is called in at least eight (8) places within WordPress core with six (6) different contexts, and leveraging it to set a default value for post_title or post_content is likely to result in lots of unintended consequences that would take a long time to identify and then correct. So I think a hook that is much more limited in scope would have less potential for inadvertent bugs.

#15 @nacin
12 years ago

  • Keywords has-patch dev-feedback 2nd-opinion removed
  • Milestone changed from Awaiting Review to 3.3

#18891 was closed as a duplicate.

I'm tempted to make this not only filterable, but also require support for 'editor', 'title,' and 'excerpt' in order to enforce each check.

It's not like a plugin can't manually specify fields that send back post_title, post_content, or post_excerpt, but this check is a serious vestige in the age of post types, so I'd like to render it as moot as possible.

#16 @nacin
12 years ago

  • Type changed from enhancement to defect (bug)

@nacin
12 years ago

#17 follow-up: @nacin
12 years ago

  • Keywords has-patch added

#18 in reply to: ↑ 17 @mikeschinkel
12 years ago

Replying to nacin:

Nice patch!

#19 @nacin
12 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [19305]:

Only enforce content/title/excerpt requirement in wp_insert_post() if the post type supports all three. Add a filter to allow this to be modified. fixes #18713.

Note: See TracTickets for help on using tickets.