![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#52422 closed defect (bug) (fixed)
Create a draft with the same slug as an existing post, the existing post will be 404.
| Reported by: | Toro_Unit | Owned by: | audrasjb |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.1 |
| Component: | Posts, Post Types | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests has-test-info early dev-feedback commit |
| Cc: | Focuses: |
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 (45)
#2
@
5 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
@
5 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.
5 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.
5 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
@
5 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
@
5 years ago
- Keywords needs-refresh has-testing-info needs-testing added
- Milestone 5.9 → 6.0
Updating the keywords:
- Marking as
needs-refreshas there are merge conflicts - Marking as
has-testing-infoas there are step-by-step instructions - Marking as
needs-testingas 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.
4 years ago
#11
@
4 years 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
- 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
Viewon "Test post". Notice that the title is "Test post 2". ✅ - Copy the link into a Private Browsing/Incognito window. Notice it 404s. ✅
- Apply 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. ✅
- PR 1333 resolves the issue.
@
4 years ago
Refresh of PR 1333. Tidied up tests, added $message parameters. Tidied up source and improved the explanation.
#12
@
4 years 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.
4 years ago
#14
follow-up:
↓ 16
@
4 years ago
- Keywords early added
- Milestone 6.0 → 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
@
4 years 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
@
4 years 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
@
4 years 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-postby 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
@
4 years 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
@
4 years 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.
4 years ago
#21
@
4 years 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.
4 years ago
#22
Trac ticket: https://core.trac.wordpress.org/ticket/52422
#23
follow-up:
↓ 24
@
4 years 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
@
4 years ago
Replying to costdev:
Whether this line is redundant is a matter for another ticket...
Right, exactly. The last patch LGTM.
#26
@
4 years 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.
4 years ago
#28
@
4 years 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.
4 years ago
#30
@
4 years ago
- Owner set to
- Status new → accepted
As per today's bug scrub, self assigning for commit.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
4 years ago
#34
PHPUnit tests were failing after the last commit, so I restarted them to make sure this PR passes the tests before commit.
4 years ago
#35
@audrasjb Seems the tests failed again. I rebased on trunk and pushed. Tests passed! 🎉
#36
@
4 years 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
@
4 years 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%.