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
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 file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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.

@@ -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_

@@ -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_

@@ -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
@@ -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.
@@ -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
@@ -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.
@@ -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
@@ -691,6 +691,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => {
replaceBlocks,
toggleSelection,
setNavigationMode,
__unstableMarkLastChangeAsPersistent,
} = dispatch( 'core/block-editor' );

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

This comment has been minimized.

Copy link
@youknowriad

youknowriad Aug 13, 2019

Contributor

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

This comment has been minimized.

Copy link
@epiqueras

epiqueras Aug 13, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@youknowriad

youknowriad Aug 14, 2019

Contributor

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

This comment has been minimized.

Copy link
@epiqueras

epiqueras Aug 14, 2019

Author Contributor

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() {
@@ -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
@@ -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,
@@ -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;
@@ -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
@@ -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 }
);

This comment has been minimized.

Copy link
@youknowriad

youknowriad Aug 22, 2019

Contributor

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...

This comment has been minimized.

Copy link
@epiqueras

epiqueras Aug 22, 2019

Author Contributor

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,
@@ -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 ];
@@ -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;
@@ -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 ) => {
// 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.
*
@@ -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 ];
@@ -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 ]
);

@@ -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 );
}

/**
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.