WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 20 months ago

#11235 new defect (bug)

Pages whose ancestors are not all "published" cannot be used as parents for other pages.

Reported by: caesarsgrunt Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.9
Component: Posts, Post Types Keywords: has-patch 2nd-opinion needs-screenshots needs-unit-tests
Focuses: Cc:

Description

Pages with trashed parents cannot be used as parents for other pages (they do not appear on the list).

Attachments (2)

11235.post.php.patch (2.3 KB) - added by muxahuk1214 20 months ago.
11235.diff (3.6 KB) - added by swissspidy 20 months ago.

Download all attachments as: .zip

Change History (14)

#1 @caesarsgrunt
8 years ago

  • Component changed from Trash to Taxonomy
  • Owner set to filosofo
  • Summary changed from Pages with trashed parents cannot be used as parents for other pages to Pages whose ancestors are not all "published" cannot be used as parents for other pages.

Correction : This does not only apply to trashed posts.

For example, changing a post's status to "pending", "draft", or "trash" will make all that post's descendants unavailable for use as parents for new posts. Also, although existing posts with them as parents will continue to work perfectly, the parent will be removed when these posts are next edited.

#2 @scribu
8 years ago

  • Keywords reporter-feedback added; needs-patch removed
  • Milestone changed from 2.9 to Future Release

It's not clear to me what the problem is.

Pages with non-published parents can't or shouldn't be allowed as parents?

#3 @caesarsgrunt
8 years ago

  • Keywords reporter-feedback removed

Create and publish two pages; Parent and Child. Set the parent of the latter to be the former. Now delete/un-publish Parent.

In the database, Child is still a child of Parent.
In the front end, on the live site, Child will now appear as a top-level page.

If you were to re-publish Parent, the taxonomy would be the same as before you un-published it.

Now there are two problems.

  1. Create a new page. Call it Grandchild. Now, although Child is still visible in the front-end and back-end, you cannot set it to be the parent of Grandchild.
  1. Edit Child. Save it. Now, the database will be changed so that Child is a root page. If you were now to re-publish Parent, Child would no longer be a child of it.

Let me know if you want any more details.

#4 @scribu
8 years ago

  • Component changed from Taxonomy to General
  • Owner filosofo deleted

Pretty clear now, thanks.

#5 @nacin
4 years ago

  • Component changed from General to Post Types

#6 @chriscct7
3 years ago

  • Keywords needs-patch added

#7 @muxahuk1214
20 months ago

  • Keywords has-patch added; needs-patch removed

Hey guys, it's my first patch @contributorsDayRiga. Could you plese review it?

This ticket was mentioned in Slack in #core by m7. View the logs.


20 months ago

#9 @adamsilverstein
20 months ago

Hey @muxahuk1214 thanks for your patch!

I did some basic testing on the patch and verified it fixed the primary issue reported on this ticket: "Pages with trashed parents cannot be used as parents for other pages" - after your patch I was able to select the child pages of trashed pages as my new page parent, and restoring the parent or publishing it restored the proper page hierarchy.

I'm concerned, however, about side effects: once trashing a parent, if I edit the child page and set a new parent for it, then later restore the original parent page, won't this override/replace my child's parent selection? This seems unexpected/buggy and should be avoided.

A few questions/suggestions:

  • Why do you think we need to add the filters?
  • What about cleanup of the _wp_post_parent_fallback meta when post parents are deleted?
  • the child post query could be a little more efficient using fields to get and pass just IDs
  • the filters could use inline docs;

#10 @adamsilverstein
20 months ago

  • Keywords 2nd-opinion needs-screenshots needs-unit-tests added

@swissspidy
20 months ago

#11 @swissspidy
20 months ago

I'm glad someone's working on such an ancient ticket, so thanks for that!

As for the patch, I can only second what Adam said.

It might make sense to move this logic into a new function that hooks into the transition_post_status_hook instead of putting it into _transition_post_status(). See 11235.diff for what I have in mind. It replaces the filters with action hooks too.

@muxahuk1214 Are you familiar with writing unit tests? If not, we could look at that together.

#12 @adamsilverstein
20 months ago

Thinking about this a little more after posting I realized the patch might affect attached media - we should test that to make sure that is not the case.

Also I wonder about changing the approach here and tackling only the narrower issue of the dropdown parent selector not showing the pages with missing parents. Fixing that dropdown would solve the primary issue raised on the ticket without as large a potential for side effects.

What do you think about updating the query for the dropdown to include these pages as selectable options?

Note: See TracTickets for help on using tickets.