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

Explore Multi-Entity Saving #18029

Merged
merged 15 commits into from
Dec 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions docs/designers-developers/developers/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,19 @@ _Related_

- hasMultiSelection in core/block-editor store.

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

Returns true if there are unsaved edits for entities other than
the editor's post, and false otherwise.

_Parameters_

- _state_ `Object`: Global application state.

_Returns_

- `boolean`: Whether there are edits or not.

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

_Related_
Expand Down
15 changes: 15 additions & 0 deletions docs/designers-developers/developers/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,21 @@ _Returns_

- `?Object`: Record.

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

Returns a map of objects with each edited
raw entity record and its corresponding edits.

The map is keyed by entity `kind => name => key => { rawRecord, edits }`.

_Parameters_

- _state_ `Object`: State tree.

_Returns_

- `null`: The map of edited records with their edits.

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

Returns the specified entity record's edits.
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,21 @@ _Returns_

- `?Object`: Record.

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

Returns a map of objects with each edited
raw entity record and its corresponding edits.

The map is keyed by entity `kind => name => key => { rawRecord, edits }`.

_Parameters_

- _state_ `Object`: State tree.

_Returns_

- `null`: The map of edited records with their edits.

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

Returns the specified entity record's edits.
Expand Down
48 changes: 48 additions & 0 deletions packages/core-data/src/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,54 @@ export function getEntityRecords( state, kind, name, query ) {
return getQueriedItems( queriedState, query );
}

/**
* Returns a map of objects with each edited
* raw entity record and its corresponding edits.
*
* The map is keyed by entity `kind => name => key => { rawRecord, edits }`.
*
* @param {Object} state State tree.
*
* @return {{ [kind: string]: { [name: string]: { [key: string]: { rawRecord: Object<string,*>, edits: Object<string,*> } } } }} The map of edited records with their edits.
*/
export const getEntityRecordChangesByRecord = createSelector(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we'll need such a selector. Each time the returned value from a selector is that complex, I feel something is wrong. Can we just replace it with a selector that checks if an entity is Dirty?
I guess right now we'd also need a selector to retrieve all dirty records (hopefully we could remove that later).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess right now we'd also need a selector to retrieve all dirty records (hopefully we could remove that later).

Yes we would need that anyways.

( state ) => {
const {
entities: { data },
} = state;
return Object.keys( data ).reduce( ( acc, kind ) => {
Object.keys( data[ kind ] ).forEach( ( name ) => {
const editsKeys = Object.keys( data[ kind ][ name ].edits ).filter( ( editsKey ) =>
hasEditsForEntityRecord( state, kind, name, editsKey )
);
if ( editsKeys.length ) {
if ( ! acc[ kind ] ) {
acc[ kind ] = {};
}
if ( ! acc[ kind ][ name ] ) {
acc[ kind ][ name ] = {};
}
editsKeys.forEach(
( editsKey ) =>
( acc[ kind ][ name ][ editsKey ] = {
rawRecord: getRawEntityRecord( state, kind, name, editsKey ),
edits: getEntityRecordNonTransientEdits(
state,
kind,
name,
editsKey
),
} )
);
}
} );

return acc;
}, {} );
},
( state ) => [ state.entities.data ]
);

/**
* Returns the specified entity record's edits.
*
Expand Down
56 changes: 55 additions & 1 deletion packages/core-data/src/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import deepFreeze from 'deep-freeze';
import {
getEntityRecord,
getEntityRecords,
getEntityRecordChangesByRecord,
getEntityRecordNonTransientEdits,
getEmbedPreview,
isPreviewEmbedFallback,
Expand Down Expand Up @@ -105,10 +106,63 @@ describe( 'getEntityRecords', () => {
} );
} );

describe( 'getEntityRecordChangesByRecord', () => {
it( 'should return a map of objects with each raw edited entity record and its corresponding edits', () => {
const state = deepFreeze( {
entities: {
config: [
{
kind: 'someKind',
name: 'someName',
transientEdits: { someTransientEditProperty: true },
},
],
data: {
someKind: {
someName: {
queriedData: {
items: {
someKey: {
someProperty: 'somePersistedValue',
someRawProperty: { raw: 'somePersistedRawValue' },
},
},
},
edits: {
someKey: {
someProperty: 'someEditedValue',
someRawProperty: 'someEditedRawValue',
someTransientEditProperty: 'someEditedTransientEditValue',
},
},
},
},
},
},
} );
expect( getEntityRecordChangesByRecord( state ) ).toEqual( {
someKind: {
someName: {
someKey: {
rawRecord: {
someProperty: 'somePersistedValue',
someRawProperty: 'somePersistedRawValue',
},
edits: {
someProperty: 'someEditedValue',
someRawProperty: 'someEditedRawValue',
},
},
},
},
} );
} );
} );

describe( 'getEntityRecordNonTransientEdits', () => {
it( 'should return an empty object when the entity does not have a loaded config.', () => {
const state = deepFreeze( {
entities: { config: {}, data: {} },
entities: { config: [], data: {} },
} );
expect(
getEntityRecordNonTransientEdits( state, 'someKind', 'someName', 'someId' )
Expand Down
1 change: 1 addition & 0 deletions packages/editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"@wordpress/viewport": "file:../viewport",
"@wordpress/wordcount": "file:../wordcount",
"classnames": "^2.2.5",
"equivalent-key-map": "^0.2.2",
"lodash": "^4.17.15",
"memize": "^1.0.5",
"react-autosize-textarea": "^3.0.2",
Expand Down
104 changes: 104 additions & 0 deletions packages/editor/src/components/entities-saved-states/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* External dependencies
*/
import { startCase } from 'lodash';
import EquivalentKeyMap from 'equivalent-key-map';

/**
* WordPress dependencies
*/
import { CheckboxControl, Modal, Button } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useSelect, useDispatch } from '@wordpress/data';
import { useState } from '@wordpress/element';

const EntitiesSavedStatesCheckbox = ( {
id,
name,
changes: { rawRecord },
checked,
setCheckedById,
} ) => (
<CheckboxControl
label={ `${ startCase( name ) }: "${ rawRecord.title ||
rawRecord.name ||
__( 'Untitled' ) }"` }
checked={ checked }
onChange={ ( nextChecked ) => setCheckedById( id, nextChecked ) }
/>
);

export default function EntitiesSavedStates( {
isOpen,
onRequestClose,
ignoredForSave = new EquivalentKeyMap(),
} ) {
const entityRecordChangesByRecord = useSelect( ( select ) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand properly, we will be receiving changes from all entities here even entities that are not edited in the editor itself (think plugin, left over from other pages if it's an SPA...).

Do you think the "editor" should register the entities it's editing somehow instead of just getting all edits from the core data store?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand properly, we will be receiving changes from all entities here even entities that are not edited in the editor itself (think plugin, left over from other pages if it's an SPA...).

Yes, I thought that wouldn't be a blocker since we only have one editor for now.

Do you think the "editor" should register the entities it's editing somehow instead of just getting all edits from the core data store?

Yes, I think the EntityProvider should trigger an action that adds the provided entity record to a list in the editor store on mount. The editor can then use this list to query/filter entity changes.

Should I do that in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure it's the responsibility of EntityProvider to do that as EntityProvider is not editor specific either.

I'm ok for this being in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but it is meant to be rendered in an editing context and it's the only way to link the two dynamically without walking the block tree.

I.e. The only way for the editor to answer: Which entities am I rendering/editing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was imagining something like the EditorProvider registering its entities and maybe the Post block, Site block registering their own. I can see how it's not great either as extra work for block authors. We can try and iterate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, and we can do it in EntityProvider in a way that does not break usage outside of an editor.

select( 'core' ).getEntityRecordChangesByRecord()
);
const { saveEditedEntityRecord } = useDispatch( 'core' );

const [ checkedById, _setCheckedById ] = useState( () => new EquivalentKeyMap() );
const setCheckedById = ( id, checked ) =>
_setCheckedById( ( prevCheckedById ) => {
const nextCheckedById = new EquivalentKeyMap( prevCheckedById );
if ( checked ) {
nextCheckedById.set( id, true );
} else {
nextCheckedById.delete( id );
}
return nextCheckedById;
} );
const saveCheckedEntities = () => {
checkedById.forEach( ( _checked, id ) => {
if ( ! ignoredForSave.has( id ) ) {
saveEditedEntityRecord(
...id.filter( ( s, i ) => i !== id.length - 1 || s !== 'undefined' )
);
}
} );
onRequestClose( checkedById );
};
return (
isOpen && (
epiqueras marked this conversation as resolved.
Show resolved Hide resolved
<Modal
title={ __( 'What do you want to save?' ) }
onRequestClose={ () => onRequestClose() }
contentLabel={ __( 'Select items to save.' ) }
>
{ Object.keys( entityRecordChangesByRecord ).map( ( changedKind ) =>
Object.keys( entityRecordChangesByRecord[ changedKind ] ).map(
( changedName ) =>
Object.keys(
entityRecordChangesByRecord[ changedKind ][ changedName ]
).map( ( changedKey ) => {
epiqueras marked this conversation as resolved.
Show resolved Hide resolved
const id = [ changedKind, changedName, changedKey ];
return (
<EntitiesSavedStatesCheckbox
key={ id.join( ' | ' ) }
id={ id }
name={ changedName }
changes={
entityRecordChangesByRecord[ changedKind ][ changedName ][
changedKey
]
}
checked={ checkedById.get( id ) }
setCheckedById={ setCheckedById }
/>
);
} )
)
) }
<Button
isPrimary
disabled={ checkedById.size === 0 }
onClick={ saveCheckedEntities }
className="editor-entities-saved-states__save-button"
>
{ __( 'Save' ) }
</Button>
</Modal>
)
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.editor-entities-saved-states__save-button {
display: block;
margin-left: auto;
margin-right: 0;
}
1 change: 1 addition & 0 deletions packages/editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export { default as TextEditorGlobalKeyboardShortcuts } from './global-keyboard-
export { default as EditorHistoryRedo } from './editor-history/redo';
export { default as EditorHistoryUndo } from './editor-history/undo';
export { default as EditorNotices } from './editor-notices';
export { default as EntitiesSavedStates } from './entities-saved-states';
export { default as ErrorBoundary } from './error-boundary';
export { default as LocalAutosaveMonitor } from './local-autosave-monitor';
export { default as PageAttributesCheck } from './page-attributes/check';
Expand Down
Loading