From 7a5f15414beb286b2d03f06f35ab2677318db2d2 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 19 May 2022 23:11:01 +0100 Subject: [PATCH] Fixed scheduled publishedAt validation being triggered unexpectedly no issue Problem: - the server stores time with second-level precision but we send milliseconds - when scheduling a post we were storing the selected publish-at time in the PublishOptions instance with non-zero milliseconds as the initial date was based on now+5mins - when we were saving after the initial schedule that millisecond-containing time was still being used resulting in a perceived time difference in our client-side and server-side validations indicating that the published_at value should be updated, but when that time was <2 mins in the future the published_at change validation was triggered resulting in errors Solution: - ensure we only set times on PublishOptions with 0 milliseconds - update client-side validations so we only trigger published_at validation when it's dirty Also fixed: - client-side validation errors were not shown on the confirm step due to a missing `.args` --- .../editor-labs/modals/publish-flow/confirm.js | 2 +- ghost/admin/app/utils/publish-options.js | 10 +++++++--- ghost/admin/app/validators/post.js | 7 ++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.js b/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.js index 481d100e4a..668a5eabff 100644 --- a/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.js +++ b/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.js @@ -51,7 +51,7 @@ export default class PublishFlowOptions extends Component { try { yield this.args.saveTask.perform(); } catch (e) { - if (e === undefined && this.publishOptions.post.errors.length !== 0) { + if (e === undefined && this.args.publishOptions.post.errors.length !== 0) { // validation error const validationError = this.args.publishOptions.post.errors.messages[0]; this.errorMessage = `Validation failed: ${validationError}`; diff --git a/ghost/admin/app/utils/publish-options.js b/ghost/admin/app/utils/publish-options.js index d60b59ba9e..d7b3f66466 100644 --- a/ghost/admin/app/utils/publish-options.js +++ b/ghost/admin/app/utils/publish-options.js @@ -41,7 +41,7 @@ export default class PublishOptions { @tracked scheduledAtUTC = this.minScheduledAt; get minScheduledAt() { - return moment.utc().add(5, 'minutes'); + return moment.utc().add(5, 'minutes').milliseconds(0); } @action @@ -59,11 +59,15 @@ export default class PublishOptions { @action setScheduledAt(date) { - if (moment.utc(date).isBefore(this.minScheduledAt)) { + // API only stores seconds so providing non-zero milliseconds can + // trigger unexpected validation when updating scheduled posts + date = moment.utc(date).milliseconds(0); + + if (date.isBefore(this.minScheduledAt)) { return; } - this.scheduledAtUTC = moment.utc(date); + this.scheduledAtUTC = date; } @action diff --git a/ghost/admin/app/validators/post.js b/ghost/admin/app/validators/post.js index 02d3a8633f..716abdc147 100644 --- a/ghost/admin/app/validators/post.js +++ b/ghost/admin/app/validators/post.js @@ -185,9 +185,7 @@ export default BaseValidator.create({ if (isEmpty(model.errors.errorsFor('publishedAtBlogTime'))) { let status = model.statusScratch || model.status; let now = moment(); - let publishedAtUTC = model.publishedAtUTC; let publishedAtBlogTZ = model.publishedAtBlogTZ; - let matchesExisting = publishedAtUTC && publishedAtBlogTZ.isSame(publishedAtUTC); let isInFuture = publishedAtBlogTZ.isSameOrAfter(now.add(2, 'minutes')); // draft/published must be in past @@ -195,9 +193,8 @@ export default BaseValidator.create({ model.errors.add('publishedAtBlogDate', 'Must be in the past'); this.invalidate(); - // scheduled must be at least 2 mins in the future - // ignore if it matches publishedAtUTC as that is likely an update of a scheduled post - } else if (status === 'scheduled' && !matchesExisting && !isInFuture) { + // scheduled must be at least 2 mins in the future when first scheduling + } else if ((model.changedAttributes().status || model.changedAttributes().publishedAtUTC) && status === 'scheduled' && !isInFuture) { model.errors.add('publishedAtBlogDate', 'Must be at least 2 mins in the future'); this.invalidate(); }