Skip to content

Commit

Permalink
refactoring according to review
Browse files Browse the repository at this point in the history
- removes superfluous actions from deleteEntityRecords
- treats invalidateCache properly
- attempts to make lookups faster for items when removing querries
  • Loading branch information
draganescu committed Jun 11, 2020
1 parent 6ce76e9 commit 9703988
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 29 deletions.
23 changes: 2 additions & 21 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,7 @@ export function* deleteEntityRecord( kind, name, recordId ) {
return;
}

yield {
type: 'DELETE_ENTITY_RECORD_START',
kind,
name,
recordId,
};

yield removeItems( kind, name, Number( recordId ) );

let error;
yield removeItems( kind, name, recordId, true );

try {
let path = `${ entity.baseURL }/${ recordId }`;
Expand All @@ -176,10 +167,8 @@ export function* deleteEntityRecord( kind, name, recordId ) {
method: 'DELETE',
} );
} catch ( _error ) {
error = _error;

const persistedRecord = yield select(
'getEntityRecord',
'getEditedEntityRecord',
kind,
name,
recordId
Expand All @@ -193,14 +182,6 @@ export function* deleteEntityRecord( kind, name, recordId ) {
false
);
}

yield {
type: 'DELETE_ENTITY_RECORD_FINISH',
kind,
name,
recordId,
error,
};
}

/**
Expand Down
11 changes: 6 additions & 5 deletions packages/core-data/src/queried-data/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,19 @@ export function receiveItems( items ) {
* Returns an action object used in signalling that entity records have been
* deleted and they need to be removed from entities state.
*
* @param {string} kind Kind of the removed entities.
* @param {string} name Name of the removed entities.
* @param {Array|Object} records Records removed.
* @param {string} kind Kind of the removed entities.
* @param {string} name Name of the removed entities.
* @param {Array|Object} records Records removed.
* @param {boolean} invalidateCache Controls whether we want to invalidate the cache.
* @return {Object} Action object.
*/
export function removeItems( kind, name, records ) {
export function removeItems( kind, name, records, invalidateCache = false ) {
return {
type: 'REMOVE_ITEMS',
items: castArray( records ),
kind,
name,
invalidateCache: false,
invalidateCache,
};
}

Expand Down
13 changes: 11 additions & 2 deletions packages/core-data/src/queried-data/reducer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
/**
* External dependencies
*/
import { map, flowRight, omit, forEach, filter } from 'lodash';
import {
map,
flowRight,
omit,
forEach,
filter,
keyBy,
toPlainObject,
} from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -154,10 +162,11 @@ const queries = ( state = {}, action ) => {
return receiveQueries( state, action );
case 'REMOVE_ITEMS':
const newState = { ...state };
const removedItems = keyBy( toPlainObject( action.items ) );
forEach( newState, ( queryItems, key ) => {
if ( newState[ key ] ) {
newState[ key ] = filter( queryItems, ( queryId ) => {
return ! action.items.includes( queryId );
return !! removedItems[ queryId ] === false;
} );
}
} );
Expand Down
2 changes: 1 addition & 1 deletion packages/core-data/src/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export function* getEntityRecords( kind, name, query = {} ) {

getEntityRecords.shouldInvalidate = ( action, kind, name ) => {
return (
action.type === 'RECEIVE_ITEMS' &&
( action.type === 'RECEIVE_ITEMS' || action.type === 'REMOVE_ITEMS' ) &&
action.invalidateCache &&
kind === action.kind &&
name === action.name
Expand Down

0 comments on commit 9703988

Please sign in to comment.