Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core-data] throwOnError option for saveEntityRecord and deleteEntityRecord actions #39258

Merged
merged 14 commits into from
Mar 16, 2022
Merged
8 changes: 5 additions & 3 deletions docs/reference-guides/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -561,12 +561,13 @@ Action triggered to delete an entity record.

_Parameters_

- _kind_ `string`: Kind of the deleted entity record.
- _name_ `string`: Name of the deleted entity record.
- _recordId_ `string`: Record ID of the deleted entity record.
- _kind_ `string`: Kind of the deleted entity.
- _name_ `string`: Name of the deleted entity.
- _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 promise.
- _options.throwOnError_ `[boolean]`: If false, this action suppresses all the exceptions. Defaults to false.

### editEntityRecord

Expand Down Expand Up @@ -734,6 +735,7 @@ _Parameters_
- _options_ `Object`: Saving options.
- _options.isAutosave_ `[boolean]`: Whether this is an autosave.
- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a promise.
- _options.throwOnError_ `[boolean]`: If false, this action suppresses all the exceptions. Defaults to false.

### undo

Expand Down
3 changes: 3 additions & 0 deletions packages/core-data/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

### New Features
– The saveEntityRecord, saveEditedEntityRecord, and deleteEntityRecord actions now accept an optional throwOnError option (defaults to false). When set to true, any exceptions occurring when the action was executing are re-thrown, causing dispatch().saveEntityRecord() to reject with an error. ([#39258](https://github.com/WordPress/gutenberg/pull/39258))
adamziel marked this conversation as resolved.
Show resolved Hide resolved

## 4.2.0 (2022-03-11)

## 4.1.2 (2022-02-23)
Expand Down
8 changes: 5 additions & 3 deletions packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,13 @@ Action triggered to delete an entity record.

_Parameters_

- _kind_ `string`: Kind of the deleted entity record.
- _name_ `string`: Name of the deleted entity record.
- _recordId_ `string`: Record ID of the deleted entity record.
- _kind_ `string`: Kind of the deleted entity.
- _name_ `string`: Name of the deleted entity.
- _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 promise.
- _options.throwOnError_ `[boolean]`: If false, this action suppresses all the exceptions. Defaults to false.

### editEntityRecord

Expand Down Expand Up @@ -237,6 +238,7 @@ _Parameters_
- _options_ `Object`: Saving options.
- _options.isAutosave_ `[boolean]`: Whether this is an autosave.
- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a promise.
- _options.throwOnError_ `[boolean]`: If false, this action suppresses all the exceptions. Defaults to false.

### undo

Expand Down
58 changes: 39 additions & 19 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,22 +209,24 @@ export function receiveEmbedPreview( url, preview ) {
/**
* Action triggered to delete an entity record.
*
* @param {string} kind Kind of the deleted entity record.
* @param {string} name Name of the deleted entity record.
* @param {string} recordId Record ID of the deleted entity record.
* @param {?Object} query Special query parameters for the
* DELETE API call.
* @param {Object} [options] Delete options.
* @param {Function} [options.__unstableFetch] Internal use only. Function to
* call instead of `apiFetch()`.
* Must return a promise.
* @param {string} kind Kind of the deleted entity.
* @param {string} name Name of the deleted entity.
* @param {string} recordId Record ID of the deleted entity.
* @param {?Object} query Special query parameters for the
* DELETE API call.
* @param {Object} [options] Delete options.
* @param {Function} [options.__unstableFetch] Internal use only. Function to
* call instead of `apiFetch()`.
* Must return a promise.
* @param {boolean} [options.throwOnError=false] If false, this action suppresses all
* the exceptions. Defaults to false.
*/
export const deleteEntityRecord = (
kind,
name,
recordId,
query,
{ __unstableFetch = apiFetch } = {}
{ __unstableFetch = apiFetch, throwOnError = false } = {}
) => async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const entityConfig = find( configs, { kind, name } );
Expand All @@ -248,6 +250,7 @@ export const deleteEntityRecord = (
recordId,
} );

let hasError = false;
try {
let path = `${ entityConfig.baseURL }/${ recordId }`;

Expand All @@ -262,6 +265,7 @@ export const deleteEntityRecord = (

await dispatch( removeItems( kind, name, recordId, true ) );
} catch ( _error ) {
hasError = true;
error = _error;
}

Expand All @@ -273,6 +277,10 @@ export const deleteEntityRecord = (
error,
} );

if ( hasError && throwOnError ) {
throw error;
}

return deletedRecord;
} finally {
dispatch.__unstableReleaseStoreLock( lock );
Expand Down Expand Up @@ -386,20 +394,26 @@ export function __unstableCreateUndoLevel() {
/**
* Action triggered to save an entity record.
*
* @param {string} kind Kind of the received entity.
* @param {string} name Name of the received entity.
* @param {Object} record Record to be saved.
* @param {Object} options Saving options.
* @param {boolean} [options.isAutosave=false] Whether this is an autosave.
* @param {Function} [options.__unstableFetch] Internal use only. Function to
* call instead of `apiFetch()`.
* Must return a promise.
* @param {string} kind Kind of the received entity.
* @param {string} name Name of the received entity.
* @param {Object} record Record to be saved.
* @param {Object} options Saving options.
* @param {boolean} [options.isAutosave=false] Whether this is an autosave.
* @param {Function} [options.__unstableFetch] Internal use only. Function to
* call instead of `apiFetch()`.
* Must return a promise.
* @param {boolean} [options.throwOnError=false] If false, this action suppresses all
* the exceptions. Defaults to false.
*/
export const saveEntityRecord = (
kind,
name,
record,
{ isAutosave = false, __unstableFetch = apiFetch } = {}
{
isAutosave = false,
__unstableFetch = apiFetch,
throwOnError = false,
} = {}
) => async ( { select, resolveSelect, dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const entityConfig = find( configs, { kind, name } );
Expand Down Expand Up @@ -445,6 +459,7 @@ export const saveEntityRecord = (
} );
let updatedRecord;
let error;
let hasError = false;
try {
const path = `${ entityConfig.baseURL }${
recordId ? '/' + recordId : ''
Expand Down Expand Up @@ -567,6 +582,7 @@ export const saveEntityRecord = (
);
}
} catch ( _error ) {
hasError = true;
error = _error;
}
dispatch( {
Expand All @@ -578,6 +594,10 @@ export const saveEntityRecord = (
isAutosave,
} );

if ( hasError && throwOnError ) {
throw error;
}

return updatedRecord;
} finally {
dispatch.__unstableReleaseStoreLock( lock );
Expand Down
131 changes: 116 additions & 15 deletions packages/core-data/src/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,68 @@ describe( 'deleteEntityRecord', () => {

expect( result ).toBe( deletedRecord );
} );

it( 'throws on error when throwOnError is true', async () => {
const entities = [
{ name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' },
];

const dispatch = Object.assign( jest.fn(), {
receiveEntityRecords: jest.fn(),
__unstableAcquireStoreLock: jest.fn(),
__unstableReleaseStoreLock: jest.fn(),
} );
// Provide entities
dispatch.mockReturnValueOnce( entities );

// Provide response
apiFetch.mockImplementation( () => {
throw new Error( 'API error' );
} );

await expect(
deleteEntityRecord(
'postType',
'post',
10,
{},
{
throwOnError: true,
}
)( { dispatch } )
).rejects.toEqual( new Error( 'API error' ) );
} );

it( 'resolves on error when throwOnError is false', async () => {
const entities = [
{ name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' },
];

const dispatch = Object.assign( jest.fn(), {
receiveEntityRecords: jest.fn(),
__unstableAcquireStoreLock: jest.fn(),
__unstableReleaseStoreLock: jest.fn(),
} );
// Provide entities
dispatch.mockReturnValueOnce( entities );

// Provide response
apiFetch.mockImplementation( () => {
throw new Error( 'API error' );
} );

await expect(
deleteEntityRecord(
'postType',
'post',
10,
{},
{
throwOnError: false,
}
)( { dispatch } )
).resolves.toBe( false );
} );
} );

describe( 'saveEditedEntityRecord', () => {
Expand Down Expand Up @@ -201,9 +263,15 @@ describe( 'saveEditedEntityRecord', () => {
} );

describe( 'saveEntityRecord', () => {
let dispatch;
beforeEach( async () => {
apiFetch.mockReset();
jest.useFakeTimers();
dispatch = Object.assign( jest.fn(), {
receiveEntityRecords: jest.fn(),
__unstableAcquireStoreLock: jest.fn(),
__unstableReleaseStoreLock: jest.fn(),
} );
} );

it( 'triggers a POST request for a new record', async () => {
Expand All @@ -215,11 +283,6 @@ describe( 'saveEntityRecord', () => {
getRawEntityRecord: () => post,
};

const dispatch = Object.assign( jest.fn(), {
receiveEntityRecords: jest.fn(),
__unstableAcquireStoreLock: jest.fn(),
__unstableReleaseStoreLock: jest.fn(),
} );
// Provide entities
dispatch.mockReturnValueOnce( configs );

Expand Down Expand Up @@ -278,6 +341,54 @@ describe( 'saveEntityRecord', () => {
expect( result ).toBe( updatedRecord );
} );

it( 'throws on error when throwOnError is true', async () => {
const post = { title: 'new post' };
const entities = [
{ name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' },
];
const select = {
getRawEntityRecord: () => post,
};

// Provide entities
dispatch.mockReturnValueOnce( entities );

// Provide response
apiFetch.mockImplementation( () => {
throw new Error( 'API error' );
} );

await expect(
saveEntityRecord( 'postType', 'post', post, {
throwOnError: true,
} )( { select, dispatch } )
).rejects.toEqual( new Error( 'API error' ) );
} );

it( 'resolves on error when throwOnError is false', async () => {
const post = { title: 'new post' };
const entities = [
{ name: 'post', kind: 'postType', baseURL: '/wp/v2/posts' },
];
const select = {
getRawEntityRecord: () => post,
};

// Provide entities
dispatch.mockReturnValueOnce( entities );

// Provide response
apiFetch.mockImplementation( () => {
throw new Error( 'API error' );
} );

await expect(
saveEntityRecord( 'postType', 'post', post, {
throwOnError: false,
} )( { select, dispatch } )
).resolves.toEqual( undefined );
} );

it( 'triggers a PUT request for an existing record', async () => {
const post = { id: 10, title: 'new post' };
const configs = [
Expand All @@ -287,11 +398,6 @@ describe( 'saveEntityRecord', () => {
getRawEntityRecord: () => post,
};

const dispatch = Object.assign( jest.fn(), {
receiveEntityRecords: jest.fn(),
__unstableAcquireStoreLock: jest.fn(),
__unstableReleaseStoreLock: jest.fn(),
} );
// Provide entities
dispatch.mockReturnValueOnce( configs );

Expand Down Expand Up @@ -364,11 +470,6 @@ describe( 'saveEntityRecord', () => {
getRawEntityRecord: () => ( {} ),
};

const dispatch = Object.assign( jest.fn(), {
receiveEntityRecords: jest.fn(),
__unstableAcquireStoreLock: jest.fn(),
__unstableReleaseStoreLock: jest.fn(),
} );
// Provide entities
dispatch.mockReturnValueOnce( configs );

Expand Down
12 changes: 3 additions & 9 deletions packages/edit-navigation/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,9 @@ export const saveNavigationPost = ( post ) => async ( {
// Save menu.
await registry
.dispatch( coreDataStore )
.saveEditedEntityRecord( 'root', 'menu', menuId );

const error = registry
.select( coreDataStore )
.getLastEntitySaveError( 'root', 'menu', menuId );

if ( error ) {
throw new Error( error.message );
}
.saveEditedEntityRecord( 'root', 'menu', menuId, {
throwOnError: true,
} );

// Save menu items.
const updatedBlocks = await dispatch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ export default function DeleteTemplate() {
...settings,
availableTemplates: newAvailableTemplates,
} );
deleteEntityRecord( 'postType', 'wp_template', template.id );
deleteEntityRecord( 'postType', 'wp_template', template.id, {
throwOnError: true,
} );
};

return (
Expand Down
Loading