Opened 5 years ago
Last modified 4 years ago
#49871 new enhancement
REST API post link should be permalink for scheduled posts
Reported by: | Jules Colle | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Permalinks | Keywords: | has-patch dev-feedback needs-testing |
Focuses: | rest-api | Cc: |
Description
Scheduled posts now return something like this:
{ "id": 34, "date": "2039-12-09T20:19:00", "slug": "message-from-the-past", "status": "future", "link": "https://example.com/?p=34", }
But I believe it makes more sense to actually return the permalink as link:
{ "id": 34, "date": "2039-12-09T20:19:00", "slug": "message-from-the-past", "status": "future", "link": "https://example.com/2039/12/09/message-from-the-past", }
The only caveat is that we will need to create 301 redirects in case the slug changes (which is the case for published posts already).
For example. if the slug becomes old-message
the new link would be https://example.com/2039/12/09/old-message
, but visiting https://example.com/2039/12/09/message-from-the-past
should redirect to the new URL.
There has been some discussion about this over here: https://github.com/WordPress/gutenberg/pull/21410#issuecomment-612132635
Attachments (2)
Change History (15)
This ticket was mentioned in Slack in #core by jules-colle. View the logs.
5 years ago
#3
@
5 years ago
- Keywords needs-patch added
Thanks @TimothyBlynJacobs, I will take it from here. @jules-colle this sounds very logical to me. Do you want to submit a patch?
#4
@
5 years ago
@Asif2BD I'll gladly take a shot at creating a patch. I'll try to submit one over the weekend.
This ticket was mentioned in Slack in #meta by ocean90. View the logs.
5 years ago
#6
@
5 years ago
- Keywords has-patch added; 2nd-opinion needs-patch removed
With patch 49871.diff applied, all links to scheduled posts are displayed as permalinks instead of guids. Tested in the WP dashboard, and also in the REST API. Works as expected.
Important: After this gets added to core, we need to refactor the changes made in this PR: https://github.com/WordPress/gutenberg/pull/21410
#7
@
5 years ago
The check for future
was added deliberately in #30910 to prevent disclosure of unpublished information about the post. Is there reason to think that those concerns no longer hold true?
#8
@
5 years ago
Good point. I wasn't aware of that. It looks like applying the patch will cause scheduled posts to automatically redirected to the permalink, as you can see here: https://bdwm.be/?p=1654 (Note: this website has the patch already applied.)
Applying the patch will undo #30910. I'll see if I can create a new patch that will prevent automatic redirection for scheduled posts.
@
5 years ago
show permalink instead of guid for scheduled posts, meanwhile ensuring that guids will not automatically redirect to the permalink version as long as the post is scheduled.
#9
@
5 years ago
Patch 49871-2.diff makes sure the effects of #30910 do no go lost.
Scheduled posts will still show the permalink instead of the guid (49871.diff), but requesting a scheduled posts with the guid (i.e. https://bdwm.be/?p=1654) will no longer auto-redirect to the permalink.
I tested this on my local dev server, and also on a live website:
https://bdwm.be/?p=1654 now no longer automatically redirects to https://bdwm.be/this-is-the-far-away-future/
#11
@
5 years ago
If someone would like to test the patch, please verify the following:
- With permalinks enabled
- a scheduled post should always be presented to the admin with a permalink. So clicking "View post" or the link presented under "What's next" (or any other place in the admin area) should show and take you to the permalink version of the post.
- Visiting a scheduled post with the guid (
example.com/?p=123
), should NOT redirect the user to the permalink version. (This is to prevent brute force guessing (#30910)) - Once the scheduled post is published, visiting the guid, should automatically redirect the user to the permalink version of the post.
- Getting future posts from the REST API should display the
post.link
as a permalink.
- Without permalinks enabled, everything should keep working as it was prior to this patch.
I can confirm all of this works as expected from my own tests.
This ticket was mentioned in Slack in #core by jules-colle. View the logs.
5 years ago
#13
@
4 years ago
- Milestone changed from Awaiting Review to Future Release
Given that we're now showing the future permalink in the editor I think it makes sense to return it from get_permalink()
. That would also reveal it in the REST API and will help simplify that editor code.
Regarding the patch, I think it works, but the loose comparison in 'future' == get_post_status( get_query_var( 'p' ) )
feels risky considering the moving parts. I'd feel better with a strict comparison.
Also some unit tests would help a lot to ensure we don't encounter regressions like #30910.
Thanks for the ticket @jules-colle and welcome to trac!
I'm moving this to the Permalinks component and keeping the REST API focus since this looks like it'd require changes to
get_post_permalink()
which checks for thefuture
post status.So I'd like to hear from the Permalink maintainers before we make any change in the REST API. cc @SergeyBiryukov, @asif2bd.