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

Editor: Update the store to use Core Data entities. #16932

Merged
merged 30 commits into from
Aug 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
241a8e1
Editor: Update the store to use Core Data entities.
aduth Jul 25, 2019
3daf3a0
Editor: Fix selector test suites.
epiqueras Aug 6, 2019
90d5e78
Editor: Fix some legacy selectors and behaviors.
epiqueras Aug 7, 2019
4bd2fb1
Editor: Fix action tests.
epiqueras Aug 7, 2019
db8bcab
Editor: Fix remaining broken unit tests.
epiqueras Aug 7, 2019
0190c97
Editor: Fix more tests.
epiqueras Aug 8, 2019
2cdd65d
Editor: Fix more e2e test behaviors.
epiqueras Aug 8, 2019
d2f2320
Editor: Fix preview functionality.
epiqueras Aug 8, 2019
24cb965
Core Data: Fix autosaves filtering.
epiqueras Aug 8, 2019
eec1666
Editor: Don't make entity dirty with initial edits.
epiqueras Aug 9, 2019
b5f9d5f
Editor: Don't save if the post is not saveable.
epiqueras Aug 9, 2019
1d48cd8
Core Data: Fix merged edits logic.
epiqueras Aug 9, 2019
1e91c9b
Core Data: Fix undo to fit e2e expected behaviors.
epiqueras Aug 9, 2019
16e69c8
Core Data: Handle more change detection and saving flows.
epiqueras Aug 9, 2019
d85aada
Block Editor: Fix undo level logic.
epiqueras Aug 9, 2019
8f70fbe
Core Data: Clean up undo reducer comment.
epiqueras Aug 9, 2019
e85a793
Editor: Make `serializeBlocks` a util.
epiqueras Aug 13, 2019
a3a7f55
Core Data: Clarify raw attribute usage.
epiqueras Aug 15, 2019
3ef3016
Core Data: Memoize .
epiqueras Aug 15, 2019
0e76e4c
Core Data: Use new raw entity record selector instead of modifying th…
epiqueras Aug 19, 2019
c271d86
Core Data: Make save notices the caller's responsibility.
epiqueras Aug 19, 2019
6c96372
Editor: Use the store key constant in actions instead of a string lit…
epiqueras Aug 19, 2019
5cde8c8
Editor: Defer serialization of blocks until save.
epiqueras Aug 19, 2019
f884aed
Editor: Fix raw content access in set up.
epiqueras Aug 20, 2019
aeff970
Editor: Revert broken test change.
epiqueras Aug 20, 2019
af24680
Editor: Make initial edits a dirtying operation.
epiqueras Aug 21, 2019
aa81118
Editor: Add comment clarifying why we set content to a new function o…
epiqueras Aug 21, 2019
4630675
Demo: Fix tests to consider the initial edits dirtying.
epiqueras Aug 21, 2019
a020241
Core Data: Set auto-drafts to drafts when autosaving them.
epiqueras Aug 21, 2019
272dfd2
Core Data: Handle receiving autosaves correctly when editing non-auto…
epiqueras Aug 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions docs/designers-developers/developers/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ _Related_

<a name="getBlocksForSerialization" href="#getBlocksForSerialization">#</a> **getBlocksForSerialization**

> **Deprecated** since Gutenberg 6.2.0.

Returns a set of blocks which are to be used in consideration of the post's
generated save content.

Expand Down Expand Up @@ -309,8 +311,7 @@ _Returns_

<a name="getEditedPostContent" href="#getEditedPostContent">#</a> **getEditedPostContent**

Returns the content of the post being edited, preferring raw string edit
before falling back to serialization of block state.
Returns the content of the post being edited.

_Parameters_

Expand Down Expand Up @@ -740,6 +741,7 @@ Return true if the current post has already been published.
_Parameters_

- _state_ `Object`: Global application state.
- _currentPost_ `?Object`: Explicit current post for bypassing registry selector.

_Returns_

Expand Down Expand Up @@ -1041,10 +1043,6 @@ _Parameters_

- _edits_ `Object`: Post attributes to edit.

_Returns_

- `Object`: Action object.

<a name="enablePublishSidebar" href="#enablePublishSidebar">#</a> **enablePublishSidebar**

Returns an action object used in signalling that the user has enabled the
Expand Down Expand Up @@ -1178,10 +1176,6 @@ _Related_
Returns an action object used in signalling that undo history should
restore last popped state.

_Returns_

- `Object`: Action object.

<a name="refreshPost" href="#refreshPost">#</a> **refreshPost**

Action generator for handling refreshing the current post.
Expand Down Expand Up @@ -1240,10 +1234,6 @@ _Parameters_
- _blocks_ `Array`: Block Array.
- _options_ `?Object`: Optional options.

_Returns_

- `Object`: Action object

<a name="resetPost" href="#resetPost">#</a> **resetPost**

Returns an action object used in signalling that the latest version of the
Expand Down Expand Up @@ -1357,10 +1347,6 @@ Action generator for trashing the current post in the editor.

Returns an action object used in signalling that undo history should pop.

_Returns_

- `Object`: Action object.

<a name="unlockPostSaving" href="#unlockPostSaving">#</a> **unlockPostSaving**

Returns an action object used to signal that post saving is unlocked.
Expand Down
16 changes: 16 additions & 0 deletions docs/designers-developers/developers/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,22 @@ _Returns_

- `?Object`: The entity record's save error.

<a name="getRawEntityRecord" href="#getRawEntityRecord">#</a> **getRawEntityRecord**

Returns the entity's record object by key,
with its attributes mapped to their raw values.

_Parameters_

- _state_ `Object`: State tree.
- _kind_ `string`: Entity kind.
- _name_ `string`: Entity name.
- _key_ `number`: Record's key.

_Returns_

- `?Object`: Object with the entity's raw attributes.

<a name="getRedoEdit" href="#getRedoEdit">#</a> **getRedoEdit**

Returns the next edit from the current undo offset
Expand Down
7 changes: 7 additions & 0 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => {
replaceBlocks,
toggleSelection,
setNavigationMode,
__unstableMarkLastChangeAsPersistent,
} = dispatch( 'core/block-editor' );

return {
Expand Down Expand Up @@ -744,6 +745,12 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => {
}
},
onReplace( blocks, indexToSelect ) {
if (
blocks.length &&
! isUnmodifiedDefaultBlock( blocks[ blocks.length - 1 ] )
) {
__unstableMarkLastChangeAsPersistent();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for this change? I wouldn't have expected this refactoring to touch the components directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need those replacements to be persistent so that undo works as expected in the E2E tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean what's changed? Why do we need this to make them persistent? How was it before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I'm surprised it ever worked for this test case specifically:

it( 'can undo asterisk transform', async () => {
	await clickBlockAppender();
	await page.keyboard.type( '1. ' );
	await pressKeyWithModifier( 'primary', 'z' );

	expect( await getEditedPostContent() ).toMatchSnapshot();
} );

If you do it at a normal pace, it works as expected and undos back to "1. ". But, if you do it super fast, the block editor never marks the last change before the transform as persistent and it goes back to "1". Could something be making this faster so now we can't rely on the time out trigerred __unstableMarkLastChangeAsPersistent?

}
replaceBlocks( [ ownProps.clientId ], blocks, indexToSelect );
},
onShiftSelection() {
Expand Down
16 changes: 16 additions & 0 deletions packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,22 @@ _Returns_

- `?Object`: The entity record's save error.

<a name="getRawEntityRecord" href="#getRawEntityRecord">#</a> **getRawEntityRecord**

Returns the entity's record object by key,
with its attributes mapped to their raw values.

_Parameters_

- _state_ `Object`: State tree.
- _kind_ `string`: Entity kind.
- _name_ `string`: Entity name.
- _key_ `number`: Record's key.

_Returns_

- `?Object`: Object with the entity's raw attributes.

<a name="getRedoEdit" href="#getRedoEdit">#</a> **getRedoEdit**

Returns the next edit from the current undo offset
Expand Down
58 changes: 47 additions & 11 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export function* editEntityRecord( kind, name, recordId, edits ) {
kind,
name
);
const record = yield select( 'getEntityRecord', kind, name, recordId );
const record = yield select( 'getRawEntityRecord', kind, name, recordId );
const editedRecord = yield select(
'getEditedEntityRecord',
kind,
Expand All @@ -147,9 +147,9 @@ export function* editEntityRecord( kind, name, recordId, edits ) {
// Clear edits when they are equal to their persisted counterparts
// so that the property is not considered dirty.
edits: Object.keys( edits ).reduce( ( acc, key ) => {
const recordValue = get( record[ key ], 'raw', record[ key ] );
const recordValue = record[ key ];
const value = mergedEdits[ key ] ?
merge( recordValue, edits[ key ] ) :
merge( {}, recordValue, edits[ key ] ) :
edits[ key ];
acc[ key ] = isEqual( recordValue, value ) ? undefined : value;
return acc;
Expand Down Expand Up @@ -235,9 +235,14 @@ export function* saveEntityRecord(
let error;
try {
const path = `${ entity.baseURL }${ recordId ? '/' + recordId : '' }`;

if ( isAutosave ) {
// Most of this autosave logic is very specific to posts.
// This is fine for now as it is the only supported autosave,
// but ideally this should all be handled in the back end,
// so the client just sends and receives objects.
const persistedRecord = yield select(
'getEntityRecord',
'getRawEntityRecord',
kind,
name,
recordId
Expand All @@ -255,18 +260,49 @@ export function* saveEntityRecord(
// to the actual persisted entity if the edits don't
// have a value.
let data = { ...persistedRecord, ...autosavePost, ...record };
data = Object.keys( data ).reduce( ( acc, key ) => {
if ( key in [ 'title', 'excerpt', 'content' ] ) {
acc[ key ] = get( data[ key ], 'raw', data[ key ] );
}
return acc;
}, {} );
data = Object.keys( data ).reduce(
( acc, key ) => {
if ( [ 'title', 'excerpt', 'content' ].includes( key ) ) {
// Edits should be the "raw" attribute values.
acc[ key ] = get( data[ key ], 'raw', data[ key ] );
}
return acc;
},
{ status: data.status === 'auto-draft' ? 'draft' : data.status }
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is very specific to the "post" autosave. I don't mind for it being in core-data right now because that's the only auto-save but in a generic way, this could be handled on the backend somehow, just send everything and let the backend do its thing.

For now maybe we can just add some comments to clarify. Also we might want to add e2e tests for the URL changes...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably a filter we could use.

updatedRecord = yield apiFetch( {
path: `${ path }/autosaves`,
method: 'POST',
data,
} );
yield receiveAutosaves( persistedRecord.id, updatedRecord );
// An autosave may be processed by the server as a regular save
// when its update is requested by the author and the post had
// draft or auto-draft status.
if ( persistedRecord.id === updatedRecord.id ) {
let newRecord = { ...persistedRecord, ...data, ...updatedRecord };
newRecord = Object.keys( newRecord ).reduce( ( acc, key ) => {
// These properties are persisted in autosaves.
if ( [ 'title', 'excerpt', 'content' ].includes( key ) ) {
// Edits should be the "raw" attribute values.
acc[ key ] = get( newRecord[ key ], 'raw', newRecord[ key ] );
} else if ( key === 'status' ) {
// Status is only persisted in autosaves when going from
// "auto-draft" to "draft".
acc[ key ] =
persistedRecord.status === 'auto-draft' &&
newRecord.status === 'draft' ?
newRecord.status :
persistedRecord.status;
} else {
// These properties are not persisted in autosaves.
acc[ key ] = get( persistedRecord[ key ], 'raw', persistedRecord[ key ] );
}
return acc;
}, {} );
yield receiveEntityRecords( kind, name, newRecord, undefined, true );
} else {
yield receiveAutosaves( persistedRecord.id, updatedRecord );
}
} else {
updatedRecord = yield apiFetch( {
path,
Expand Down
14 changes: 9 additions & 5 deletions packages/core-data/src/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ function entity( entityConfig ) {
// If the edited value is still different to the persisted value,
// keep the edited value in edits.
if (
// Edits are the "raw" attribute values, but records may have
// objects with more properties, so we use `get` here for the
// comparison.
! isEqual( edits[ key ], get( record[ key ], 'raw', record[ key ] ) )
) {
acc[ key ] = edits[ key ];
Expand Down Expand Up @@ -330,18 +333,19 @@ export function undo( state = UNDO_INITIAL_STATE, action ) {
edits: { ...state.flattenedUndo, ...action.meta.undo.edits },
},
];
} else {
// Clear potential redos, because this only supports linear history.
nextState = state.slice( 0, state.offset || undefined );
nextState.flattenedUndo = state.flattenedUndo;
nextState.offset = 0;
return nextState;
}

// Clear potential redos, because this only supports linear history.
nextState = state.slice( 0, state.offset || undefined );
nextState.offset = 0;

nextState.push( {
kind: action.kind,
name: action.name,
recordId: action.recordId,
edits: { ...nextState.flattenedUndo, ...action.edits },
edits: { ...action.edits, ...state.flattenedUndo },
} );

return nextState;
Expand Down
52 changes: 36 additions & 16 deletions packages/core-data/src/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,34 @@ export function getEntityRecord( state, kind, name, key ) {
return get( state.entities.data, [ kind, name, 'queriedData', 'items', key ] );
}

/**
* Returns the entity's record object by key,
* with its attributes mapped to their raw values.
*
* @param {Object} state State tree.
* @param {string} kind Entity kind.
* @param {string} name Entity name.
* @param {number} key Record's key.
*
* @return {Object?} Object with the entity's raw attributes.
*/
export const getRawEntityRecord = createSelector(
( state, kind, name, key ) => {
const record = getEntityRecord( state, kind, name, key );
return (
record &&
Object.keys( record ).reduce( ( acc, _key ) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabbing is a little wild here. Not sure why ESLint didn't catch this.

// Because edits are the "raw" attribute values,
// we return those from record selectors to make rendering,
// comparisons, and joins with edits easier.
acc[ _key ] = get( record[ _key ], 'raw', record[ _key ] );
return acc;
}, {} )
);
},
( state ) => [ state.entities.data ]
);

/**
* Returns the Entity's records.
*
Expand Down Expand Up @@ -156,8 +184,7 @@ export function getEntityRecordEdits( state, kind, name, recordId ) {
export const getEntityRecordNonTransientEdits = createSelector(
( state, kind, name, recordId ) => {
const { transientEdits = {} } = getEntity( state, kind, name );
const edits =
getEntityRecordEdits( state, kind, name, recordId ) || [];
const edits = getEntityRecordEdits( state, kind, name, recordId ) || [];
return Object.keys( edits ).reduce( ( acc, key ) => {
if ( ! transientEdits[ key ] ) {
acc[ key ] = edits[ key ];
Expand Down Expand Up @@ -194,16 +221,10 @@ export function hasEditsForEntityRecord( state, kind, name, recordId ) {
* @return {Object?} The entity record, merged with its edits.
*/
export const getEditedEntityRecord = createSelector(
( state, kind, name, recordId ) => {
const record = getEntityRecord( state, kind, name, recordId );
return {
...Object.keys( record ).reduce( ( acc, key ) => {
acc[ key ] = get( record[ key ], 'raw', record[ key ] );
return acc;
}, {} ),
...getEntityRecordEdits( state, kind, name, recordId ),
};
},
( state, kind, name, recordId ) => ( {
...getRawEntityRecord( state, kind, name, recordId ),
...getEntityRecordEdits( state, kind, name, recordId ),
} ),
( state ) => [ state.entities.data ]
);

Expand Down Expand Up @@ -237,12 +258,11 @@ export function isAutosavingEntityRecord( state, kind, name, recordId ) {
* @return {boolean} Whether the entity record is saving or not.
*/
export function isSavingEntityRecord( state, kind, name, recordId ) {
const { pending, isAutosave } = get(
return get(
state.entities.data,
[ kind, name, 'saving', recordId ],
{}
[ kind, name, 'saving', recordId, 'pending' ],
false
);
return Boolean( pending && ! isAutosave );
}

/**
Expand Down
Loading