Skip to content

Commit

Permalink
Block Directory: Fix "no result" ui flash (#22783)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
StevenDufresne authored Jun 22, 2020
1 parent 01fab7e commit a3e1255
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export default compose( [
const downloadableItems = hasPermission
? getDownloadableBlocks( filterValue )
: [];
const isLoading = isRequestingDownloadableBlocks();
const isLoading = isRequestingDownloadableBlocks( filterValue );

return {
downloadableItems,
Expand Down
19 changes: 7 additions & 12 deletions packages/block-directory/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 15 additions & 4 deletions packages/block-directory/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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;
}

/**
Expand Down
36 changes: 17 additions & 19 deletions packages/block-directory/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
} );
} );

Expand Down
61 changes: 58 additions & 3 deletions packages/block-directory/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
getNewBlockTypes,
getUnusedBlockTypes,
isInstalling,
isRequestingDownloadableBlocks,
} from '../selectors';

describe( 'selectors', () => {
Expand All @@ -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 = {
Expand Down Expand Up @@ -143,9 +199,8 @@ describe( 'selectors', () => {
describe( 'getDownloadableBlocks', () => {
const state = {
downloadableBlocks: {
isRequestingDownloadableBlocks: false,
results: {
boxer: [ downloadableBlock ],
boxer: {
results: [ downloadableBlock ],
},
},
};
Expand Down

0 comments on commit a3e1255

Please sign in to comment.