#21112 closed enhancement (fixed)
Add pre_wp_unique_post_slug to override post slug handling
Reported by: | coffee2code | Owned by: | pento |
---|---|---|---|
Milestone: | 5.1 | Priority: | normal |
Severity: | normal | Version: | 3.4 |
Component: | Permalinks | Keywords: | has-patch has-dev-note |
Focuses: | Cc: |
Description
As originally proposed in the comments of #20480 (and in fact further back in a comment for #14111), a pre_wp_unique_post_slug
filter early in wp_unique_post_slug()
could be useful and justified for some use cases rather than relying on the existing and late-firing wp_unique_post_slug
filter for a couple reasons:
wp_unique_post_slug()
(and thus this new filter) is not high use, so won't impact performancewp_unique_post_slug()
performs database queries (perhaps repeatedly until a unique slug is found), fires other filters, etc, all of which may be rendered moot if a hooking function's intent is to fully implement its own slug generation/handling logic. The pre slug could short-circuit that unnecessary effort.
The attached patch introduces the new filter in addition to adding related phpDocs.
Attachments (5)
Change History (36)
#3
@
10 years ago
Added 21112.2.diff as a refresh of 21112.diff in order to:
- Add inline docs for filter
- Add unit test
- Add braces
- Remove unnecessary additions to function's docblock
#4
@
10 years ago
- Keywords reporter-feedback added
Out of interest, can you give a use case for this filter?
#5
@
10 years ago
- Keywords reporter-feedback removed
Well, likely most uses for the existing wp_unique_post_slug
filter would be better served using the proposed pre_wp_unique_post_slug
filter for the simple reason that if you are mucking around with unique slug detection (and regeneration due to a conflict), you're abandoning the built-in work being performed by wp_unique_post_slug()
.
Being early-firing (pre_wp_unique_post_slug
) versus late-firing (wp_unique_post_slug
) means you can skip the queries, filters firing, and code execution that you know will always be disregarded.
Some examples (geared towards custom-crafting the slugs the site uses) include:
- changing of certain terms when appearing in slugs:
e.g. "wp-is-fun" => "wordpress-is-fun", "wp-cant-be-shuttered" => "wp-cannot-be-shuttered" - removal of certain terms/phrases from appearing in slugs:
e.g. "the-one-where-i-go-camping" => "where-i-go-camping" - force prepending of word(s) to all, or contextually to some, slugs:
e.g. "wp-is-fun" => "mysite-wp-is-fun" - custom slug truncation length
(Core has a 200 char limit, and while the internally used_truncate_post_slug()
supports customizing that length via an argument, it is not otherwise filterable or customizable) - custom duplicate slug suffix
(core attempts "-N" where N is a number tried until a unique slug is found)
e.g. "the-weekly-report-15" => "the-weekly-report-fifteen" - custom slug rules per post type
In the end, yes, all this can be achieved with the existing wp_unique_post_slug
filter, but it's a waste to run the 1(+n) queries and code execution in situations where custom handling will negate it all.
#7
@
9 years ago
Just to add a bit more emphasis on this: a library we're using creates posts to keep track of scheduled events (wp's cron is not good enough). By default the post_name is not populated, but even when it is, because WordPress would do it on ajax, there's no authenticated user, and the post is pending. wp_insert_post
strips post_name
even if one was passed through [here](https://core.trac.wordpress.org/browser/trunk/src/wp-includes/post.php#L3014).
Because there's no way to bypass this, on a large site when there are 60,000+ entries with the same empty post_name, the server just comes to a grinding halt. It's also not great with hosts like WPEngine, that have a hard limit for script execution. It's 60 seconds in WPEngine's case.
Currently the only way for us to do this is to essentially copy the entire wp_insert_post
function, and remove all the things that would hit wp_unique_post_slug
and call our version of it from within the library. It's not very WordPress way to be honest.
Being able to short circuit the function would be AMAZING.
This ticket was mentioned in Slack in #docs by morganestes. View the logs.
8 years ago
#9
@
8 years ago
- Milestone changed from Awaiting Review to Future Release
- Owner set to johnbillion
- Status changed from new to reviewing
#10
@
8 years ago
Chiming in here, to basically echo what @javorszky mentioned above, as this:
[…] on a large site when there are 60,000+ entries with the same empty post_name, the server just comes to a grinding halt. It's also not great with hosts like WPEngine, that have a hard limit for script execution. It's 60 seconds in WPEngine's case.
is exactly what we're experiencing at the moment :[
#11
@
8 years ago
My thoughts:
- Needs seems genuine
- Patch seems reasonable
- Approach is similar to others we use in core
- This could go in as-is, or the patch could be refreshed to include phpdoc in the tests like @chriscct7 mentioned
Paging @johnbillion, who marked himself as owner. Able to see this through?
#13
@
7 years ago
I hotfixed this on a huge site with Subscriptions on. The result was that renewals went from around ~200 / hr to ~980 / hr.
Can we please get this into WP Core officially?
<puppy eyes>
This ticket was mentioned in Slack in #core by javorszky. View the logs.
7 years ago
#15
@
7 years ago
Recently bumped into this as well.
@johnbillion, can you please consider to move this ticket forward?
This ticket was mentioned in Slack in #core by javorszky. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by javorszky. View the logs.
7 years ago
#19
@
7 years ago
- Keywords needs-docs removed
Removing needs-docs
, as test in 21112.2.diff
has inline documentation.
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
6 years ago
#22
@
6 years ago
Some notes on the patch and inline docs:
- Third-person singular verbs should be used for the summary, describing what is being filtered. This should be more like "Filters the unique post slug..." than "Filter to allow setting the unique post slug..." See the example at https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#language.
- Version should be using three digits, see https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-functions-class-methods. No need to change it right now because it can be adjusted before committing.
- Missing period after
Post parent ID
- We usually use
null
forpre_
filters like in this case and then explicitly check whether the variable isnull
or something else.
#23
@
6 years ago
- Keywords needs-refresh added
- Milestone changed from 5.0 to 5.1
Per https://make.wordpress.org/core/2018/10/08/wordpress-5-0-for-contributors-and-committers/
See also Slack discussion here https://wordpress.slack.com/archives/C02RQBWTW/p1538986854000100
The 21112.4.diff will also need refreshing to updatethe PHPDoc @since
block to 5.1
and updating per @swissspidy's comment above
@
6 years ago
Refreshed the patch per the above comments, along with some additional documentation tweaks.
#24
@
6 years ago
- Keywords needs-refresh removed
Looks good to me!
I don't see anything that would hold this back.
#25
@
6 years ago
- Owner changed from johnbillion to swissspidy
Reassigning to @swissspidy to move this along for 5.1
#30
@
6 years ago
No, only security fixes are backported to branches older than the current major version.
#31
@
6 years ago
- Keywords has-dev-note added
This was mentioned in a 5.1 dev note: https://make.wordpress.org/core/2019/01/23/miscellaneous-developer-focused-changes-in-5-1/
Refreshed the 21112.diff patch. See associated comment for details.