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.
WordPress · Jul 8, 2021 · a6908dc · a6908dc
1 parent 14624ba
commit a6908dc
Show file tree

Hide file tree

Showing 6 changed files with 202 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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,15 +150,15 @@ 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',
 			onClick: this.createOnClick( onClickButton ),
 		};
 
 		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
@@ -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 {
 								/>
 							</div>
 							<div className="editor-post-publish-panel__header-cancel-button">
-								<Button onClick={ onClose } variant="secondary">
+								<Button
+									disabled={ isSavingNonPostEntityChanges }
+									onClick={ onClose }
+									variant="secondary"
+								>
 									{ __( 'Cancel' ) }
 								</Button>
 							</div>
@@ -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
@@ -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
@@ -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 = {