From 4cd149236cb6f87ad25dd94b42b36663f64182b7 Mon Sep 17 00:00:00 2001 From: Steven Dufresne Date: Fri, 24 Jan 2020 13:14:12 +0900 Subject: [PATCH] Block Directory: Refactor the reducer by breaking out the block management actions into their own reducer. (#19330) * Block Directory: Refactory the reducer by break out the block management actions into their own reducer. * Moved hasPermission into its own reducer. * Also remove the 'items' list as it's not being used * Update the getInstalledBlockTypes selector to point to the new reducer that manages installs. * Update typo in test. * Remove the lodash dependency in the selectors. It isn\'t necessary. --- packages/block-directory/src/store/reducer.js | 44 ++++++-- .../block-directory/src/store/selectors.js | 9 +- .../src/store/test/fixtures/index.js | 23 ++++ .../block-directory/src/store/test/reducer.js | 100 ++++++++++++++++++ .../src/store/test/selectors.js | 21 ++++ 5 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 packages/block-directory/src/store/test/fixtures/index.js create mode 100644 packages/block-directory/src/store/test/reducer.js create mode 100644 packages/block-directory/src/store/test/selectors.js diff --git a/packages/block-directory/src/store/reducer.js b/packages/block-directory/src/store/reducer.js index 8dc4f3438c318..a8644e8fc1a60 100644 --- a/packages/block-directory/src/store/reducer.js +++ b/packages/block-directory/src/store/reducer.js @@ -13,10 +13,8 @@ import { combineReducers } from '@wordpress/data'; */ export const downloadableBlocks = ( state = { results: {}, - hasPermission: true, filterValue: undefined, isRequestingDownloadableBlocks: true, - installedBlockTypes: [], }, action ) => { switch ( action.type ) { case 'FETCH_DOWNLOADABLE_BLOCKS' : @@ -30,21 +28,29 @@ export const downloadableBlocks = ( state = { results: Object.assign( {}, state.results, { [ action.filterValue ]: action.downloadableBlocks, } ), - hasPermission: true, isRequestingDownloadableBlocks: false, }; - case 'SET_INSTALL_BLOCKS_PERMISSION' : - return { - ...state, - items: action.hasPermission ? state.items : [], - hasPermission: action.hasPermission, - }; + } + return state; +}; + +/** + * Reducer managing the installation and deletion of blocks. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Updated state. + */ +export const blockManagement = ( state = { + installedBlockTypes: [], +}, action ) => { + switch ( action.type ) { case 'ADD_INSTALLED_BLOCK_TYPE' : return { ...state, installedBlockTypes: [ ...state.installedBlockTypes, action.item ], }; - case 'REMOVE_INSTALLED_BLOCK_TYPE' : return { ...state, @@ -54,6 +60,24 @@ export const downloadableBlocks = ( state = { return state; }; +/** + * Reducer returns whether the user can install blocks. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Updated state. + */ +export function hasPermission( state = true, action ) { + if ( action.type === 'SET_INSTALL_BLOCKS_PERMISSION' ) { + return action.hasPermission; + } + + return state; +} + export default combineReducers( { downloadableBlocks, + blockManagement, + hasPermission, } ); diff --git a/packages/block-directory/src/store/selectors.js b/packages/block-directory/src/store/selectors.js index 635bd37b81941..7a1778b1cc09a 100644 --- a/packages/block-directory/src/store/selectors.js +++ b/packages/block-directory/src/store/selectors.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { get } from 'lodash'; - /** * Returns true if application is requesting for downloadable blocks. * @@ -37,7 +32,7 @@ export function getDownloadableBlocks( state, filterValue ) { * @return {boolean} User has permission to install blocks. */ export function hasInstallBlocksPermission( state ) { - return state.downloadableBlocks.hasPermission; + return state.hasPermission; } /** @@ -48,5 +43,5 @@ export function hasInstallBlocksPermission( state ) { * @return {Array} Block type items. */ export function getInstalledBlockTypes( state ) { - return get( state, [ 'downloadableBlocks', 'installedBlockTypes' ], [] ); + return state.blockManagement.installedBlockTypes; } diff --git a/packages/block-directory/src/store/test/fixtures/index.js b/packages/block-directory/src/store/test/fixtures/index.js new file mode 100644 index 0000000000000..bbbfbdaa6f9ed --- /dev/null +++ b/packages/block-directory/src/store/test/fixtures/index.js @@ -0,0 +1,23 @@ +export const downloadableBlock = { + name: 'boxer/boxer', + title: 'Boxer', + description: 'Boxer is a Block that puts your WordPress posts into boxes on a page.', + id: 'boxer-block', + rating: 5, + ratingCount: 1, + activeInstalls: 0, + authorBlockRating: 5, + authorBlockCount: '1', + author: 'CK Lee', + icon: 'block-default', + assets: [ + 'https://plugins.svn.wordpress.org/boxer-block/trunk/build/index.js', + 'https://plugins.svn.wordpress.org/boxer-block/trunk/build/view.js', + ], + humanizedUpdated: '3 months ago', +}; + +export const installedItem = { + id: 'boxer-block', + name: 'boxer/boxer', +}; diff --git a/packages/block-directory/src/store/test/reducer.js b/packages/block-directory/src/store/test/reducer.js new file mode 100644 index 0000000000000..f221a3bfa023d --- /dev/null +++ b/packages/block-directory/src/store/test/reducer.js @@ -0,0 +1,100 @@ +/** + * External dependencies + */ +import deepFreeze from 'deep-freeze'; + +/** + * Internal dependencies + */ +import { + downloadableBlocks, + blockManagement, + hasPermission, +} from '../reducer'; +import { installedItem, downloadableBlock } from './fixtures'; + +describe( 'state', () => { + describe( 'downloadableBlocks()', () => { + it( 'should update state to reflect active search', () => { + const initialState = deepFreeze( { + isRequestingDownloadableBlocks: false, + } ); + const state = downloadableBlocks( initialState, { + type: 'FETCH_DOWNLOADABLE_BLOCKS', + } ); + + expect( state.isRequestingDownloadableBlocks ).toBe( true ); + } ); + + it( 'should update state to reflect search results have returned', () => { + const state = downloadableBlocks( undefined, { + type: 'RECEIVE_DOWNLOADABLE_BLOCKS', + filterValue: downloadableBlock.title, + downloadableBlocks: [ downloadableBlock ], + } ); + + expect( state.isRequestingDownloadableBlocks ).toBe( false ); + } ); + + it( 'should set user\'s search term and save results', () => { + const state = downloadableBlocks( undefined, { + type: 'RECEIVE_DOWNLOADABLE_BLOCKS', + filterValue: downloadableBlock.title, + downloadableBlocks: [ downloadableBlock ], + } ); + expect( state.results ).toHaveProperty( downloadableBlock.title ); + expect( state.results[ downloadableBlock.title ] ).toHaveLength( 1 ); + + // It should append to the results + const updatedState = downloadableBlocks( state, { + type: 'RECEIVE_DOWNLOADABLE_BLOCKS', + filterValue: 'Test 1', + downloadableBlocks: [ downloadableBlock ], + } ); + + expect( Object.keys( updatedState.results ) ).toHaveLength( 2 ); + } ); + } ); + + describe( 'blockManagement()', () => { + it( 'should start with an empty installedBlockTypes List', () => { + const state = blockManagement( undefined, { + type: 'NOOP_TYPE', + } ); + expect( state.installedBlockTypes ).toEqual( [] ); + } ); + + it( 'should add item to the installedBlockTypesList', () => { + const initialState = deepFreeze( { installedBlockTypes: [] } ); + const state = blockManagement( initialState, { + type: 'ADD_INSTALLED_BLOCK_TYPE', + item: installedItem, + } ); + + expect( state.installedBlockTypes ).toHaveLength( 1 ); + } ); + + it( 'should remove item from the installedBlockTypesList', () => { + const initialState = deepFreeze( { + installedBlockTypes: [ installedItem ], + } ); + const state = blockManagement( initialState, { + type: 'REMOVE_INSTALLED_BLOCK_TYPE', + item: installedItem, + } ); + + expect( state.installedBlockTypes ).toHaveLength( 0 ); + } ); + } ); + + describe( 'hasPermission()', () => { + it( 'should update permissions appropriately', () => { + const state = hasPermission( true, { + type: 'SET_INSTALL_BLOCKS_PERMISSION', + hasPermission: false, + } ); + + expect( state ).toBe( false ); + } ); + } ); +} ); diff --git a/packages/block-directory/src/store/test/selectors.js b/packages/block-directory/src/store/test/selectors.js new file mode 100644 index 0000000000000..b907328e3a420 --- /dev/null +++ b/packages/block-directory/src/store/test/selectors.js @@ -0,0 +1,21 @@ +/** + * Internal dependencies + */ +import { + getInstalledBlockTypes, +} from '../selectors'; + +describe( 'selectors', () => { + describe( 'getInstalledBlockTypes', () => { + it( 'should retrieve the block types that are installed', () => { + const blockTypes = [ 'fake-type' ]; + const state = { + blockManagement: { + installedBlockTypes: blockTypes, + }, + }; + const installedBlockTypes = getInstalledBlockTypes( state ); + expect( installedBlockTypes ).toEqual( blockTypes ); + } ); + } ); +} );