From 3f943d9d33b6fb4ab0454192d69dba8b099cc830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Thu, 23 Oct 2025 13:09:43 +0200 Subject: [PATCH 1/7] Multiline::saveRows() returns void --- demos/form-control/multiline.php | 4 +++- src/Form/Control/Multiline.php | 7 +------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/demos/form-control/multiline.php b/demos/form-control/multiline.php index 8de33913a5..a63ec6b01f 100644 --- a/demos/form-control/multiline.php +++ b/demos/form-control/multiline.php @@ -58,7 +58,9 @@ $form->onSubmit(static function (Form $form) use ($multiline) { $rows = $multiline->model->atomic(static function () use ($multiline) { - return $multiline->saveRows()->model->export(); + $multiline->saveRows(); + + return $multiline->model->export(); }); $rows = array_map(static fn ($row) => $form->getApp()->uiPersistence->typecastSaveRow($multiline->model, $row), $rows); diff --git a/src/Form/Control/Multiline.php b/src/Form/Control/Multiline.php index a64d284235..012e4e0870 100644 --- a/src/Form/Control/Multiline.php +++ b/src/Form/Control/Multiline.php @@ -466,10 +466,7 @@ public function validate(array $rows, array $mlids): array return $rowErrors; } - /** - * @return $this - */ - public function saveRows(): self + public function saveRows(): void { $model = $this->model; @@ -494,8 +491,6 @@ public function saveRows(): self $entity->save(); } } - - return $this; } /** From 8df92739c6cb444a36bb73a67517f49c3fc3bfe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Thu, 23 Oct 2025 13:11:41 +0200 Subject: [PATCH 2/7] do not typecast readOnly fields - they cannot be set using Model::set --- src/Form/Control/Multiline.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Form/Control/Multiline.php b/src/Form/Control/Multiline.php index 012e4e0870..c352409382 100644 --- a/src/Form/Control/Multiline.php +++ b/src/Form/Control/Multiline.php @@ -353,6 +353,13 @@ private function decodeInput(string $json): array foreach ($rowDataWithMlid as $row) { $mlids[] = $row['__atkml']; unset($row['__atkml']); + + foreach ($row as $k => $v) { + if ($this->model->getField($k)->readOnly) { + unset($row[$k]); + } + } + $rowData[] = $this->typecastUiLoadRow($row); } From d316c0efb995915fac9172771cc4ed7658eefd6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Wed, 29 Oct 2025 00:41:00 +0100 Subject: [PATCH 3/7] workaround autoincrement for custom ID type for ContainsXxx save --- demos/init-db.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/demos/init-db.php b/demos/init-db.php index d75c7ea03a..067d5d5fa2 100644 --- a/demos/init-db.php +++ b/demos/init-db.php @@ -284,6 +284,29 @@ protected function init(): void $this->getIdField()->type = WrappedIdType::NAME; + // workaround autoincrement for custom ID type for ContainsXxx save + // https://github.com/atk4/data/blob/6.0.0/src/Persistence/Array_.php#L324 + $this->onHookShort(Model::HOOK_BEFORE_INSERT, function (&$data) { + $persistence = $this->getModel()->getPersistence(); + if ($persistence instanceof Persistence\Array_) { + $idField = $this->getIdField(); + + if (isset($data[$idField->shortName])) { + return; + } + + assert($this->getModel()->containedInEntity !== null); + + $origIdFieldType = $idField->type; + $idField->type = 'bigint'; + try { + $data[$idField->shortName] = new WrappedId($persistence->generateNewId($this->getModel())); + } finally { + $idField->type = $origIdFieldType; + } + } + }); + $this->initPreventModification(); if ($this->getPersistence()->getDatabasePlatform() instanceof PostgreSQLPlatform || class_exists(CodeCoverage::class, false)) { From 2f19d861c111070f8cd91dcbc11649748647e3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Wed, 29 Oct 2025 00:41:18 +0100 Subject: [PATCH 4/7] Save Multiline using structured changes --- src/Form/Control/Multiline.php | 131 +++++------------------------- src/Form/Control/TheirChanges.php | 77 ++++++++++++++++++ 2 files changed, 97 insertions(+), 111 deletions(-) create mode 100644 src/Form/Control/TheirChanges.php diff --git a/src/Form/Control/Multiline.php b/src/Form/Control/Multiline.php index c352409382..ba60732387 100644 --- a/src/Form/Control/Multiline.php +++ b/src/Form/Control/Multiline.php @@ -161,8 +161,8 @@ class Multiline extends Form\Control /** @var list The fields names used in each row. */ public $rowFields; - /** @var list> The data sent for each row. */ - protected $rowData; + /** The changes set by self::setInputValue(). */ + public TheirChanges $changes; /** @var int The max number of records (rows) that can be added to Multiline. 0 means no limit. */ public $rowLimit = 0; @@ -295,53 +295,6 @@ public function getInputValue(): string return $this->getApp()->encodeJson($rowsUi); } - /** - * Same as Persistence::typecastSaveRow() but allow null in not-nullable fields. - * - * @param array $row - * - * @return array - */ - private function typecastContainedSaveRow(array $row): array - { - $res = []; - foreach ($row as $fieldName => $value) { - $field = $this->model->getField($fieldName); - - $res[$field->getPersistenceName()] = $value === null - ? null - : $this->model->getPersistence()->typecastSaveField($field, $value); - } - - return $res; - } - - /** - * @param \Closure(): void $fx - */ - private function invokeWithContainsXxxNormalizeHookIgnored(\Closure $fx): void - { - // TOD hack to supress "Contained model data cannot be modified directly" exception - // https://github.com/atk4/data/blob/fca1cd55b0/src/Reference/ContainsBase.php#L58-L81 - $ourModel = $this->entityField->getModel(); - \Closure::bind(static function () use ($fx, $ourModel) { - $spot = Model::HOOK_NORMALIZE; - $priority = \PHP_INT_MIN; - $normalizeHooks = $ourModel->hooks[$spot][$priority] ?? []; - foreach (array_keys($normalizeHooks) as $hookIndex) { - $ourModel->hooks[$spot][$priority][$hookIndex][0] = static fn () => null; - } - - try { - $fx(); - } finally { - foreach (array_keys($normalizeHooks) as $hookIndex) { - $ourModel->hooks[$spot][$priority][$hookIndex][0] = $normalizeHooks[$hookIndex][0]; - } - } - }, null, Model::class)(); - } - /** * @return array{list>, list} */ @@ -371,56 +324,34 @@ public function setInputValue(string $value): void { [$rowData, $mlids] = $this->decodeInput($value); - $this->rowData = $rowData; - if ($this->rowData !== []) { - $this->rowErrors = $this->validate($this->rowData, $mlids); + if ($rowData !== []) { + $this->rowErrors = $this->validate($rowData, $mlids); if ($this->rowErrors !== []) { throw new ValidationException([$this->shortName => 'Multiline error']); } } - $rowsRaw = array_map(fn ($v) => $this->typecastContainedSaveRow($v), $this->rowData); + $refModelOrEntity = $this->entityField->getField()->hasReference() + ? $this->entityField->getField()->getReference()->ref($this->entityField->getEntity()) + : $this->model; - // mimic ContainsOne save format - // https://github.com/atk4/data/blob/6.0.0/src/Reference/ContainsOne.php#L37 - // TODO replace with something like "schedule model save task" and then drop self::saveRows() method - if ($rowsRaw === []) { - $value = ''; - } else { - foreach ($rowsRaw as $k => $rowRaw) { - $idFieldRawName = $this->model->getIdField()->getPersistenceName(); - if ($rowRaw[$idFieldRawName] === null) { - $refModel = $this->entityField->getField()->hasReference() - ? $this->entityField->getField()->getReference()->ref($this->entityField->getEntity())->getModel(true) - : $this->model; - - // TODO to pass Behat tests purely - if (!$this->entityField->getField()->hasReference() && $this->entityField->getField()->type !== 'json' && !$refModel->getPersistence() instanceof Persistence\Array_) { - continue; - } + $changes = new TheirChanges(); - $idField = $refModel->getIdField(); - $origIdFieldType = $idField->type; - $idField->type = 'bigint'; - try { - $rowsRaw[$k][$idFieldRawName] = Persistence\Array_::assertInstanceOf($refModel->getPersistence())->generateNewId($refModel); - } finally { - $idField->type = $origIdFieldType; - } - } - } + // TODO this is dangerous, deleted row IDs should be passed from UI + $idsToDelete = array_filter(array_column($rowData, $refModelOrEntity->idField), static fn ($v) => $v !== null); + foreach ($refModelOrEntity->getModel(true)->createIteratorBy($refModelOrEntity->idField, 'not in', $idsToDelete) as $entity) { + $changes->deletes[] = [$refModelOrEntity->idField => $entity->getId()]; + } - if ($this->isOneToOne()) { - assert(count($rowsRaw) === 1); - $rowsRaw = array_first($rowsRaw); + foreach ($rowData as $row) { + if ($row[$refModelOrEntity->idField] === null) { + $changes->inserts[] = $row; + } else { + $changes->updates[] = [[$refModelOrEntity->idField => $row[$refModelOrEntity->idField]], $row]; } - - $value = $this->getApp()->encodeJson($rowsRaw); } - $this->invokeWithContainsXxxNormalizeHookIgnored(function () use ($value) { - parent::setInputValue($value); - }); + $this->changes = $changes; } /** @@ -475,29 +406,7 @@ public function validate(array $rows, array $mlids): array public function saveRows(): void { - $model = $this->model; - - // delete removed rows - // TODO this is dangerous, deleted row IDs should be passed from UI - $idsToDelete = array_filter(array_column($this->rowData, $model->idField), static fn ($v) => $v !== null); - foreach ($model->createIteratorBy($model->idField, 'not in', $idsToDelete) as $entity) { - $entity->delete(); - } - - foreach ($this->rowData as $row) { - $entity = $row[$model->idField] !== null - ? $model->load($row[$model->idField]) - : $model->createEntity(); - foreach ($row as $fieldName => $value) { - if ($model->getField($fieldName)->isEditable()) { - $entity->set($fieldName, $value); - } - } - - if (!$entity->isLoaded() || $entity->getDirtyRef() !== []) { - $entity->save(); - } - } + $this->changes->saveTo($this->model); } /** diff --git a/src/Form/Control/TheirChanges.php b/src/Form/Control/TheirChanges.php new file mode 100644 index 0000000000..05ef0f9d88 --- /dev/null +++ b/src/Form/Control/TheirChanges.php @@ -0,0 +1,77 @@ +> */ + public array $inserts = []; + + /** @var list, array}> */ + public array $updates = []; + + /** @var list> */ + public array $deletes = []; + + /** + * @param array $oldData + */ + protected function loadEntity(Model $modelOrEntity, array $oldData): Model + { + $entity = $modelOrEntity->isEntity() + ? $modelOrEntity + : $modelOrEntity->load($oldData[$modelOrEntity->idField]); + + foreach ($oldData as $k => $v) { + if (!$entity->compare($k, $v)) { + throw (new Exception('Field value does not match expected value')) + ->addMoreInfo('entity', $entity) + ->addMoreInfo('field', $k) + ->addMoreInfo('valueExpected', $v) + ->addMoreInfo('valueActual', $entity->get($k)); + } + } + + return $entity; + } + + public function saveTo(Model $theirModelOrEntity): void + { + $theirModelOrEntity->atomic(function () use ($theirModelOrEntity) { + foreach ($this->deletes as $oldData) { + $entity = $this->loadEntity($theirModelOrEntity, $oldData); + + $entity->delete(); + } + + foreach ($this->updates as [$oldData, $newData]) { + $entity = $this->loadEntity($theirModelOrEntity, $oldData); + + $entity->setMulti($newData); + $entity->save(); + } + + foreach ($this->inserts as $newData) { + $entity = $theirModelOrEntity->isEntity() + ? $theirModelOrEntity + : $theirModelOrEntity->createEntity(); + $this->loadEntity($entity, [$theirModelOrEntity->idField => null]); + + if (($newData[$theirModelOrEntity->idField] ?? null) === null) { + unset($newData[$theirModelOrEntity->idField]); + } + + $entity->setMulti($newData); + $entity->save(); + } + }); + } +} From 8ffdde6b8a526bffbd2fbac9ccbe0273980ed8f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Wed, 29 Oct 2025 00:50:56 +0100 Subject: [PATCH 5/7] autosave ref data using after_save hook --- .../multiline-containsmany-single.php | 5 +++ docs/multiline.md | 2 +- src/Form/Control/Multiline.php | 31 ++++++++++++------- src/Form/Control/TheirChanges.php | 17 ++++++++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/demos/form-control/multiline-containsmany-single.php b/demos/form-control/multiline-containsmany-single.php index 04ca6ef3c6..f1157547d3 100644 --- a/demos/form-control/multiline-containsmany-single.php +++ b/demos/form-control/multiline-containsmany-single.php @@ -14,5 +14,10 @@ $form = Form::addTo($app); $form->setEntity((new MultilineDelivery($app->db))->createEntity()); $form->onSubmit(static function (Form $form) { + // save ContainsXxx data to JSON + // https://github.com/atk4/data/blob/6.0.0/src/Reference/ContainsOne.php#L29-L40 + $form->entity->save(); + $form->entity->setNull($form->entity->idField); + return new JsToast($form->getApp()->encodeJson($form->entity->get())); }); diff --git a/docs/multiline.md b/docs/multiline.md index 123a6e0f34..90a45cee07 100644 --- a/docs/multiline.md +++ b/docs/multiline.md @@ -125,7 +125,7 @@ $userForm->onSubmit(function (Form $form) use ($ml) { // save emails record related to current user $ml->saveRows(); - return new JsToast(var_export($ml->model->export(), true)); + return new JsToast(json_encode($ml->model->export())); }); ``` diff --git a/src/Form/Control/Multiline.php b/src/Form/Control/Multiline.php index ba60732387..c1dbf18895 100644 --- a/src/Form/Control/Multiline.php +++ b/src/Form/Control/Multiline.php @@ -276,17 +276,17 @@ private function isOneToOne(): bool #[\Override] public function getInputValue(): string { - $refModelOrEntity = $this->entityField->getField()->hasReference() + $theirModelOrEntity = $this->entityField->getField()->hasReference() ? $this->entityField->getField()->getReference()->ref($this->entityField->getEntity()) : $this->model; - if ($refModelOrEntity->isEntity()) { - $refModelOrEntity = $refModelOrEntity->isLoaded() - ? [$refModelOrEntity] + if ($theirModelOrEntity->isEntity()) { + $theirModelOrEntity = $theirModelOrEntity->isLoaded() + ? [$theirModelOrEntity] : []; } $rows = []; - foreach ($refModelOrEntity as $row) { + foreach ($theirModelOrEntity as $row) { $rows[] = $row->get(); } @@ -331,27 +331,34 @@ public function setInputValue(string $value): void } } - $refModelOrEntity = $this->entityField->getField()->hasReference() + $theirModelOrEntity = $this->entityField->getField()->hasReference() ? $this->entityField->getField()->getReference()->ref($this->entityField->getEntity()) : $this->model; $changes = new TheirChanges(); // TODO this is dangerous, deleted row IDs should be passed from UI - $idsToDelete = array_filter(array_column($rowData, $refModelOrEntity->idField), static fn ($v) => $v !== null); - foreach ($refModelOrEntity->getModel(true)->createIteratorBy($refModelOrEntity->idField, 'not in', $idsToDelete) as $entity) { - $changes->deletes[] = [$refModelOrEntity->idField => $entity->getId()]; + $idsToDelete = array_filter(array_column($rowData, $theirModelOrEntity->idField), static fn ($v) => $v !== null); + foreach ($theirModelOrEntity->getModel(true)->createIteratorBy($theirModelOrEntity->idField, 'not in', $idsToDelete) as $entity) { + $changes->deletes[] = [$theirModelOrEntity->idField => $entity->getId()]; } foreach ($rowData as $row) { - if ($row[$refModelOrEntity->idField] === null) { + if ($row[$theirModelOrEntity->idField] === null) { $changes->inserts[] = $row; } else { - $changes->updates[] = [[$refModelOrEntity->idField => $row[$refModelOrEntity->idField]], $row]; + $changes->updates[] = [[$theirModelOrEntity->idField => $row[$theirModelOrEntity->idField]], $row]; } } $this->changes = $changes; + + if ($this->entityField->getField()->hasReference()) { + $changes->saveOnSave( + $this->entityField->getEntity(), + $this->entityField->getField()->getReference() + ); + } } /** @@ -406,6 +413,8 @@ public function validate(array $rows, array $mlids): array public function saveRows(): void { + assert(!$this->entityField->getField()->hasReference()); + $this->changes->saveTo($this->model); } diff --git a/src/Form/Control/TheirChanges.php b/src/Form/Control/TheirChanges.php index 05ef0f9d88..222502d8fb 100644 --- a/src/Form/Control/TheirChanges.php +++ b/src/Form/Control/TheirChanges.php @@ -6,6 +6,7 @@ use Atk4\Data\Exception; use Atk4\Data\Model; +use Atk4\Data\Reference; /** * TODO move to atk4/data and allow deep/nested changes. @@ -74,4 +75,20 @@ public function saveTo(Model $theirModelOrEntity): void } }); } + + public function saveOnSave(Model $ourEntity, Reference $theirReference): void + { + $ourEntity->assertIsEntity(); + $theirReference->assertOurModelOrEntity($ourEntity); + + $hookIndex = $ourEntity->onHook(Model::HOOK_AFTER_SAVE, function (Model $m) use ($ourEntity, $theirReference, &$hookIndex) { + assert($m === $ourEntity); // prevent cloning + + $ourEntity->removeHook(Model::HOOK_AFTER_SAVE, $hookIndex, true); + + $theirModelOrEntity = $theirReference->ref($m); + + $this->saveTo($theirModelOrEntity); + }); + } } From 2ddc57df4094ba40bbe291b66f2699c4883d1416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Wed, 29 Oct 2025 01:20:40 +0100 Subject: [PATCH 6/7] add row delete test using "their reference changes" --- tests-behat/multiline.feature | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests-behat/multiline.feature b/tests-behat/multiline.feature index e661955be6..d15acb027d 100644 --- a/tests-behat/multiline.feature +++ b/tests-behat/multiline.feature @@ -102,6 +102,14 @@ Feature: Multiline Then I check if text in "//table//tr[1]/td[3]" match regex "~\"atk_afp_tbl__box\":8[,}]~" Then I check if text in "//table//tr[1]/td[3]" match regex "~\{\"atk_afp_tbl__id\":2,[^{}]*\"atk_afp_tbl__item\":\"Mango\",~" + When I click using selector "//table//tr[1]//i.icon.edit" + When I click using selector "(//div.modal.active//div.field[label[text()='Items']]//tbody//input[@type=\"checkbox\"])[1]" + When I click using selector "//div.modal.active//div.field[label[text()='Items']]//tfoot//button[i.trash.icon]" + When I press Modal button "Save" + Then Toast display should contain text "Record has been saved!" + Then I check if text in "//table//tr[1]/td[2]" match regex "~^\{\"atk_afp_tbl__id\":1,[^{}]*\"atk_afp_tbl__item\":\"Melon\",~" + Then I check if text in "//table//tr[1]/td[3]" match regex "~^\[\{\"atk_afp_tbl__id\":2,[^{}]*\"atk_afp_tbl__item\":\"Mango\",~" + When I click using selector "//table//tr[1]//i.icon.trash" When I press Modal button "Ok" Then Toast display should contain text "Record has been deleted!" From b7e0822dc7da526985c3c8f743146c959e47707a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Wed, 29 Oct 2025 01:44:52 +0100 Subject: [PATCH 7/7] impl. simple save in interactive/loader2.php demo --- demos/interactive/loader2.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/demos/interactive/loader2.php b/demos/interactive/loader2.php index 46c1e97381..60ac5866a3 100644 --- a/demos/interactive/loader2.php +++ b/demos/interactive/loader2.php @@ -9,6 +9,7 @@ use Atk4\Ui\Columns; use Atk4\Ui\Form; use Atk4\Ui\Grid; +use Atk4\Ui\Js\JsToast; use Atk4\Ui\Loader; use Atk4\Ui\Text; use Atk4\Ui\View; @@ -35,4 +36,9 @@ $form = Form::addTo($p); $form->setEntity($country->load($id)); + $form->onSubmit(static function (Form $form) { + $message = $form->entity->getUserAction('edit')->execute(); + + return new JsToast($message); + }); });