From a6908dcc4cde747ee2879317597f73179bf6e4d1 Mon Sep 17 00:00:00 2001 From: Bernie Reiter Date: Thu, 8 Jul 2021 13:26:56 +0200 Subject: [PATCH] PostPublishButton: Disable if saving non-post entities (#33140) We're currently not disabling the Publish/Save/Update button while saving non-post entity changes (such as changes made to a reusable block, or to general site settings through a special block such as the Site Title block). To fix this, this commit introduces a selector called `isSavingNonPostEntityChanges`, and a few helpers. --- packages/core-data/src/selectors.js | 50 +++++++++++++++++ packages/core-data/src/test/selectors.js | 49 +++++++++++++++++ .../components/post-publish-button/index.js | 29 ++++++---- .../components/post-publish-panel/index.js | 9 ++- packages/editor/src/store/selectors.js | 23 ++++++++ packages/editor/src/store/test/selectors.js | 55 +++++++++++++++++++ 6 files changed, 202 insertions(+), 13 deletions(-) diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index e4397b2df277f..6c7fc62127140 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -316,6 +316,56 @@ export const __experimentalGetDirtyEntityRecords = createSelector( ( state ) => [ state.entities.data ] ); +/** + * Returns the list of entities currently being saved. + * + * @param {Object} state State tree. + * + * @return {[{ title: string, key: string, name: string, kind: string }]} The list of records being saved. + */ +export const __experimentalGetEntitiesBeingSaved = createSelector( + ( state ) => { + const { + entities: { data }, + } = state; + const recordsBeingSaved = []; + Object.keys( data ).forEach( ( kind ) => { + Object.keys( data[ kind ] ).forEach( ( name ) => { + const primaryKeys = Object.keys( + data[ kind ][ name ].saving + ).filter( ( primaryKey ) => + isSavingEntityRecord( state, kind, name, primaryKey ) + ); + + if ( primaryKeys.length ) { + const entity = getEntity( state, kind, name ); + primaryKeys.forEach( ( primaryKey ) => { + const entityRecord = getEditedEntityRecord( + state, + kind, + name, + primaryKey + ); + recordsBeingSaved.push( { + // We avoid using primaryKey because it's transformed into a string + // when it's used as an object key. + key: + entityRecord[ + entity.key || DEFAULT_ENTITY_KEY + ], + title: entity?.getTitle?.( entityRecord ) || '', + name, + kind, + } ); + } ); + } + } ); + } ); + return recordsBeingSaved; + }, + ( state ) => [ state.entities.data ] +); + /** * Returns the specified entity record's edits. * diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index 68878692351a0..586d39985e0c5 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -12,6 +12,7 @@ import { hasEntityRecords, getEntityRecords, __experimentalGetDirtyEntityRecords, + __experimentalGetEntitiesBeingSaved, getEntityRecordNonTransientEdits, getEmbedPreview, isPreviewEmbedFallback, @@ -396,6 +397,54 @@ describe( '__experimentalGetDirtyEntityRecords', () => { } ); } ); +describe( '__experimentalGetEntitiesBeingSaved', () => { + it( "should return a map of objects with each raw entity record that's being saved", () => { + const state = deepFreeze( { + entities: { + config: [ + { + kind: 'someKind', + name: 'someName', + transientEdits: { someTransientEditProperty: true }, + }, + ], + data: { + someKind: { + someName: { + queriedData: { + items: { + default: { + someKey: { + someProperty: 'somePersistedValue', + someRawProperty: { + raw: 'somePersistedRawValue', + }, + id: 'someKey', + }, + }, + }, + itemIsComplete: { + default: { + someKey: true, + }, + }, + }, + saving: { + someKey: { + pending: true, + }, + }, + }, + }, + }, + }, + } ); + expect( __experimentalGetEntitiesBeingSaved( state ) ).toEqual( [ + { kind: 'someKind', name: 'someName', key: 'someKey', title: '' }, + ] ); + } ); +} ); + describe( 'getEntityRecordNonTransientEdits', () => { it( 'should return an empty object when the entity does not have a loaded config.', () => { const state = deepFreeze( { diff --git a/packages/editor/src/components/post-publish-button/index.js b/packages/editor/src/components/post-publish-button/index.js index 36b67b9756d45..39e2030040350 100644 --- a/packages/editor/src/components/post-publish-button/index.js +++ b/packages/editor/src/components/post-publish-button/index.js @@ -103,21 +103,24 @@ export class PostPublishButton extends Component { onToggle, visibility, hasNonPostEntityChanges, + isSavingNonPostEntityChanges, } = this.props; const isButtonDisabled = - isSaving || - forceIsSaving || - ! isSaveable || - isPostSavingLocked || - ( ! isPublishable && ! forceIsDirty ); + ( isSaving || + forceIsSaving || + ! isSaveable || + isPostSavingLocked || + ( ! isPublishable && ! forceIsDirty ) ) && + ( ! hasNonPostEntityChanges || isSavingNonPostEntityChanges ); const isToggleDisabled = - isPublished || - isSaving || - forceIsSaving || - ! isSaveable || - ( ! isPublishable && ! forceIsDirty ); + ( isPublished || + isSaving || + forceIsSaving || + ! isSaveable || + ( ! isPublishable && ! forceIsDirty ) ) && + ( ! hasNonPostEntityChanges || isSavingNonPostEntityChanges ); let publishStatus; if ( ! hasPublishAction ) { @@ -147,7 +150,7 @@ export class PostPublishButton extends Component { }; const buttonProps = { - 'aria-disabled': isButtonDisabled && ! hasNonPostEntityChanges, + 'aria-disabled': isButtonDisabled, className: 'editor-post-publish-button', isBusy: ! isAutoSaving && isSaving && isPublished, variant: 'primary', @@ -155,7 +158,7 @@ export class PostPublishButton extends Component { }; const toggleProps = { - 'aria-disabled': isToggleDisabled && ! hasNonPostEntityChanges, + 'aria-disabled': isToggleDisabled, 'aria-expanded': isOpen, className: 'editor-post-publish-panel__toggle', isBusy: isSaving && isPublished, @@ -210,6 +213,7 @@ export default compose( [ getCurrentPostType, getCurrentPostId, hasNonPostEntityChanges, + isSavingNonPostEntityChanges, } = select( editorStore ); const _isAutoSaving = isAutosavingPost(); return { @@ -229,6 +233,7 @@ export default compose( [ postType: getCurrentPostType(), postId: getCurrentPostId(), hasNonPostEntityChanges: hasNonPostEntityChanges(), + isSavingNonPostEntityChanges: isSavingNonPostEntityChanges(), }; } ), withDispatch( ( dispatch ) => { diff --git a/packages/editor/src/components/post-publish-panel/index.js b/packages/editor/src/components/post-publish-panel/index.js index c2092bd70e339..9cd93d1230415 100644 --- a/packages/editor/src/components/post-publish-panel/index.js +++ b/packages/editor/src/components/post-publish-panel/index.js @@ -62,6 +62,7 @@ export class PostPublishPanel extends Component { isPublishSidebarEnabled, isScheduled, isSaving, + isSavingNonPostEntityChanges, onClose, onTogglePublishSidebar, PostPublishExtension, @@ -97,7 +98,11 @@ export class PostPublishPanel extends Component { />
-
@@ -140,6 +145,7 @@ export default compose( [ isEditedPostBeingScheduled, isEditedPostDirty, isSavingPost, + isSavingNonPostEntityChanges, } = select( editorStore ); const { isPublishSidebarEnabled } = select( editorStore ); const postType = getPostType( getEditedPostAttribute( 'type' ) ); @@ -156,6 +162,7 @@ export default compose( [ isPublished: isCurrentPostPublished(), isPublishSidebarEnabled: isPublishSidebarEnabled(), isSaving: isSavingPost(), + isSavingNonPostEntityChanges: isSavingNonPostEntityChanges(), isScheduled: isCurrentPostScheduled(), }; } ), diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 9b9b5c57b2fdb..e1ef7783cd6fb 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -802,6 +802,29 @@ export const isSavingPost = createRegistrySelector( ( select ) => ( state ) => { ); } ); +/** + * Returns true if non-post entities are currently being saved, or false otherwise. + * + * @param {Object} state Global application state. + * + * @return {boolean} Whether non-post entities are being saved. + */ +export const isSavingNonPostEntityChanges = createRegistrySelector( + ( select ) => ( state ) => { + const entitiesBeingSaved = select( + coreStore + ).__experimentalGetEntitiesBeingSaved(); + const { type, id } = getCurrentPost( state ); + return some( + entitiesBeingSaved, + ( entityRecord ) => + entityRecord.kind !== 'postType' || + entityRecord.name !== type || + entityRecord.key !== id + ); + } +); + /** * Returns true if a previous post save was attempted successfully, or false * otherwise. diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 650764f9dcb45..b59975fbc8436 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -40,6 +40,13 @@ selectorNames.forEach( ( name ) => { ); }, + __experimentalGetEntitiesBeingSaved() { + return ( + state.__experimentalGetEntitiesBeingSaved && + state.__experimentalGetEntitiesBeingSaved() + ); + }, + getEntityRecordEdits() { const present = state.editor && state.editor.present; let edits = present && present.edits; @@ -170,6 +177,7 @@ const { getCurrentPostAttribute, getEditedPostAttribute, isSavingPost, + isSavingNonPostEntityChanges, didPostSaveRequestSucceed, didPostSaveRequestFail, getSuggestedPostFormat, @@ -2085,6 +2093,53 @@ describe( 'selectors', () => { } ); } ); + describe( 'isSavingNonPostEntityChanges', () => { + it( 'should return true if changes to an arbitrary entity are being saved', () => { + const state = { + currentPost: { id: 1, type: 'post' }, + __experimentalGetEntitiesBeingSaved() { + return [ + { kind: 'someKind', name: 'someName', key: 'someKey' }, + ]; + }, + }; + expect( isSavingNonPostEntityChanges( state ) ).toBe( true ); + } ); + it( 'should return false if the only changes being saved are for the current post', () => { + const state = { + currentPost: { id: 1, type: 'post' }, + __experimentalGetEntitiesBeingSaved() { + return [ { kind: 'postType', name: 'post', key: 1 } ]; + }, + }; + expect( isSavingNonPostEntityChanges( state ) ).toBe( false ); + } ); + it( 'should return true if changes to multiple posts are being saved', () => { + const state = { + currentPost: { id: 1, type: 'post' }, + __experimentalGetEntitiesBeingSaved() { + return [ + { kind: 'postType', name: 'post', key: 1 }, + { kind: 'postType', name: 'post', key: 2 }, + ]; + }, + }; + expect( isSavingNonPostEntityChanges( state ) ).toBe( true ); + } ); + it( 'should return true if changes to multiple posts of different post types are being saved', () => { + const state = { + currentPost: { id: 1, type: 'post' }, + __experimentalGetEntitiesBeingSaved() { + return [ + { kind: 'postType', name: 'post', key: 1 }, + { kind: 'postType', name: 'wp_template', key: 1 }, + ]; + }, + }; + expect( isSavingNonPostEntityChanges( state ) ).toBe( true ); + } ); + } ); + describe( 'didPostSaveRequestSucceed', () => { it( 'should return true if the post save request is successful', () => { const state = {