Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 2, 2018

Conversation

kadamwhite
Copy link
Contributor

@kadamwhite kadamwhite commented Sep 17, 2018

Description

This addresses the post date label issue described in #7195 with an alternative to the previously-proposed solution in PR #8395.

Rather than forcing a new property into the API to work around the nuances of how dates are stored in the database, we can use what @earnjam and @TimothyBJacobs proposed in https://core.trac.wordpress.org/ticket/39953 and infer that a post's date is "floating" (i.e. that it should be published immediately) by comparing the date modified to the listed publish date. In that Trac thread @TimothyBJacobs explained,

If the date and the modified date are the same, then we're still working on the post because those are both updated at every write. As mentioned we don't have an actual created_at date. This would save us having to add the new field. We couldn't figure out a time when those two dates wouldn't match, and in testing it looked like it would work to fulfill Gutenberg's needs.

Note that this works in Gutenberg because on save, we only send back values which have been edited. To fully address this issue throughout WordPress, changes will be needed in Trac to ensure that we apply the same logic when preparing posts to be saved to the database.

How has this been tested?

I created this patch with @earnjam at the WCNYC contributor day, and I have tested it by creating several posts to ensure various combinations of draft and auto-draft correctly reflect manually-specified dates and the Immediately placeholder. To reproduce these tests,

  1. Create a new post and look at the date label. It should display "Immediately."
  2. Update the title (or any other non-date field) and save a draft. Post should still display Publish: "Immediately."
  3. Test publish vs manually-specify date:
  • To test that date is set properly when publishing, publish the post and ensure the date that is displayed on the front-end and when reloading the post in the editor reflects the time at which the post was published.
  • To test that the date is set properly when manually specifying a date, click "Immediately" and set a manual date at which the post should be published. Reload the page and ensure the provided date still displays.

Screenshots

image

Types of changes

Bug fix for PostScheduleLabel component.
Adds unit tests for 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.

Move logic to determine whether a post should be considered to have a
floating date from the PostScheduleLabel component into a new selector
isEditedPostDateFloating.
@kadamwhite
Copy link
Contributor Author

kadamwhite commented Sep 27, 2018

@youknowriad I have added a commit to introduce a isEditedPostDateFloating selector, which produces a value to be consumed within the component instead of relying on the component code to apply the date logic.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, Code looks good, I'll just do some testing a bit after (unless someone beats me to it) and approve.

dateI18n( settings.formats.datetime, date ) :
__( 'Immediately' );
}

export default withSelect( ( select ) => {
return {
date: select( 'core/editor' ).getEditedPostAttribute( 'date' ),
floating: select( 'core/editor' ).isEditedPostDateFloating(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: maybe isFloating to clarify that it's a boolean?

@kadamwhite
Copy link
Contributor Author

Good point about the property naming, I have made that change 👍

*
* @return {boolean} Whether the edited post has a floating date value.
*/
export function isEditedPostDateFloating( state ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this new selector to generate some changes in the docs if you run npm run docs:build. Can you commit these changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this new selector to generate some changes in the docs if you run npm run docs:build. Can you commit these changes?

This was never done and there are local changes to commit on master after running the command.

Please review #10234

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 my bad, @aduth , hadn't had time to circle back to this and had missed that piece of feedback. I'll know for next time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries @kadamwhite

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@youknowriad youknowriad added this to the 4.0 milestone Sep 27, 2018
@youknowriad youknowriad merged commit af53caa into master Oct 2, 2018
@youknowriad youknowriad deleted the fix/7195-infer-publish-immediately branch October 2, 2018 10:54
@mtias mtias added the [Feature] Saving Related to saving functionality label Oct 9, 2018
@mtias
Copy link
Member

mtias commented Oct 9, 2018

Thanks for the work here @kadamwhite.

ockham added a commit that referenced this pull request Jan 13, 2021
… edited (#28127)

The algorithm to infer the "floating" status of post dates have been introduced in #9967. It works well but it should be using the persisted post status and not the edited one. 

Co-authored-by: Bernie Reiter <ockham@raz.or.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Saving Related to saving functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants