diff --git a/docs/designers-developers/developers/data/data-core.md b/docs/designers-developers/developers/data/data-core.md index f5e06a7659e038..ad91567b20ab71 100644 --- a/docs/designers-developers/developers/data/data-core.md +++ b/docs/designers-developers/developers/data/data-core.md @@ -140,7 +140,9 @@ _Returns_ # **getEntityRecord** -Returns the Entity's record object by key. +Returns the Entity's record object by key. Returns `null` if the value is not +yet received, undefined if the value entity is known to not exist, or the +entity object if it exists and is received. _Parameters_ @@ -148,10 +150,11 @@ _Parameters_ - _kind_ `string`: Entity kind. - _name_ `string`: Entity name. - _key_ `number`: Record's key +- _query_ `?Object`: Optional query. _Returns_ -- `?Object`: Record. +- `(?Object|undefined)`: Record. # **getEntityRecordChangesByRecord** @@ -336,6 +339,22 @@ _Returns_ - `boolean`: Whether the entity record has edits or not. +# **hasEntityRecords** + +Returns true if records have been received for the given set of parameters, +or false otherwise. + +_Parameters_ + +- _state_ `Object`: State tree +- _kind_ `string`: Entity kind. +- _name_ `string`: Entity name. +- _query_ `?Object`: Optional terms query. + +_Returns_ + +- `boolean`: Whether entity records have been received. + # **hasFetchedAutosaves** Returns true if the REST request for autosaves has completed. diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 5b5cda9580b62f..64288b941a6538 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -353,7 +353,9 @@ _Returns_ # **getEntityRecord** -Returns the Entity's record object by key. +Returns the Entity's record object by key. Returns `null` if the value is not +yet received, undefined if the value entity is known to not exist, or the +entity object if it exists and is received. _Parameters_ @@ -361,10 +363,11 @@ _Parameters_ - _kind_ `string`: Entity kind. - _name_ `string`: Entity name. - _key_ `number`: Record's key +- _query_ `?Object`: Optional query. _Returns_ -- `?Object`: Record. +- `(?Object|undefined)`: Record. # **getEntityRecordChangesByRecord** @@ -549,6 +552,22 @@ _Returns_ - `boolean`: Whether the entity record has edits or not. +# **hasEntityRecords** + +Returns true if records have been received for the given set of parameters, +or false otherwise. + +_Parameters_ + +- _state_ `Object`: State tree +- _kind_ `string`: Entity kind. +- _name_ `string`: Entity name. +- _query_ `?Object`: Optional terms query. + +_Returns_ + +- `boolean`: Whether entity records have been received. + # **hasFetchedAutosaves** Returns true if the REST request for autosaves has completed. diff --git a/packages/core-data/src/queried-data/get-query-parts.js b/packages/core-data/src/queried-data/get-query-parts.js index de8a5311619ddd..3c3f0fb5882c77 100644 --- a/packages/core-data/src/queried-data/get-query-parts.js +++ b/packages/core-data/src/queried-data/get-query-parts.js @@ -6,17 +6,20 @@ import { addQueryArgs } from '@wordpress/url'; /** * Internal dependencies */ -import { withWeakMapCache } from '../utils'; +import { withWeakMapCache, getNormalizedCommaSeparable } from '../utils'; /** * An object of properties describing a specific query. * * @typedef {Object} WPQueriedDataQueryParts * - * @property {number} page The query page (1-based index, default 1). - * @property {number} perPage Items per page for query (default 10). - * @property {string} stableKey An encoded stable string of all non-pagination - * query parameters. + * @property {number} page The query page (1-based index, default 1). + * @property {number} perPage Items per page for query (default 10). + * @property {string} stableKey An encoded stable string of all non- + * pagination, non-fields query parameters. + * @property {?(string[])} fields Target subset of fields to derive from + * item objects. + * @property {?(number[])} include Specific item IDs to include. */ /** @@ -36,6 +39,8 @@ export function getQueryParts( query ) { stableKey: '', page: 1, perPage: 10, + fields: null, + include: null, }; // Ensure stable key by sorting keys. Also more efficient for iterating. @@ -49,10 +54,19 @@ export function getQueryParts( query ) { case 'page': parts[ key ] = Number( value ); break; + case 'per_page': parts.perPage = Number( value ); break; + case 'include': + parts.include = getNormalizedCommaSeparable( value ).map( Number ); + break; + + case '_fields': + parts.fields = getNormalizedCommaSeparable( value ); + break; + default: // While it could be any deterministic string, for simplicity's // sake mimic querystring encoding for stable key. diff --git a/packages/core-data/src/queried-data/reducer.js b/packages/core-data/src/queried-data/reducer.js index 36da4465bb922d..588a35f95a1c06 100644 --- a/packages/core-data/src/queried-data/reducer.js +++ b/packages/core-data/src/queried-data/reducer.js @@ -85,6 +85,46 @@ function items( state = {}, action ) { return state; } +/** + * Reducer tracking item completeness, keyed by ID. A complete item is one for + * which all fields are known. This is used in supporting `_fields` queries, + * where not all properties associated with an entity are necessarily returned. + * In such cases, completeness is used as an indication of whether it would be + * safe to use queried data for a non-`_fields`-limited request. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Next state. + */ +export function itemIsComplete( state = {}, action ) { + const { type, query, key = DEFAULT_ENTITY_KEY } = action; + if ( type !== 'RECEIVE_ITEMS' ) { + return state; + } + + // An item is considered complete if it is received without an associated + // fields query. Ideally, this would be implemented in such a way where the + // complete aggregate of all fields would satisfy completeness. Since the + // fields are not consistent across all entity types, this would require + // introspection on the REST schema for each entity to know which fields + // compose a complete item for that entity. + const isCompleteQuery = ! query || ! Array.isArray( getQueryParts( query ).fields ); + + return { + ...state, + ...action.items.reduce( ( result, item ) => { + const itemId = item[ key ]; + + // Defer to completeness if already assigned. Technically the + // data may be outdated if receiving items for a field subset. + result[ itemId ] = state[ itemId ] || isCompleteQuery; + + return result; + }, {} ), + }; +} + /** * Reducer tracking queries state, keyed by stable query key. Each reducer * query object includes `itemIds` and `requestingPageByPerPage`. @@ -134,5 +174,6 @@ const queries = flowRight( [ export default combineReducers( { items, + itemIsComplete, queries, } ); diff --git a/packages/core-data/src/queried-data/selectors.js b/packages/core-data/src/queried-data/selectors.js index c545f07a434c40..239930e5f9bb07 100644 --- a/packages/core-data/src/queried-data/selectors.js +++ b/packages/core-data/src/queried-data/selectors.js @@ -27,13 +27,21 @@ const queriedItemsCacheByState = new WeakMap(); * @return {?Array} Query items. */ function getQueriedItemsUncached( state, query ) { - const { stableKey, page, perPage } = getQueryParts( query ); + const { stableKey, page, perPage, include, fields } = getQueryParts( query ); - if ( ! state.queries[ stableKey ] ) { - return null; + let itemIds; + if ( Array.isArray( include ) && ! stableKey ) { + // If the parsed query yields a set of IDs, but otherwise no filtering, + // it's safe to consider targeted item IDs as the include set. This + // doesn't guarantee that those objects have been queried, which is + // accounted for below in the loop `null` return. + itemIds = include; + // TODO: Avoid storing the empty stable string in reducer, since it + // can be computed dynamically here always. + } else if ( state.queries[ stableKey ] ) { + itemIds = state.queries[ stableKey ]; } - const itemIds = state.queries[ stableKey ]; if ( ! itemIds ) { return null; } @@ -47,7 +55,43 @@ function getQueriedItemsUncached( state, query ) { const items = []; for ( let i = startOffset; i < endOffset; i++ ) { const itemId = itemIds[ i ]; - items.push( state.items[ itemId ] ); + if ( Array.isArray( include ) && ! include.includes( itemId ) ) { + continue; + } + + if ( ! state.items.hasOwnProperty( itemId ) ) { + return null; + } + + const item = state.items[ itemId ]; + + let filteredItem; + if ( Array.isArray( fields ) ) { + filteredItem = {}; + + for ( let f = 0; f < fields.length; f++ ) { + // Abort the entire request if a field is missing from the item. + // This accounts for the fact that queried items are stored by + // stable key without an associated fields query. Other requests + // may have included fewer fields properties. + const field = fields[ f ]; + if ( ! item.hasOwnProperty( field ) ) { + return null; + } + + filteredItem[ field ] = item[ field ]; + } + } else { + // If expecting a complete item, validate that completeness, or + // otherwise abort. + if ( ! state.itemIsComplete[ itemId ] ) { + return null; + } + + filteredItem = item; + } + + items.push( filteredItem ); } return items; diff --git a/packages/core-data/src/queried-data/test/get-query-parts.js b/packages/core-data/src/queried-data/test/get-query-parts.js index f950a6e61b13ec..c97328dfa057ee 100644 --- a/packages/core-data/src/queried-data/test/get-query-parts.js +++ b/packages/core-data/src/queried-data/test/get-query-parts.js @@ -11,6 +11,32 @@ describe( 'getQueryParts', () => { page: 2, perPage: 2, stableKey: '', + fields: null, + include: null, + } ); + } ); + + it( 'parses out `include` ID filtering', () => { + const parts = getQueryParts( { include: [ 1 ] } ); + + expect( parts ).toEqual( { + page: 1, + perPage: 10, + stableKey: '', + fields: null, + include: [ 1 ], + } ); + } ); + + it( 'parses out `_fields` property filtering', () => { + const parts = getQueryParts( { _fields: 'content', a: 1 } ); + + expect( parts ).toEqual( { + page: 1, + perPage: 10, + stableKey: 'a=1', + fields: [ 'content' ], + include: null, } ); } ); @@ -23,6 +49,8 @@ describe( 'getQueryParts', () => { page: 1, perPage: 10, stableKey: '%3F=%26&b=2', + fields: null, + include: null, } ); } ); @@ -33,6 +61,8 @@ describe( 'getQueryParts', () => { page: 1, perPage: 10, stableKey: 'a%5B0%5D=1&a%5B1%5D=2', + fields: null, + include: null, } ); } ); @@ -45,6 +75,8 @@ describe( 'getQueryParts', () => { page: 1, perPage: 10, stableKey: 'b=2', + fields: null, + include: null, } ); } ); @@ -55,6 +87,8 @@ describe( 'getQueryParts', () => { page: 1, perPage: -1, stableKey: 'b=2', + fields: null, + include: null, } ); } ); } ); diff --git a/packages/core-data/src/queried-data/test/reducer.js b/packages/core-data/src/queried-data/test/reducer.js index 018c0e99803e74..a15c34cc3085fa 100644 --- a/packages/core-data/src/queried-data/test/reducer.js +++ b/packages/core-data/src/queried-data/test/reducer.js @@ -8,6 +8,7 @@ import deepFreeze from 'deep-freeze'; */ import reducer, { getMergedItemIds, + itemIsComplete, } from '../reducer'; describe( 'getMergedItemIds', () => { @@ -89,12 +90,82 @@ describe( 'getMergedItemIds', () => { } ); } ); +describe( 'itemIsComplete', () => { + it( 'should assign received items as complete if no associated query', () => { + const original = deepFreeze( {} ); + const state = itemIsComplete( original, { + type: 'RECEIVE_ITEMS', + items: [ + { id: 1, content: 'chicken', author: 'bob' }, + ], + } ); + + expect( state ).toEqual( { + 1: true, + } ); + } ); + + it( 'should assign received items as complete if non-fields-filtering query', () => { + const original = deepFreeze( {} ); + const state = itemIsComplete( original, { + type: 'RECEIVE_ITEMS', + query: { + per_page: 5, + }, + items: [ + { id: 1, content: 'chicken', author: 'bob' }, + ], + } ); + + expect( state ).toEqual( { + 1: true, + } ); + } ); + + it( 'should assign received items as incomplete if fields-filtering query', () => { + const original = deepFreeze( {} ); + const state = itemIsComplete( original, { + type: 'RECEIVE_ITEMS', + query: { + _fields: 'content', + }, + items: [ + { id: 1, content: 'chicken' }, + ], + } ); + + expect( state ).toEqual( { + 1: false, + } ); + } ); + + it( 'should defer to existing completeness when receiving filtered query', () => { + const original = deepFreeze( { + 1: true, + } ); + const state = itemIsComplete( original, { + type: 'RECEIVE_ITEMS', + query: { + _fields: 'content', + }, + items: [ + { id: 1, content: 'chicken' }, + ], + } ); + + expect( state ).toEqual( { + 1: true, + } ); + } ); +} ); + describe( 'reducer', () => { it( 'returns a default value of its combined keys defaults', () => { const state = reducer( undefined, {} ); expect( state ).toEqual( { items: {}, + itemIsComplete: {}, queries: {}, } ); } ); @@ -103,6 +174,7 @@ describe( 'reducer', () => { const original = deepFreeze( { items: {}, queries: {}, + itemIsComplete: {}, } ); const state = reducer( original, { type: 'RECEIVE_ITEMS', @@ -116,6 +188,9 @@ describe( 'reducer', () => { items: { 1: { id: 1, name: 'abc' }, }, + itemIsComplete: { + 1: true, + }, queries: { 's=a': [ 1 ], }, @@ -126,6 +201,7 @@ describe( 'reducer', () => { const original = deepFreeze( { items: {}, queries: {}, + itemIsComplete: {}, } ); const state = reducer( original, { type: 'RECEIVE_ITEMS', @@ -138,6 +214,9 @@ describe( 'reducer', () => { items: { 1: { id: 1, name: 'abc' }, }, + itemIsComplete: { + 1: true, + }, queries: {}, } ); } ); diff --git a/packages/core-data/src/queried-data/test/selectors.js b/packages/core-data/src/queried-data/test/selectors.js index 4d6b4f59509ff3..32ccd2462fcc18 100644 --- a/packages/core-data/src/queried-data/test/selectors.js +++ b/packages/core-data/src/queried-data/test/selectors.js @@ -7,6 +7,7 @@ describe( 'getQueriedItems', () => { it( 'should return null if requesting but no item IDs', () => { const state = { items: {}, + itemIsComplete: {}, queries: {}, }; @@ -21,6 +22,10 @@ describe( 'getQueriedItems', () => { 1: { id: 1 }, 2: { id: 2 }, }, + itemIsComplete: { + 1: true, + 2: true, + }, queries: { '': [ 1, 2 ], }, @@ -40,6 +45,10 @@ describe( 'getQueriedItems', () => { 1: { id: 1 }, 2: { id: 2 }, }, + itemIsComplete: { + 1: true, + 2: true, + }, queries: [ 1, 2 ], }; @@ -48,4 +57,109 @@ describe( 'getQueriedItems', () => { expect( resultA ).toBe( resultB ); } ); + + it( 'should return items queried by include', () => { + const state = { + items: { + 1: { id: 1 }, + 2: { id: 2 }, + }, + itemIsComplete: { + 1: true, + 2: true, + }, + queries: { + '': [ 1, 2 ], + }, + }; + + const result = getQueriedItems( state, { include: [ 1 ] } ); + + expect( result ).toEqual( [ + { id: 1 }, + ] ); + } ); + + it( 'should dynamically construct fields-filtered item from available data', () => { + const state = { + items: { + 1: { + id: 1, + content: 'chicken', + author: 'bob', + }, + 2: { + id: 2, + content: 'ribs', + author: 'sally', + }, + }, + itemIsComplete: { + 1: true, + 2: true, + }, + queries: { + '': [ 1, 2 ], + }, + }; + + const result = getQueriedItems( state, { _fields: [ 'content' ] } ); + + expect( result ).toEqual( [ + { content: 'chicken' }, + { content: 'ribs' }, + ] ); + } ); + + it( 'should return null if attempting to filter by yet-unknown fields', () => { + const state = { + items: { + 1: { + id: 1, + author: 'bob', + }, + 2: { + id: 2, + author: 'sally', + }, + }, + itemIsComplete: { + 1: false, + 2: false, + }, + queries: { + '': [ 1, 2 ], + }, + }; + + const result = getQueriedItems( state, { _fields: [ 'content' ] } ); + + expect( result ).toBe( null ); + } ); + + it( 'should return null if querying non-filtered data for incomplete item', () => { + const state = { + items: { + 1: { + id: 1, + author: 'bob', + }, + 2: { + id: 2, + author: 'sally', + }, + }, + itemIsComplete: { + 1: false, + 2: false, + }, + queries: { + '': [ 1, 2 ], + }, + }; + + const result = getQueriedItems( state ); + + expect( result ).toBe( null ); + } ); } ); diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 670cb4adf2a750..12f73498e36499 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { find, includes, get, hasIn, compact } from 'lodash'; +import { find, includes, get, hasIn, compact, uniq } from 'lodash'; /** * WordPress dependencies @@ -21,8 +21,9 @@ import { receiveUserPermission, receiveAutosaves, } from './actions'; -import { getKindEntities } from './entities'; -import { apiFetch, resolveSelect } from './controls'; +import { getKindEntities, DEFAULT_ENTITY_KEY } from './entities'; +import { apiFetch, select, resolveSelect } from './controls'; +import { getNormalizedCommaSeparable } from './utils'; /** * Requests authors from the REST API. @@ -43,18 +44,58 @@ export function* getCurrentUser() { /** * Requests an entity's record from the REST API. * - * @param {string} kind Entity kind. - * @param {string} name Entity name. - * @param {number} key Record's key + * @param {string} kind Entity kind. + * @param {string} name Entity name. + * @param {number|string} key Record's key + * @param {Object|undefined} query Optional object of query parameters to + * include with request. */ -export function* getEntityRecord( kind, name, key = '' ) { +export function* getEntityRecord( kind, name, key = '', query ) { const entities = yield getKindEntities( kind ); const entity = find( entities, { kind, name } ); if ( ! entity ) { return; } - const record = yield apiFetch( { path: `${ entity.baseURL }/${ key }?context=edit` } ); - yield receiveEntityRecords( kind, name, record ); + + if ( query !== undefined && query._fields ) { + // If requesting specific fields, items and query assocation to said + // records are stored by ID reference. Thus, fields must always include + // the ID. + query = { + ...query, + _fields: uniq( [ + ...( getNormalizedCommaSeparable( query._fields ) || [] ), + entity.key || DEFAULT_ENTITY_KEY, + ] ).join(), + }; + } + + // Disable reason: While true that an early return could leave `path` + // unused, it's important that path is derived using the query prior to + // additional query modifications in the condition below, since those + // modifications are relevant to how the data is tracked in state, and not + // for how the request is made to the REST API. + + // eslint-disable-next-line @wordpress/no-unused-vars-before-return + const path = addQueryArgs( entity.baseURL + '/' + key, { + ...query, + context: 'edit', + } ); + + if ( query !== undefined ) { + query = { ...query, include: [ key ] }; + + // The resolution cache won't consider query as reusable based on the + // fields, so it's tested here, prior to initiating the REST request, + // and without causing `getEntityRecords` resolution to occur. + const hasRecords = yield select( 'hasEntityRecords', kind, name, query ); + if ( hasRecords ) { + return; + } + } + + const record = yield apiFetch( { path } ); + yield receiveEntityRecords( kind, name, record, query ); } /** @@ -70,6 +111,20 @@ export function* getEntityRecords( kind, name, query = {} ) { if ( ! entity ) { return; } + + if ( query._fields ) { + // If requesting specific fields, items and query assocation to said + // records are stored by ID reference. Thus, fields must always include + // the ID. + query = { + ...query, + _fields: uniq( [ + ...( getNormalizedCommaSeparable( query._fields ) || [] ), + entity.key || DEFAULT_ENTITY_KEY, + ] ).join(), + }; + } + const path = addQueryArgs( entity.baseURL, { ...query, context: 'edit', diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index a4ea47ac8c745e..2e0ae852af60f8 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -2,7 +2,7 @@ * External dependencies */ import createSelector from 'rememo'; -import { map, find, get, filter, compact, defaultTo } from 'lodash'; +import { first, map, find, get, filter, compact, defaultTo } from 'lodash'; /** * WordPress dependencies @@ -94,17 +94,39 @@ export function getEntity( state, kind, name ) { } /** - * Returns the Entity's record object by key. + * Returns the Entity's record object by key. Returns `null` if the value is not + * yet received, undefined if the value entity is known to not exist, or the + * entity object if it exists and is received. * - * @param {Object} state State tree - * @param {string} kind Entity kind. - * @param {string} name Entity name. - * @param {number} key Record's key + * @param {Object} state State tree + * @param {string} kind Entity kind. + * @param {string} name Entity name. + * @param {number} key Record's key + * @param {?Object} query Optional query. * - * @return {Object?} Record. + * @return {Object?|undefined} Record. */ -export function getEntityRecord( state, kind, name, key ) { - return get( state.entities.data, [ kind, name, 'queriedData', 'items', key ] ); +export function getEntityRecord( state, kind, name, key, query ) { + // Queried data state is prepopulated for all known entities. If this is not + // assigned for the given parameters, then it is known to not exist. Thus, a + // return value of `undefined` is used instead of `null` (where `null` is + // otherwise used to represent an unknown state). + const queriedState = get( state.entities.data, [ kind, name, 'queriedData' ] ); + if ( ! queriedState ) { + return; + } + + if ( query === undefined ) { + // If expecting a complete item, validate that completeness. + if ( ! queriedState.itemIsComplete[ key ] ) { + return null; + } + + return queriedState.items[ key ] || null; + } + + query = { ...query, include: [ key ] }; + return first( getQueriedItems( queriedState, query ) ) || null; } /** @@ -135,17 +157,36 @@ export const getRawEntityRecord = createSelector( ( state ) => [ state.entities.data ] ); +/** + * Returns true if records have been received for the given set of parameters, + * or false otherwise. + * + * @param {Object} state State tree + * @param {string} kind Entity kind. + * @param {string} name Entity name. + * @param {?Object} query Optional terms query. + * + * @return {boolean} Whether entity records have been received. + */ +export function hasEntityRecords( state, kind, name, query ) { + return Array.isArray( getEntityRecords( state, kind, name, query ) ); +} + /** * Returns the Entity's records. * - * @param {Object} state State tree - * @param {string} kind Entity kind. - * @param {string} name Entity name. - * @param {?Object} query Optional terms query. + * @param {Object} state State tree + * @param {string} kind Entity kind. + * @param {string} name Entity name. + * @param {?Object} query Optional terms query. * * @return {Array} Records. */ export function getEntityRecords( state, kind, name, query ) { + // Queried data state is prepopulated for all known entities. If this is not + // assigned for the given parameters, then it is known to not exist. Thus, a + // return value of an empty array is used instead of `null` (where `null` is + // otherwise used to represent an unknown state). const queriedState = get( state.entities.data, [ kind, name, 'queriedData' ] ); if ( ! queriedState ) { return []; diff --git a/packages/core-data/src/test/reducer.js b/packages/core-data/src/test/reducer.js index c585e4f4415a91..0cbb71ba14a9fb 100644 --- a/packages/core-data/src/test/reducer.js +++ b/packages/core-data/src/test/reducer.js @@ -39,10 +39,14 @@ describe( 'terms()', () => { } ); describe( 'entities', () => { + // See also unit tests at `queried-data/test/reducer.js`, which are more + // thorough in testing the behavior of what is tracked here as the + // `queriedData` property on a kind/name nested object pair. + it( 'returns the default state for all defined entities', () => { const state = entities( undefined, {} ); - expect( state.data.root.postType.queriedData ).toEqual( { items: {}, queries: {} } ); + expect( state.data.root.postType.queriedData ).toEqual( { items: {}, itemIsComplete: {}, queries: {} } ); } ); it( 'returns with received post types by slug', () => { @@ -59,6 +63,10 @@ describe( 'entities', () => { b: { slug: 'b', title: 'beach' }, s: { slug: 's', title: 'sun' }, }, + itemIsComplete: { + b: true, + s: true, + }, queries: {}, } ); } ); @@ -72,6 +80,9 @@ describe( 'entities', () => { items: { w: { slug: 'w', title: 'water' }, }, + itemIsComplete: { + w: true, + }, queries: {}, }, }, @@ -90,6 +101,10 @@ describe( 'entities', () => { w: { slug: 'w', title: 'water' }, b: { slug: 'b', title: 'beach' }, }, + itemIsComplete: { + w: true, + b: true, + }, queries: {}, } ); } ); diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index ae75557d40f1a9..1004423c732e8c 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -8,6 +8,7 @@ import deepFreeze from 'deep-freeze'; */ import { getEntityRecord, + hasEntityRecords, getEntityRecords, getEntityRecordChangesByRecord, getEntityRecordNonTransientEdits, @@ -21,7 +22,7 @@ import { } from '../selectors'; describe( 'getEntityRecord', () => { - it( 'should return undefined for unknown record’s key', () => { + it( 'should return undefined for unknown entity kind, name', () => { const state = deepFreeze( { entities: { data: { @@ -29,6 +30,7 @@ describe( 'getEntityRecord', () => { postType: { queriedData: { items: {}, + itemIsComplete: {}, queries: {}, }, }, @@ -36,7 +38,26 @@ describe( 'getEntityRecord', () => { }, }, } ); - expect( getEntityRecord( state, 'root', 'postType', 'post' ) ).toBe( undefined ); + expect( getEntityRecord( state, 'foo', 'bar', 'baz' ) ).toBe( undefined ); + } ); + + it( 'should return null for unknown record’s key', () => { + const state = deepFreeze( { + entities: { + data: { + root: { + postType: { + queriedData: { + items: {}, + itemIsComplete: {}, + queries: {}, + }, + }, + }, + }, + }, + } ); + expect( getEntityRecord( state, 'root', 'postType', 'post' ) ).toBe( null ); } ); it( 'should return a record by key', () => { @@ -49,6 +70,9 @@ describe( 'getEntityRecord', () => { items: { post: { slug: 'post' }, }, + itemIsComplete: { + post: true, + }, queries: {}, }, }, @@ -58,10 +82,103 @@ describe( 'getEntityRecord', () => { } ); expect( getEntityRecord( state, 'root', 'postType', 'post' ) ).toEqual( { slug: 'post' } ); } ); + + it( 'should return null if no item received, filtered item requested', () => { + } ); + + it( 'should return filtered item if incomplete item received, filtered item requested', () => { + } ); + + it( 'should return null if incomplete item received, complete item requested', () => { + + } ); + + it( 'should return filtered item if complete item received, filtered item requested', () => { + const state = deepFreeze( { + entities: { + data: { + postType: { + post: { + queriedData: { + items: { + 1: { id: 1, content: 'chicken', author: 'bob' }, + }, + itemIsComplete: { + 1: true, + }, + queries: {}, + }, + }, + }, + }, + }, + } ); + expect( getEntityRecord( state, 'postType', 'post', 1, { _fields: 'content' } ) ).toEqual( { content: 'chicken' } ); + } ); +} ); + +describe( 'hasEntityRecords', () => { + it( 'returns false if entity records have not been received', () => { + const state = deepFreeze( { + entities: { + data: { + root: { + postType: { + queriedData: { + items: {}, + itemIsComplete: {}, + queries: {}, + }, + }, + }, + }, + }, + } ); + + expect( hasEntityRecords( state, 'root', 'postType' ) ).toBe( false ); + } ); + + it( 'returns true if the entity configuration is not known', () => { + const state = deepFreeze( { + entities: { + data: {}, + }, + } ); + + expect( hasEntityRecords( state, 'root', 'postType' ) ).toBe( true ); + } ); + + it( 'returns true if entity records have been received', () => { + const state = deepFreeze( { + entities: { + data: { + root: { + postType: { + queriedData: { + items: { + post: { slug: 'post' }, + page: { slug: 'page' }, + }, + itemIsComplete: { + post: true, + page: true, + }, + queries: { + '': [ 'post', 'page' ], + }, + }, + }, + }, + }, + }, + } ); + + expect( hasEntityRecords( state, 'root', 'postType' ) ).toBe( true ); + } ); } ); describe( 'getEntityRecords', () => { - it( 'should return an null by default', () => { + it( 'should return null by default', () => { const state = deepFreeze( { entities: { data: { @@ -69,6 +186,7 @@ describe( 'getEntityRecords', () => { postType: { queriedData: { items: {}, + itemIsComplete: {}, queries: {}, }, }, @@ -79,6 +197,16 @@ describe( 'getEntityRecords', () => { expect( getEntityRecords( state, 'root', 'postType' ) ).toBe( null ); } ); + it( 'should return an empty array for an unknown entity configuration', () => { + const state = deepFreeze( { + entities: { + data: {}, + }, + } ); + + expect( getEntityRecords( state, 'root', 'postType' ) ).toEqual( [] ); + } ); + it( 'should return all the records', () => { const state = deepFreeze( { entities: { @@ -90,6 +218,10 @@ describe( 'getEntityRecords', () => { post: { slug: 'post' }, page: { slug: 'page' }, }, + itemIsComplete: { + post: true, + page: true, + }, queries: { '': [ 'post', 'page' ], }, @@ -127,6 +259,9 @@ describe( 'getEntityRecordChangesByRecord', () => { someRawProperty: { raw: 'somePersistedRawValue' }, }, }, + itemIsComplete: { + someKey: true, + }, }, edits: { someKey: { diff --git a/packages/core-data/src/utils/conservative-map-item.js b/packages/core-data/src/utils/conservative-map-item.js index 370c3cf0861fd6..629347247bd1ff 100644 --- a/packages/core-data/src/utils/conservative-map-item.js +++ b/packages/core-data/src/utils/conservative-map-item.js @@ -33,5 +33,15 @@ export default function conservativeMapItem( item, nextItem ) { if ( ! hasChanges ) { return item; } + + // Only at this point, backfill properties from the original item which + // weren't explicitly set into the result above. This is an optimization + // to allow `hasChanges` to return early. + for ( const key in item ) { + if ( ! result.hasOwnProperty( key ) ) { + result[ key ] = item[ key ]; + } + } + return result; } diff --git a/packages/core-data/src/utils/get-normalized-comma-separable.js b/packages/core-data/src/utils/get-normalized-comma-separable.js new file mode 100644 index 00000000000000..62ff206fa59760 --- /dev/null +++ b/packages/core-data/src/utils/get-normalized-comma-separable.js @@ -0,0 +1,20 @@ +/** + * Given a value which can be specified as one or the other of a comma-separated + * string or an array, returns a value normalized to an array of strings, or + * null if the value cannot be interpreted as either. + * + * @param {string|string[]|*} value + * + * @return {?(string[])} Normalized field value. + */ +function getNormalizedCommaSeparable( value ) { + if ( typeof value === 'string' ) { + return value.split( ',' ); + } else if ( Array.isArray( value ) ) { + return value; + } + + return null; +} + +export default getNormalizedCommaSeparable; diff --git a/packages/core-data/src/utils/index.js b/packages/core-data/src/utils/index.js index 7adb57e48d5d7f..77eaa730630760 100644 --- a/packages/core-data/src/utils/index.js +++ b/packages/core-data/src/utils/index.js @@ -1,4 +1,5 @@ export { default as conservativeMapItem } from './conservative-map-item'; +export { default as getNormalizedCommaSeparable } from './get-normalized-comma-separable'; export { default as ifMatchingAction } from './if-matching-action'; export { default as onSubKey } from './on-sub-key'; export { default as replaceAction } from './replace-action'; diff --git a/packages/core-data/src/utils/test/conservative-map-item.js b/packages/core-data/src/utils/test/conservative-map-item.js index 5f42cc73fdde78..dd63fb218f46d8 100644 --- a/packages/core-data/src/utils/test/conservative-map-item.js +++ b/packages/core-data/src/utils/test/conservative-map-item.js @@ -30,4 +30,15 @@ describe( 'conservativeMapItem', () => { expect( result.b ).toBe( nextItem.b ); expect( result ).toEqual( { a: [ {} ], b: [ 2 ] } ); } ); + + it( 'merges to the original item', () => { + const item = { a: [ 1 ], b: [ 2 ] }; + const nextItem = { c: [ 3 ], d: [ 4 ] }; + const result = conservativeMapItem( item, nextItem ); + + expect( result ).not.toBe( item ); + expect( result.a ).toBe( item.a ); + expect( result.b ).toBe( item.b ); + expect( result ).toEqual( { a: [ 1 ], b: [ 2 ], c: [ 3 ], d: [ 4 ] } ); + } ); } ); diff --git a/packages/core-data/src/utils/test/get-normalized-comma-separable.js b/packages/core-data/src/utils/test/get-normalized-comma-separable.js new file mode 100644 index 00000000000000..3e0a662b8f47cc --- /dev/null +++ b/packages/core-data/src/utils/test/get-normalized-comma-separable.js @@ -0,0 +1,24 @@ +/** + * Internal dependencies + */ +import getNormalizedCommaSeparable from '../get-normalized-comma-separable'; + +describe( 'getNormalizedCommaSeparable', () => { + it( 'returns a given array verbatim', () => { + const result = getNormalizedCommaSeparable( [ 'a', 'b' ] ); + + expect( result ).toEqual( [ 'a', 'b' ] ); + } ); + + it( 'returns a given string as an array of comma-separated parts', () => { + const result = getNormalizedCommaSeparable( 'a,b' ); + + expect( result ).toEqual( [ 'a', 'b' ] ); + } ); + + it( 'returns null if not an array or comma-separated string', () => { + const result = getNormalizedCommaSeparable( 10 ); + + expect( result ).toBe( null ); + } ); +} );