From 9d676d6d31f8713a7b6696cf0e913e35d7eb9e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Nykl=C3=AD=C4=8Dek?= <60318239+ONyklicek@users.noreply.github.com> Date: Sun, 6 Sep 2026 12:34:57 +0200 Subject: [PATCH] Dehydrate an action modal's form data on submit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write path has had a named seam since ADR 0021 — a field shapes its own value on the way out — but it had exactly two hosts: Form::save() through SaveHandler, and an editable table cell through updateTableCell(). An action modal was not one of them. submitActionModal() / callMountedAction() read mountedActions.{depth}.data and handed it to the callback untouched, so the same schema wrote different values depending on which host persisted it: Select::dehydrateState()'s documented '' -> null rule simply did not apply inside a modal, and a cleared numeric field reached the column as '' — SQLSTATE[22007] "Incorrect decimal value: ''" on any strict MySQL. The schema walk moves out of SaveHandler into Forms\Runtime\StateDehydrator, the canonical owner both hosts now call, and the action runtime gains the seam next to the validation one it already had: dehydrateMountedActionFormData(), a no-op in core, overridden by the wire-forms bridge, so a form-free host is unaffected. Wizards dehydrate every step, not only the one on screen at submit; repeater children are dehydrated per item. Footer actions are deliberately excluded — they read the form mid-edit and write back into the same bag, so dehydrating there would hand the callback a value the form no longer holds, and would run a FileUpload's store on a form the user never submitted. TextInput had no dehydration at all, so numeric() was a rendering hint and nothing more — this half reached Form::save() too. A field whose HTML type is number now stores null when it is left empty; every other type is untouched on purpose, because '' is a legitimate string and nulling it would break a non-nullable text column rather than save it. The tests are mutation-checked: removing either call site fails them. --- CHANGELOG.md | 2 + docs/core/actions.md | 2 + docs/cs/core/actions.md | 2 + docs/cs/forms/custom-fields.md | 12 +- docs/cs/forms/fields/text-input.md | 17 ++ docs/forms/custom-fields.md | 13 +- docs/forms/fields/text-input.md | 18 ++ .../Actions/Concerns/InteractsWithActions.php | 20 +++ .../Actions/InteractsWithActionsTest.php | 22 +++ packages/forms/src/Components/TextInput.php | 27 ++- .../src/Concerns/InteractsWithActionForms.php | 54 ++++++ packages/forms/src/Concerns/WithActions.php | 5 +- .../forms/src/Forms/Runtime/SaveHandler.php | 115 +------------ .../src/Forms/Runtime/StateDehydrator.php | 129 ++++++++++++++ .../Feature/ActionFormDehydrationTest.php | 159 ++++++++++++++++++ .../tests/Unit/Components/TextInputTest.php | 21 +++ .../Unit/Runtime/StateDehydratorTest.php | 59 +++++++ .../src/Concerns/InteractsWithTableModals.php | 4 + packages/table/src/Concerns/WithTable.php | 1 + .../Feature/ActionModalDehydrationTest.php | 99 +++++++++++ 20 files changed, 667 insertions(+), 114 deletions(-) create mode 100644 packages/forms/src/Forms/Runtime/StateDehydrator.php create mode 100644 packages/forms/tests/Feature/ActionFormDehydrationTest.php create mode 100644 packages/forms/tests/Unit/Runtime/StateDehydratorTest.php create mode 100644 packages/table/tests/Feature/ActionModalDehydrationTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index fc179b1d..7784003f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to the Wire ecosystem will be documented in this file. ## [1.17.4] ### Fixed +- **An action modal handed its callback raw widget state, so a cleared number input arrived as `''` and killed the insert: `SQLSTATE[22007] Incorrect decimal value: '' for column 'total_discount_percent'`.** The write path has had a named seam since ADR 0021 — a field shapes its own value on the way out, which is how a cleared `Select` becomes `null` instead of `''` and how a `DateTimePicker` applies its storage format and zone — but the seam had exactly two hosts: `Form::save()` (through `SaveHandler`) and an editable table cell (`updateTableCell()`). **An action modal was not one of them.** `submitActionModal()` / `callMountedAction()` read `mountedActions.{depth}.data` and passed it to the callback untouched, so the same schema wrote different values depending on which host persisted it: `Select::dehydrateState()`'s documented `'' → null` rule simply did not apply inside a modal, and `->form([...])` plus `->action(fn (array $data) => Model::create($data))` — the shape every create modal has — put an empty string into whatever column the user had cleared. The schema walk moved out of `SaveHandler` into `Forms\Runtime\StateDehydrator`, the canonical owner both hosts now call, and the action runtime gained the seam next to the validation one it already had (`dehydrateMountedActionFormData()`, a no-op in core, overridden by the wire-forms bridge, so a form-free host is unaffected). Wizards dehydrate every step, not just the one on screen at submit; repeater children are dehydrated per item. Footer actions are deliberately excluded: they read the form mid-edit and write back into the same bag, so dehydrating there would hand the callback a value the form no longer holds — and would run a `FileUpload`'s store on a form the user never submitted. +- **`TextInput` had no dehydration at all, so `->numeric()` was a rendering hint and nothing more.** An emptied `` submits `''`, which no numeric column can hold: MySQL in strict mode refuses the write and a lenient driver silently stores `0`. Neither is what an author who left an optional amount blank asked for, and unlike the modal bug above this one reached `Form::save()` too. A field whose HTML type is `number` (`numeric()`, `integer()`, `type('number')`) now stores `null` when it is left empty. Every other type is untouched on purpose — `''` is a legitimate string, and nulling it would break a non-nullable text column rather than save it. - **An action modal prefilled from an enum-cast column died on `Select::getOptionLabel(): Argument #1 ($value) must be of type string|int|null, App\Enums\Status given`.** `fillFormUsing(fn ($record) => ['status' => $record->status])` is the obvious way to write a prefill, and on a column with an enum cast the attribute is the case object, not its backing value. That object went into the modal's form-data bag untouched: `HasModal::getFormDefaults()` returned `$overrides + $seed` raw, and the bag is written **straight into Livewire state** by the host (`WithActions`, `InteractsWithTableActions`, `WithTable`) — the one seeding path that never passes through the form runtime, whose `StateManager::fill()` has collapsed enums to scalars all along. So the enum reached the field's own render, where `getSelectedOptionLabels()` handed it to a `string|int|null` parameter and PHP fataled — a 500 on opening the modal, before the user could touch anything. Two things were wrong with it beyond the crash: an enum instance cannot round-trip through Livewire state to the browser, and a `` submits `''`, and no numeric column can + * hold that: MySQL in strict mode rejects the write outright ("Incorrect + * decimal value: ''") and a lenient driver silently stores 0. Neither is + * what an author who left an optional amount blank asked for. This is the + * same rule Select already applies to its placeholder choice. + * + * Text is deliberately untouched: `''` is a legitimate string value, and + * turning it into null would break a non-nullable column that holds one. + */ + public function dehydrateState(mixed $state, ?Model $record = null): mixed + { + if ($this->inputType !== 'number') { + return $state; + } + + return (is_string($state) && trim($state) === '') ? null : $state; + } + // ─── Constraints ─────────────────────────────────────────────── /** Set the minimum numeric value (a value or a `$get`-aware Closure). */ diff --git a/packages/forms/src/Concerns/InteractsWithActionForms.php b/packages/forms/src/Concerns/InteractsWithActionForms.php index ecbc4c77..ee5af018 100644 --- a/packages/forms/src/Concerns/InteractsWithActionForms.php +++ b/packages/forms/src/Concerns/InteractsWithActionForms.php @@ -4,6 +4,7 @@ namespace NyonCode\WireForms\Concerns; +use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\Validator; use NyonCode\WireCore\Actions\Action; use NyonCode\WireCore\Actions\ActionHalt; @@ -11,6 +12,7 @@ use NyonCode\WireCore\Actions\HeaderAction; use NyonCode\WireCore\Actions\ModalStep; use NyonCode\WireForms\Forms\Form; +use NyonCode\WireForms\Forms\Runtime\StateDehydrator; use Throwable; /** @@ -184,6 +186,58 @@ protected function validateMountedActionForm(): void $this->actionModalFormInstance?->validate(); } + /** + * Dehydrate the submitted bag through the active modal's own schema, so an + * action callback receives what the form would have persisted — a cleared + * Select as null rather than '', a date in its storage format and zone, a + * FileUpload as its stored path. + * + * Without this the two write paths disagree: the same schema saved through + * Form::save() goes through {@see StateDehydrator}, while an action modal + * handed the raw Livewire state straight to the callback. + * + * A wizard shares one bag across its steps, so every step's schema gets a + * pass — the fields of a step that is not on screen at submit time still + * own their values. + * + * @param array $data + * @return array + */ + protected function dehydrateMountedActionFormData(array $data): array + { + [$action, $context] = $this->resolveCurrentActionForm(); + + // Defensive: submit paths resolve a non-null action before they get here. + // @codeCoverageIgnoreStart + if ($action === null) { + return $data; + } + // @codeCoverageIgnoreEnd + + $record = $context instanceof Model ? $context : null; + $statePath = $this->actionFrameStatePath($this->topActionFrameIndex()); + + if ($action->hasMultipleSteps()) { + for ($step = 0; $step < $action->getStepCount(); $step++) { + $form = $action->getStepFormInstance($this, $context, $step, $statePath); + + if ($form instanceof Form) { + $data = StateDehydrator::dehydrate($form->getSchema(), $data, $record); + } + } + + return $data; + } + + // Reuse the instance validation already resolved for this frame. + $form = $this->actionModalFormInstance + ?? $this->buildModalActionFormInstance($action, $context); + + return $form instanceof Form + ? StateDehydrator::dehydrate($form->getSchema(), $data, $record) + : $data; + } + // ========================================== // Wizard stepping // ========================================== diff --git a/packages/forms/src/Concerns/WithActions.php b/packages/forms/src/Concerns/WithActions.php index 21287480..dfab868b 100644 --- a/packages/forms/src/Concerns/WithActions.php +++ b/packages/forms/src/Concerns/WithActions.php @@ -48,6 +48,7 @@ trait WithActions // (no-op in core so a form-free host still works standalone). use InteractsWithActionForms, InteractsWithActions { InteractsWithActionForms::validateMountedActionForm insteadof InteractsWithActions; + InteractsWithActionForms::dehydrateMountedActionFormData insteadof InteractsWithActions; InteractsWithActionForms::resolveHaltModalForm insteadof InteractsWithActions; InteractsWithActionForms::getActionModalFormInstance insteadof InteractsWithActions; InteractsWithActionForms::getActionModalFormInstanceForDepth insteadof InteractsWithActions; @@ -209,7 +210,9 @@ public function callMountedAction(): void $this->runStandaloneAction( $action, $record instanceof Model ? $record : null, - $this->getMountedActionFormData(), + // The callback gets what the form would have persisted, not the raw + // widget state — the same seam Form::save() runs (ADR 0021). + $this->dehydrateMountedActionFormData($this->getMountedActionFormData()), (array) $this->getMountedActionState('arguments', []), ); diff --git a/packages/forms/src/Forms/Runtime/SaveHandler.php b/packages/forms/src/Forms/Runtime/SaveHandler.php index 7b22637d..10ff28d9 100644 --- a/packages/forms/src/Forms/Runtime/SaveHandler.php +++ b/packages/forms/src/Forms/Runtime/SaveHandler.php @@ -15,8 +15,6 @@ use NyonCode\WireCore\Core\Plugin\Hooks\FormSavingPayload; use NyonCode\WireCore\Core\Plugin\PluginManager; use NyonCode\WireCore\Foundation\Components\LayoutComponent; -use NyonCode\WireCore\Foundation\Contracts\DehydratesState; -use NyonCode\WireForms\Components\Field; use NyonCode\WireForms\Components\MorphToSelect; use NyonCode\WireForms\Components\Repeater; use NyonCode\WireForms\Components\Tags; @@ -59,7 +57,11 @@ public function save(): mixed // FileUpload moves validated temporary uploads to permanent storage (so // an abandoned form leaves no orphan) and how a date field applies its // storage format and timezone. - $data = $this->dehydrateFields($data); + $data = StateDehydrator::dehydrate( + $this->config->schema, + $data, + $this->config->model instanceof Model ? $this->config->model : null, + ); // 3. Plugin hook: form.saving (can modify data) if (app()->bound(PluginManager::class)) { @@ -186,113 +188,6 @@ private function persist(array $data): mixed return $instance; } - /** - * Move any pending file uploads in the data to permanent storage and replace - * them with their stored paths, keeping already-stored paths as-is (merge). - * - * @param array $data - * @return array - */ - /** - * Apply every field's own dehydration to the data about to be persisted. - * - * @param array $data - * @return array - */ - private function dehydrateFields(array $data): array - { - $model = $this->config->model instanceof Model ? $this->config->model : null; - - // Top-level fields (not nested inside a repeater). - foreach ($this->collectDehydratingFields($this->config->schema) as $field) { - $name = $field->getName(); - - if (! array_key_exists($name, $data)) { - continue; - } - - $data[$name] = $field->dehydrateState($data[$name], $model); - } - - // Repeater children: a DehydratesState child (FileUpload storing its - // upload, DateTimePicker applying format/timezone) lives under the - // repeater key as an array of items, so the top-level pass never reaches - // it. Without this a nested file is never moved to permanent storage and a - // nested date keeps its raw wire value. - foreach ($this->dehydratingRepeaters($this->config->schema) as $repeater) { - $name = $repeater->getName(); - - if (! isset($data[$name]) || ! is_array($data[$name])) { - continue; - } - - $childFields = $this->collectDehydratingFields($repeater->getSchema()); - - foreach ($data[$name] as $index => $item) { - if (! is_array($item)) { - continue; - } - - foreach ($childFields as $child) { - $childName = $child->getName(); - - if (array_key_exists($childName, $item)) { - $data[$name][$index][$childName] = $child->dehydrateState($item[$childName], $model); - } - } - } - } - - return $data; - } - - /** - * Repeaters anywhere in the schema (used to dehydrate their child fields). - * - * @param array $schema - * @return array - */ - private function dehydratingRepeaters(array $schema): array - { - $repeaters = []; - - foreach ($schema as $component) { - if ($component instanceof Repeater) { - $repeaters[] = $component; - } elseif ($component instanceof LayoutComponent) { - $repeaters = array_merge($repeaters, $this->dehydratingRepeaters($component->getSchema())); - } - } - - return $repeaters; - } - - /** - * Collect every field that dehydrates its own state, traversing nested layouts. - * - * Only a Field carries the name that keys the data array — a layout could - * implement the contract without one. - * - * @param array $schema - * @return array - */ - private function collectDehydratingFields(array $schema): array - { - $fields = []; - - foreach ($schema as $component) { - if ($component instanceof DehydratesState && $component instanceof Field) { - $fields[] = $component; - } elseif ($component instanceof LayoutComponent && ! $component instanceof Repeater) { - // Repeaters are handled per-item by dehydratingRepeaters(); their - // children must not be flattened into the top-level key match. - $fields = array_merge($fields, $this->collectDehydratingFields($component->getSchema())); - } - } - - return $fields; - } - /** * Collect the field names of all relationship-backed repeaters in the schema, * traversing nested layout components. diff --git a/packages/forms/src/Forms/Runtime/StateDehydrator.php b/packages/forms/src/Forms/Runtime/StateDehydrator.php new file mode 100644 index 00000000..5124cb4c --- /dev/null +++ b/packages/forms/src/Forms/Runtime/StateDehydrator.php @@ -0,0 +1,129 @@ + $schema + * @param array $data + * @return array + */ + public static function dehydrate(array $schema, array $data, ?Model $record = null): array + { + // Top-level fields (not nested inside a repeater). + foreach (self::dehydratingFields($schema) as $field) { + $name = $field->getName(); + + if (! array_key_exists($name, $data)) { + continue; + } + + $data[$name] = $field->dehydrateState($data[$name], $record); + } + + // Repeater children: a DehydratesState child (FileUpload storing its + // upload, DateTimePicker applying format/timezone) lives under the + // repeater key as an array of items, so the top-level pass never reaches + // it. Without this a nested file is never moved to permanent storage and a + // nested date keeps its raw wire value. + foreach (self::dehydratingRepeaters($schema) as $repeater) { + $name = $repeater->getName(); + + if (! isset($data[$name]) || ! is_array($data[$name])) { + continue; + } + + $childFields = self::dehydratingFields($repeater->getSchema()); + + foreach ($data[$name] as $index => $item) { + if (! is_array($item)) { + continue; + } + + foreach ($childFields as $child) { + $childName = $child->getName(); + + if (array_key_exists($childName, $item)) { + $data[$name][$index][$childName] = $child->dehydrateState($item[$childName], $record); + } + } + } + } + + return $data; + } + + /** + * Every field that dehydrates its own state, traversing nested layouts. + * + * Only a Field carries the name that keys the data array — a layout could + * implement the contract without one. + * + * @param array $schema + * @return array + */ + private static function dehydratingFields(array $schema): array + { + $fields = []; + + foreach ($schema as $component) { + if ($component instanceof DehydratesState && $component instanceof Field) { + $fields[] = $component; + } elseif ($component instanceof LayoutComponent && ! $component instanceof Repeater) { + // Repeaters are handled per-item by dehydratingRepeaters(); their + // children must not be flattened into the top-level key match. + $fields = array_merge($fields, self::dehydratingFields($component->getSchema())); + } + } + + return $fields; + } + + /** + * Repeaters anywhere in the schema (used to dehydrate their child fields). + * + * @param array $schema + * @return array + */ + private static function dehydratingRepeaters(array $schema): array + { + $repeaters = []; + + foreach ($schema as $component) { + if ($component instanceof Repeater) { + $repeaters[] = $component; + } elseif ($component instanceof LayoutComponent) { + $repeaters = array_merge($repeaters, self::dehydratingRepeaters($component->getSchema())); + } + } + + return $repeaters; + } +} diff --git a/packages/forms/tests/Feature/ActionFormDehydrationTest.php b/packages/forms/tests/Feature/ActionFormDehydrationTest.php new file mode 100644 index 00000000..01a76788 --- /dev/null +++ b/packages/forms/tests/Feature/ActionFormDehydrationTest.php @@ -0,0 +1,159 @@ +|null What the action callback actually received. */ + public ?array $submitted = null; + + protected function actions(): array + { + return [$this->editAction(), $this->wizardAction(), $this->repeaterAction()]; + } + + public function editAction(): Action + { + return Action::make('edit') + ->form([ + TextInput::make('discount')->numeric(), + TextInput::make('note'), + Select::make('status')->options(['draft' => 'Draft', 'published' => 'Published']), + ]) + ->action(fn (array $data) => $this->submitted = $data); + } + + public function wizardAction(): Action + { + return Action::make('wizard') + ->steps([ + ModalStep::make('amounts') + ->schema([TextInput::make('discount')->numeric()]), + ModalStep::make('details') + ->schema([TextInput::make('note')]), + ]) + ->action(fn (array $data) => $this->submitted = $data); + } + + public function repeaterAction(): Action + { + return Action::make('repeater') + ->form([ + Repeater::make('rows')->schema([ + TextInput::make('quantity')->numeric(), + TextInput::make('label'), + ]), + ]) + ->action(fn (array $data) => $this->submitted = $data); + } + + public function render(): string + { + return <<<'BLADE' +
+ +
+ BLADE; + } +} + +/** + * The submitted bag as the action callback received it. + * + * Read back off the instance rather than asserted with assertSet(): Livewire + * compares loosely, so `'' == null` passes and a test written that way stays + * green with the whole seam removed — which is how it was first written here. + * + * @return array + */ +function submittedBag(Testable $component): array +{ + return $component->instance()->submitted ?? []; +} + +it('stores a cleared number input as null, leaving text alone', function () { + $component = Livewire::test(ActionDehydrationHost::class) + ->call('mountAction', 'edit') + ->set('mountedActions.0.data.discount', '') + ->set('mountedActions.0.data.note', '') + ->call('callMountedAction'); + + $bag = submittedBag($component); + + expect($bag['discount'])->toBeNull() + // '' is a legitimate string: a non-nullable text column holds one. + ->and($bag['note'])->toBe(''); +}); + +it('keeps a number that was actually entered', function () { + $component = Livewire::test(ActionDehydrationHost::class) + ->call('mountAction', 'edit') + ->set('mountedActions.0.data.discount', '12.5') + ->call('callMountedAction'); + + expect(submittedBag($component)['discount'])->toBe('12.5'); +}); + +it('applies a fields own dehydration to the action bag, not just to Form::save', function () { + // Select has implemented the seam since ADR 0021; only the save path ran it. + $component = Livewire::test(ActionDehydrationHost::class) + ->call('mountAction', 'edit') + ->set('mountedActions.0.data.status', '') + ->call('callMountedAction'); + + expect(submittedBag($component)['status'])->toBeNull(); +}); + +it('dehydrates every wizard step, not only the one on screen at submit', function () { + $component = Livewire::test(ActionDehydrationHost::class) + ->call('mountAction', 'wizard') + ->set('mountedActions.0.data.discount', '') + ->call('nextActionModalStep') + ->set('mountedActions.0.data.note', 'ok') + ->call('callMountedAction'); + + $bag = submittedBag($component); + + expect($bag['discount'])->toBeNull() + ->and($bag['note'])->toBe('ok'); +}); + +it('dehydrates repeater children per item', function () { + $component = Livewire::test(ActionDehydrationHost::class) + ->call('mountAction', 'repeater') + ->set('mountedActions.0.data.rows', [ + ['quantity' => '', 'label' => 'first'], + ['quantity' => '3', 'label' => ''], + ]) + ->call('callMountedAction'); + + expect(submittedBag($component)['rows'])->toBe([ + ['quantity' => null, 'label' => 'first'], + ['quantity' => '3', 'label' => ''], + ]); +}); diff --git a/packages/forms/tests/Unit/Components/TextInputTest.php b/packages/forms/tests/Unit/Components/TextInputTest.php index 72a40766..ab5c62d1 100644 --- a/packages/forms/tests/Unit/Components/TextInputTest.php +++ b/packages/forms/tests/Unit/Components/TextInputTest.php @@ -334,3 +334,24 @@ public function getLabel(): ?string test('the search preset sets the search input type', function () { expect(TextInput::make('q')->search()->getInputType())->toBe('search'); }); + +/* + * Dehydration (ADR 0021). An emptied `` submits '', which + * no numeric column can hold: MySQL in strict mode refuses the write and a + * lenient driver stores 0. Text is left alone — '' is a value a string column + * legitimately holds, and nulling it would break a non-nullable one. + */ +test('a cleared numeric input dehydrates to null', function (TextInput $field, mixed $state, mixed $expected) { + expect($field->dehydrateState($state))->toBe($expected); +})->with([ + 'numeric, cleared' => [fn () => TextInput::make('amount')->numeric(), '', null], + 'numeric, whitespace only' => [fn () => TextInput::make('amount')->numeric(), ' ', null], + 'integer, cleared' => [fn () => TextInput::make('count')->integer(), '', null], + 'type(number), cleared' => [fn () => TextInput::make('amount')->type('number'), '', null], + 'numeric, entered' => [fn () => TextInput::make('amount')->numeric(), '12.5', '12.5'], + 'numeric, zero' => [fn () => TextInput::make('amount')->numeric(), '0', '0'], + 'numeric, already null' => [fn () => TextInput::make('amount')->numeric(), null, null], + // '' survives on every non-number type, including the ones that look numeric. + 'text, cleared' => [fn () => TextInput::make('name'), '', ''], + 'tel, cleared' => [fn () => TextInput::make('phone')->tel(), '', ''], +]); diff --git a/packages/forms/tests/Unit/Runtime/StateDehydratorTest.php b/packages/forms/tests/Unit/Runtime/StateDehydratorTest.php new file mode 100644 index 00000000..b8717c07 --- /dev/null +++ b/packages/forms/tests/Unit/Runtime/StateDehydratorTest.php @@ -0,0 +1,59 @@ +schema([TextInput::make('discount')->numeric()])]; + + expect(StateDehydrator::dehydrate($schema, ['discount' => ''])) + ->toBe(['discount' => null]); +}); + +it('dehydrates the children of a repeater nested in a layout component', function () { + $schema = [ + Section::make('Rows')->schema([ + Repeater::make('rows')->schema([TextInput::make('quantity')->numeric()]), + ]), + ]; + + expect(StateDehydrator::dehydrate($schema, ['rows' => [['quantity' => '']]])) + ->toBe(['rows' => [['quantity' => null]]]); +}); + +it('leaves keys the schema does not name alone', function () { + $schema = [TextInput::make('discount')->numeric()]; + + expect(StateDehydrator::dehydrate($schema, ['note' => ''])) + ->toBe(['note' => '']); +}); + +it('skips a repeater whose bag entry is missing or is not a list of rows', function (mixed $bag, mixed $expected) { + // A repeater key can be absent (never touched) or hold a scalar (a host that + // wrote the bag itself). Neither is a set of rows to walk. + $schema = [Repeater::make('rows')->schema([TextInput::make('quantity')->numeric()])]; + + expect(StateDehydrator::dehydrate($schema, $bag))->toBe($expected); +})->with([ + 'key absent' => [['other' => 1], ['other' => 1]], + 'not an array' => [['rows' => ''], ['rows' => '']], +]); + +it('skips a repeater row that is not an array of fields', function () { + $schema = [Repeater::make('rows')->schema([TextInput::make('quantity')->numeric()])]; + + expect(StateDehydrator::dehydrate($schema, ['rows' => ['scrap', ['quantity' => '']]])) + ->toBe(['rows' => ['scrap', ['quantity' => null]]]); +}); diff --git a/packages/table/src/Concerns/InteractsWithTableModals.php b/packages/table/src/Concerns/InteractsWithTableModals.php index 8f583ca4..25bd8066 100644 --- a/packages/table/src/Concerns/InteractsWithTableModals.php +++ b/packages/table/src/Concerns/InteractsWithTableModals.php @@ -84,6 +84,10 @@ public function submitActionModal(): void $this->validateMountedActionForm(); + // The callback gets what the form would have persisted, not the raw + // widget state — the same seam Form::save() runs (ADR 0021). + $formData = $this->dehydrateMountedActionFormData($formData); + $stackVersionBefore = $this->actionStackVersion; // Execute action diff --git a/packages/table/src/Concerns/WithTable.php b/packages/table/src/Concerns/WithTable.php index 4bad6544..4e04d7b9 100644 --- a/packages/table/src/Concerns/WithTable.php +++ b/packages/table/src/Concerns/WithTable.php @@ -85,6 +85,7 @@ trait WithTable // engine's defaults. use InteractsWithActionForms, InteractsWithActions, InteractsWithTableActions { InteractsWithActionForms::validateMountedActionForm insteadof InteractsWithActions; + InteractsWithActionForms::dehydrateMountedActionFormData insteadof InteractsWithActions; InteractsWithActionForms::resolveHaltModalForm insteadof InteractsWithActions; InteractsWithActionForms::getActionModalFormInstance insteadof InteractsWithActions; InteractsWithActionForms::getActionModalFormInstanceForDepth insteadof InteractsWithActions; diff --git a/packages/table/tests/Feature/ActionModalDehydrationTest.php b/packages/table/tests/Feature/ActionModalDehydrationTest.php new file mode 100644 index 00000000..ba9b1bf6 --- /dev/null +++ b/packages/table/tests/Feature/ActionModalDehydrationTest.php @@ -0,0 +1,99 @@ +|null */ + public ?array $submitted = null; + + public function table(Table $table): Table + { + return $table + ->model(AmdOffer::class) + ->paginated(false) + ->columns([TextColumn::make('title')]) + ->headerActions([ + HeaderAction::make('create') + ->form([TextInput::make('discount')->numeric(), TextInput::make('title')]) + ->action(fn (array $data) => $this->submitted = $data), + ]) + ->actions([ + Action::make('edit') + ->form([TextInput::make('discount')->numeric()]) + ->action(fn (array $data) => $this->submitted = $data), + ]); + } + + public function render() + { + return $this->getTableProperty(); + } +} + +beforeEach(function () { + config()->set('app.key', 'base64:'.base64_encode(random_bytes(32))); + + Schema::dropIfExists('amd_offers'); + Schema::create('amd_offers', function (Blueprint $table): void { + $table->id(); + $table->string('title')->nullable(); + $table->decimal('discount', 5, 2)->nullable(); + }); +}); + +it('hands a header action the value the form would have persisted', function () { + $component = Livewire::test(AmdComponent::class) + ->call('openHeaderActionModal', 'create') + ->set('tableState.modal.actions.0.data.discount', '') + ->set('tableState.modal.actions.0.data.title', '') + ->call('submitActionModal'); + + $bag = $component->instance()->submitted ?? []; + + // A cleared number input is null; '' would die on a decimal column under a + // strict SQL mode ("Incorrect decimal value: ''"). + expect($bag['discount'])->toBeNull() + ->and($bag['title'])->toBe(''); +}); + +it('hands a row action the same', function () { + $offer = AmdOffer::query()->create(['title' => 'One', 'discount' => 10]); + + $component = Livewire::test(AmdComponent::class) + ->call('openActionModal', (string) $offer->id, 'edit') + ->set('tableState.modal.actions.0.data.discount', '') + ->call('submitActionModal'); + + expect(($component->instance()->submitted ?? [])['discount'])->toBeNull(); +});