#47911 closed enhancement (fixed)
redirect_guess_404_permalink does not consider public custom posts status
| Reported by: | goaroundagain | Owned by: | peterwilsoncc |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.0 |
| Component: | Canonical | Version: | 5.2.2 |
| Severity: | normal | Keywords: | has-patch has-unit-tests early add-to-field-guide |
| Cc: | Focuses: |
Description
At the moment the redirect_guess_404_permalink function does only consider the default "publish" post status and there is no way around because "publish" ist hard coded:
$post_id = $wpdb->get_var( "SELECT ID FROM $wpdb->posts WHERE $where AND post_status = 'publish'" );
From register_post_status: https://codex.wordpress.org/Function_Reference/register_post_status
public: (bool) (optional) Whether posts of this status should be shown in the front end of the site.
For this reason, all post with a custom post status set to public will not be found and return an 404.
I think custom post status with public set to true should also be considered.
Attachments (4)
Change History (29)
#3
@
7 years ago
- Keywords close removed
Sorry for the confusion, its not fixed. Correct link to the linke in 5.2.2: https://core.trac.wordpress.org/browser/tags/5.2.2/src/wp-includes/canonical.php#L686
@
7 years ago
Easy fix, just query all public post stati and change the query to IN linke already done with post types in https://core.trac.wordpress.org/browser/tags/5.2.2/src/wp-includes/canonical.php#L673
#5
@
7 years ago
- Milestone Awaiting Review → 5.3
- Owner set to
- Status new → reviewing
Related: #47574
#6
@
7 years ago
@SergeyBiryukov Is this one still on your radar for 5.3? If not, can me move it to a future milestone?
This ticket was mentioned in Slack in #core by marybaum. View the logs.
7 years ago
#8
@
7 years ago
- Keywords needs-testing needs-unit-tests added
- Milestone 5.3 → Future Release
Moving this to a Future Release as we're now in beta3 for 5.3 and this change will need testing and possible unit tests.
@
5 years ago
The previous diff accidentally removed unnecessary blank link so I updated it again. Basically, this diff is still to update the code provided by @goaroundagain and add a unit test for this ticket
#10
@
5 years ago
- Keywords needs-testing removed
I tested the patch and it still applies cleanly against trunk. It looks good on my side: custom post statuses are took into account.
#11
@
5 years ago
- Keywords needs-unit-tests removed
Some unit tests were proposed, so let's remove this keyword.
#12
@
5 years ago
- Milestone 5.8 → 5.9
Today is 5.8 Beta 1. As work continues on this ticket, punting to 5.9.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
5 years ago
#16
@
5 years ago
- Keywords early added
- Milestone 5.9 → 6.0
- Type defect (bug) → enhancement
As per today's bug scrub, this enhancement should be considered earlier in the release cycle. Moving to milestone 6.0 with the early workflow keyword.
#17
@
4 years ago
- Keywords needs-refresh added
The public attribute when registering post statuses is slightly odd. A post status can be registered as public while not actually been publicly queryable.
<?php register_post_status( 'guess404', [ 'public' => true, 'publicly_queryable' => false, 'internal' => true, 'protected' => true, ] );
I suggest the following:
<?php $public_statuses = array_filter( get_post_stati(), 'is_post_status_viewable' );
The IN value will need to run through esc_sql() too. It's mostly a safe bet that the data will be in a form that doesn't need escaping, but for a single function call it's possible to be 100% sure, so let's do that.
This may need a dev note too.
This ticket was mentioned in Slack in #core by costdev. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
#20
@
4 years ago
- Keywords needs-dev-note added
- Owner changed from to
Thanks @goaroundagain for reporting this. We discussed this ticket during a recent bug-scrub session. Based on the feedback received from the team we're updating the ticket with the changes:
- Changing owner from @SergeyBiryukov to @peterwilsoncc since Peter volunteered to push up to a PR. He also wrote the code.
- Adding keyword needs-dev-note
Props to @peterwilsoncc
Cheers!
This ticket was mentioned in PR #2470 on WordPress/wordpress-develop by peterwilsoncc.
4 years ago
#21
- Keywords needs-refresh removed
#22
@
4 years ago
In the linked pull request:
- built upon the previous work in 47911_htdat_3.diff
- now uses
is_post_status_viewable()to determine public post statuses - added
esc_sqlto the SQLIN() - extended tests to include custom public, private, internal and protected statuses
Is pre_redirect_guess_404_permalink adequate in terms of filters or should a filter for the post statuses to include be added?
Naming suggestions are welcome if you think the filter is needed.
peterwilsoncc commented on PR #2470:
4 years ago
#24
Commited in https://core.trac.wordpress.org/changeset/53043
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Link to the line in canonical.php: https://core.trac.wordpress.org/browser/tags/5.2.1/src/wp-includes/canonical.php#L686