From adeef741fb235b74f2c0e011f94075a80e2151e4 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 4 May 2022 10:30:37 +0100 Subject: [PATCH] Added first iteration of saving to new publish flow refs https://github.com/TryGhost/Team/issues/1542 - extracted before/after save routines in the editor controller into separate actions - allows saving to occur in the publish flow without it needing any editor-specific knowledge - allows for easier cleanup of email related logic from the editor save tasks later on - added `saveTask` to `PublishOptions` - applies the selected options to the post model where they correspond to model attributes and keeps the previous values in memory so the changes can be undone on failure - this keeps the local model state in sync because if a publish fails we want the editor to continue showing the draft state, non-scheduled publish time, and not have an unexpected email-only state - saves the post model directly passing `adapterOptions` so the save request query params match the chosen publish options - added a `saveTask` to the `` component - passed through to the `publish-flow` modal and is triggered by the confirm button on the confirmation screen - runs the before/afterSave arguments passed in from the editor - runs the `saveTask` on `PublishOptions` which handles everything needed to change status and send emails - polls the post after saving to wait for the attached email to switch to submitted/failed which lets us show a failure message and retry button as required (message + retry not yet implemented) - adds "complete" state to publish flow once save has finished - confirms what just happened based on saved post data rather than chosen publish options - has a link to the view the post --- ghost/admin/app/adapters/post.js | 6 +- .../editor-labs/modals/publish-flow.hbs | 7 +- .../editor-labs/modals/publish-flow.js | 10 ++ .../modals/publish-flow/complete.hbs | 50 +++++++ .../modals/publish-flow/confirm.hbs | 9 +- .../editor-labs/publish-management.js | 141 ++++++++++++++++-- ghost/admin/app/controllers/editor.js | 116 ++++++++------ ghost/admin/app/templates/editor.hbs | 6 +- 8 files changed, 280 insertions(+), 65 deletions(-) create mode 100644 ghost/admin/app/components/editor-labs/modals/publish-flow/complete.hbs diff --git a/ghost/admin/app/adapters/post.js b/ghost/admin/app/adapters/post.js index 0826e8f9ed..7e42f56b57 100644 --- a/ghost/admin/app/adapters/post.js +++ b/ghost/admin/app/adapters/post.js @@ -6,9 +6,11 @@ export default class Post extends ApplicationAdapter { const url = this.buildURL(modelName, id, snapshot, requestType, query); const parsedUrl = new URL(url); - if (snapshot?.adapterOptions?.sendEmailWhenPublished) { - let emailRecipientFilter = snapshot.adapterOptions.sendEmailWhenPublished; + // TODO: cleanup sendEmailWhenPublished when removing publishingFlow flag + let emailRecipientFilter = snapshot?.adapterOptions?.emailRecipientFilter + || snapshot?.adapterOptions?.sendEmailWhenPublished; + if (emailRecipientFilter) { if (emailRecipientFilter === 'status:free,status:-free') { emailRecipientFilter = 'all'; } diff --git a/ghost/admin/app/components/editor-labs/modals/publish-flow.hbs b/ghost/admin/app/components/editor-labs/modals/publish-flow.hbs index 4dd20f4151..d58ed54caa 100644 --- a/ghost/admin/app/components/editor-labs/modals/publish-flow.hbs +++ b/ghost/admin/app/components/editor-labs/modals/publish-flow.hbs @@ -9,10 +9,15 @@ {{#if this.isConfirming}} + {{else if this.isComplete}} + {{else}} + Your {{post.displayName}} is + {{#if post.isScheduled}} + scheduled + {{else if post.emailOnly}} + emailed + {{else}} + live + {{~/if~}} + ! + + +

+ {{#if post.isScheduled}} + {{#let (moment-site-tz post.publishedAtUTC) as |scheduledAt|}} + On + + {{moment-format scheduledAt "D MMM YYYY"}} + at + {{moment-format scheduledAt "HH:mm"}} + + your + {{/let}} + {{else}} + Your + {{/if}} + + {{post.displayName}} was + + {{#if post.email}} + delivered to + + {{pluralize post.email.emailCount "member"}} + + {{#if post.emailOnly}} + and was not + {{else}} + and was + {{/if}} + {{/if}} + + published on your site. +

+ +
+ + Open {{post.displayName}} in new tab +
+{{/let}} \ No newline at end of file diff --git a/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.hbs b/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.hbs index d33869cb54..58749a9d28 100644 --- a/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.hbs +++ b/ghost/admin/app/components/editor-labs/modals/publish-flow/confirm.hbs @@ -12,6 +12,7 @@ at {{moment-format scheduledAt "HH:mm"}} + your {{/let}} {{else}} Your @@ -41,6 +42,12 @@

- +
\ No newline at end of file diff --git a/ghost/admin/app/components/editor-labs/publish-management.js b/ghost/admin/app/components/editor-labs/publish-management.js index 12f7fdf00a..e72e45689e 100644 --- a/ghost/admin/app/components/editor-labs/publish-management.js +++ b/ghost/admin/app/components/editor-labs/publish-management.js @@ -1,13 +1,17 @@ import Component from '@glimmer/component'; +import EmailFailedError from 'ghost-admin/errors/email-failed-error'; import PublishFlowModal from './modals/publish-flow'; import PublishOptionsResource from 'ghost-admin/helpers/publish-options'; import moment from 'moment'; import {action, get} from '@ember/object'; import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; +import {task, timeout} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; import {use} from 'ember-could-get-used-to-this'; +const CONFIRM_EMAIL_POLL_LENGTH = 1000; +const CONFIRM_EMAIL_MAX_POLL_LENGTH = 15 * 1000; + export class PublishOptions { // passed in services config = null; @@ -18,6 +22,8 @@ export class PublishOptions { post = null; user = null; + @tracked totalMemberCount = 0; + get isLoading() { return this.setupTask.isRunning; } @@ -30,6 +36,10 @@ export class PublishOptions { return this.publishType !== 'send'; } + get willOnlyEmail() { + return this.publishType === 'send'; + } + // publish date ------------------------------------------------------------ @tracked isScheduled = false; @@ -75,10 +85,10 @@ export class PublishOptions { get publishTypeOptions() { return [{ - value: 'publish+send', - label: 'Publish and email', - display: 'Publish and email', - confirmButton: 'Publish and send', + value: 'publish+send', // internal + label: 'Publish and email', // shown in expanded options + display: 'Publish and email', // shown in option title + confirmButton: 'Publish and send', // shown in confirm step disabled: this.emailDisabled }, { value: 'publish', @@ -108,13 +118,14 @@ export class PublishOptions { // publish type dropdown is shown but email options are disabled get emailDisabled() { - const mailgunConfigured = get(this.settings, 'mailgunIsConfigured') - || get(this.config, 'mailgunIsConfigured'); + const mailgunIsNotConfigured = !get(this.settings, 'mailgunIsConfigured') + && !get(this.config, 'mailgunIsConfigured'); + + const hasNoMembers = this.totalMemberCount === 0; - // TODO: check members count // TODO: check email limit - return !mailgunConfigured; + return mailgunIsNotConfigured || hasNoMembers; } @action @@ -157,7 +168,7 @@ export class PublishOptions { this.store = store; this.user = user; - // these need to be set here rather than class-level properties because + // this needs to be set here rather than a class-level property because // unlike Ember-based classes the services are not injected so can't be // used until after they are assigned above this.allNewsletters = this.store.peekAll('newsletter'); @@ -181,7 +192,9 @@ export class PublishOptions { @task *fetchRequiredDataTask() { // total # of members - used to enable/disable email - const countTotalMembers = this.store.query('member', {limit: 1}).then(res => res.meta.pagination.total); + const countTotalMembers = this.store.query('member', {limit: 1}).then((res) => { + this.totalMemberCount = res.meta.pagination.total; + }); // email limits // TODO: query limit service @@ -193,10 +206,64 @@ export class PublishOptions { } // saving ------------------------------------------------------------------ + + revertableModelProperties = ['status', 'publishedAtUTC', 'emailOnly']; + + @task({drop: true}) + *saveTask() { + this._applyModelChanges(); + + const adapterOptions = {}; + + if (this.willEmail) { + adapterOptions.newsletterId = this.newsletter.id; + // TODO: replace with real filter + adapterOptions.emailRecipientFilter = 'status:free,status:-free'; + } + + try { + return yield this.post.save({adapterOptions}); + } catch (e) { + this._revertModelChanges(); + throw e; + } + } + + // Publishing/scheduling is a side-effect of changing model properties. + // We don't want to get into a situation where we've applied these changes + // but they haven't been saved because that would result in confusing UI. + // + // Here we apply those changes from the selected publish options but keep + // track of the previous values in case saving fails. We can't use ED's + // rollbackAttributes() because it would also rollback any other unsaved edits + _applyModelChanges() { + // store backup of original values in case we need to revert + this._originalModelValues = {}; + this.revertableModelProperties.forEach((property) => { + this._originalModelValues[property] = this.post[property]; + }); + + this.post.status = this.isScheduled ? 'scheduled' : 'published'; + + if (this.post.isScheduled) { + this.post.publishedAtUTC = this.scheduledAtUTC; + } + + this.post.emailOnly = this.publishType === 'email'; + } + + _revertModelChanges() { + this.revertableModelProperties.forEach((property) => { + this.post[property] = this._originalModelValues[property]; + }); + } } +/* Component -----------------------------------------------------------------*/ + // This component exists for the duration of the editor screen being open. -// It's used to store the selected publish options and control the publishing flow. +// It's used to store the selected publish options and control the +// publishing flow modal. export default class PublishManagement extends Component { @service modals; @@ -218,8 +285,56 @@ export default class PublishManagement extends Component { this.publishOptions.resetPastScheduledAt(); this.publishFlowModal = this.modals.open(PublishFlowModal, { - publishOptions: this.publishOptions + publishOptions: this.publishOptions, + saveTask: this.saveTask }); } } + + @task + *saveTask() { + // clean up blank editor cards + // apply cloned mobiledoc + // apply scratch values + // generate slug if needed (should never happen - publish flow can't be opened on new posts) + yield this.args.beforeSave(); + + // apply publish options (with undo on failure) + // save with the required query params for emailing + const result = yield this.publishOptions.saveTask.perform(); + + // perform any post-save cleanup for the editor + yield this.args.afterSave(result); + + // if emailed, wait until it has been submitted so we can show a failure message if needed + if (this.publishOptions.post.email) { + yield this.confirmEmailTask.perform(); + } + + return result; + } + + @task + *confirmEmailTask() { + const post = this.publishOptions.post; + + let pollTimeout = 0; + if (post.email && post.email.status !== 'submitted') { + while (pollTimeout < CONFIRM_EMAIL_MAX_POLL_LENGTH) { + yield timeout(CONFIRM_EMAIL_POLL_LENGTH); + pollTimeout += CONFIRM_EMAIL_POLL_LENGTH; + + yield post.reload(); + + if (post.email.status === 'submitted') { + break; + } + if (post.email.status === 'failed') { + throw new EmailFailedError(post.email.error); + } + } + } + + return true; + } } diff --git a/ghost/admin/app/controllers/editor.js b/ghost/admin/app/controllers/editor.js index aa22e7059e..e10c5dff1b 100644 --- a/ghost/admin/app/controllers/editor.js +++ b/ghost/admin/app/controllers/editor.js @@ -265,7 +265,7 @@ export default class EditorController extends Controller { keyboardEvent?.preventDefault(); if (this.post.isDraft) { - this.send('openPostPreviewModal'); + this.openPostPreviewModal(); } else { window.open(this.post.previewUrl, '_blank', 'noopener'); } @@ -444,7 +444,8 @@ export default class EditorController extends Controller { /* Public tasks ----------------------------------------------------------*/ // separate task for autosave so that it doesn't override a manual save - @dropTask *autosaveTask() { + @dropTask + *autosaveTask() { if (!this.get('saveTask.isRunning')) { return yield this.saveTask.perform({ silent: true, @@ -461,7 +462,7 @@ export default class EditorController extends Controller { let isNew = this.get('post.isNew'); let status; - this.send('cancelAutosave'); + this.cancelAutosave(); if (options.backgroundSave && !this.hasDirtyAttributes) { return; @@ -496,43 +497,11 @@ export default class EditorController extends Controller { } } - // ensure we remove any blank cards when performing a full save - if (!options.backgroundSave) { - if (this._koenig) { - this._koenig.cleanup(); - this.set('hasDirtyAttributes', true); - } - } - - // Set the properties that are indirected - // set mobiledoc equal to what's in the editor but create a copy so that - // nested objects/arrays don't keep references which can mean that both - // scratch and mobiledoc get updated simultaneously - this.set('post.mobiledoc', JSON.parse(JSON.stringify(this.post.scratch || null))); + // set manually here instead of in beforeSaveTask because the + // new publishing flow sets the post status manually on publish this.set('post.status', status); - // Set a default title - if (!this.get('post.titleScratch').trim()) { - this.set('post.titleScratch', DEFAULT_TITLE); - } - - this.set('post.title', this.get('post.titleScratch')); - this.set('post.customExcerpt', this.get('post.customExcerptScratch')); - this.set('post.footerInjection', this.get('post.footerExcerptScratch')); - this.set('post.headerInjection', this.get('post.headerExcerptScratch')); - this.set('post.metaTitle', this.get('post.metaTitleScratch')); - this.set('post.metaDescription', this.get('post.metaDescriptionScratch')); - this.set('post.ogTitle', this.get('post.ogTitleScratch')); - this.set('post.ogDescription', this.get('post.ogDescriptionScratch')); - this.set('post.twitterTitle', this.get('post.twitterTitleScratch')); - this.set('post.twitterDescription', this.get('post.twitterDescriptionScratch')); - this.set('post.emailSubject', this.get('post.emailSubjectScratch')); - - if (!this.get('post.slug')) { - this.saveTitleTask.cancelAll(); - - yield this.generateSlugTask.perform(); - } + yield this.beforeSaveTask.perform(); try { let post = yield this._savePostTask.perform(options); @@ -577,7 +546,7 @@ export default class EditorController extends Controller { // re-throw if we have a general server error if (error && !isInvalidError(error)) { - this.send('error', error); + this.error(error); return; } @@ -592,6 +561,50 @@ export default class EditorController extends Controller { } } + @task + *beforeSaveTask(options = {}) { + // ensure we remove any blank cards when performing a full save + if (!options.backgroundSave) { + if (this._koenig) { + this._koenig.cleanup(); + this.set('hasDirtyAttributes', true); + } + } + + // TODO: There's no need for (at least) most of these scratch values. + // Refactor so we're setting model attributes directly + + // Set the properties that are indirected + + // Set mobiledoc equal to what's in the editor but create a copy so that + // nested objects/arrays don't keep references which can mean that both + // scratch and mobiledoc get updated simultaneously + this.set('post.mobiledoc', JSON.parse(JSON.stringify(this.post.scratch || null))); + + // Set a default title + if (!this.get('post.titleScratch').trim()) { + this.set('post.titleScratch', DEFAULT_TITLE); + } + + this.set('post.title', this.get('post.titleScratch')); + this.set('post.customExcerpt', this.get('post.customExcerptScratch')); + this.set('post.footerInjection', this.get('post.footerExcerptScratch')); + this.set('post.headerInjection', this.get('post.headerExcerptScratch')); + this.set('post.metaTitle', this.get('post.metaTitleScratch')); + this.set('post.metaDescription', this.get('post.metaDescriptionScratch')); + this.set('post.ogTitle', this.get('post.ogTitleScratch')); + this.set('post.ogDescription', this.get('post.ogDescriptionScratch')); + this.set('post.twitterTitle', this.get('post.twitterTitleScratch')); + this.set('post.twitterDescription', this.get('post.twitterDescriptionScratch')); + this.set('post.emailSubject', this.get('post.emailSubjectScratch')); + + if (!this.get('post.slug')) { + this.saveTitleTask.cancelAll(); + + yield this.generateSlugTask.perform(); + } + } + /* * triggered by a user manually changing slug */ @@ -671,7 +684,8 @@ export default class EditorController extends Controller { } // convenience method for saving the post and performing post-save cleanup - @task *_savePostTask(options = {}) { + @task + *_savePostTask(options = {}) { let {post} = this; const previousEmailOnlyValue = this.post.emailOnly; @@ -695,6 +709,13 @@ export default class EditorController extends Controller { throw error; } + this.afterSave(post); + + return post; + } + + @action + afterSave(post) { this.notifications.closeAlerts('post.save'); // remove any unsaved tags @@ -718,11 +739,10 @@ export default class EditorController extends Controller { if (titlesMatch && bodiesMatch) { this.set('hasDirtyAttributes', false); } - - return post; } - @task *saveTitleTask() { + @task + *saveTitleTask() { let post = this.post; let currentTitle = post.get('title'); let newTitle = post.get('titleScratch').trim(); @@ -747,7 +767,8 @@ export default class EditorController extends Controller { this.ui.updateDocumentTitle(); } - @enqueueTask *generateSlugTask() { + @enqueueTask + *generateSlugTask() { let title = this.get('post.titleScratch'); // Only set an "untitled" slug once per post @@ -772,7 +793,8 @@ export default class EditorController extends Controller { } // load supplementel data such as the members count in the background - @restartableTask *backgroundLoaderTask() { + @restartableTask + *backgroundLoaderTask() { yield this.store.query('snippet', {limit: 'all'}); } @@ -861,7 +883,7 @@ export default class EditorController extends Controller { // if an autosave is scheduled, cancel it, save then transition if (this._autosaveRunning) { - this.send('cancelAutosave'); + this.cancelAutosave(); this.autosaveTask.cancelAll(); await this.autosaveTask.perform(); @@ -898,7 +920,7 @@ export default class EditorController extends Controller { // make sure the save tasks aren't still running in the background // after leaving the edit route - this.send('cancelAutosave'); + this.cancelAutosave(); if (post) { // clear post of any unsaved, client-generated tags diff --git a/ghost/admin/app/templates/editor.hbs b/ghost/admin/app/templates/editor.hbs index 34c27de3c7..a4ff2711c4 100644 --- a/ghost/admin/app/templates/editor.hbs +++ b/ghost/admin/app/templates/editor.hbs @@ -46,7 +46,11 @@ data-test-contributor-save /> {{else}} {{#if (feature "publishingFlow")}} - + {{else}}