WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#13179 closed defect (bug) (fixed)

Saving a custom post type with post_parent resets to zero

Reported by: mikeschinkel Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:

Description

If you set post_parent in code for a custom post type and then edit the child if resets the post_parent to zero.

Use-case: I have a custom post type of "store" and another of "location." I have a meta box that allows me to add locations on the store edit page and I set the location's post_parent to the post_ID of the store. All works fine until I go to edit a location that has a post_parent set save it will wipe the post parent and thus create an orphan.

(fyi, my locations do not have page-attributes so setting the parent is impossible from the location edit UI anyway.)

While this is not "supported" behavior yet I'd think that saving it should not overwrite a value already set. I went through the code and I'm not sure how to correct this. It seems that edit ignores what's in the database and only goes with what is in $_POST. As such, it might be a significant modification.

Something to consider?

Attachments (2)

Adds_a_hidden_input_for_$post_parent.diff (943 bytes) - added by mikeschinkel 4 years ago.
Adds a hidden <input> for $post_parent to maintain its value during save.
13179.diff (907 bytes) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 follow-up: mikeschinkel4 years ago

  • Component changed from General to Post Types

FYI, I was about to get around the problem by adding in a view-only metabox that had a hidden "parent_id" input element:

add_action('add_meta_boxes_location','my_add_meta_boxes_location');
function my_add_meta_boxes_location($location) {
	if ($location->post_parent) {
		add_meta_box('location_parent','store','my_location_parent_meta_box','location','side');
	}
}

function my_location_parent_meta_box($location,$metabox) {
	$store = get_post($location->post_parent);
	$html = <<<HTML
<div id="div" class="postbox">
	<h4>{$store->post_title}</h4>
	<input type="hidden" id="parent_id" name="parent_id" value="{$store->ID}" />
</div>
HTML;
	echo $html;
}

It would still be nice it editing did not overwrite values, but unless and until that if fixed this metabox can be used.

mikeschinkel4 years ago

Adds a hidden <input> for $post_parent to maintain its value during save.

comment:2 mikeschinkel4 years ago

  • Cc mikeschinkel@… added
  • Keywords has-patch dev-feedback added

I've added a patch that simply includes a single <input> for $parent_id that allows the post parent to be maintained.

Not sure if there will cause side effects elsewhere; any reason why parent was not already included in the form? There is code in edit_post() within /wp-admin/includes/post.php that has the comment "Reunite any orphaned attachments with their parent" but it seems that the code is just related to auto-drafts (but I'm not sure of this.)

BTW, _wp_translate_postdata() in /wp-admin/includes/post.php captures the $_POST value of 'parent_id' and translates it to 'post_parent' so I had to use 'parent_id' instead of the more standard 'post_parent.' Anyone know why it was done this way?

As I think this is effectively a bug I am hoping it can make it into 3.0.

comment:3 in reply to: ↑ 1 mikeschinkel4 years ago

Replying to mikeschinkel:

FYI, I was about to get around the problem by adding in a view-only metabox that had a hidden "parent_id" input element:

Note that in my example code above, "location" is my custom post type.

comment:4 follow-up: nacin4 years ago

  • Milestone changed from Unassigned to 3.1

Too late for 3.0.

comment:5 kevinB4 years ago

  • Cc kevinB added

comment:6 in reply to: ↑ 4 mikeschinkel4 years ago

Replying to nacin:

Too late for 3.0.

How about 3.1? I just spent 4 hours debugging a problem that was all because of the post_parent kept getting reset to zero (it caused tons of downstream data corruption so that's why it took so long to figure out.)

This is a really trivial change...

comment:7 nacin4 years ago

  • Milestone changed from Awaiting Triage to 3.1

comment:8 automattor3 years ago

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

(In [16657]) Prevent post_parent from being reset when there's no page attributes box. Props mikeschinkel. Fixes #13179

comment:9 solarissmoke3 years ago

  • Keywords has-patch dev-feedback removed
  • Resolution fixed deleted
  • Severity changed from normal to major
  • Status changed from closed to reopened

Unfortunately this breaks things.

There are now two elements with identical id="parent_id" and name="parent_id" in the HTML for the post editor when editing pages. One is a hidden element in the head and the other is in the "Parent" meta box.

comment:10 nacin3 years ago

If we kill the ID here, we should be fine in terms of validation, javascript, etc (not sure why else, or why this is 'major'). The name in the meta box will override the earlier name in the hidden input.

Last edited 3 years ago by nacin (previous) (diff)

nacin3 years ago

comment:11 nacin3 years ago

  • Keywords has-patch added

comment:12 solarissmoke3 years ago

  • Severity changed from major to normal

Oops wasn't supposed to change the severity

When submitting, the POST data contains parent_id twice - is this acceptable?

comment:13 solarissmoke3 years ago

Never mind, you've answered that.

comment:14 markjaquith3 years ago

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

(In [17445]) Do not output an HTML ID on the (potentially duplicate) hidden parent_id field. props nacin. fixes #13179 for trunk

comment:15 markjaquith3 years ago

(In [17446]) Do not output an HTML ID on the (potentially duplicate) hidden parent_id field. props nacin. fixes #13179 for 3.1

comment:16 scribu3 years ago

Follow-up: #16673

Note: See TracTickets for help on using tickets.