Skip to content

Commit

Permalink
Remove optimistic updates to solve the template revert issue (#27797)
Browse files Browse the repository at this point in the history
  • Loading branch information
youknowriad authored Dec 18, 2020
1 parent c597dd7 commit 66496e5
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 92 deletions.
55 changes: 0 additions & 55 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,6 @@ export function* saveEntityRecord(
};
let updatedRecord;
let error;
let persistedEntity;
let currentEdits;
try {
const path = `${ entity.baseURL }${
recordId ? '/' + recordId : ''
Expand Down Expand Up @@ -512,31 +510,6 @@ export function* saveEntityRecord(
};
}

// Get the full local version of the record before the update,
// to merge it with the edits and then propagate it to subscribers
persistedEntity = yield controls.select(
'core',
'__experimentalGetEntityRecordNoResolver',
kind,
name,
recordId
);
currentEdits = yield controls.select(
'core',
'getEntityRecordEdits',
kind,
name,
recordId
);
yield receiveEntityRecords(
kind,
name,
{ ...persistedEntity, ...edits },
undefined,
// This must be false or it will trigger a GET request in parallel to the PUT/POST below
false
);

updatedRecord = yield apiFetch( {
path,
method: recordId ? 'PUT' : 'POST',
Expand All @@ -552,34 +525,6 @@ export function* saveEntityRecord(
}
} catch ( _error ) {
error = _error;

// If we got to the point in the try block where we made an optimistic update,
// we need to roll it back here.
if ( persistedEntity && currentEdits ) {
yield receiveEntityRecords(
kind,
name,
persistedEntity,
undefined,
true
);
yield editEntityRecord(
kind,
name,
recordId,
{
...currentEdits,
...( yield controls.select(
'core',
'getEntityRecordEdits',
kind,
name,
recordId
) ),
},
{ undoIgnore: true }
);
}
}
yield {
type: 'SAVE_ENTITY_RECORD_FINISH',
Expand Down
17 changes: 0 additions & 17 deletions packages/core-data/src/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,7 @@ describe( 'saveEntityRecord', () => {
'SAVE_ENTITY_RECORD_START'
);

// Should select __experimentalGetEntityRecordNoResolver selector (as opposed to getEntityRecord)
// see https://github.com/WordPress/gutenberg/pull/19752#discussion_r368498318.
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
expect( fulfillment.next().value.selectorName ).toBe(
'__experimentalGetEntityRecordNoResolver'
);
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
const receiveItems = fulfillment.next().value;
expect( receiveItems.type ).toBe( 'RECEIVE_ITEMS' );
expect( receiveItems.invalidateCache ).toBe( false );
const { value: apiFetchAction } = fulfillment.next( {} );
expect( apiFetchAction.request ).toEqual( {
path: '/wp/v2/posts',
Expand Down Expand Up @@ -173,11 +164,6 @@ describe( 'saveEntityRecord', () => {
'SAVE_ENTITY_RECORD_START'
);
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
const receiveItems = fulfillment.next().value;
expect( receiveItems.type ).toBe( 'RECEIVE_ITEMS' );
expect( receiveItems.invalidateCache ).toBe( false );
const { value: apiFetchAction } = fulfillment.next( {} );
expect( apiFetchAction.request ).toEqual( {
path: '/wp/v2/posts/10',
Expand Down Expand Up @@ -222,9 +208,6 @@ describe( 'saveEntityRecord', () => {
'SAVE_ENTITY_RECORD_START'
);
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
expect( fulfillment.next().value.type ).toBe( '@@data/SELECT' );
expect( fulfillment.next().value.type ).toBe( 'RECEIVE_ITEMS' );
const { value: apiFetchAction } = fulfillment.next( {} );
expect( apiFetchAction.request ).toEqual( {
path: '/wp/v2/types/page',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ function PostTemplate() {
const { template, isEditing, isFSETheme } = useSelect( ( select ) => {
const {
getEditedPostAttribute,
__unstableIsAutodraftPost,
getCurrentPostType,
getCurrentPost,
} = select( editorStore );
const { __experimentalGetTemplateForLink } = select( coreStore );
const { isEditingTemplate } = select( editPostStore );
Expand All @@ -30,7 +30,7 @@ function PostTemplate() {
template:
isFSEEnabled &&
link &&
! __unstableIsAutodraftPost() &&
getCurrentPost().status !== 'auto-draft' &&
getCurrentPostType() !== 'wp_template'
? __experimentalGetTemplateForLink( link )
: null,
Expand Down
6 changes: 2 additions & 4 deletions packages/edit-post/src/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ function Editor( {
const { getEntityRecord, __experimentalGetTemplateForLink } = select(
'core'
);
const { getEditorSettings, __unstableIsAutodraftPost } = select(
'core/editor'
);
const { getEditorSettings, getCurrentPost } = select( 'core/editor' );
const { getBlockTypes } = select( blocksStore );
const postObject = getEntityRecord( 'postType', postType, postId );
const isFSETheme = getEditorSettings().isFSETheme;
Expand All @@ -87,7 +85,7 @@ function Editor( {
template:
isFSETheme &&
postObject &&
! __unstableIsAutodraftPost() &&
getCurrentPost().status !== 'auto-draft' &&
postType !== 'wp_template'
? __experimentalGetTemplateForLink( postObject.link )
: null,
Expand Down
14 changes: 0 additions & 14 deletions packages/editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1269,20 +1269,6 @@ export function getEditorBlocks( state ) {
return getEditedPostAttribute( state, 'blocks' ) || EMPTY_ARRAY;
}

/**
* Checks whether a post is an auto-draft ignoring the optimistic transaction.
* This selector shouldn't be necessary. It's currently used as a workaround
* to avoid template resolution for auto-drafts which has a backend bug.
*
* @param {Object} state State.
* @return {boolean} Whether the post is "auto-draft" on the backend.
*/
export function __unstableIsAutodraftPost( state ) {
const post = getCurrentPost( state );
const isSaving = isSavingPost( state );
return isSaving || post.status === 'auto-draft';
}

/**
* A block selection object.
*
Expand Down

0 comments on commit 66496e5

Please sign in to comment.