Show "Publish: Immediately" as date for new drafts without a date #8395

earnjam · 2018-08-02T15:04:02Z

Description

This fixes the issue with the post date label discovered on #7195.

Need some feedback/discussion on the REST API portion. See my comment on #7195 for reasons on needing the modification. It's a bit hackish to use a null value for date_gmt, but that's the way that WordPress has traditionally handled it, so it's just a continuation of that.

How has this been tested?

Create a new post and look at the date label

Screenshots

Look at the publish panel on the side. This is immediately on creation of a new post right now. It sets the date the draft was first created. This is not the date that will be used when publishing.

Fixed version. When no date has been set, it shows "Immediately" instead.

Types of changes

Filters the REST API response to check the post for a post_date_gmt value of 0000-00-00 00:00:00 and sets date_gmt value to null if so
Bug fix on PostScheduleLabel component

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

earnjam · 2018-08-03T15:29:56Z

I'm having second thoughts about this method, as modifying the response here might break back compatibility.

An alternative proposed to core is a date_floating flag.

The other possibility is to check if the date and modified values are identical on a draft or auto-draft. They should likely only be identical when a true post date hasn't been set yet, but that feels like a potentially invalid assumption (though with very low risk if wrong)

earnjam · 2018-08-06T18:06:55Z

Updated to use a date_floating flag on the REST API response based on discussions on https://core.trac.wordpress.org/ticket/39953

desrosj

I think the addition of flag makes sense over filtering the response here. Few points:

There is a closure being used that should become a regular function definition.
Not sure that get_post()->post_date_gmt will always return a proper WP_Post object.

earnjam · 2018-08-06T19:28:07Z

Removed the closure, wasn't thinking there. 🙈

The get_callback function can take a parameter that is an array of values of the post in the REST API response. I used that to pass an ID to the get_post() call. That seems safer because if there isn't a post with that ID, then it would have erred out before it got to that point.

Quick Summary

Posts that have a status of draft or auto-draft and do not have a date explicitly set by the user, have a date that is considered floating. Essentially this means a true post date hasn't been set, and will either be set automatically when the post is published or can be set manually by the user. When the date is floating, we should be showing "Publish: Immediately" instead of a date that may not be accurate once published.

WordPress core traditionally uses a saved value of 0000-00-00 00:00:00 for post_date_gmt to determine if the post has a floating date.

About 18 months ago, we added a shim to force the REST API to return a value for the date_gmt because its easier to work with than the localized date. See https://core.trac.wordpress.org/changeset/40108

This essentially means there is no way to know for certain that a post has a floating date. Based on some discussions in trac ticket #39953 it seems like adding a date_floating flag is the best solution for backwards compatibility. That's what I've done here.

earnjam · 2018-08-07T15:44:38Z

I'm not sure why Travis won't update here. I manually re-ran the build and all checks passed: https://travis-ci.org/WordPress/gutenberg/builds/412776921

earnjam · 2018-08-07T15:53:50Z

@danielbachhuber When you have a minute, can I get your thoughts on this approach since it will require a change to the REST API? See my last comment above for a quick summary and related links to trac.

danielbachhuber · 2018-08-07T19:34:26Z

When you have a minute, can I get your thoughts on this approach

Ugh, what an awful issue. I've read the Trac ticket and will give it some thought. I think @joehoyle might be at WC Pub contributor day on Friday too.

earnjam · 2018-08-07T20:11:48Z

Thanks. I don't like the solution for sure, but I'm struggling to come up with a better alternative.

danielbachhuber · 2018-08-17T16:11:02Z

lib/rest-api.php

+ */
+function gutenberg_add_date_floating_flag() {
+	$post_types = get_post_types( array( 'show_in_rest' => true ), 'names' );
+	register_rest_field( $post_types, 'date_floating', array( 'get_callback' => 'gutenberg_get_date_floating_flag' ) );


Should this be flagged as a read-only field?

danielbachhuber

I don't love the date_floating attribute name, but I don't see a way around it at this point.

Could we get PHPUnit tests added for the behavior of date_floating, so we have explicit usage described?

kadamwhite · 2018-09-16T19:52:48Z

Based on the most recent conversation in https://core.trac.wordpress.org/ticket/39953 I would prefer we move forward with the solution of inferring this flag when the modified date is the same as the post date, and update the PostScheduleLabel component accordingly. I have proposed a small patch for the Gutenberg side of things in that ticket, which I will prepare as a separate PR.

kadamwhite · 2018-09-17T13:58:54Z

I propose #9967 as an alternative approach which should supersede this PR.

* Close #7195 * Close #8395 * Close #9030 * Close #9967 * Fix #10182

earnjam · 2018-09-26T10:53:19Z

Closing this in favor of #9967

earnjam added 2 commits August 2, 2018 11:01

Check date_gmt to determine post-schedule label

82353c6

Filter REST API to allow null value for date_gmt when not set in db

b37b3d9

earnjam changed the title ~~Show I~~ Show "Publish: Immediately" as date for new drafts without a date Aug 2, 2018

earnjam added 2 commits August 2, 2018 11:23

Fix code style issues

af0b3ca

Use camelCase for dateGmt

fe6e55f

earnjam added the REST API Interaction Related to REST API label Aug 2, 2018

Merge branch 'master' into fix/7195-floating-publish-date-label

33c2f5c

earnjam added the [Status] In Progress Tracking issues with work in progress label Aug 4, 2018

earnjam added 2 commits August 6, 2018 11:15

Merge branch 'master' into fix/7195-floating-publish-date-label

1b12d00

Use date_floating value to determine proper date label

6b09f2f

earnjam removed the [Status] In Progress Tracking issues with work in progress label Aug 6, 2018

desrosj requested changes Aug 6, 2018

View reviewed changes

earnjam added 3 commits August 6, 2018 14:33

Remove closure

2803609

Fix docblock

259eec5

Remove unnecessary quotes from property

a8a2f8e

earnjam added the Core REST API Task Task for Core REST API efforts label Aug 6, 2018

earnjam requested a review from desrosj August 7, 2018 15:45

danielbachhuber self-assigned this Aug 7, 2018

danielbachhuber reviewed Aug 17, 2018

View reviewed changes

danielbachhuber requested changes Aug 17, 2018

View reviewed changes

danielbachhuber assigned earnjam and unassigned danielbachhuber Aug 17, 2018

danielbachhuber added the [Status] In Progress Tracking issues with work in progress label Aug 17, 2018

kadamwhite mentioned this pull request Sep 17, 2018

Show "Publish: Immediately" for new drafts by inferring floating date #9967

Merged

4 tasks

tofumatt mentioned this pull request Sep 26, 2018

Post never appears as set to publish "immediately" #10182

Closed

tofumatt added a commit that referenced this pull request Sep 26, 2018

fix: Show "Publish Immediately"

8509802

* Close #7195 * Close #8395 * Close #9030 * Close #9967 * Fix #10182

tofumatt mentioned this pull request Sep 26, 2018

fix: Show "Publish Immediately" #10183

Closed

earnjam closed this Sep 26, 2018

earnjam removed the [Status] In Progress Tracking issues with work in progress label Sep 26, 2018

earnjam deleted the fix/7195-floating-publish-date-label branch September 26, 2018 10:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show "Publish: Immediately" as date for new drafts without a date #8395

Show "Publish: Immediately" as date for new drafts without a date #8395

earnjam commented Aug 2, 2018 •

edited

Loading

earnjam commented Aug 3, 2018

earnjam commented Aug 6, 2018

desrosj left a comment

earnjam commented Aug 6, 2018

earnjam commented Aug 7, 2018

earnjam commented Aug 7, 2018

danielbachhuber commented Aug 7, 2018

earnjam commented Aug 7, 2018

danielbachhuber Aug 17, 2018

danielbachhuber left a comment

kadamwhite commented Sep 16, 2018

kadamwhite commented Sep 17, 2018

earnjam commented Sep 26, 2018

Show "Publish: Immediately" as date for new drafts without a date #8395

Show "Publish: Immediately" as date for new drafts without a date #8395

Conversation

earnjam commented Aug 2, 2018 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

earnjam commented Aug 3, 2018

earnjam commented Aug 6, 2018

desrosj left a comment

Choose a reason for hiding this comment

earnjam commented Aug 6, 2018

Quick Summary

earnjam commented Aug 7, 2018

earnjam commented Aug 7, 2018

danielbachhuber commented Aug 7, 2018

earnjam commented Aug 7, 2018

danielbachhuber Aug 17, 2018

Choose a reason for hiding this comment

danielbachhuber left a comment

Choose a reason for hiding this comment

kadamwhite commented Sep 16, 2018

kadamwhite commented Sep 17, 2018

earnjam commented Sep 26, 2018

earnjam commented Aug 2, 2018 •

edited

Loading