REST API support switching draft to unscheduled

[mnelson4]

Ticket #39953 allows REST API clients to send a post's date in a POST request, and either leave the post as "date floating" (draft that says 'Publish immediately') switch it to be "non-date-floating" (draft that says 'Schedule for...').

However, REST API clients cannot make a request to switch the post BACK to being "date floating" (mentioned on https://core.trac.wordpress.org/ticket/39953#comment:25 and https://core.trac.wordpress.org/ticket/39953#comment:34).

This option isn't available in WordPress' default editor either (see https://core.trac.wordpress.org/ticket/8368#comment:20). However, from my testing, WordPress.com's Calypso editor DOES allow this. Also there's a very old ticket suggesting it be added: #8368, so it seems reasonable that the REST API should support this (and maybe Gutenberg too someday).

So how should a REST API client specify they want to switch a post back to date floating (say "Publish immediately" in the classic editor)?

Should they provide a post's date set to null, false, '', or maybe even to the same value as modified? (I suggest that last one because that's how you leave a currently-"date floating" post as-is, see https://core.trac.wordpress.org/ticket/39953#comment:25)

[jnylen0]

Should they provide a post's date set to null, false, '', or maybe even to the same value as modified?

No, the API and its clients should use a separate field for this purpose.

If you were writing a new piece of code today that manipulates various date values, and also has a flag to control a specific date-related behavior, independent of the actual values of the dates, would you re-use a special value of one of the date variables to control this independent and separate flag, or use a different variable?

The correct answer is a different variable. Except that in this case, the decision made will end up affecting the design and implementation of any project that uses the REST API.

This same mistake was made 10 years ago, but please don't let that confuse the issue, and please don't perpetuate the mistake further. I suggest closing this ticket as a duplicate of #39953.

[mnelson4]

No, the API and its clients should use a separate field for this purpose.

Thanks for the feedback @jnylen0 and I mostly agreed, but you can see on #39953 that there's been quite a bit of resistance to adding a new field (not from me! See https://core.trac.wordpress.org/ticket/39953#comment:16. But I'm pretty un-opinionated and just want to help find a solution most people are happy with).

I suggest closing this ticket as a duplicate of #39953.

I opened this as a branch off of that ticket (see https://core.trac.wordpress.org/ticket/39953#comment:39) so that the currently-requested part of the feature (the ability to leave posts as "date-floating") can proceed un-impeded by this related-but-IMO-separate discussion. I'm pretty sure @kadamwhite ok'd this approach: ​https://wordpress.slack.com/archives/C02RQC26G/p1537482913000100.

Having said that, if there's a complete reversal on #39953, and we do end up using a date_floating field, and we support this functionality using that field, then ya, we should close this ticket.

[jnylen0]

The most convincing argument against a new field that I've seen so far is "feels like we shouldn't need it".

In principle I agree, but the way WP has worked for years is different. The task is then one of choosing the least ugly solution, with a design that makes things easier for API clients and encourages them to write clean, straightforward code themselves.

[kadamwhite]

Respectfully, adding a field for the sole purpose of changing one piece of UI state in client interfaces is tremendous overkill. To springboard off your argument, the way WP has worked for years to my mind has been to embrace contextual solutions to historical inconsistencies, not force additional data and properties into classes, databases or API responses to perpetuate those inconsistencies. The intent of the REST API is to provide a consistent facade layer over those historical inconsistencies, not to expose those inconsistencies to clients through strange new response properties.

I do not regard this as perpetuating the mistakes of the past; I think we have already perpetuated those mistakes by returning a "false" date when a post is floating, rather than providing null in that instance. That's an inconsistency we can address with v3 of the REST API, but I hold the ship has sailed for v2, and my goal therefore is to ensure that things work as consistently and predictably as possible when consuming the existing data.

I am in favor of this ticket, and believe the appropriate response to accept to restore floating date status would be null, as that has the most explicit meaning of "unset this" in other APIs. However @danielbachhuber has requested that we validate whether we accept '' or false for unsetting values elsewhere in the API, to ensure internal consistency.

[kadamwhite]

If you were writing a new piece of code today that manipulates various date values, and also has a flag to control a specific date-related behavior, independent of the actual values of the dates, would you re-use a special value of one of the date variables to control this independent and separate flag, or use a different variable?

Put another way, as I understand the existing behavior, this argument inaccurate; the value of the date in the database, namely 0000-00-00T00:00:00, _is_ the actual value that represents date floating. The presence of that zeroed-out value is a placeholder for a specific publish date's non-existence, i.e. the state of having floating date. That state is not stored as a separate flag in the data model, and representing it as such would further confuse the situation. Better, then, to do what is proposed in this ticket, and accept a null value when writing a post, which will be converted to the zeroed-out placeholder on save.

[jnylen0]

the way WP has worked for years to my mind has been to embrace contextual solutions to historical inconsistencies

When this involves encouraging API clients to write convoluted code, that's where I stop being convinced that it's the right solution.

I originally changed the behavior of the date_gmt field in the API because posts should always have a usable UTC date field. I did that because working with UTC and possibly a timezone is the only provably correct way to handle dates, and so the date_gmt field should always mean the UTC date and nothing else.

I think we have already perpetuated those mistakes by returning a "false" date when a post is floating, rather than providing null in that instance

It's not a null date, though, because you still have the creation date of the draft. The root problem here is the lack of a clearly designed data model from the start. Some more thoughts on that here.

---

Anyway, I'm not going to be spending any further time on this. It's a tricky issue, and I trust y'all to come up with a solution that works.

[aldavigdis]

There is a discussion about the same topic for Gutenberg over at ​https://github.com/WordPress/gutenberg/issues/12048. Perhaps it would be a good idea to coordinate on this and prioritise?

[adamsilverstein]

I am in favor of this ticket, and believe the appropriate response to accept to restore floating date status would be null,

Accepting null makes sense: inserting null for these crates the "floating" date (it is the default database value for these fields).

One potential issue however is that null is the same as not sending any value at all for the field, right? This might lead to unexpected behavior if clients weren't always sending the date_gmt field. I'm curious what precedent exists in other endpoints and APIs.

[adamsilverstein]

44975.diff​ is a first pass at accepting an empty string for date or dage_gmt to reset the dates to floating ("publish immediately"). Needs tests, maybe a status check?

[aldavigdis]

Excellent! I'm going to have a better look at the Gutenberg bits later this week.

[adamsilverstein]

In 44975.2.diff​ Add some unit tests.

[kadamwhite]

One potential issue however is that null is the same as not sending any value at all for the field, right?

Not sending any value for the field at all would, I would expect based on experience with both this and other APIs, result in no change to the record date of any sort. Sending null would either unset a previously specified date, or, if the date was already floating, cause no change.

[adamsilverstein]

Let's try to land a fix for this ticket in 5.3.

[adamsilverstein]

In 44975.3.diff​:

• Use a null value in the test.
• update doc blocks

In my testing sending a null value works just like sending a blank string. I updated the test to use null and adjusted the doc blocks a little as well.

Matching draft PR (with tests running): ​https://github.com/WordPress/wordpress-develop/pull/81

[adamsilverstein]

@kadamwhite or @timothyblynjacobs can you please review 44975.3.diff​ when you have a chance!

[adamsilverstein]

44975.4.diff​ updated date types in schma" 'type' => [ 'string', 'null' ],

[TimothyBlynJacobs]

Uploaded a patch that supports validation for type being a list. Then, the post controller can specify that the date/date_gmt is a 'type' => array( 'string', 'null' ). Then, in prepare_item_for_database check that the parameter was included in the request and set to null.

This introduces WP_REST_Request::has_param to make it simpler to check if a parameter with a null value was included in the request. The settings controller ( which also checks for null values ) works around this by using array_key_exists on the existing ::get_params() method.

[Rarst]

Was asked to comment from Date/Time component perspective and... it's kinda highly not obvious.

1. Sending null to null a date value makes sense on REST API level.
2. However underlying concept of null date in core is a mess.

"Zeroed" date is used as null value in database schema, but it's actually a valid date in PHP 7. So the legacy logic built on top of that is very likely in some state of broken at the moment.

So I suppose... If you want to literally zero a date then send null and call it done. Something is probably broken about it though.

Actual solution would really be designing a proper state of the post for this and how it transitions, which very likely would need to be dedicated and explicit, not derived from date fields.

PS for an extra level of not fun UTC dates specifically are unsuitable for scheduling purposes, because users schedule local time and any future time zone states relative from UTC are unknown.

[adamsilverstein]

44975.5.diff​ worked for null in my testing - sending anull value for date_gmt reset the date correctly. Screencast - ​https://cl.ly/e8bb21e13ac3 - note I did have to reload to show the correct date, this is like a gutenberg issue though and might already be resolved in the plugin/5.3.

Checking the database, i see the expected 0values for the post - ​https://cl.ly/8968c001c517/MySQL_5.7.26_wpdev.localhostdevelopwordpresswp_posts_2019-09-23_08-13-24.jpg

One question: is the intention to also allow an empty string to reset or only null? when I tested with empty string the save was rejected with the error Updating failed. Error message: Invalid parameter(s): date_gmt - ​https://cl.ly/7d968427a5ff

[TimothyBlynJacobs]

Thank you for the feedback @Rarst!

However underlying concept of null date in core is a mess.

In the patch we specifically set post_date and post_date_gmt to null if the REST request parameter value is null. However, we could instead set that to 0000-00-00 00:00:00 if that'd be better. AFAICT, they are handled the ​same.

So I suppose... If you want to literally zero a date then send null and call it done. Something is probably broken about it though.

Yeah, we want the field in the DB to be set zeroed. The null value isn't persisted beyond the API layer. Do you have ideas what might be broken? IIRC from when we were researching this last year, it looks like passing an "empty" value to wp_insert_post is the correct way to do this?

Actual solution would really be designing a proper state of the post for this and how it transitions, which very likely would need to be dedicated and explicit, not derived from date fields.

Did you see @kadamwhite's comment?

---

44975.5.diff​ worked for null in my testing - sending a null value for date_gmt reset the date correctly.

Awesome!

One question: is the intention to also allow an empty string to reset or only null?

Yeah, after ​discussing in #core-restapi we'll only allow null and not try to convert an empty string to null. This also matches behavior of how we handle null in other endpoints like Settings and Meta fields.

---

Something I noticed when looking at the patch again, I think we need to check against the item schema for$schema['properties']['date(_gmt)'] the same way we do for the regular post date updates. I uploaded a patch to add that.

[Rarst]

In the patch we specifically set post_date and post_date_gmt to null if the REST request parameter value is null. However, we could instead set that to 0000-00-00 00:00:00 if that'd be better.

I don't think that makes a particular difference.

Do you have ideas what might be broken?

Not specifically and I don't have it in me to start untangling that mess at the moment.

Did you see @kadamwhite's comment?

Yeah, we chatted some out of thread as well.

Basically this is a mess on top of a mess. If your goal is to be most consistent with existing layers of mess then sending null through REST API to reset date fields is probably passable solution.

I'd like to see this behavior entirely untangled from date fields (come on, this is not some peace of classical beauty, this is weird ancient kludge of crazy implementation detail) but that's probably not happening in context of REST API.

[kadamwhite]

The external interface being introduced here (accept null to reset date) I believe to be solid, and we can address any internal inconsistencies we discover as a bugfix without changing that new interface. Marking for commit for 5.3; thank you all for your input!

[kadamwhite]

In 46249:

REST API: Pass "null" as the post date property to reset post to initial "floating" date value.

Props TimothyBlynJacobs, adamsilverstein, jnylen0, mnelson4.
Fixes #44975.

Trac Ticket: https://core.trac.wordpress.org/ticket/44975

[TimothyBlynJacobs]

In 47811:

REST API: Add @since entries for rest_validate_value_from_schema().

See #49572, #48818, #44949, #50053, #48820, #49720, #42961, #44975, #43392, #38583.