[WIP] Widgets editor integration with batch processing #26205
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adamziel
added
[Package] Core data
|
Size Change: +497 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
adamziel
force-pushed
the
update/integrate-batch-processing-with-data-layer
branch
from
192e827 to
83f8427
Compare
adamziel
force-pushed
the
update/integrate-batch-processing-with-data-layer
branch
from
4ef39e7 to
358e5f3
Compare
adamziel
commented
Oct 16, 2020
| @@ -131,7 +131,7 @@ public function serve_batch_request( WP_REST_Request $batch_request ) { | |||
| $allow_batch = ! empty( rest_get_server()->get_route_options( $route )['allow_batch'] ); | |||
| } | |||
|
|
|||
| if ( ! $allow_batch ) { | |||
| if ( 0 && ! $allow_batch ) { | |||
There was a problem hiding this comment.
temporary to make the endpoint work for testing
adamziel
force-pushed
the
update/integrate-batch-processing-with-data-layer
branch
from
358e5f3 to
83532bf
Compare
adamziel
force-pushed
the
update/integrate-batch-processing-with-data-layer
branch
from
83532bf to
cc54114
Compare
adamziel
changed the title
adamziel
commented
Oct 16, 2020
| } ); | ||
|
|
||
| if ( response.failed ) { | ||
| throw response.responses.map( ( { body } ) => body ); |
There was a problem hiding this comment.
if the error is general and we don't have granular responses this function could just throw it as it is and batch-processing will propagate it to each caller
adamziel
commented
Oct 16, 2020
Comment on lines
+36
to
+40
| return dispatch( 'core/batch-processing' ).enqueueItemAndAutocommit( | ||
| 'WIDGETS_API_FETCH', | ||
| 'default', | ||
| request | ||
| ); |
There was a problem hiding this comment.
Alternatively this could call something like enqueueItemAndWaitForResults and get a promise that would be resolved once processBatch is called explicitly and finishes running.
adamziel
commented
Oct 16, 2020
Comment on lines
+19
to
+22
| dispatch( 'core/batch-processing' ).registerProcessor( | ||
| 'WIDGETS_API_FETCH', | ||
| batchProcessor | ||
| ); |
There was a problem hiding this comment.
I have mixed feelings about storing callbacks in redux state. That may not be a problem we need to solve now, or may not be a problem at all.
adamziel
commented
Oct 16, 2020
| @@ -70,7 +70,7 @@ export function* commitTransaction( batchId, transactionId ) { | |||
| exception, | |||
| results; | |||
| try { | |||
| results = yield* processTransaction( batch, transactionId ); | |||
| results = yield processTransaction( batch, transactionId ); | |||
TimothyBJacobs
force-pushed
the
update/data-layer-batch-requests-use-state
branch
from
b40cf06 to
e36d90e
Compare
|
I've integrated these changes into #26086. I think we should consolidate into that PR since there isn't much point of supporting batching without using the new endpoints. |
adamziel
force-pushed
the
update/data-layer-batch-requests-use-state
branch
from
e36d90e to
3c3b2e7
Compare
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. * 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]>
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]>
aristath
deleted the
update/integrate-batch-processing-with-data-layer
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚧 UNDER CONSTRUCTION 🚧
Integration of data layer with #26164
This is super rudimentary, a better approach is coming