#52422 closed defect (bug) (fixed)
Create a draft with the same slug as an existing post, the existing post will be 404.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Posts, Post Types | Keywords: | has-patch has-unit-tests has-testing-info early dev-feedback commit |
Focuses: | Cc: |
Description
Step-by-step reproduction instructions
- create custom post type. and set
'show_in_rest' => true
<?php function custom_init() { $args = array( 'public' => true, 'label' => 'Books', 'show_in_rest' => true, ); register_post_type( 'book', $args ); } add_action( 'init', 'custom_init' );
- Publish new post of the post type.
- Create draft post of the post type with same slug.
- Go to the published post. but show draft post. if logged out, 404.
Using the block editor, a page can also create drafts with the same slug, which causes the same problem.
Attachments (3)
Change History (44)
#2
@
2 years ago
I've attempted to reproduce this on both 5.6 and trunk without success. I've tried with both a CPT per the original post and standard posts per the follow up comment.
Are either of you able to reproduce this with all plugins disabled and using a default theme such as Twenty Twenty-One?
#3
@
2 years ago
@peterwilsoncc Thanks.
No plugins are used, and the theme is Twenty Twenty One. reproduced on WordPress 5.5 and 5.6.
When creating a post, please use the block editor.
Quick Edit or Classic Editor don't allow us to create a post with the same slug as an existing post.
This ticket was mentioned in PR #1333 on WordPress/wordpress-develop by torounit.
2 years ago
#4
- Keywords has-patch has-unit-tests added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/52422.
Using the block editor, a draft can have the same slug as a post that already exists.
There is a part in WP_Query that uses get_page_by_path, so it may load the wrong post.
A hack similar to wp_ajax_inline_save, fixed so that the slug is different.
2 years ago
#5
@torounit
I think the problem with this bug is not that it can be saved with the same slug, but that articles that are not published are called.
#6
@
2 years ago
get_page_by_path is used in WP_Query.
- Draft page ( id: 1, slug: foo, status: draft )
- Published page ( id: 2, slug: foo, status: publish )
If the above two posts exist, get_page_by_path will get the draft page. That post will be set to queried_object.
https://github.com/WordPress/wordpress-develop/blob/5.7.2/src/wp-includes/class-wp-query.php#L1001
#9
@
19 months ago
- Keywords needs-refresh has-testing-info needs-testing added
- Milestone changed from 5.9 to 6.0
Updating the keywords:
- Marking as
needs-refresh
as there are merge conflicts - Marking as
has-testing-info
as there are step-by-step instructions - Marking as
needs-testing
as I too am unable to reproduce the reported problem
I'm not able to reproduce this problem either. It needs more investigation. As 5.9 Beta 1 is in 2 days, moving it to 6.0.
This ticket was mentioned in Slack in #core by costdev. View the logs.
14 months ago
#11
@
14 months ago
- Keywords needs-testing removed
Test Report
Environment
- Server: Apache (Linux)
- WordPress: 6.0-alpha-52448-src
- Browser: Chrome 99.0.4844.84
- OS: Windows 10
- Theme: Twenty Seventeen
- Plugins: None activated
Steps to Reproduce
- Navigate to
Settings > Permalinks
. - Ensure the permalink structure is set to
Post name
. ClickSave Changes
. - Navigate to
Posts > Add New
. - Name the post "Test post". Click
Publish
. - Navigate to
Posts > Add New
. - Name the post "Test post 2". Click
Save draft
. - On the right hand side, scroll down and edit the slug from "test-post-2" to "test-post". Click
Save draft
. - Navigate to
Posts
. - Click
View
on "Test post". Notice that the title is "Test post 2". ✅ - Copy the link into a Private Browsing/Incognito window. Notice it 404s. ✅
- Apply 52422.refresh-of-PR-1333.diff (a refresh of PR 1333).
- Run
npm run build:dev
. - Trash and permanently remove "Test post" and "Test post 2".
- Repeat steps 1-7. Notice that "test-post" changes to "test-post-2", preventing the use of an existing slug. ✅
- Continue to step 8 through 10. Notice that both issues in 9 and 10 no longer occur. ✅
Results
- Issue reproduced. ✅
- 52422.refresh-of-PR-1333.diff (a refresh of PR 1333) resolves the issue.
@
14 months ago
Refresh of PR 1333. Tidied up tests, added $message parameters. Tidied up source and improved the explanation.
#12
@
13 months ago
- Keywords commit added; needs-refresh removed
Patch still applies cleanly against trunk. Adding commit
.
This ticket was mentioned in Slack in #core by costdev. View the logs.
13 months ago
#14
follow-up:
↓ 16
@
13 months ago
- Keywords early added
- Milestone changed from 6.0 to 6.1
As 6.0 RC1 is starting in less than an hour, I'm moving this to the 6.1 milestone and adding the early
keyword so that it has plenty of testing/soak time.
Committers: This can be committed to trunk
after we branch 6.0.
To clarify: 52422.refresh-of-PR-1333.diff is the commit candidate.
#15
@
13 months ago
Test Report
Server: macOs 12.2
WordPress: 6.1-alpha-53344-src
Browser: Safari 15.4
OS: macOs 12.2
Theme: Twenty Twenty-One / Twenty Twenty-Two
Plugins: None activated
I'm able to reproduce the issue and the PR above fixed it in my tests.
Before the fix: watch the screen recodring
After the fix: watch the screen recording
#16
in reply to:
↑ 14
@
13 months ago
Replying to costdev:
To clarify: 52422.refresh-of-PR-1333.diff is the commit candidate.
Probably missing something but why the $prepared_post->post_type = $this->post_type;
is removed in the patches?
#17
@
13 months ago
Thanks, @costdev!
Test Report
Patch 52422 (PR 1333 refresh via @costdev) 👍🏻
Env
- WordPress 6.1-alpha-53344-src
- Safari 15.4
- Chrome 101.0.4951.54
- macOS 12.3.1 (Monterey)
- Theme: Twenty Twenty-Two
- Gutenberg DISABLED 🔴
Steps to Test
- Ensure permalink structure is set to "Post name" in Settings > Permalinks.
- Under Posts > Add New create a new post with the title "Test Post" and Publish.
- In another tab, verify that the post is published at
<your site url>/test-post/
. - Navigate to Posts > Add New and create another new post with the title "Test Post 2", and click Save draft (DO NOT click Publish).
- Find the "Permalink" metabox on the right-hand side of the post editor and update the "URL Slug" to
test-post
by removing the-2
-- this will match the slug of the first new post. Click Save draft again (DO NOT click Publish). - In the second tab, refresh the page at
<your site url>/test-post/
. - Observe that the post title displayed is now "Test Post 2", indicating that the draft post is overriding the original post's slug. 🐞
- Open the page in Private Browsing/Incognito mode and observe that a 404 error (not found) is returned, as the draft post is not published. 🐞
- Apply patch. 🛠
- From the main Posts page, "Move to Trash" and "Delete Permanently" both "Test Post" and "Test Post 2".
- Repeat Steps 2 through 5.
- Observe that the "Permalink" metabox in the post editor refreshes the "VIEW POST" link to
<your site url>/test-post-2/
. ✅ - In the second tab, refresh the page at
<your site url>/test-post/
. - Observe that the post title remains "Test Post", indicating the draft has not overridden the published post's slug. ✅
- In Private Browsing/Incognito, observe that the originally published post is displayed (404 error has been mitigated). ✅
Expected Results
- I was able to reproduce the reported issue (see Steps 7-8). ✅
- The patch corrects the identified issue (see Steps 12-15). ✅
Additional Notes
- The above reproduction steps do not apply to Pages, but only to Posts. Pages seem unaffected by this issue (i.e. a draft page does not override an existing published page slug).
#18
@
13 months ago
- Keywords commit removed
As there is a valid question about why $prepared_post->post_type = $this->post_type;
is removed in the patch, I'm removing the commit
keyword until code review is finished. once finished, then commit
can be applied to alert it's ready to be committed.
#19
@
13 months ago
- Keywords dev-feedback added
I'm not sure why $prepared_post->post_type = $this->post_type;
was removed. There's no context in the original PR and 52422.refresh-of-PR-1333.diff is just a refresh with some improvements, source formatting and explanations.
Whether or not this line is redundant, the change doesn't appear to belong within this ticket's scope.
@Toro_Unit Can you provide any information about why this line was removed?
This ticket was mentioned in Slack in #core by costdev. View the logs.
12 months ago
#21
@
12 months ago
Following up on comment 16, $prepared_post->post_type = $this->post_type;
doesn't appear to have any effect.
I added the line back in along with some logging before and after it. Results below.
Before: wp_block | After: wp_block Before: wp_block | After: wp_block Before: page | After: page Before: page | After: page Before: post | After: post Before: post | After: post Before: post | After: post // ...continues with `post | post` for a while.
I believe this is because WP_REST_Posts_Controller::prepare_item_for_database()
sets the post type at this location.
Whether this line is redundant is a matter for another ticket, and @rachelbaker might be able to give some insight on this.
Until then, I'll add the line back in and submit a PR to show CI passing.
This ticket was mentioned in PR #2752 on WordPress/wordpress-develop by costdev.
12 months ago
#22
Trac ticket: https://core.trac.wordpress.org/ticket/52422
#23
follow-up:
↓ 24
@
12 months ago
CI is passing on the PR. @hellofromTonya, @azaozz, is there anything else you think this needs before we can re-add commit
?
#24
in reply to:
↑ 23
@
12 months ago
Replying to costdev:
Whether this line is redundant is a matter for another ticket...
Right, exactly. The last patch LGTM.
#26
@
12 months ago
@costdev Thank you update patch !!!
I can't recall why $prepared_post->post_type = $this->post_type; was removed.
I think it is a mistake. ( Or it could have been removed because it would not affect anything. )
I rolledback tha line and tested it and confirmed that the problem is fixed for both page and custom post type.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
12 months ago
#28
@
12 months ago
Thanks @Toro_Unit for reporting this. We reviewed this ticket during a recent bug-scrub session. Based on the feedback received from the team we are confident that Colin's patch can potentially solve the problem.
Props to @Toro_Unit
Cheers!
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
12 months ago
#30
@
12 months ago
- Owner set to audrasjb
- Status changed from new to accepted
As per today's bug scrub, self assigning for commit
.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
12 months ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
12 months ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
11 months ago
11 months ago
#34
PHPUnit tests were failing after the last commit, so I restarted them to make sure this PR passes the tests before commit.
11 months ago
#35
@audrasjb Seems the tests failed again. I rebased on trunk
and pushed. Tests passed! 🎉
#36
@
11 months ago
- Keywords commit removed
I added a note to the PR but it didn't come through here.
I think the PR should focus on the logic around when the REST API uses get_sample_permalink()
rather than change the logic using wp_unique_post_slug()
.
This will ensure all the appropriate filters fire too.
#37
@
11 months ago
@peterwilsoncc
This hack is also used for quick editing.
I think the best way to do this is to modify wp_insert_post and remove this hack.
It also happened in default posts.
%postname%
.