WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 3 months 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)

49871.diff (678 bytes) - added by Jules Colle 5 months ago.
show permalink instead of guid for scheduled posts
49871-2.diff (1.3 KB) - added by Jules Colle 5 months 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.

Download all attachments as: .zip

Change History (15)

#1 @TimothyBlynJacobs
6 months ago

  • Component changed from REST API to Permalinks
  • Version changed from trunk to 4.7

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 the future post status.

So I'd like to hear from the Permalink maintainers before we make any change in the REST API. cc @SergeyBiryukov, @asif2bd.

This ticket was mentioned in Slack in #core by jules-colle. View the logs.


6 months ago

#3 @Asif2BD
6 months 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 @Jules Colle
6 months 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 months ago

@Jules Colle
5 months ago

show permalink instead of guid for scheduled posts

#6 @Jules Colle
5 months 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 @dlh
5 months 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 @Jules Colle
5 months ago

Good point. I wasn't aware of that. It looks like applying the patch will cause scheduled posts to automatically redirect the guid-link 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.

Last edited 5 months ago by Jules Colle (previous) (diff)

@Jules Colle
5 months 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 @Jules Colle
5 months 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/

#10 @Asif2BD
5 months ago

  • Keywords dev-feedback needs-testing added

#11 @Jules Colle
5 months 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.

Last edited 5 months ago by Jules Colle (previous) (diff)

This ticket was mentioned in Slack in #core by jules-colle. View the logs.


5 months ago

#13 @earnjam
3 months 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.

Note: See TracTickets for help on using tickets.