From 06f2447845608efccd6046bbb29bcc60ea51f1fd Mon Sep 17 00:00:00 2001 From: Adam Zielinski Date: Wed, 1 Sep 2021 17:49:06 +0200 Subject: [PATCH] Refactor deleteEntityRecord to use thunks instead of generators (#34386) * Refactor deleteEntityRecord to use thunks instead of generators * Remove await from two synchronous dispatch calls * Adjust tests --- docs/reference-guides/data/data-core.md | 2 +- packages/core-data/README.md | 2 +- packages/core-data/src/actions.js | 45 ++++++---------- packages/core-data/src/test/actions.js | 68 +++++++++++++++---------- 4 files changed, 60 insertions(+), 57 deletions(-) diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index cfecc33470ab6..ce93ad2aedea7 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -535,7 +535,7 @@ _Parameters_ - _recordId_ `string`: Record ID of the deleted entity. - _query_ `?Object`: Special query parameters for the DELETE API call. - _options_ `[Object]`: Delete options. -- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a promise. ### editEntityRecord diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 4e906e79bb1be..82754d2cbb711 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -69,7 +69,7 @@ _Parameters_ - _recordId_ `string`: Record ID of the deleted entity. - _query_ `?Object`: Special query parameters for the DELETE API call. - _options_ `[Object]`: Delete options. -- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a promise. ### editEntityRecord diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index 49c624977e48d..d06bc6d9d391c 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -8,7 +8,7 @@ import { v4 as uuid } from 'uuid'; * WordPress dependencies */ import { controls } from '@wordpress/data'; -import { apiFetch, __unstableAwaitPromise } from '@wordpress/data-controls'; +import { __unstableAwaitPromise } from '@wordpress/data-controls'; import triggerFetch from '@wordpress/api-fetch'; import { addQueryArgs } from '@wordpress/url'; @@ -162,16 +162,16 @@ export function receiveEmbedPreview( url, preview ) { * @param {Object} [options] Delete options. * @param {Function} [options.__unstableFetch] Internal use only. Function to * call instead of `apiFetch()`. - * Must return a control descriptor. + * Must return a promise. */ -export function* deleteEntityRecord( +export const deleteEntityRecord = ( kind, name, recordId, query, - { __unstableFetch = null } = {} -) { - const entities = yield getKindEntities( kind ); + { __unstableFetch = triggerFetch } = {} +) => async ( { dispatch } ) => { + const entities = await dispatch( getKindEntities( kind ) ); const entity = find( entities, { kind, name } ); let error; let deletedRecord = false; @@ -179,21 +179,19 @@ export function* deleteEntityRecord( return; } - const lock = yield controls.dispatch( - STORE_NAME, - '__unstableAcquireStoreLock', + const lock = await dispatch.__unstableAcquireStoreLock( STORE_NAME, [ 'entities', 'data', kind, name, recordId ], { exclusive: true } ); try { - yield { + dispatch( { type: 'DELETE_ENTITY_RECORD_START', kind, name, recordId, - }; + } ); try { let path = `${ entity.baseURL }/${ recordId }`; @@ -202,40 +200,29 @@ export function* deleteEntityRecord( path = addQueryArgs( path, query ); } - const options = { + deletedRecord = await __unstableFetch( { path, method: 'DELETE', - }; - if ( __unstableFetch ) { - deletedRecord = yield __unstableAwaitPromise( - __unstableFetch( options ) - ); - } else { - deletedRecord = yield apiFetch( options ); - } + } ); - yield removeItems( kind, name, recordId, true ); + await dispatch( removeItems( kind, name, recordId, true ) ); } catch ( _error ) { error = _error; } - yield { + dispatch( { type: 'DELETE_ENTITY_RECORD_FINISH', kind, name, recordId, error, - }; + } ); return deletedRecord; } finally { - yield controls.dispatch( - STORE_NAME, - '__unstableReleaseStoreLock', - lock - ); + dispatch.__unstableReleaseStoreLock( lock ); } -} +}; /** * Returns an action object that triggers an diff --git a/packages/core-data/src/test/actions.js b/packages/core-data/src/test/actions.js index 302645687391d..9b242cadcf862 100644 --- a/packages/core-data/src/test/actions.js +++ b/packages/core-data/src/test/actions.js @@ -49,46 +49,62 @@ describe( 'editEntityRecord', () => { } ); describe( 'deleteEntityRecord', () => { + beforeEach( async () => { + apiFetch.mockReset(); + jest.useFakeTimers(); + } ); + it( 'triggers a DELETE request for an existing record', async () => { - const post = 10; + const deletedRecord = { title: 'new post', id: 10 }; const entities = [ { name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' }, ]; - const fulfillment = deleteEntityRecord( 'postType', 'post', post ); - // Trigger generator - fulfillment.next(); + const dispatch = Object.assign( jest.fn(), { + receiveEntityRecords: jest.fn(), + __unstableAcquireStoreLock: jest.fn(), + __unstableReleaseStoreLock: jest.fn(), + } ); + // Provide entities + dispatch.mockReturnValueOnce( entities ); - // Acquire lock - expect( fulfillment.next( entities ).value.type ).toBe( - '@@data/DISPATCH' - ); + // Provide response + apiFetch.mockImplementation( () => deletedRecord ); - // Start - expect( fulfillment.next().value.type ).toEqual( - 'DELETE_ENTITY_RECORD_START' - ); + const result = await deleteEntityRecord( + 'postType', + 'post', + deletedRecord.id + )( { dispatch } ); - // delete api call - const { value: apiFetchAction } = fulfillment.next(); - expect( apiFetchAction.request ).toEqual( { + expect( apiFetch ).toHaveBeenCalledTimes( 1 ); + expect( apiFetch ).toHaveBeenCalledWith( { path: '/wp/v2/posts/10', method: 'DELETE', } ); - expect( fulfillment.next().value.type ).toBe( 'REMOVE_ITEMS' ); - - expect( fulfillment.next().value.type ).toBe( - 'DELETE_ENTITY_RECORD_FINISH' + expect( dispatch ).toHaveBeenCalledTimes( 4 ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'DELETE_ENTITY_RECORD_START', + kind: 'postType', + name: 'post', + recordId: 10, + } ); + expect( dispatch ).toHaveBeenCalledWith( { + type: 'DELETE_ENTITY_RECORD_FINISH', + kind: 'postType', + name: 'post', + recordId: 10, + error: undefined, + } ); + expect( dispatch.__unstableAcquireStoreLock ).toHaveBeenCalledTimes( + 1 + ); + expect( dispatch.__unstableReleaseStoreLock ).toHaveBeenCalledTimes( + 1 ); - // Release lock - expect( fulfillment.next().value.type ).toEqual( '@@data/DISPATCH' ); - - expect( fulfillment.next() ).toMatchObject( { - done: true, - value: undefined, - } ); + expect( result ).toBe( deletedRecord ); } ); } );