From 4a95e50b554b8c9d2b1adde0daab22691d5342a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Fri, 6 Jan 2023 12:54:09 +0100 Subject: [PATCH 1/2] use local object, never typecast internal data --- demos/interactive/cardtable.php | 9 +++++- src/CardTable.php | 19 +++++------ src/Table.php | 57 +++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 14 deletions(-) diff --git a/demos/interactive/cardtable.php b/demos/interactive/cardtable.php index e3c61519ae..4020539009 100644 --- a/demos/interactive/cardtable.php +++ b/demos/interactive/cardtable.php @@ -12,4 +12,11 @@ Header::addTo($app, ['Card displays read-only data of a single record']); -CardTable::addTo($app)->setModel((new Stat($app->db))->loadAny()); +$entity = (new Stat($app->db))->loadAny(); +$entity->project_code .= ' no reload'; + +CardTable::addTo($app)->setModel($entity); + +// CardTable uses internally atk4_local_object type which uses weak references, +// force GC to test the data are kept referenced correctly +gc_collect_cycles(); diff --git a/src/CardTable.php b/src/CardTable.php index c0fe44f2e4..4a5ba1e75f 100644 --- a/src/CardTable.php +++ b/src/CardTable.php @@ -4,6 +4,7 @@ namespace Atk4\Ui; +use Atk4\Data\Field; use Atk4\Data\Model; /** @@ -34,12 +35,12 @@ public function setModel(Model $entity, array $columns = null): void } $data = []; - foreach ($entity->get() as $key => $value) { + foreach (array_keys($entity->get()) as $key) { if (in_array($key, $columns, true)) { $data[] = [ 'id' => $key, 'field' => $entity->getField($key)->getCaption(), - 'value' => $this->getApp()->uiPersistence->typecastSaveField($entity->getField($key), $value), + 'value' => new Model\EntityFieldPair($entity, $key), ]; } } @@ -51,17 +52,13 @@ public function setModel(Model $entity, array $columns = null): void $this->_bypass = false; } - $this->addDecorator('value', [Table\Column\Multiformat::class, function (Model $row) use ($entity) { - $field = $entity->getField($row->getId()); - $ret = $this->decoratorFactory( - $field, - $field->type === 'boolean' ? [Table\Column\Status::class, ['positive' => [true, 'Yes'], 'negative' => [false, 'No']]] : [] - ); - if ($ret instanceof Table\Column\Money) { - $ret->attr['all']['class'] = ['single line']; + $this->addDecorator('value', [Table\Column\Multiformat::class, function (Model $row, Field $field) { + $c = $this->decoratorFactory($field); + if ($c instanceof Table\Column\Money) { + $c->attr['all']['class'] = ['single line']; } - return [$ret]; + return [$c]; }]); } } diff --git a/src/Table.php b/src/Table.php index 861a07cd98..8864dcb88b 100644 --- a/src/Table.php +++ b/src/Table.php @@ -7,6 +7,7 @@ use Atk4\Core\Factory; use Atk4\Data\Field; use Atk4\Data\Model; +use Atk4\Data\Model\EntityFieldPair; use Atk4\Ui\Js\Jquery; use Atk4\Ui\Js\JsChain; use Atk4\Ui\Js\JsExpression; @@ -389,6 +390,40 @@ public function setModel(Model $model, array $columns = null): void } } + public function setSource(array $data, $fields = null): Model + { + // mainly for CardTable to support different field type for different row 1/2 + // related with https://github.com/atk4/data/blob/4.0.0/tests/Persistence/StaticTest.php#L142 + $dataWithObjects = []; + if (is_array(reset($data))) { + foreach ($data as $rowKey => $row) { + foreach ($row as $k => $v) { + if ($v instanceof EntityFieldPair) { + $dataWithObjects[$row['id']][$k] = $v; + $data[$rowKey][$k] = null; + } + } + } + } + + $model = parent::setSource($data, $fields); + + foreach ($dataWithObjects as $id => $row) { + $entity = $model->load($id); + foreach ($row as $k => $v) { + $model->getField($k)->type = 'atk4_local_object'; + $entity->set($k, $v); + } + $entity->save(); + } + $model->onHook(Model::HOOK_BEFORE_LOAD, function () use ($dataWithObjects) { + // useless hook, but make sure the $dataWithObjects is kept referenced from $model + $count = count($dataWithObjects); + }); + + return $model; + } + protected function renderView(): void { if (!$this->columns) { @@ -420,8 +455,26 @@ protected function renderView(): void // the same in Lister class $modelBackup = $this->model; try { - foreach ($this->model as $this->model) { - $this->currentRow = $this->model; + foreach ($this->model as $entityOrig) { + // mainly for CardTable to support different field type for different row 2/2 + $entityCloned = (clone $entityOrig->getModel())->createEntity(); + $entityCloned->setId($entityOrig->getId()); + \Closure::bind(function () use ($entityOrig, $entityCloned) { + foreach ($entityOrig->data as $k => $v) { + $field = $entityCloned->getField($k); + if ($field->type === 'atk4_local_object' && $v instanceof EntityFieldPair) { + $field->type = $v->getField()->type; + $field->enum = $v->getField()->enum; + $field->values = $v->getField()->values; + $field->ui = $v->getField()->ui; + $v = $v->get(); + } + $entityCloned->data[$k] = $v; + } + }, null, Model::class)(); + + $this->model = $entityCloned; + $this->currentRow = $entityCloned; // TODO we should either drop currentRow property or never update model property if ($this->hook(self::HOOK_BEFORE_ROW) === false) { continue; } From 9dc8705b015d678bc50845dcf9fff531f949b11c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Fri, 6 Jan 2023 12:55:48 +0100 Subject: [PATCH 2/2] fix to pass original entity for table column render --- demos/init-db.php | 9 +++++++-- src/Table.php | 25 ++++--------------------- src/Table/Column/Multiformat.php | 7 +++++++ 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/demos/init-db.php b/demos/init-db.php index 5280f87717..b07550b354 100644 --- a/demos/init-db.php +++ b/demos/init-db.php @@ -270,10 +270,15 @@ protected function init(): void 'type' => 'string', 'ui' => [ 'form' => [Form\Control\Line::class], - 'table' => [Table\Column\CountryFlag::class], + 'table' => [Table\Column\CountryFlag::class, 'nameField' => $this->fieldName()->client_country], ], ]) - ->addField($this->fieldName()->client_country, Country::hinting()->fieldName()->name); + ->addField($this->fieldName()->client_country, Country::hinting()->fieldName()->name, [ + // caption needs to be set explicitly because HasOneSql::addField() uses the value from referenced field + // https://github.com/atk4/data/blob/4.0.0/src/Reference/HasOneSql.php#L87 + // https://github.com/atk4/data/pull/447/files#r1063408315 + 'caption' => 'Client Country Name', + ]); $this->addField($this->fieldName()->is_commercial, ['type' => 'boolean']); $this->addField($this->fieldName()->currency, ['values' => ['EUR' => 'Euro', 'USD' => 'US Dollar', 'GBP' => 'Pound Sterling']]); diff --git a/src/Table.php b/src/Table.php index 8864dcb88b..6a47335d5b 100644 --- a/src/Table.php +++ b/src/Table.php @@ -392,7 +392,7 @@ public function setModel(Model $model, array $columns = null): void public function setSource(array $data, $fields = null): Model { - // mainly for CardTable to support different field type for different row 1/2 + // mainly for CardTable to support different field type for different row // related with https://github.com/atk4/data/blob/4.0.0/tests/Persistence/StaticTest.php#L142 $dataWithObjects = []; if (is_array(reset($data))) { @@ -455,26 +455,9 @@ protected function renderView(): void // the same in Lister class $modelBackup = $this->model; try { - foreach ($this->model as $entityOrig) { - // mainly for CardTable to support different field type for different row 2/2 - $entityCloned = (clone $entityOrig->getModel())->createEntity(); - $entityCloned->setId($entityOrig->getId()); - \Closure::bind(function () use ($entityOrig, $entityCloned) { - foreach ($entityOrig->data as $k => $v) { - $field = $entityCloned->getField($k); - if ($field->type === 'atk4_local_object' && $v instanceof EntityFieldPair) { - $field->type = $v->getField()->type; - $field->enum = $v->getField()->enum; - $field->values = $v->getField()->values; - $field->ui = $v->getField()->ui; - $v = $v->get(); - } - $entityCloned->data[$k] = $v; - } - }, null, Model::class)(); - - $this->model = $entityCloned; - $this->currentRow = $entityCloned; // TODO we should either drop currentRow property or never update model property + foreach ($this->model as $entity) { + $this->model = $entity; + $this->currentRow = $entity; // TODO we should either drop currentRow property or never update model property if ($this->hook(self::HOOK_BEFORE_ROW) === false) { continue; } diff --git a/src/Table/Column/Multiformat.php b/src/Table/Column/Multiformat.php index c50a4d4265..7e5c5ef023 100644 --- a/src/Table/Column/Multiformat.php +++ b/src/Table/Column/Multiformat.php @@ -31,6 +31,13 @@ public function getDataCellHtml(Field $field = null, array $attr = []): string public function getHtmlTags(Model $row, ?Field $field): array { + // mainly for CardTable to support different field type for different row + $fieldValue = $field->get($row); + if ($fieldValue instanceof Model\EntityFieldPair) { + $row = $fieldValue->getEntity(); + $field = $fieldValue->getField(); + } + $decorators = ($this->callback)($row, $field); // we need to smartly wrap things up $name = $field->shortName;