Augment RESET_BLOCKS action instead of replacing it

Keep the RESET_BLOCKS logic in the reducer by instead using a higher-order reducer to augment RESET_BLOCKS with a list of client IDs that should remain in state as they are referenced by a reusable block.
WordPress · Nov 13, 2018 · 7d3d116 · 7d3d116
1 parent 0c92c67
commit 7d3d116
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 86 deletions.
diff --git a/packages/editor/src/store/reducer.js b/packages/editor/src/store/reducer.js
@@ -15,6 +15,8 @@ import {
 	isEqual,
 	overSome,
 	get,
+	map,
+	pick,
 } from 'lodash';
 
 /**
@@ -101,31 +103,6 @@ function getFlattenedBlocks( blocks ) {
 	return flattenedBlocks;
 }
 
-/**
- * Given a block order map object, returns *all* of the block client IDs that are
- * a descendant of the given root client ID.
- *
- * Calling this with `rootClientId` set to `''` results in a list of client IDs
- * that are in the post. That is, it excludes blocks like fetched reusable
- * blocks which are stored into state but not visible.
- *
- * @param {Object}  blocksOrder  Object that maps block client IDs to a list of
- *                               nested block client IDs.
- * @param {?string} rootClientId The root client ID to search. Defaults to ''.
- *
- * @return {Array} List of descendant client IDs.
- */
-function getNestedBlockClientIds( blocksOrder, rootClientId = '' ) {
-	const attachedClientIds = [];
-	if ( blocksOrder[ rootClientId ] ) {
-		blocksOrder[ rootClientId ].forEach( ( clientId ) => {
-			attachedClientIds.push( clientId );
-			attachedClientIds.push( ...getNestedBlockClientIds( blocksOrder, clientId ) );
-		} );
-	}
-	return attachedClientIds;
-}
-
 /**
  * Returns an object against which it is safe to perform mutating operations,
  * given the original object and its current working copy.
@@ -237,29 +214,18 @@ const withInnerBlocksRemoveCascade = ( reducer ) => ( state, action ) => {
 };
 
 /**
- * Higher-order reducer which targets the combined blocks reducer and handles
- * the `RESET_BLOCKS` action. When dispatched, this action will replace all
- * blocks that exist in the post, leaving blocks that exist only in state (e.g.
- * reusable blocks) alone.
+ * Higher-order reducer targeting the combined reducer, augmenting the
+ * RESET_BLOCKS action with a list of client IDs that should remain in editor
+ * state as they are being referenced by a reusable block.
  *
  * @param {Function} reducer Original reducer function.
  *
  * @return {Function} Enhanced reducer function.
  */
-const withBlockReset = ( reducer ) => ( state, action ) => {
+const withReusableBlockClientIdsOnReset = ( reducer ) => ( state, action ) => {
 	if ( state && action.type === 'RESET_BLOCKS' ) {
-		const visibleClientIds = getNestedBlockClientIds( state.order );
-		return {
-			...state,
-			byClientId: {
-				...omit( state.byClientId, visibleClientIds ),
-				...getFlattenedBlocks( action.blocks ),
-			},
-			order: {
-				...omit( state.order, visibleClientIds ),
-				...mapBlockOrder( action.blocks ),
-			},
-		};
+		const reusableBlockClientIds = map( state.reusableBlocks.data, 'clientId' );
+		action = { ...action, reusableBlockClientIds };
 	}
 
 	return reducer( state, action );
@@ -334,8 +300,6 @@ export const editor = flow( [
 	blocks: flow( [
 		combineReducers,
 
-		withBlockReset,
-
 		// Track whether changes exist, resetting at each post save. Relies on
 		// editor initialization firing post reset as an effect.
 		withChangeDetection( {
@@ -345,6 +309,12 @@ export const editor = flow( [
 	] )( {
 		byClientId( state = {}, action ) {
 			switch ( action.type ) {
+				case 'RESET_BLOCKS':
+					return {
+						...pick( state, action.reusableBlockClientIds ),
+						...getFlattenedBlocks( action.blocks ),
+					};
+
 				case 'SETUP_EDITOR_STATE':
 					return getFlattenedBlocks( action.blocks );
 
@@ -447,6 +417,12 @@ export const editor = flow( [
 
 		order( state = {}, action ) {
 			switch ( action.type ) {
+				case 'RESET_BLOCKS':
+					return {
+						...pick( state, action.reusableBlockClientIds ),
+						...mapBlockOrder( action.blocks ),
+					};
+
 				case 'SETUP_EDITOR_STATE':
 					return mapBlockOrder( action.blocks );
 
@@ -1201,7 +1177,11 @@ export function autosave( state = null, action ) {
 	return state;
 }
 
-export default optimist( combineReducers( {
+export default flow( [
+	combineReducers,
+	withReusableBlockClientIdsOnReset,
+	optimist,
+] )( {
 	editor,
 	initialEdits,
 	currentPost,
@@ -1219,4 +1199,4 @@ export default optimist( combineReducers( {
 	autosave,
 	settings,
 	postSavingLock,
-} ) );
+} );
diff --git a/packages/editor/src/store/test/reducer.js b/packages/editor/src/store/test/reducer.js
@@ -1074,55 +1074,30 @@ describe( 'state', () => {
 		} );
 
 		describe( 'blocks', () => {
-			it( 'should not reset any blocks that are not in the post', () => {
-				const actions = [
-					{
-						type: 'RESET_BLOCKS',
-						blocks: [
-							{
-								clientId: 'block1',
-								innerBlocks: [
-									{ clientId: 'block11', innerBlocks: [] },
-									{ clientId: 'block12', innerBlocks: [] },
-								],
-							},
-						],
-					},
-					{
-						type: 'RECEIVE_BLOCKS',
-						blocks: [
-							{
-								clientId: 'block2',
-								innerBlocks: [
-									{ clientId: 'block21', innerBlocks: [] },
-									{ clientId: 'block22', innerBlocks: [] },
-								],
-							},
-						],
-					},
-				];
-				const original = deepFreeze( actions.reduce( editor, undefined ) );
-
+			it( 'should not reset blocks referenced by a reusable block', () => {
+				const original = deepFreeze( editor( undefined, {
+					type: 'RESET_BLOCKS',
+					blocks: [
+						{ clientId: 'block1', innerBlocks: [] },
+						{ clientId: 'block2', innerBlocks: [] },
+					],
+				} ) );
 				const state = editor( original, {
 					type: 'RESET_BLOCKS',
 					blocks: [
-						{
-							clientId: 'block3',
-							innerBlocks: [
-								{ clientId: 'block31', innerBlocks: [] },
-								{ clientId: 'block32', innerBlocks: [] },
-							],
-						},
+						{ clientId: 'block3', innerBlocks: [] },
 					],
+					reusableBlockClientIds: [ 'block2' ],
 				} );
 
 				expect( state.present.blocks.byClientId ).toEqual( {
 					block2: { clientId: 'block2' },
-					block21: { clientId: 'block21' },
-					block22: { clientId: 'block22' },
 					block3: { clientId: 'block3' },
-					block31: { clientId: 'block31' },
-					block32: { clientId: 'block32' },
+				} );
+				expect( state.present.blocks.order ).toEqual( {
+					'': [ 'block3' ],
+					block2: [],
+					block3: [],
 				} );
 			} );