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

Use the new deleteEntityRecord to delete menus #22428

Merged

Conversation

draganescu
Copy link
Contributor

@draganescu draganescu commented May 18, 2020

Description

Closes #22340 This PR implements deleting menus using entity delete. It depends on #21557

How has this been tested?

Tested locally by:

  1. Creating menus
  2. Deleting menus

@github-actions
Copy link

github-actions bot commented May 18, 2020

Size Change: -109 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/edit-navigation/index.js 9.76 kB -109 B (1%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.37 kB 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 108 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.59 kB 0 B
build/block-library/editor.css 7.6 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 8.04 kB 0 B
build/block-library/style.css 8.04 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/index.js 197 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.62 kB 0 B
build/core-data/index.js 11.7 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.5 kB 0 B
build/edit-post/style.css 5.5 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.02 kB 0 B
build/edit-site/style.css 3.02 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 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.js 44.9 kB 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.73 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.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 663 B 0 B
build/nux/style.css 660 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@draganescu draganescu requested a review from epiqueras May 20, 2020 14:07
@draganescu draganescu force-pushed the try/delete-menus-with-entities branch from 200c70e to 2118c52 Compare May 21, 2020 11:42
@draganescu draganescu marked this pull request as ready for review May 21, 2020 11:44
@adamziel adamziel force-pushed the add/delete-items-nav-items-php branch from 7f3aa1e to c059655 Compare May 21, 2020 14:36
@draganescu draganescu force-pushed the try/delete-menus-with-entities branch 3 times, most recently from 7bf5640 to dcb6b9e Compare May 22, 2020 16:52
@draganescu
Copy link
Contributor Author

@adamziel 's #22603 makes this useless. Not sure whether to close it or not.

@adamziel
Copy link
Contributor

adamziel commented May 29, 2020

@draganescu #22603 is all about menu items and this one is about menus. Using batch requests for menu items makes sense since the model of interaction with them is "customize a lot of entities and save them all at once". On the other hand, the model of interaction with menus is more like "customize and save only entity at a time" so this PR is not only useful, but would also potentially solve #22340.

@draganescu draganescu force-pushed the try/delete-menus-with-entities branch from 0d381e2 to 3a4f26b Compare June 26, 2020 15:11
@adamziel adamziel merged this pull request into add/delete-items-nav-items-php Jun 30, 2020
@adamziel adamziel deleted the try/delete-menus-with-entities branch June 30, 2020 09:46
draganescu added a commit that referenced this pull request Jun 30, 2020
* 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 added a commit that referenced this pull request Jul 28, 2020
* reset, delete nav menu items

there is a missing state preservation when menus are changed on the menus editor component

* fixing a bad merge

* no invalidateCache and refactored according to review

* revert change to getMergedItemIds

* refactor according to self review

* deletes query items, adds tests for new entity methods

also adds changelog

* refactored for properly deleteing entity ids from querries

* updates tests

* moved the REMOVE_ITEMS reducer to receive all query keys

props @aduth

* removes the need to send query on delete

* updates some comments

also removes cruft from a test

* refactoring according to review

- removes superfluous actions from deleteEntityRecords
- treats invalidateCache properly
- attempts to make lookups faster for items when removing querries

* do not clear cache on delete

* fixes test after reming superfluous actions from deleteEntityRecord

* makes a proper POJO for removing items from queries and invalidates the cache after items are removed

* fix changelog and add deleteQueryParams to deleteRecord

* fixes test

* Rename deleteQueryParams to query for consistency

* Use the new deleteEntityRecord to delete menus (#22428)

* 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

* fixes query param's type for consistency

* comment linting, removed useless catch logic for deleteEntityRecord, removed useless string conversion

* try implement error handling for delete

* fixed the intentional typo and the unintentional one

* updates and fixes according to review

* rename remove items' action id collection

* makes notices unique in menu editor

* Update packages/core-data/src/queried-data/actions.js

Co-authored-by: Enrique Piqueras <epiqueras@users.noreply.github.com>

* fixes bugs introduced by renaming items to itemsIds in the remove items action

also improves tests and made sure the notifications always have unique ids

* lint

* moves noticeId creation inside effect

* updated according to review

- test delete for final shape when generator is done
- simplify filter removing items from query

* lint

Co-authored-by: Enrique Piqueras <epiqueras@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants