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

[ CoreData Entities ] add support for delete operations #21557

Merged
merged 32 commits into from
Jul 28, 2020

Conversation

draganescu
Copy link
Contributor

@draganescu draganescu commented Apr 13, 2020

Description

Adds support for delete operations to entities. For a testing example use #22428

Screenshots

2020-04-13 15-13-04 2020-04-13 15_13_59

Types of changes

New feature to the entity actions and reducers that enable one to call delete[EntityName]

@github-actions
Copy link

github-actions bot commented Apr 13, 2020

Size Change: -1.01 MB (87%) 🏆

Total Size: 1.15 MB

Filename Size Change
build/a11y/index.js 0 B -1.14 kB (0%)
build/annotations/index.js 0 B -3.62 kB (0%)
build/api-fetch/index.js 0 B -3.4 kB (0%)
build/autop/index.js 0 B -2.82 kB (0%)
build/blob/index.js 0 B -620 B (0%)
build/block-directory/index.js 0 B -7.39 kB (0%)
build/block-directory/style-rtl.css 953 B +12 B (1%)
build/block-directory/style.css 952 B +10 B (1%)
build/block-editor/index.js 0 B -109 kB (0%)
build/block-editor/style-rtl.css 10.8 kB +126 B (1%)
build/block-editor/style.css 10.8 kB +124 B (1%)
build/block-library/editor-rtl.css 7.63 kB +224 B (2%)
build/block-library/editor.css 7.63 kB +222 B (2%)
build/block-library/index.js 0 B -129 kB (0%)
build/block-library/style-rtl.css 7.83 kB -211 B (2%)
build/block-library/style.css 7.83 kB -211 B (2%)
build/block-library/theme-rtl.css 728 B -2 B (0%)
build/block-library/theme.css 729 B -3 B (0%)
build/block-serialization-default-parser/index.js 0 B -1.88 kB (0%)
build/block-serialization-spec-parser/index.js 0 B -3.1 kB (0%)
build/blocks/index.js 0 B -48.2 kB (0%)
build/components/index.js 0 B -198 kB (0%)
build/components/style-rtl.css 15.7 kB -251 B (1%)
build/components/style.css 15.6 kB -247 B (1%)
build/compose/index.js 0 B -9.65 kB (0%)
build/core-data/index.js 0 B -11.4 kB (0%)
build/data-controls/index.js 0 B -1.29 kB (0%)
build/data/index.js 0 B -8.44 kB (0%)
build/date/index.js 0 B -5.47 kB (0%)
build/deprecated/index.js 0 B -772 B (0%)
build/dom-ready/index.js 0 B -569 B (0%)
build/dom/index.js 0 B -3.19 kB (0%)
build/edit-navigation/index.js 0 B -9.88 kB (0%)
build/edit-navigation/style-rtl.css 1.08 kB +58 B (5%) 🔍
build/edit-navigation/style.css 1.08 kB +58 B (5%) 🔍
build/edit-post/index.js 0 B -303 kB (0%)
build/edit-post/style-rtl.css 5.61 kB +99 B (1%)
build/edit-post/style.css 5.61 kB +98 B (1%)
build/edit-site/index.js 0 B -16.6 kB (0%)
build/edit-site/style-rtl.css 3.06 kB +18 B (0%)
build/edit-site/style.css 3.06 kB +20 B (0%)
build/edit-widgets/index.js 0 B -9.32 kB (0%)
build/edit-widgets/style-rtl.css 2.45 kB +26 B (1%)
build/edit-widgets/style.css 2.45 kB +26 B (1%)
build/editor/index.js 0 B -44.8 kB (0%)
build/editor/style-rtl.css 3.78 kB -73 B (1%)
build/editor/style.css 3.78 kB -80 B (2%)
build/element/index.js 0 B -4.65 kB (0%)
build/escape-html/index.js 0 B -733 B (0%)
build/format-library/index.js 0 B -7.72 kB (0%)
build/hooks/index.js 0 B -2.13 kB (0%)
build/html-entities/index.js 0 B -622 B (0%)
build/i18n/index.js 0 B -3.56 kB (0%)
build/is-shallow-equal/index.js 0 B -710 B (0%)
build/keyboard-shortcuts/index.js 0 B -2.52 kB (0%)
build/keycodes/index.js 0 B -1.94 kB (0%)
build/list-reusable-blocks/index.js 0 B -3.12 kB (0%)
build/list-reusable-blocks/style-rtl.css 476 B +26 B (5%) 🔍
build/list-reusable-blocks/style.css 476 B +25 B (5%) 🔍
build/media-utils/index.js 0 B -5.29 kB (0%)
build/notices/index.js 0 B -1.79 kB (0%)
build/nux/index.js 0 B -3.4 kB (0%)
build/plugins/index.js 0 B -2.56 kB (0%)
build/primitives/index.js 0 B -1.5 kB (0%)
build/priority-queue/index.js 0 B -788 B (0%)
build/redux-routine/index.js 0 B -2.85 kB (0%)
build/rich-text/index.js 0 B -14 kB (0%)
build/server-side-render/index.js 0 B -2.67 kB (0%)
build/shortcode/index.js 0 B -1.69 kB (0%)
build/token-list/index.js 0 B -1.28 kB (0%)
build/url/index.js 0 B -4.06 kB (0%)
build/viewport/index.js 0 B -1.85 kB (0%)
build/warning/index.js 0 B -1.14 kB (0%)
build/wordcount/index.js 0 B -1.17 kB (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.63 kB 0 B
build/block-editor/index.min.js 125 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.3 kB 0 B
build/components/index.min.js 200 kB 0 B
build/compose/index.min.js 9.68 kB 0 B
build/core-data/index.min.js 11.8 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.45 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.9 kB 0 B
build/edit-post/index.min.js 304 kB 0 B
build/edit-site/index.min.js 16.9 kB 0 B
build/edit-widgets/index.min.js 9.37 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.min.js 45.3 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.js 3.12 kB 0 B
build/media-utils/index.min.js 5.33 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@draganescu draganescu force-pushed the add/delete-items-nav-items-php branch from 40bb71d to 26a70f8 Compare April 27, 2020 15:11
@draganescu
Copy link
Contributor Author

I've added a snackbar notice for removing menu items. The problem I am facing is that while in #21486 I was able to curtail the cache of the entity store by creating local state out of the inital get menus call, here the edit menu component is remounted and the menu items are taken from the cache again.

As a quick fix we could not use entities :) or otherwise we need the delete option in place. In absence, the delete works but on switching between menus the cache wrongly shows deleted menu items too.

cc @noisysocks

@draganescu draganescu force-pushed the add/delete-items-nav-items-php branch 2 times, most recently from 43edd43 to f010401 Compare April 30, 2020 14:00
@draganescu draganescu force-pushed the add/delete-items-nav-items-php branch from f010401 to ffb83d4 Compare May 5, 2020 09:22
Copy link
Contributor

@adamziel adamziel left a comment

Choose a reason for hiding this comment

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

I left a few small notes

packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
@draganescu draganescu changed the title [ experimental/nav-menus ] delete nav menu items [ experimental/nav-menus ] WIP: delete nav menu items May 6, 2020
@draganescu draganescu marked this pull request as draft May 12, 2020 05:25
@adamziel adamziel force-pushed the add/delete-items-nav-items-php branch from 93c0e09 to c6896f7 Compare May 15, 2020 11:46
@adamziel
Copy link
Contributor

@draganescu I think I got it to work, the culprit was the invalidateCache: true sent with the REMOVE_ITEMS action

@draganescu draganescu changed the title [ experimental/nav-menus ] WIP: delete nav menu items [ experimental/nav-menus ] delete nav menu items May 15, 2020
@draganescu draganescu marked this pull request as ready for review May 15, 2020 13:31
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
@noisysocks
Copy link
Member

This is looking really good!

We need to add unit tests for the two new actions (removeItems and deleteEntityRecord) as well as the new branch (REMOVE_ITEMS) in the reducer.

Let's also add a note to @wordpress/core-data's CHANGELOG.md.

@adamziel
Copy link
Contributor

@draganescu I rebased, tested a few scenarios, and here's what happened:

2020-05-21 16-32-41 2020-05-21 16_36_11

2020-05-21 16-45-59 2020-05-21 16_48_04

@draganescu
Copy link
Contributor Author

draganescu commented May 21, 2020

Apparently nested items break the deletions and it appears to be a bug in the hook because all the nested items get deleted.

draganescu and others added 3 commits June 30, 2020 15:52
* adds delete menu with entity delete

* updates the delete and removes the stateMenus

* passes the new force query param

* fix bug with resetting current menu after delete
@draganescu draganescu force-pushed the add/delete-items-nav-items-php branch from d91d165 to 5abdf63 Compare June 30, 2020 13:15
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
@draganescu draganescu marked this pull request as draft July 2, 2020 14:48
@draganescu draganescu marked this pull request as ready for review July 2, 2020 15:28
packages/core-data/CHANGELOG.md Show resolved Hide resolved
packages/core-data/src/actions.js Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/actions.js Show resolved Hide resolved
packages/core-data/src/queried-data/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/actions.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/test/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/queried-data/test/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/test/actions.js Outdated Show resolved Hide resolved
draganescu and others added 3 commits July 9, 2020 20:00
…ms action

also improves tests and made sure the notifications always have unique ids
packages/core-data/src/queried-data/reducer.js Outdated Show resolved Hide resolved
packages/core-data/src/test/actions.js Outdated Show resolved Hide resolved
- test delete for final shape when generator is done
- simplify filter removing items from query
@draganescu draganescu merged commit b67cd9a into master Jul 28, 2020
@draganescu draganescu deleted the add/delete-items-nav-items-php branch July 28, 2020 09:26
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Core data /packages/core-data [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants