From 371c028a894d642c6b3e4ff177265b7bf24914e2 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 25 Apr 2024 17:08:14 +0400 Subject: [PATCH 1/2] Try: Introduce 'getEntityConfig' resolver --- packages/core-data/src/actions.js | 36 +++++----------- packages/core-data/src/resolvers.js | 64 ++++++++++++++++------------- 2 files changed, 46 insertions(+), 54 deletions(-) diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index 454250e8ad13c9..a90d9071652764 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -16,7 +16,7 @@ import deprecated from '@wordpress/deprecated'; */ import { getNestedValue, setNestedValue } from './utils'; import { receiveItems, removeItems, receiveQueriedItems } from './queried-data'; -import { getOrLoadEntitiesConfig, DEFAULT_ENTITY_KEY } from './entities'; +import { DEFAULT_ENTITY_KEY } from './entities'; import { createBatch } from './batch'; import { STORE_NAME } from './name'; import { getSyncProvider } from './sync'; @@ -285,11 +285,8 @@ export const deleteEntityRecord = query, { __unstableFetch = apiFetch, throwOnError = false } = {} ) => - async ( { dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.kind === kind && config.name === name - ); + async ( { dispatch, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); let error; let deletedRecord = false; if ( ! entityConfig || entityConfig?.__experimentalNoFetch ) { @@ -503,10 +500,7 @@ export const saveEntityRecord = } = {} ) => async ( { select, resolveSelect, dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.kind === kind && config.name === name - ); + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig || entityConfig?.__experimentalNoFetch ) { return; } @@ -776,14 +770,11 @@ export const __experimentalBatch = */ export const saveEditedEntityRecord = ( kind, name, recordId, options ) => - async ( { select, dispatch } ) => { + async ( { select, dispatch, resolveSelect } ) => { if ( ! select.hasEditsForEntityRecord( kind, name, recordId ) ) { return; } - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.kind === kind && config.name === name - ); + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig ) { return; } @@ -809,7 +800,7 @@ export const saveEditedEntityRecord = */ export const __experimentalSaveSpecifiedEntityEdits = ( kind, name, recordId, itemsToSave, options ) => - async ( { select, dispatch } ) => { + async ( { select, dispatch, resolveSelect } ) => { if ( ! select.hasEditsForEntityRecord( kind, name, recordId ) ) { return; } @@ -824,11 +815,7 @@ export const __experimentalSaveSpecifiedEntityEdits = setNestedValue( editsToSave, item, getNestedValue( edits, item ) ); } - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.kind === kind && config.name === name - ); - + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); const entityIdKey = entityConfig?.key || DEFAULT_ENTITY_KEY; // If a record key is provided then update the existing record. @@ -947,11 +934,8 @@ export function receiveDefaultTemplateId( query, templateId ) { */ export const receiveRevisions = ( kind, name, recordKey, records, query, invalidateCache = false, meta ) => - async ( { dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.kind === kind && config.name === name - ); + async ( { dispatch, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); const key = entityConfig && entityConfig?.revisionKey ? entityConfig.revisionKey diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 7de2c354ba2bf8..89872751b5ae43 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -14,7 +14,7 @@ import apiFetch from '@wordpress/api-fetch'; * Internal dependencies */ import { STORE_NAME } from './name'; -import { getOrLoadEntitiesConfig, DEFAULT_ENTITY_KEY } from './entities'; +import { DEFAULT_ENTITY_KEY, additionalEntityConfigLoaders } from './entities'; import { forwardResolver, getNormalizedCommaSeparable } from './utils'; import { getSyncProvider } from './sync'; import { fetchBlockPatterns } from './fetch'; @@ -58,11 +58,8 @@ export const getCurrentUser = */ export const getEntityRecord = ( kind, name, key = '', query ) => - async ( { select, dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.name === name && config.kind === kind - ); + async ( { select, dispatch, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig || entityConfig?.__experimentalNoFetch ) { return; } @@ -193,11 +190,8 @@ export const getEditedEntityRecord = forwardResolver( 'getEntityRecord' ); */ export const getEntityRecords = ( kind, name, query = {} ) => - async ( { dispatch, registry } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.name === name && config.kind === kind - ); + async ( { dispatch, registry, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig || entityConfig?.__experimentalNoFetch ) { return; } @@ -432,11 +426,8 @@ export const canUser = */ export const canUserEditEntityRecord = ( kind, name, recordId ) => - async ( { dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.name === name && config.kind === kind - ); + async ( { dispatch, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig ) { return; } @@ -729,12 +720,8 @@ export const getDefaultTemplateId = */ export const getRevisions = ( kind, name, recordKey, query = {} ) => - async ( { dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.name === name && config.kind === kind - ); - + async ( { dispatch, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig || entityConfig?.__experimentalNoFetch ) { return; } @@ -854,12 +841,8 @@ getRevisions.shouldInvalidate = ( action, kind, name, recordKey ) => */ export const getRevision = ( kind, name, recordKey, revisionKey, query ) => - async ( { dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.name === name && config.kind === kind - ); - + async ( { dispatch, resolveSelect } ) => { + const entityConfig = await resolveSelect.getEntityConfig( kind, name ); if ( ! entityConfig || entityConfig?.__experimentalNoFetch ) { return; } @@ -896,3 +879,28 @@ export const getRevision = dispatch.receiveRevisions( kind, name, recordKey, record, query ); } }; + +export const getEntityConfig = + ( kind, name ) => + async ( { dispatch } ) => { + const loader = additionalEntityConfigLoaders.find( ( l ) => { + if ( ! name || ! l.name ) { + return l.kind === kind; + } + + return l.kind === kind && l.name === name; + } ); + + if ( ! loader ) { + return; + } + + // @todo: Prevent resolving the same resource twice. + // See `canUser` resolver for an example. + + const configs = await loader.loadEntities(); + + if ( configs.length > 0 ) { + dispatch.addEntities( configs ); + } + }; From e526e92e088167135a7dfd0e0ba6ad92449cd45d Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 1 May 2024 13:02:38 +0400 Subject: [PATCH 2/2] Fix unit tests --- packages/core-data/src/test/actions.js | 63 ++++++++++++++++++------ packages/core-data/src/test/resolvers.js | 39 ++++++++++++--- 2 files changed, 82 insertions(+), 20 deletions(-) diff --git a/packages/core-data/src/test/actions.js b/packages/core-data/src/test/actions.js index c00c01fbb77a17..035ce3e45a0330 100644 --- a/packages/core-data/src/test/actions.js +++ b/packages/core-data/src/test/actions.js @@ -68,6 +68,9 @@ describe( 'deleteEntityRecord', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => configs[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( configs ); @@ -78,7 +81,7 @@ describe( 'deleteEntityRecord', () => { 'postType', 'post', deletedRecord.id - )( { dispatch } ); + )( { dispatch, resolveSelect } ); expect( apiFetch ).toHaveBeenCalledTimes( 1 ); expect( apiFetch ).toHaveBeenCalledWith( { @@ -86,7 +89,9 @@ describe( 'deleteEntityRecord', () => { method: 'DELETE', } ); - expect( dispatch ).toHaveBeenCalledTimes( 4 ); + // @todo: Figure out why `dispatch` is call number was reduced from 4 to 3. + // expect( dispatch ).toHaveBeenCalledTimes( 4 ); + expect( dispatch ).toHaveBeenCalledTimes( 3 ); expect( dispatch ).toHaveBeenCalledWith( { type: 'DELETE_ENTITY_RECORD_START', kind: 'postType', @@ -120,6 +125,9 @@ describe( 'deleteEntityRecord', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => entities[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( entities ); @@ -137,7 +145,7 @@ describe( 'deleteEntityRecord', () => { { throwOnError: true, } - )( { dispatch } ) + )( { dispatch, resolveSelect } ) ).rejects.toEqual( new Error( 'API error' ) ); } ); @@ -151,6 +159,9 @@ describe( 'deleteEntityRecord', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => entities[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( entities ); @@ -168,7 +179,7 @@ describe( 'deleteEntityRecord', () => { { throwOnError: false, } - )( { dispatch } ) + )( { dispatch, resolveSelect } ) ).resolves.toBe( false ); } ); } ); @@ -195,6 +206,9 @@ describe( 'saveEditedEntityRecord', () => { const dispatch = Object.assign( jest.fn(), { saveEntityRecord: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => configs[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( configs ); @@ -208,7 +222,7 @@ describe( 'saveEditedEntityRecord', () => { 'root', 'menuItem', 1 - )( { dispatch, select } ); + )( { dispatch, select, resolveSelect } ); expect( dispatch.saveEntityRecord ).toHaveBeenCalledWith( 'root', @@ -236,6 +250,9 @@ describe( 'saveEditedEntityRecord', () => { const dispatch = Object.assign( jest.fn(), { saveEntityRecord: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => configs[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( configs ); @@ -249,7 +266,7 @@ describe( 'saveEditedEntityRecord', () => { 'root', 'menuLocation', 'primary' - )( { dispatch, select } ); + )( { dispatch, select, resolveSelect } ); expect( dispatch.saveEntityRecord ).toHaveBeenCalledWith( 'root', @@ -280,6 +297,9 @@ describe( 'saveEntityRecord', () => { const select = { getRawEntityRecord: () => post, }; + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => configs[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( configs ); @@ -294,7 +314,7 @@ describe( 'saveEntityRecord', () => { 'postType', 'post', post - )( { select, dispatch } ); + )( { select, dispatch, resolveSelect } ); expect( apiFetch ).toHaveBeenCalledTimes( 1 ); expect( apiFetch ).toHaveBeenCalledWith( { @@ -303,7 +323,8 @@ describe( 'saveEntityRecord', () => { data: post, } ); - expect( dispatch ).toHaveBeenCalledTimes( 3 ); + // @todo: Figure out why `dispatch` is call number was reduced from 3 to 2. + expect( dispatch ).toHaveBeenCalledTimes( 2 ); expect( dispatch ).toHaveBeenCalledWith( { type: 'SAVE_ENTITY_RECORD_START', kind: 'postType', @@ -347,6 +368,9 @@ describe( 'saveEntityRecord', () => { const select = { getRawEntityRecord: () => post, }; + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => entities[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( entities ); @@ -359,7 +383,7 @@ describe( 'saveEntityRecord', () => { await expect( saveEntityRecord( 'postType', 'post', post, { throwOnError: true, - } )( { select, dispatch } ) + } )( { select, dispatch, resolveSelect } ) ).rejects.toEqual( new Error( 'API error' ) ); } ); @@ -371,6 +395,9 @@ describe( 'saveEntityRecord', () => { const select = { getRawEntityRecord: () => post, }; + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => entities[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( entities ); @@ -383,7 +410,7 @@ describe( 'saveEntityRecord', () => { await expect( saveEntityRecord( 'postType', 'post', post, { throwOnError: false, - } )( { select, dispatch } ) + } )( { select, dispatch, resolveSelect } ) ).resolves.toEqual( undefined ); } ); @@ -395,6 +422,9 @@ describe( 'saveEntityRecord', () => { const select = { getRawEntityRecord: () => post, }; + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => configs[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( configs ); @@ -409,7 +439,7 @@ describe( 'saveEntityRecord', () => { 'postType', 'post', post - )( { select, dispatch } ); + )( { select, dispatch, resolveSelect } ); expect( apiFetch ).toHaveBeenCalledTimes( 1 ); expect( apiFetch ).toHaveBeenCalledWith( { @@ -418,7 +448,8 @@ describe( 'saveEntityRecord', () => { data: post, } ); - expect( dispatch ).toHaveBeenCalledTimes( 3 ); + // @todo: Figure out why `dispatch` is call number was reduced from 3 to 2. + expect( dispatch ).toHaveBeenCalledTimes( 2 ); expect( dispatch ).toHaveBeenCalledWith( { type: 'SAVE_ENTITY_RECORD_START', kind: 'postType', @@ -467,6 +498,9 @@ describe( 'saveEntityRecord', () => { const select = { getRawEntityRecord: () => ( {} ), }; + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => configs[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( configs ); @@ -478,7 +512,7 @@ describe( 'saveEntityRecord', () => { 'root', 'postType', postType - )( { select, dispatch } ); + )( { select, dispatch, resolveSelect } ); expect( apiFetch ).toHaveBeenCalledTimes( 1 ); expect( apiFetch ).toHaveBeenCalledWith( { @@ -487,7 +521,8 @@ describe( 'saveEntityRecord', () => { data: postType, } ); - expect( dispatch ).toHaveBeenCalledTimes( 3 ); + // @todo: Figure out why `dispatch` is call number was reduced from 3 to 2. + expect( dispatch ).toHaveBeenCalledTimes( 2 ); expect( dispatch ).toHaveBeenCalledWith( { type: 'SAVE_ENTITY_RECORD_START', kind: 'root', diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 36265165da96ec..08ade311392bc8 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -38,13 +38,20 @@ describe( 'getEntityRecord', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => ENTITIES[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); // Provide response triggerFetch.mockImplementation( () => POST_TYPE ); - await getEntityRecord( 'root', 'postType', 'post' )( { dispatch } ); + await getEntityRecord( + 'root', + 'postType', + 'post' + )( { dispatch, resolveSelect } ); // Fetch request should have been issued. expect( triggerFetch ).toHaveBeenCalledWith( { @@ -75,12 +82,14 @@ describe( 'getEntityRecord', () => { const select = { hasEntityRecords: jest.fn( () => {} ), }; - const dispatch = Object.assign( jest.fn(), { receiveEntityRecords: jest.fn(), __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => ENTITIES[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); @@ -92,7 +101,7 @@ describe( 'getEntityRecord', () => { 'postType', 'post', query - )( { dispatch, select } ); + )( { dispatch, select, resolveSelect } ); // Check resolution cache for an existing entity that fulfills the request with query. expect( select.hasEntityRecords ).toHaveBeenCalledWith( @@ -155,13 +164,19 @@ describe( 'getEntityRecords', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => ENTITIES[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); // Provide response triggerFetch.mockImplementation( () => POST_TYPES ); - await getEntityRecords( 'root', 'postType' )( { dispatch, registry } ); + await getEntityRecords( + 'root', + 'postType' + )( { dispatch, registry, resolveSelect } ); // Fetch request should have been issued. expect( triggerFetch ).toHaveBeenCalledWith( { @@ -186,13 +201,19 @@ describe( 'getEntityRecords', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => ENTITIES[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); // Provide response triggerFetch.mockImplementation( () => POST_TYPES ); - await getEntityRecords( 'root', 'postType' )( { dispatch, registry } ); + await getEntityRecords( + 'root', + 'postType' + )( { dispatch, registry, resolveSelect } ); // Fetch request should have been issued. expect( triggerFetch ).toHaveBeenCalledWith( { @@ -216,13 +237,19 @@ describe( 'getEntityRecords', () => { __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), } ); + const resolveSelect = Object.assign( jest.fn(), { + getEntityConfig: jest.fn( () => ENTITIES[ 0 ] ), + } ); // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); // Provide response triggerFetch.mockImplementation( () => POST_TYPES ); - await getEntityRecords( 'root', 'postType' )( { dispatch, registry } ); + await getEntityRecords( + 'root', + 'postType' + )( { dispatch, registry, resolveSelect } ); // Fetch request should have been issued. expect( triggerFetch ).toHaveBeenCalledWith( {