[Live] add batch action controller (2) #5
Conversation
| // OR body of the request is JSON | ||
| $requestData = json_decode($request->getContent(), true, 512, \JSON_THROW_ON_ERROR); | ||
| $data = $requestData['data'] ?? []; | ||
| $request->attributes->set('_component_data', $requestData); |
There was a problem hiding this comment.
The actions would be a sibling of args. So probably it should be set separately, like below with args. It feels a little weird to put that special case here... but we already do it for args. So there are two special cases: multiple actions (so we look for actions) or a single action (so we look for args).
Alternatively, if this bothers us enough (though it's an invisible detail to the user), we could be more consistent between single and multiple actions - i.e. by ALWAYS sending actions=[] in the json (e.g. actions: [{name: 'save', args: {foo: 'bar'}}]), even if there is just one action (and then POST to /_components/the-name instead of /_components/the-name/save.
There was a problem hiding this comment.
I think this is more clear with the parseDataFor() method. It always returns a normalized array of data.
|
|
||
| if (!$request->attributes->get('_component_default_action', false) && !AsLiveComponent::isActionAllowed($component, $action)) { | ||
| throw new NotFoundHttpException(sprintf('The action "%s" either doesn\'t exist or is not allowed in "%s". Make sure it exist and has the LiveAction attribute above it.', $action, \get_class($component))); | ||
| } |
There was a problem hiding this comment.
We could move this up into onKernelRequest. It's inside of that method that we're reading the action from the user input, which is what we need to validate. We did it down here because this is where we finally, for sure, know the final controller. But if we moved the check up to onKernelRequest(), and then someone (via a listener) decided to change from one action to another action on their live component, who cares? That's not user input doing that.
Btw: motivation for this is potential simplification... but there are a lot of moving pieces, so I'm not sure how it would all "land".
There was a problem hiding this comment.
This wouldn't catch the case where a batch request is sent with invalid action (because the request event is skipped for sub-requests). Or are you thinking for _batch to loop over and validate?
I can't move this up because it requires a component instance. For non-batch requests, this isn't available until onKernelController.
kbond
force-pushed
the
batch-action-controller-2
branch
3 times, most recently
from
331ac28 to
d0edf9a
Compare
kbond
force-pushed
the
batch-action-controller-2
branch
from
d0edf9a to
896f48f
Compare
|
Updated to use |
…th "queued" changes (weaverryan, kbond) This PR was merged into the 2.x branch. Discussion ---------- [WIP] [Live] Make Ajax calls happen in serial, with "queued" changes | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | Tickets | None | License | MIT Hi! Consider the following situation: A) Model update is made: Ajax call starts B) An action is triggered BEFORE the Ajax call from (A) finishes. Previously, we would start a 2nd Ajax call for (B) before the Ajax call from (A) finished... meaning two Ajax calls were happening at the same time. There are a few problems with this: (i) Ajax call (B) is missing any potential data changes from Ajax call (A) and (i) complexity of multiple things loading at once, and potentially finishing in a different order. This PR simplifies things, which matches Livewire's behavior anyways. Now, the action from (B) will WAIT until the Ajax call from (A) finishes and THEN start. In fact, while an Ajax call is being made, all changes (potentially multiple model updates or actions) will be "queued" and then all set at once on the next request. TODO: * [X] Update the backend: for POST requests, the "data" was moved under a `data` key in JSON. Previously the entire body was the "data". * [X] Update the backend: for POST/action requests, action "arguments" were moved from query parameters to an `args` key in the JSON. * [x] Frontend: add tests for the `batch` action Ajax calls * [x] Update the backend: a new fake `/batch` action needs to be added that can handle multiple actions at once * [ ] A new `updatedModels` is sent on the ajax requests. If the signature fails, use this to give a better error message about what readonly properties were just modified. (in fact, this is the only purpose of sending this new field to the backend at this time). * [X] ~~Pass a `includeUpdatedModels` value to the Stimulus controller ONLY when in `dev` mode.~~ For consistency, we will always pass the `updatedModels` in our Ajax requests, though this is intended to be "internal" and we won't use it other than to throw better errors. * [X] ~~(Optional) If the backend has an unexpected exception (i.e. 5xx or maybe some 4xx where we mark that this is a problem we should show the developer), then render the entire HTML exception page (in dev mode only) so the user can see the error.~~ Done in symfony#467 Cheers! Commits ------- 4fbcebe cs fix and small tweaks a0c4d3f use `Request::toArray()` 13d7110 mildly reworking to follow the logic of what is happening more clearly (#5) b47ece1 [Live] add batch action controller 8b1e879 Refactoring towards single request & pending updates
# This is the 1st commit message: WIP heavy refactoring to Component Initial "hook" system used to reset model field after re-render Adding a 2nd hook to handle window unloaded reinit polling after re-render Adding Component proxy # This is the commit message #2: fixing some tests # This is the commit message #3: Refactoring loading to a hook # This is the commit message #4: fixing tests # This is the commit message #5: rearranging # This is the commit message #6: Refactoring polling to a separate class
Second attempt...