WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#18713 closed defect (bug) (fixed)

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

Reported by: mikeschinkel Owned by: 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 3 years ago.
Adds a wp_translate_postdata hook
post.php.diff (429 bytes) - added by sirzooro 3 years ago.
New pre_wp_insert_post_data filter
18713.diff (663 bytes) - added by nacin 3 years ago.
18713.2.diff (873 bytes) - added by nacin 2 years ago.

Download all attachments as: .zip

Change History (23)

mikeschinkel3 years ago

Adds a wp_translate_postdata hook

comment:1 mikeschinkel3 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.

comment:2 tollmanz3 years ago

  • Cc tollmanz added

comment:3 sirzooro3 years ago

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

Related: #18692

comment:4 sirzooro3 years ago

  • Cc sirzooro added

comment:5 anointed3 years ago

  • Cc anointed added

comment:6 naomicbush3 years ago

  • Cc naomicbush added

sirzooro3 years ago

New pre_wp_insert_post_data filter

comment:7 mikeschinkel3 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?

comment:9 in reply to: ↑ 8 sirzooro3 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...

comment:10 follow-up: prettyboymp3 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?

comment:11 in reply to: ↑ 10 sirzooro3 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.

nacin3 years ago

comment:12 nacin3 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.

comment:13 mikeschinkel3 years ago

@nacin: Nice patch.

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

comment:14 mikeschinkel3 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.

comment:15 nacin2 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.

comment:16 nacin2 years ago

  • Type changed from enhancement to defect (bug)

nacin2 years ago

comment:17 follow-up: nacin2 years ago

  • Keywords has-patch added

comment:18 in reply to: ↑ 17 mikeschinkel2 years ago

Replying to nacin:

Nice patch!

comment:19 nacin2 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.