From a3e1255ebb70f595fcebfa59095a5ea39d198908 Mon Sep 17 00:00:00 2001 From: Steven Dufresne Date: Mon, 22 Jun 2020 10:36:28 +0900 Subject: [PATCH] Block Directory: Fix "no result" ui flash (#22783) * Change the way we check for pending searches from boolean to integer. * Fix reducer tests. * Add tests for the selector. * Change the pending search parameter and add tests to the reducer. * Add space between tests to maintain spacing consistency. * Change structure of downloadable blocks to use store results & pending status. * Clean up variable name. * Remove redundant test. * Add test case for retrieving request status and update text to be clearer. * Deep freeze initial state for downloadable blocks test. * Add back in tests removed in rebase. * Move the test back into its area. --- .../downloadable-blocks-panel/index.js | 2 +- packages/block-directory/src/store/reducer.js | 19 +++--- .../block-directory/src/store/selectors.js | 19 ++++-- .../block-directory/src/store/test/reducer.js | 36 ++++++----- .../src/store/test/selectors.js | 61 ++++++++++++++++++- 5 files changed, 98 insertions(+), 39 deletions(-) diff --git a/packages/block-directory/src/components/downloadable-blocks-panel/index.js b/packages/block-directory/src/components/downloadable-blocks-panel/index.js index 08d346d6774a58..df2060eb265816 100644 --- a/packages/block-directory/src/components/downloadable-blocks-panel/index.js +++ b/packages/block-directory/src/components/downloadable-blocks-panel/index.js @@ -94,7 +94,7 @@ export default compose( [ const downloadableItems = hasPermission ? getDownloadableBlocks( filterValue ) : []; - const isLoading = isRequestingDownloadableBlocks(); + const isLoading = isRequestingDownloadableBlocks( filterValue ); return { downloadableItems, diff --git a/packages/block-directory/src/store/reducer.js b/packages/block-directory/src/store/reducer.js index 52e83a8e4db982..fabc863e72a482 100644 --- a/packages/block-directory/src/store/reducer.js +++ b/packages/block-directory/src/store/reducer.js @@ -11,27 +11,22 @@ import { combineReducers } from '@wordpress/data'; * * @return {Object} Updated state. */ -export const downloadableBlocks = ( - state = { - results: {}, - isRequestingDownloadableBlocks: true, - }, - action -) => { +export const downloadableBlocks = ( state = {}, action ) => { switch ( action.type ) { case 'FETCH_DOWNLOADABLE_BLOCKS': return { ...state, - isRequestingDownloadableBlocks: true, + [ action.filterValue ]: { + isRequesting: true, + }, }; case 'RECEIVE_DOWNLOADABLE_BLOCKS': return { ...state, - results: { - ...state.results, - [ action.filterValue ]: action.downloadableBlocks, + [ action.filterValue ]: { + results: action.downloadableBlocks, + isRequesting: false, }, - isRequestingDownloadableBlocks: false, }; } return state; diff --git a/packages/block-directory/src/store/selectors.js b/packages/block-directory/src/store/selectors.js index 81d6e8ed89c1ed..1e9e93b31bbb37 100644 --- a/packages/block-directory/src/store/selectors.js +++ b/packages/block-directory/src/store/selectors.js @@ -12,11 +12,19 @@ import hasBlockType from './utils/has-block-type'; * Returns true if application is requesting for downloadable blocks. * * @param {Object} state Global application state. + * @param {string} filterValue Search string. + * * * @return {Array} Downloadable blocks */ -export function isRequestingDownloadableBlocks( state ) { - return state.downloadableBlocks.isRequestingDownloadableBlocks; +export function isRequestingDownloadableBlocks( state, filterValue ) { + if ( + ! state.downloadableBlocks[ filterValue ] || + ! state.downloadableBlocks[ filterValue ].isRequesting + ) { + return false; + } + return state.downloadableBlocks[ filterValue ].isRequesting; } /** @@ -28,10 +36,13 @@ export function isRequestingDownloadableBlocks( state ) { * @return {Array} Downloadable blocks */ export function getDownloadableBlocks( state, filterValue ) { - if ( ! state.downloadableBlocks.results[ filterValue ] ) { + if ( + ! state.downloadableBlocks[ filterValue ] || + ! state.downloadableBlocks[ filterValue ].results + ) { return []; } - return state.downloadableBlocks.results[ filterValue ]; + return state.downloadableBlocks[ filterValue ].results; } /** diff --git a/packages/block-directory/src/store/test/reducer.js b/packages/block-directory/src/store/test/reducer.js index 897217ea0f85b4..14ccd5e2eb66c0 100644 --- a/packages/block-directory/src/store/test/reducer.js +++ b/packages/block-directory/src/store/test/reducer.js @@ -17,46 +17,44 @@ import { blockTypeInstalled, downloadableBlock } from './fixtures'; describe( 'state', () => { describe( 'downloadableBlocks()', () => { it( 'should update state to reflect active search', () => { - const initialState = deepFreeze( { - isRequestingDownloadableBlocks: false, - } ); + const initialState = deepFreeze( {} ); + const filterValue = 'Awesome Block'; + const state = downloadableBlocks( initialState, { type: 'FETCH_DOWNLOADABLE_BLOCKS', - filterValue: 'test', + filterValue, } ); - expect( state.isRequestingDownloadableBlocks ).toEqual( true ); + expect( state[ filterValue ].isRequesting ).toEqual( true ); } ); it( 'should update state to reflect search results have returned', () => { const query = downloadableBlock.title; - const state = downloadableBlocks( undefined, { + const initialState = deepFreeze( { + [ query ]: { + isRequesting: true, + }, + } ); + + const state = downloadableBlocks( initialState, { type: 'RECEIVE_DOWNLOADABLE_BLOCKS', filterValue: query, downloadableBlocks: [ downloadableBlock ], } ); - expect( state.isRequestingDownloadableBlocks ).toEqual( false ); + expect( state[ query ].isRequesting ).toEqual( false ); } ); it( "should set user's search term and save results", () => { const query = downloadableBlock.title; - const state = downloadableBlocks( undefined, { + const initialState = deepFreeze( {} ); + const state = downloadableBlocks( initialState, { type: 'RECEIVE_DOWNLOADABLE_BLOCKS', filterValue: query, downloadableBlocks: [ downloadableBlock ], } ); - expect( state.results ).toHaveProperty( query ); - expect( state.results[ query ] ).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 ); + expect( state ).toHaveProperty( query ); + expect( state[ query ].results ).toHaveLength( 1 ); } ); } ); diff --git a/packages/block-directory/src/store/test/selectors.js b/packages/block-directory/src/store/test/selectors.js index 690bec529018a8..14d72e7c7e178b 100644 --- a/packages/block-directory/src/store/test/selectors.js +++ b/packages/block-directory/src/store/test/selectors.js @@ -15,6 +15,7 @@ import { getNewBlockTypes, getUnusedBlockTypes, isInstalling, + isRequestingDownloadableBlocks, } from '../selectors'; describe( 'selectors', () => { @@ -31,6 +32,61 @@ describe( 'selectors', () => { } ); } ); + describe( 'isRequestingDownloadableBlocks', () => { + it( 'should return false if no requests have been made for the block', () => { + const filterValue = 'Awesome Block'; + + const state = { + downloadableBlocks: {}, + }; + const isRequesting = isRequestingDownloadableBlocks( + state, + filterValue + ); + + expect( isRequesting ).toEqual( false ); + } ); + + it( 'should return false if there are no pending requests for the block', () => { + const filterValue = 'Awesome Block'; + + const state = { + downloadableBlocks: { + [ filterValue ]: { + isRequesting: false, + }, + }, + }; + const isRequesting = isRequestingDownloadableBlocks( + state, + filterValue + ); + + expect( isRequesting ).toEqual( false ); + } ); + + it( 'should return true if the block has a pending request', () => { + const filterValue = 'Awesome Block'; + + const state = { + downloadableBlocks: { + [ filterValue ]: { + isRequesting: true, + }, + 'previous-search-keyword': { + isRequesting: false, + }, + }, + }; + const isRequesting = isRequestingDownloadableBlocks( + state, + filterValue + ); + + expect( isRequesting ).toEqual( true ); + } ); + } ); + describe( 'getNewBlockTypes', () => { it( 'should retrieve the block types that are installed and in the post content', () => { getNewBlockTypes.registry = { @@ -143,9 +199,8 @@ describe( 'selectors', () => { describe( 'getDownloadableBlocks', () => { const state = { downloadableBlocks: { - isRequestingDownloadableBlocks: false, - results: { - boxer: [ downloadableBlock ], + boxer: { + results: [ downloadableBlock ], }, }, };