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

First pass at using the new sidebars and widget endpoints. #26086

Conversation

TimothyBJacobs
Copy link
Member

Description

Use the new sidebars and widget endpoints introduced in #25958.

How has this been tested?

Manual testing, PHPUnit testing.

Types of changes

Refactoring.

Right now the saves are all separate requests. This is blocked on #26024.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@TimothyBJacobs
Copy link
Member Author

PHPUnit tests failures are unrelated see, #26085.

@helen
Copy link
Member

helen commented Oct 16, 2020

The first time I tried to save after switching to this branch and building I got There was an error. can't access property "id", g is undefined, after that I got the correct Widgets saved. message. Just wanted to note that in case it's reproducible and needs to be fixed.

@TimothyBJacobs TimothyBJacobs force-pushed the update/use-widgets-endpoint branch from 649d777 to a53126f Compare October 17, 2020 21:46
@TimothyBJacobs
Copy link
Member Author

The first time I tried to save after switching to this branch and building I got There was an error. can't access property "id", g is undefined, after that I got the correct Widgets saved. message. Just wanted to note that in case it's reproducible and needs to be fixed.

This happens when the response returned when trying to create an endpoint doesn't return a valid response. This happened in particular because RSGW generates a fatal error when calling wp_kses.

I've added a specific error message when saving a widget fails due to this reason, Could not save the widget "%s". An unexpected error occurred.. But I'm not sure what should happen here to resolve the error for the user. Should we prompt them to remove that widget?

@TimothyBJacobs TimothyBJacobs force-pushed the update/use-widgets-endpoint branch 2 times, most recently from 0e4f26b to 26549c1 Compare October 18, 2020 22:37
@TimothyBJacobs TimothyBJacobs changed the base branch from master to update/data-layer-batch-requests-use-state October 19, 2020 00:05
adamziel and others added 12 commits October 19, 2020 14:41
We need to get the full list of widget ids from the client, we can't use the
returned widget ids from the batch because not all widgets will be dirty.

Also implements basic error handling for when the batch fails. A snackbar is
added with the names of the widgets that failed to save. We also now propagate
a dummy error to the apiFetch callers for the requests that didn't have a
validation error, but need something to short circuit their handling.
@adamziel adamziel force-pushed the update/use-widgets-endpoint branch from c38f6b6 to 8eced9b Compare October 19, 2020 12:47
@adamziel adamziel force-pushed the update/data-layer-batch-requests-use-state branch from e36d90e to 3c3b2e7 Compare October 19, 2020 12:52
@adamziel
Copy link
Contributor

Merging with base branch, let's consider this as a single PR.

@adamziel adamziel merged commit 1d80d48 into WordPress:update/data-layer-batch-requests-use-state Oct 19, 2020
adamziel added a commit that referenced this pull request Oct 19, 2020
* Draft batch-processing module

* committing/rudimentary API_FETCH integration

* Make batch processing functional!

* Roll back integration changes

* Cleanup package.json

* Update package-lock.json

* Update terminology

* yield from commitTransaction

* Correct yield from

* Move the comment where it belongs

* First pass at enqueueItemAndWaitForResults.

* Draft batch-processing module

* committing/rudimentary API_FETCH integration

* Make batch processing functional!

* Roll back integration changes

* Cleanup package.json

* Update package-lock.json

* Update terminology

* yield from commitTransaction

* Correct yield from

* Move the comment where it belongs

* update package-lock.json

* First pass at using the new sidebars and widget endpoints. (#26086)

* Draft batch-processing module

* committing/rudimentary API_FETCH integration

* Make batch processing functional!

* Roll back integration changes

* Cleanup package.json

* Update package-lock.json

* Update terminology

* yield from commitTransaction

* Correct yield from

* Move the comment where it belongs

* First pass at enqueueItemAndWaitForResults.

* First pass at using the new sidebars and widget endpoints.

* Add test for saving multiple widgets in a row

* Generate better error message when creating fails

* Add basic batch integration.

Works off of #26205.

* Allow mixing HTTP methods in a batch.

* Fix disappearing data, add basic error handling.

We need to get the full list of widget ids from the client, we can't use the
returned widget ids from the batch because not all widgets will be dirty.

Also implements basic error handling for when the batch fails. A snackbar is
added with the names of the widgets that failed to save. We also now propagate
a dummy error to the apiFetch callers for the requests that didn't have a
validation error, but need something to short circuit their handling.

* yield from the widget area saving.

* Move batch-processing into edit-widgets package

Co-authored-by: Adam Zieliński <[email protected]>

* remove separate batch-processing package

* Update batch processing tests

* Resolve batch errors in a safe, well-ordered way

* Fix unit tests

* Wire progress indicator to specific widgets

* Fix authorship after merge/rebase (1/2)

* Fix authorship after merge/rebase (2/2)

* Don't mark the method property as required.

We don't evaluate nested defaults yet, so this caused errors.

* Allow updating widgets without including the id_base or number.

* Don't start GET /wp/v2/widgets?per_page=-1 if there are batch requests already in progress

* Process newWidgetClientIds in correct order

* Fix package.json

* remove batch-processing package

Co-authored-by: Timothy Jacobs <[email protected]>
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