fix: [#20954] Scheduled posts display the correct url

[kirilzh]

Description

Fix #20954
Currently links are set from link key from

api: /wp/v2/posts/:id 

{
   "id":79,
   "guid":{
      "rendered":"http:\/\/localhost:8888\/?p=79",
      "raw":"http:\/\/localhost:8888\/?p=79"
   },
   "slug":"potato-salad",
   "status":"future",
   "link":"http:\/\/localhost:8888\/?p=79",
   "title":{
      "raw":"potato salad",
      "rendered":"potato salad"
   },
   "permalink_template":"http:\/\/localhost:8888\/2020\/04\/17\/%postname%\/",
   "generated_slug":"potato-salad"
}

For scheduled posts this does not represent their respective live URLs. Hence, I changed the source post link. For scheduled posts that have postname in the URL the slug is injected. I chose to use permalink_template as it corresponds to the setting chosen by the user.

How has this been tested?

From Settings -> Permalinks -> checked all options under custom structure. When scheduling a post the permalink_template creates the real url and with %postname% as a template.

Screenshots

Types of changes

Non-breaking change. Bug fix.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[kirilzh]

Ready to be reviewed

[paaljoachim]

It would be great to get this added to 5.4.1.
As it fixes scheduled posts so they show the correct URL depending on which permalink setting is used.

@youknowriad @jorgefilipecosta @aduth

[pwkip]

I tested this with normal posts and @kirilzh's fix seems to be working fine.

I was wondering if the "View post" links should also be updated accordingly..

These links are still pointing to /?p=[post-id]. But with the classic editor plugin enabled, this is also the case. So I guess there's no need to update these.

On the other hand... I think it's a valid idea to actually generate the final URL for scheduled posts as the link attribute in the REST API. So for example: instead of

api: /wp/v2/posts/:id 
{
   "id":79,
   "status":"future",
   "link":"http:\/\/localhost:8888\/?p=79",
   "generated_slug":"potato-salad"
}

Actually have this return

api: /wp/v2/posts/:id 
{
   "id":79,
   "status":"future",
   "link":"http:\/\/localhost:8888\/2020\/04\/17\/potato-salad\/",
   "generated_slug":"potato-salad"
}

[earnjam]

We can't update the REST API link attribute, or change the href because the future URL isn't live/routable yet.

[pwkip]

@earnjam I'm not 100% sure what you mean by "the URL isn't live/routable". I think the same is true for the temporary link, no?

After creating a scheduled post (with the classic editor), I can confirm the following:

• there are 2 URLs you can access
  • http://example.com/?p=35 (temporary link)
  • http://example.com/2020/05/my-new-post (permalink)
• both URLs will return the exact same page. (see screenshots below)

I'd like to learn why, if these URLs both work in the same way and point towards the same (in some cases non-existent) resource, why we can't have the permalink set as link from the start? WordPress is even presenting the permalink to the user after creating a scheduled post.

Admin accesses the scheduled post via permalink:

Admin accesses the scheduled post via temporary link:

Visitor tries to access the scheduled post via permalink:

Visitor tries to access the scheduled post via temporary link:

So what am I missing?

[mcsf]

Thanks for this fix. It looks good, though I've requested a couple of changes.

Edit: I see now that a previous review points out the same thing.
One thing to note is that this won't fix the URL shown in the snackbar upon saving a scheduled post. It doesn't necessarily have to, but I think that merits discussion:

[mcsf]

Though a bit long, something like getFuturePostUrl might be clearer.

[kirilzh]

fixed

[mcsf]

I would update the first sentence based on my suggestion for getFuturePostUrl. The second sentence is IMO unnecessary.

[kirilzh]

changed description to

* Returns URL for a future post.

[mcsf]

@earnjam I'm not 100% sure what you mean by "the URL isn't live/routable". I think the same is true for the temporary link, no?

I think the distinction in question (correct me if I'm wrong) was about how, once a post is published, WordPress guarantees that a permalink will keep pointing to the post even if the permalink is changed. That is:

1. Publish a post as /2020/04/09/abc
2. Change the slug to def
3. Both /2020/04/09/abc and /2020/04/09/def will point to the post

In contrast, when dealing with scheduled posts I am not sure what kind of guarantees, if any, the system can make about ensuring the longevity of these generated URLs.

[pwkip]

I think the question that needs to be answered first is this:
Should WP ever generate a permalink to a post that is not published yet?

• If yes: why not make this the default behaviour, and put the permalink at the source (i.e. the post.link in the REST API)
• If not: is this PR a good idea then? Since it would encourage users to use a link that is not guaranteed to work after the post is published.

EDIT: I just think it's confusing to present the user with 2 URLs

1. http://example.com/?p=35 (temporary link)
2. http://example.com/2020/05/my-new-post (permalink)

I think we need to decide on which format is the recommended one for scheduled posts.

I personally like the permalink best, but I agree with @mcsf that the original link should keep working if the slug changes. I just checked this, and that doesn't seem to be the case now. So we might need to change that too if the permalink should become the standard link.

[earnjam]

@earnjam I'm not 100% sure what you mean by "the URL isn't live/routable". I think the same is true for the temporary link, no?

Sorry, I was thinking along the lines of for non-authenticated users, but it doesn't matter for them, as they'll get a 401 when trying to make a request for a scheduled post from the REST API. Neither style link for scheduled posts work for non-authenticated users, and since they couldn't access the post on the endpoint anyway, they don't need to be considered as to which to return/show.

Scheduled are very similar to drafts in their visibility. The primary difference between the two is the presence of a future publish date, and that the slug for a scheduled post has already been passed through wp_unique_post_slug() to prevent conflicts. Drafts don't have their slugs firmed yet, so they aren't guaranteed to be unique. This means we can't make the href for drafts be the estimated pretty permalink because it might point to other content, or return 404 when the slug isn't actually saved yet and is just being estimated in the editor from the post title. That's something that I think warrants further conversation in another issue/PR, as it would be helpful to return unique slugs for drafts to the editor so users know whether they will encounter a slug conflict before they publish. But it could be confusing when it's not a functional URL. Either way it doesn't matter with regards to this PR.

As far as for the link attribute on the REST API for scheduled posts, I do think it is more representative of the shape of the post to show the pretty permalink than the plain permalink. Doing that would also make the editor code simpler here. However, as @mcsf noted, not creating a 301 redirect when the slug changes prior to publishing is something that should be considered if we're going to begin returning it while scheduled.

[pwkip]

@mcsf , @earnjam, @kirilzh do you all agree that the pretty permalink should be set as the post.link in the REST API (including 301 redirects for changed slugs)?

If so, I can open a trac ticket for this.

[earnjam]

I think open the Trac ticket for sure. That way we can center that discussion around the REST API change.

For the purposes of this PR, I think it's fine to display the pretty permalink for scheduled posts in the editor using the parts supplied in the response (permalink structure + post name). If the REST API is changed, then we can refactor to just use that value directly.

[pwkip]

Trac ticket opened: https://core.trac.wordpress.org/ticket/49871

[mcsf]

@kirilzh: any changes you'd like to make, or is this ready to merge?

[kirilzh]

it's ready to be merged

[paaljoachim]

A last test using gutenberg.run.

Initial test.
New post.
Add a title.
Some text in a paragraph block.
Set one week ahead to activate Schedule... button.

Post address:
http://hkd7wa1i.gutenberg.run/2020/04/20/a-test/

Going back to adjust the post.
Resaving. Then viewing post. This is the URL I see:
http://hkd7wa1i.gutenberg.run/?p=11

I was expecting to see the correct permalink structure also on adjusting and updating the scheduled post.

[mcsf]

Going back to adjust the post.
Resaving. Then viewing post. This is the URL I see:
http://hkd7wa1i.gutenberg.run/?p=11

I was expecting to see the correct permalink structure also on adjusting and updating the scheduled post.

There's a lot of room for improvement in the overall experience, as also noted in #21410 (comment). The question is whether this small step of showing the future URL in the post-schedule view is worth it on its own. I lean towards "yes".

[paaljoachim]

Having the initial scheduled post show a pretty permalink and have all following edited version of the same post show a plain permalink is not a great experience (and makes it feel like a bug).
I am hoping that we could get this issue fully solved, so we do not need to go back to it again.

Schedule a post forward in time and view it on the frontend and see the URL reflect the permalink option one has selected under the permalink settings. On first viewing of the post and on editing the post in other viewings after it.

Hopefully we can get some additional voices in here that have some knowledge to share.
Thanks.

@earnjam

[pwkip]

I also hope we can move the trac ticket forward and change the scheduled post link to the permalink at get_post_permalink(). I'll do my best to follow up on it and create a patch. But I have a feeling it could take a while because this change might have an impact on other code and some third-party plugins.

For now I'm in for merging this PR, because I believe it's a step in the right direction.
Once it's merged, I think we should add a comment in the trac ticket that this PR should be reverted after the change is done in get_post_permalink().

[jorgefilipecosta]

In my tests, this PR seems like an improvement so I think it would be good to merge. It is no perfect solution yet but it seems better than what we have now.

[mcsf]

In my tests, this PR seems like an improvement so I think it would be good to merge. It is no perfect solution yet but it seems better than what we have now.

Thanks for weighing in. Yeah, I'd like to merge so that we can improve the other aspects of the UX. The decisions made in the REST API ticket will also have an impact on what our next steps are.

[jorgefilipecosta]

Given the discussion we had, I guess it would be good to test this approach in the plugin for some time. I'm removing the "Backport to WP Core" label.