WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#13490 closed defect (bug) (worksforme)

Permalinks Issues for Custom Post Types with Parents

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

Description

When a custom post type is assigned a value for post_parent via a custom metabox then it's permalink fails because the rewrite rules for register_post_type() don't allow paths longer than two segments.

The URL can be fixed by removing the parent path segment via the "post_type_link" hook but doing so does not fix the "sample" URL displayed in the editor. This confuses my end user because the URL he sees is not the actual URL that works and there is no hook to allow me to fix this.

We could add a filter on the return value for get_sample_permalink() but it feels like that's just patching the problem because it requires two hooks to be coded to addedd this problem instead of allowing WP core to recognize the pattern of having custom post types with parents. So I don't know what the proper solution is which is why I am not attaching a patch here.

To understand my use case I have a custom post type called "Restaurant" and another called "Restaurant Location." Clearly a Restaurant is a parent of a Restaurant Location, right?

My URL templates are like this:

http://example.com/restaurants/%restaurant%/
http://example.com/restaurant-locations/%restaurant-location%/ 

And my actual URLs might look like this:

http://example.com/restaurants/johns-seafood/
http://example.com/restaurant-locations/johns-seafood-miami/ 

What shows up in the editor is this with the latter segment editable (notice how the base segment is "restaurant-locations" and not even "restaurants":

http://example.com/restaurant-locations/johns-seafood/johns-seafood-miami/ 

BTW, I'd really prefer to be able to have the following but to have the following would take a lot more UX surgery on the core and the admin editor than I think is smart for a plugin to attempt to perform (assuming it is even possible):

http://example.com/restaurants/johns-seafood/
http://example.com/restaurants/johns-seafood/miami/ 

Change History (10)

comment:1 follow-up: dd324 years ago

  • Component changed from General to Post Types
  • Keywords reporter-feedback added
  • Version set to 3.0

When a custom post type is assigned a value for post_parent via a custom metabox then it's permalink fails because the rewrite rules for register_post_type() don't allow paths longer than two segments.

If you set it to be hierarchical, then it should allow an unlimited path length, the same as pages, the only catch being it must be /<rewrite slug>/../../../../../ etc.

After registering a post type, you need to flush the rewrite rules in order for the new rules to take effect, This is probably whats caught you, you've set it up first without the hierarchical flag set, then changed it, which has enabled the UI, but the rules are still only for non-hierarchical.

For the rest of restaurant-locations vs restaurants, I dont fully understand, To me it sounds like you want to have the latter as child "pages" of the location? If so, thats fully supported -as the same post type-, If you want them to be different post types, you need to handle the permalinks yourself, through add_rewrite_rule(), as the post type URL rewriting only handles the basic /slug/...../ permastructure, nothing fancy, But WordPress has nothing stoping you from using something fancy, you just need to write the rule yourself.

comment:2 in reply to: ↑ 1 ; follow-up: mikeschinkel4 years ago

Replying to dd32:

If you set it to be hierarchical, then it should allow an unlimited path length, the same as pages, the only catch being it must be /<rewrite slug>/../../../../../ etc.

Thanks for the response. I wasn't having any issue with path length. Or maybe I misunderstand a correlation?

After registering a post type, you need to flush the rewrite rules in order for the new rules to take effect, This is probably whats caught you, you've set it up first without the hierarchical flag set, then changed it, which has enabled the UI, but the rules are still only for non-hierarchical.

I'm pretty sure that is not the issue. Here is my code (currently in the theme and not in a plugin while I work to get it right after which I plan to move to a plugin); note the flushing of rewrite rules on every page load:

add_action('init', 'tyc_post_types');
function tyc_post_types() {
	register_post_type('restaurant',
		array(
			'label'           => __('Restaurants'),
			'singular_label'  => __('Restaurant'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant',
			'rewrite'         => array('slug' => 'restaurants'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				'excerpts',
				'thumbnail',
				'custom-fields',
				'editor',
				),
			)
	);
	register_post_type('restaurant-location',
		array(
			'label'           => __('Locations'),
			'singular_label'  => __('Location'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant-location',
			'rewrite'         => array('slug' => 'restaurant-locations'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				'editor',
				'excerpt',
				'custom-fields',
				),
			)
	);

	register_post_type('restaurant-menu',
		array(
			'label'           => __('Menus'),
			'singular_label'  => __('Menu'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant-menu',
			'rewrite'         => array('slug' => 'restaurant-menus'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				),
			)
	);

	global $wp_rewrite;
	$wp_rewrite->flush_rules(false);

}

So I'm pretty sure it's not a case of setting without the hierarchical flag set then changed it but having the rules still being non-hierarchical. I'm guessing maybe you are making an assumption that it works as expected but that it actually isn't working as expected? (Forgive me if my guess is wrong and it's just me misunderstanding somehow.) Maybe you could take my code above and test it?

For the rest of restaurant-locations vs restaurants, I dont fully understand, To me it sounds like you want to have the latter as child "pages" of the location?

Having restaurants as children of locations? No, the other way around; locations as children of restaurants.

If so, thats fully supported -as the same post type-, If you want them to be different post types, you need to handle the permalinks yourself, through add_rewrite_rule(), as the post type URL rewriting only handles the basic /slug/...../ permastructure, nothing fancy, But WordPress has nothing stoping you from using something fancy, you just need to write the rule yourself.

Okay, yes it is possible to come around and add more rewrite rules but it's not possible (AFAICT) to assign a more complex rule using register_post_type()so we end up with vestigial rules in the list, and a long list of rules can be a performance drain for page load so I don't want to have excess rewrite rules that are never used.

Also, there's another aspect of the ticket I don't think you picked up on and that's the strong interest in having URLs that have the same slug, i.e. "miami" is the location for all three restaurants in this case:

http://example.com/restaurants/johns-seafood/miami/ 
http://example.com/restaurants/marys-ale-house/miami/ 
http://example.com/restaurants/big-jims/miami/ 

Currently WordPress doesn't handle duplicate slugs and wants to make it look like this which is sub-optimal for UX:

http://example.com/restaurants/johns-seafood/miami/ 
http://example.com/restaurants/marys-ale-house/miami-2/ 
http://example.com/restaurants/big-jims/miami-3/ 

comment:3 mikeschinkel4 years ago

  • Cc mikeschinkel@… added

comment:4 mikeschinkel4 years ago

Okay, so I put some more time into this and am further confirming my concerns. (However, if I'm doing it wrong I will be graciously happy to learn of what I am doing wrong.)

So I've updated the code to look like this:

add_action('init', 'tyc_post_types');
function tyc_post_types() {
	global $wp,$wp_rewrite;
	$wp->add_query_var('restaurant');
	$wp_rewrite->add_rewrite_tag('%restaurant%', '([^/]+)','restaurant=');

	register_post_type('restaurant-menu',
		array(
			'label'           => __('Menus'),
			'singular_label'  => __('Menu'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant-menu',
			'rewrite'         => array('slug' => 'restaurant-menus'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				),
			)
	);

	$wp->add_query_var('restaurant-menu');
	$wp_rewrite->add_rewrite_tag('%restaurant-menu%', '([^/]+)','restaurant-menu=');
	$wp_rewrite->add_permastruct('restaurant-menu', 'restaurants/%restaurant%/menus/%restaurant-menu%');

	register_post_type('restaurant-location',
		array(
			'label'           => __('Locations'),
			'singular_label'  => __('Location'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant-location',
			'rewrite'         => array('slug' => 'restaurant-locations'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				'editor',
				'excerpt',
				'custom-fields',
				),
			)
	);

	$wp->add_query_var('restaurant-location');
	$wp_rewrite->add_rewrite_tag('%restaurant-location%', '([^/]+)','restaurant-location=');
	$wp_rewrite->add_permastruct('restaurant-location', 'restaurants/%restaurant%/%restaurant-location%');

	register_post_type('restaurant',
		array(
			'label'           => __('Restaurants'),
			'singular_label'  => __('Restaurant'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant',
			'rewrite'         => array('slug' => 'restaurants'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				'excerpts',
				'thumbnail',
				'custom-fields',
				'editor',
				),
			)
	);
	global $wp_rewrite;
	$wp_rewrite->flush_rules(false);

}

With this code this URL for Restaurant Menus works (I've not yet tried to get Restaurant Locations to work yet):

http://thetasteyoucrave.dev/restaurants/basmatis/menus/dinner-menu/

But the edit screen has this as a URL (where "dinner-menu" is editable) and the "View Menu" has the same link and of course doesn't work:

http://thetasteyoucrave.dev/restaurants/%restaurant%/menus/basmatis/dinner-menu/

As they say, pictures are worth 1000 words so here is the edit screen:

http://img.skitch.com/20100522-q6bxt9c18jhnsmpgqt8feb1hyf.png

And here's after clicking the "View Menu" button link:

http://img.skitch.com/20100522-jjsw8dsjmr1jc72fsw799685dc.png

I tried correcting the problem with a post_type_link hook modifying only a "sample" link but it didn't help; results were the same.

It seems the major culprits here are the get_sample_permalink() and/or the get_sample_permalink_html() functions.

comment:5 mikeschinkel4 years ago

Okay. Ultimately to get it to work here's what I had to do (and I'm not 100% sure all use-cases are tested.) I had to 'preg_replace()' the html filtered by the 'get_sample_permalink_html' hook (which is fragile and is likely to break with other plugin code that uses the term "menus" in URLs) and I had to do some serious surgery on the 'post_type_link' hook:

<?php

add_filter('get_sample_permalink_html','tyc_get_sample_permalink_html',10,4);
function tyc_get_sample_permalink_html($return, $id, $new_title, $new_slug) {
	if (strpos($return,'</span>/menus/</span>')!==false) {
		$return = str_replace('</span>/menus/</span>','</span>/</span>',$return);
		$return = preg_replace('#http://([^/]+)/restaurants/([^/]+)/<span#','http://$1/restaurants/$2/menus/<span',$return);
		$return = preg_replace('#http://([^/]+)/restaurants/([^/]+)/([^/]+)/menus/#','http://$1/restaurants/$2/menus/$3/\'',$return);
	}
	return $return;
}
add_filter('post_type_link','tyc_post_type_links',10,3);
function tyc_post_type_links($post_link, $id, $leavename) {
	static $post_links;
	if (!isset($post_links[$post_link])) {
		$parts = explode('/',$post_link);
		if ($parts[4]=='%restaurant-dummy%') {
			if ($parts[5]=='menus') {           // For Post Type: restaurant-menu
				$parts[4] = $parts[6];            // Move restaurant name to where the dummy is
				array_splice($parts,6,1);         // Remove the now second occurance of restaurant name
			} else {                            // For Post Type: restaurant-location
				array_splice($parts,4,1);         // Remove the '%restaurant-dummy%' placeholder
			}
		}
		$post_links[$post_link] = implode('/',$parts);
	}
	return $post_links[$post_link];
}

add_action('init', 'tyc_post_types');
function tyc_post_types() {
	global $wp,$wp_rewrite;
	$wp->add_query_var('restaurant-dummy');
	$wp_rewrite->add_rewrite_tag('%restaurant-dummy%', '([^/]+)','restaurant-dummy=');

	register_post_type('restaurant-menu',
		array(
			'label'           => __('Menus'),
			'singular_label'  => __('Menu'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant-menu',
			'rewrite'         => array('slug' => 'restaurant-menus'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				),
			)
	);

	$wp->add_query_var('restaurant-menu');
	$wp_rewrite->add_rewrite_tag('%restaurant-menu%', '([^/]+)','restaurant-menu=');
	$wp_rewrite->add_permastruct('restaurant-menu', 'restaurants/%restaurant-dummy%/menus/%restaurant-menu%');

	register_post_type('restaurant-location',
		array(
			'label'           => __('Locations'),
			'singular_label'  => __('Location'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant-location',
			'rewrite'         => array('slug' => 'restaurant-locations'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				'editor',
				'excerpt',
				'custom-fields',
				),
			)
	);

	$wp->add_query_var('restaurant-location');
	$wp_rewrite->add_rewrite_tag('%restaurant-location%', '([^/]+)','restaurant-location=');
	$wp_rewrite->add_permastruct('restaurant-location', 'restaurants/%restaurant-dummy%/%restaurant-location%');

	register_post_type('restaurant',
		array(
			'label'           => __('Restaurants'),
			'singular_label'  => __('Restaurant'),
			'public'          => true,
			'show_ui'         => true,
			'query_var'       => 'restaurant',
			'rewrite'         => array('slug' => 'restaurants'),
			'hierarchical'    => true,
			'supports'        => array(
				'title',
				'excerpts',
				'thumbnail',
				'custom-fields',
				'editor',
				),
			)
	);
	global $wp_rewrite;
	$wp_rewrite->flush_rules(false);
}

Maybe I'm tackling it the wrong way, but if not don't you agree that this level of code is extreme for something as simple as URL patterns like this?

http://example.com/restaurants/mcdonalds/              ===> Restaurant
http://example.com/restaurants/mcdonalds/downtown/     ===> Restaurant Location
http://example.com/restaurants/mcdonalds/menus/lunch/  ===> Restaurant Menu

BTW, my solution still does not address the problem of unique slugs required, i.e.:

http://example.com/restaurants/mcdonalds/menus/lunch/
http://example.com/restaurants/burger-king/menus/lunch-2/
http://example.com/restaurants/wendys/menus/lunch-3/

Versus the more desirable (at least for my current two clients assuming my own preference is unimportant):

http://example.com/restaurants/mcdonalds/menus/lunch/
http://example.com/restaurants/burger-king/menus/lunch/
http://example.com/restaurants/wendys/menus/lunch/

comment:6 mikeschinkel4 years ago

Here's a resolution for the duplicate slugs (but I wonder why I need to set this because I defined my custom post types as hierarchical; maybe this is something that needs to be done as the result of setting hierarchical=true when calling register_post_tye(), or maybe not?):

add_filter('hierarchical_post_types','tyc_hierarchical_post_types');
function tyc_hierarchical_post_types($post_types) {
	$post_types = array_merge(array('restaurant-menu','restaurant-location'),$post_types);
	return $post_types;
}

Of course now my restaurant-dummy parameters are showing how they are not working for me because now my location pages are ignoring the restaurant path segment and are loading all locations with the same name regardless of the restaurant. But at least this will be an easy hack to apply.

comment:7 mikeschinkel4 years ago

So, in the interest of bringing it full circle for others wanting to solve this problem (and hopefully for the core team to analyze and address to enable others in the future to deal with this in an easier manner) here is the code to make the menu and location pages only display one record:

add_filter('parse_query','tyc_parse_query');
function tyc_parse_query($query) {
	if (isset($query->query_vars['restaurant-dummy'])) {
		$restaurant = get_page_by_path($query->query_vars['restaurant-dummy'],OBJECT,'restaurant');
		$query->query_vars['post_parent'] = $restaurant->ID;
		$query->query_vars['restaurant'] = $query->query_vars['restaurant-dummy'];
		unset($query->query_vars['restaurant-dummy']);
		
		// Not sure if I really need to update ->query, but why not? Some plugin probably uses it. 
		$query->query['post_parent'] = $restaurant->ID;
		$query->query['restaurant'] = $query->query['restaurant-dummy'];
		unset($query->query['restaurant-dummy']);

	}
	return $query;
}

NOTE: After editing the slug but before pressing publish the "View Location" and "View Menu" buttons don't work for Restaurant Location and Restaurant Menu, respectively. Ideally if this gets addressed in core they would be disabled or their hyperlink modified after clicking "Ok" to save the slug.

comment:8 in reply to: ↑ 2 mikeschinkel4 years ago

Replying to mikeschinkel:

Also, there's another aspect of the ticket I don't think you picked up on and that's the strong interest in having URLs that have the same slug, i.e. "miami" is the location for all three restaurants in this case:

http://example.com/restaurants/johns-seafood/miami/ 
http://example.com/restaurants/marys-ale-house/miami/ 
http://example.com/restaurants/big-jims/miami/ 

Currently WordPress doesn't handle duplicate slugs and wants to make it look like this which is sub-optimal for UX:

http://example.com/restaurants/johns-seafood/miami/ 
http://example.com/restaurants/marys-ale-house/miami-2/ 
http://example.com/restaurants/big-jims/miami-3/ 

Point of note, I was wrong about this. it seems that if you hook 'hierarchical_post_type' and add your custom post types it does the trick.

comment:9 follow-up: dd324 years ago

  • Milestone Unassigned deleted
  • Resolution set to worksforme
  • Status changed from new to closed

The problem with this ticket, Is that you're not separating out the issues you're having.

No. Custom post types default rewrite rules are not supposed to have post_type1/post_type2/post_type3, As you've done however, theres nothing stopping you manually adding a rewrite rule to achiece this.

The reason the tokens are not being replaced in the url, is most likely because only one post is being requested, The post hierarchical code is for a SINGLE post type, as in post1/post2/post3, all of the same post_type, The same as pages in core, You cant mix them.

Finally, Yes, The core rewrite code is limited in the URL's it can create and handle without a plugin/theme extending the rewrite rules, This is expected, it covers 99% of use-cases, and allows for authors to define their own. The system is limited in how it can parse urls, and as you know, theres another ticket for that.

I cant follow the essay of this ticket to determine if there is actually any bugs i've not covered, Trac is for reporting issues, specific issues, You might be better off on wp-hackers for working out how to do some things.

Okay, yes it is possible to come around and add more rewrite rules but it's not possible (AFAICT) to assign a more complex rule using register_post_type()so we end up with vestigial rules in the list, and a long list of rules can be a performance drain for page load so I don't want to have excess rewrite rules that are never used.

Set the rewrite arguement to false to prevent the default rules being created - Reading the register_post_type() function or its documentation will reveal this.

comment:10 in reply to: ↑ 9 mikeschinkel4 years ago

Replying to dd32:

The problem with this ticket, Is that you're not separating out the issues you're having.

Fair enough. I'll break them out as I have time.

No. Custom post types default rewrite rules are not supposed to have post_type1/post_type2/post_type3, As you've done however, theres nothing stopping you manually adding a rewrite rule to achiece this.

Can we avoid talk of "supposed to have" and "not supposed to have?" It just clouds the issue.

There is a use-case and I'm trying to solve it.

The reason the tokens are not being replaced in the url, is most likely because only one post is being requested, The post hierarchical code is for a SINGLE post type, as in post1/post2/post3, all of the same post_type, The same as pages in core, You cant mix them.

There are many valid use-cases that will emerge with custom post types where this will be needed. I have two clients of two who need this, and every personal project I have needs it too. I understand that the code might currently not contemplate such usage but the purpose of my ticket it to present these use-cases as useful requirements.

Finally, Yes, The core rewrite code is limited in the URL's it can create and handle without a plugin/theme extending the rewrite rules, This is expected, it covers 99% of use-cases, and allows for authors to define their own.

My expectation is that 99% of use-cases for blogging are covered but that probably 10% of use-case as a CMS are currently covered. I believe we'll see a lot of demand for this type of URL and post-to-post relationship. But no need to take my word for it, we can just wait to see how many others demand it.

The system is limited in how it can parse urls, and as you know, theres another ticket for that.

Yes I'm all too aware. At least one other ticket for this I submitted. That was a bigger picture ticket; this was a tactical ticket.

I cant follow the essay of this ticket to determine if there is actually any bugs i've not covered, Trac is for reporting issues, specific issues, You might be better off on wp-hackers for working out how to do some things.

I appreciate that this is long so I will break it up as I have time.

As for wp-hackers list, I don't really think that list is productive anymore. Too many competing visions for how it should be used and too many people annoyed by people who don't use it the way they want it used. Better to discuss issues elsewhere I think.

Set the rewrite arguement to false to prevent the default rules being created - Reading the register_post_type() function or its documentation will reveal this.

Your response is a bit dismissive here, don't you think? Consider you are chastising me for not "finding a needle in the haystack." I have read the function, I've traced through it many times with a debugger, and I have read the documentation, many times but I've never recognized that behavior. I guess I'm just asking for a bit more respect on this point.

Note: See TracTickets for help on using tickets.