Skip to content

Commit

Permalink
Typecast ID for UI strictly using UI persistence (#2168)
Browse files Browse the repository at this point in the history
  • Loading branch information
mvorisek authored Feb 14, 2024
1 parent 4d77a98 commit 868fbb3
Show file tree
Hide file tree
Showing 29 changed files with 239 additions and 104 deletions.
2 changes: 1 addition & 1 deletion demos/_unit-test/modal-error.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
$button = Button::addTo($p)->set('Test ModalExecutor load PHP error');
$executor = ModalExecutor::assertInstanceOf($p->getExecutorFactory()->createExecutor($country->getUserAction('edit'), $button));
if (\Closure::bind(static fn () => $executor->loader, null, ModalExecutor::class)()->cb->isTriggered()) {
$executor->stickyGet($executor->name, '-1');
$executor->stickyGet($executor->name, '99000'); // no such record so load will fail
}
$button->on('click', $executor);
});
Expand Down
4 changes: 2 additions & 2 deletions demos/basic/breadcrumb.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
$model = new Country($app->db);
$model->setLimit(15);

$id = $crumb->stickyGet('country_id');
if ($id) {
$id = $app->uiPersistence->typecastLoadField($model->getField($model->idField), $crumb->stickyGet('country_id'));
if ($id !== null) {
// perhaps we edit individual country?
$model = $model->load($id);
$crumb->addCrumb($model->name, []);
Expand Down
6 changes: 3 additions & 3 deletions demos/collection/multitable.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function setModel(Model $model, array $route = []): void
$selections = $this->explodeSelectionValue($this->getApp()->tryGetRequestQueryParam($this->name) ?? '');

if ($selections !== []) {
$table->js(true)->find('tr[data-id=' . $selections[0] . ']')->addClass('active');
$table->js(true)->find('tr[data-id=' . $this->getApp()->uiPersistence->typecastSaveField($this->model->getField($this->model->idField), $selections[0]) . ']')->addClass('active');
}

$makeJsReloadFx = function (array $path): JsReload {
Expand All @@ -65,7 +65,7 @@ public function setModel(Model $model, array $route = []): void
$table->on('click', 'tr', $jsReload);

while ($id = array_shift($selections)) {
$path[] = $id;
$path[] = $this->getApp()->uiPersistence->typecastSaveField($this->model->getField($this->model->idField), $id);
$pushModel = new $model($model->getPersistence());
$pushModel = $pushModel->tryLoad($id);
if ($pushModel === null) {
Expand All @@ -87,7 +87,7 @@ public function setModel(Model $model, array $route = []): void
$table->setModel($pushModel->setLimit(10), [$pushModel->titleField]);

if ($selections !== []) {
$table->js(true)->find('tr[data-id=' . $selections[0] . ']')->addClass('active');
$table->js(true)->find('tr[data-id=' . $this->getApp()->uiPersistence->typecastSaveField($this->model->getField($this->model->idField), $selections[0]) . ']')->addClass('active');
}

$jsReload = $makeJsReloadFx($path);
Expand Down
4 changes: 1 addition & 3 deletions demos/form-control/dropdown-plus.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
$form->addControl('product_id', [Form\Control\DropdownCascade::class, 'cascadeFrom' => 'sub_category_id', 'reference' => SubCategory::hinting()->fieldName()->Products]);

$form->onSubmit(static function (Form $form) use ($app) {
$message = $app->encodeJson($form->model->get());
$message = $app->encodeJson($app->uiPersistence->typecastSaveRow($form->model, $form->model->get()));

$view = new Message('Values: ');
$view->setApp($form->getApp());
Expand All @@ -53,7 +53,6 @@
'model' => (new Country($app->db))->setLimit(25),
'renderRowFunction' => static function (Country $row) {
return [
'value' => $row->getId(),
'title' => $row->getTitle() . ' (' . $row->iso3 . ')',
];
},
Expand All @@ -66,7 +65,6 @@
'model' => (new File($app->db))->setLimit(25),
'renderRowFunction' => static function (File $row) {
return [
'value' => $row->getId(),
'title' => $row->getTitle(),
'icon' => $row->is_folder ? 'folder' : 'file',
];
Expand Down
4 changes: 2 additions & 2 deletions demos/form-control/lookup.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
$model = new Model($app->db, ['table' => 'test']);

// lookup without plus button
$model->hasOne('country1', ['model' => [Country::class]]);
$model->hasOne('country1', ['model' => [Country::class], 'type' => WrappedIdType::NAME]);

// lookup with plus button
$model->hasOne('country2', ['model' => [Country::class], 'ui' => ['form' => ['plus' => true]]]);
$model->hasOne('country2', ['model' => [Country::class], 'type' => WrappedIdType::NAME, 'ui' => ['form' => ['plus' => true]]]);

$form->setModel($model->createEntity());

Expand Down
9 changes: 1 addition & 8 deletions demos/form-control/multiline.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,7 @@
return $multiline->saveRows()->model->export();
});

// TODO typecast using https://github.com/atk4/ui/pull/1991 once merged
foreach ($rows as $kRow => $row) {
foreach ($row as $kV => $v) {
if ($v instanceof \DateTime) {
$rows[$kRow][$kV] = $form->getApp()->uiPersistence->typecastSaveField($multiline->model->getField($kV), $row[$kV]);
}
}
}
$rows = array_map(static fn ($row) => $form->getApp()->uiPersistence->typecastSaveRow($multiline->model, $row), $rows);

return new JsToast($form->getApp()->encodeJson($rows));
});
4 changes: 2 additions & 2 deletions demos/form/form2.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ protected function init(): void
$this->addField('name', ['required' => true]);
$this->addField('surname', ['ui' => ['placeholder' => 'e.g. Smith']]);
$this->addField('gender', ['enum' => ['M', 'F']]);
$this->hasOne('country_lookup_id', ['model' => [Country::class]]); // this works fast
$this->hasOne('country_dropdown_id', ['model' => [Country::class], 'ui' => ['form' => new Form\Control\Dropdown()]]); // this works slow
$this->hasOne('country_lookup_id', ['model' => [Country::class], 'type' => WrappedIdType::NAME]); // this works fast
$this->hasOne('country_dropdown_id', ['model' => [Country::class], 'type' => WrappedIdType::NAME, 'ui' => ['form' => new Form\Control\Dropdown()]]); // this works slow
}

#[\Override]
Expand Down
116 changes: 116 additions & 0 deletions demos/init-db.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
use Atk4\Ui\Exception;
use Atk4\Ui\Form;
use Atk4\Ui\Table;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\DBAL\Types\Type as DbalType;
use Mvorisek\Atk4\Hintable\Data\HintablePropertyDef;
use SebastianBergmann\CodeCoverage\CodeCoverage;

Expand All @@ -24,6 +26,77 @@
->addMoreInfo('PDO error', $e->getMessage());
}

/**
* Improve testing by using non-scalar ID with custom DBAL type.
*/
class WrappedId
{
private const MIN_VALUE = 1;
private const MAX_VALUE = 100_000;

private int $id;

public function __construct(int $id)
{
if ($id < self::MIN_VALUE || $id > self::MAX_VALUE) {
throw (new Exception('ID value is outside supported range'))
->addMoreInfo('value', $id);
}

$this->id = $id;
}

public function getId(): int
{
return $this->id;
}
}

class WrappedIdType extends DbalType
{
public const NAME = 'atk4_ui_demos_id';

#[\Override]
public function getName(): string
{
return self::NAME;
}

#[\Override]
public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform): string
{
return DbalType::getType('integer')->getSQLDeclaration($fieldDeclaration, $platform);
}

#[\Override]
public function convertToDatabaseValue($value, AbstractPlatform $platform): ?int
{
if ($value === null) {
return null;
}

return DbalType::getType('integer')->convertToDatabaseValue($value->getId(), $platform);
}

#[\Override]
public function convertToPHPValue($value, AbstractPlatform $platform): ?object
{
if ($value === null) {
return null;
}

return new WrappedId(DbalType::getType('integer')->convertToPHPValue($value, $platform));
}

#[\Override]
public function requiresSQLCommentHint(AbstractPlatform $platform): bool
{
return true;
}
}

DbalType::addType(WrappedIdType::NAME, WrappedIdType::class);

trait ModelPreventModificationTrait
{
protected function isAllowDbModifications(): bool
Expand Down Expand Up @@ -124,6 +197,12 @@ protected function initPreventModification(): void

/**
* Improve testing by using prefixed real field and SQL names.
*
* @method (static|null) tryLoad(WrappedId $id = null) remove parentheses around return type once https://github.com/phpstan/phpstan/issues/10548 is fixed
* @method static load(WrappedId $id)
* @method \Traversable<WrappedId, static> getIterator()
* @method WrappedId insert(array<string, mixed> $row)
* @method static delete(WrappedId $id = null)
*/
class ModelWithPrefixedFields extends Model
{
Expand Down Expand Up @@ -199,6 +278,8 @@ protected function init(): void

parent::init();

$this->getField($this->idField)->type = WrappedIdType::NAME;

$this->initPreventModification();

if ($this->getPersistence()->getDatabasePlatform() instanceof PostgreSQLPlatform || class_exists(CodeCoverage::class, false)) {
Expand All @@ -216,6 +297,41 @@ public function addField(string $name, $seed = []): Field

return parent::addField($name, $seed);
}

#[\Override]
public function hasOne(string $link, array $defaults = [])
{
// TODO remove once HasOne reference can infer type from their model
if (!isset($defaults['type'])) {
$defaults['type'] = WrappedIdType::NAME;
}

return parent::hasOne($link, $defaults);
}

#[\Override]
public function getId(): ?WrappedId
{
return parent::getId();
}

/**
* @param WrappedId|($allowNull is true ? null : never) $value
*/
#[\Override] // @phpstan-ignore-line
public function setId($value, bool $allowNull = true)
{
return parent::setId($value, $allowNull);
}

/**
* @return \Traversable<WrappedId, static>
*/
#[\Override]
public function createIteratorBy($field, $operator = null, $value = null): \Traversable
{
return parent::createIteratorBy(...'func_get_args'());
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion demos/layout/layout-panel.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
Header::addTo($app, ['UserAction Friendly', 'size' => 4, 'subHeader' => 'Panel can run model action.']);

$panel3 = Panel\Right::addTo($app);
$countryId = $panel3->stickyGet('id');
$countryId = $app->uiPersistence->typecastLoadField($country->getField($country->idField), $panel3->stickyGet('id'));
$msg = Message::addTo($panel3, ['Run Country model action below.']);

$deck = View::addTo($app, ['ui' => 'cards']);
Expand Down
4 changes: 2 additions & 2 deletions demos/layout/layouts.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
['page' => ['layouts_nolayout'], 'title' => 'HTML without layout'],
['page' => ['layouts_manual'], 'title' => 'Manual layout'],
['page' => ['../basic/header', 'layout' => Layout\Centered::class], 'title' => 'Centered layout'],
['page' => ['layouts_admin'], 'title' => 'Admin Layout'],
['page' => ['layouts_error'], 'title' => 'Exception Error'],
['page' => ['layouts_admin'], 'title' => 'Admin layout'],
['page' => ['layouts_error'], 'title' => 'Unhandled error'],
];

// layout
Expand Down
5 changes: 1 addition & 4 deletions docs/form-control.md
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,6 @@ This function is called with each model record and needs to return an array:
```
$dropdown->renderRowFunction = function (Model $record) {
return [
'value' => $record->idField,
'title' => $record->getTitle() . ' (' . $record->get('subtitle') . ')',
];
}
Expand All @@ -390,7 +389,6 @@ You can also use this function to add an Icon to a record:
```
$dropdown->renderRowFunction = function (Model $record) {
return [
'value' => $record->idField,
'title' => $record->getTitle() . ' (' . $record->get('subtitle') . ')',
'icon' => $record->get('value') > 100 ? 'money' : 'coins',
];
Expand Down Expand Up @@ -426,7 +424,6 @@ With the according renderRowFunction:
```
function (Model $record) {
return [
'value' => $record->getId(),
'title' => $record->getTitle,
'icon' => $record->value > 100 ? 'money' : 'coins',
'someOtherField' => $record->get('SomeOtherField'),
Expand All @@ -435,7 +432,7 @@ function (Model $record) {
}
```

Of course, the tags `value`, `title`, `icon`, `someOtherField` and `someOtherField2` need to be set in my_dropdown.html.
Of course, the tags `title`, `icon`, `someOtherField` and `someOtherField2` need to be set in my_dropdown.html.

### Usage with $values property

Expand Down
2 changes: 1 addition & 1 deletion src/Card.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public function setModel(Model $entity, array $fields = null): void
$fields = array_keys($this->model->getFields(['editable', 'visible']));
}

$this->template->trySet('dataId', (string) $this->model->getId());
$this->template->trySet('dataId', $this->getApp()->uiPersistence->typecastSaveField($this->model->getField($this->model->idField), $this->model->getId()));

View::addTo($this->getSection(), [$entity->getTitle(), 'class.header' => true]);
$this->getSection()->addFields($entity, $fields, $this->useLabel, $this->useTable);
Expand Down
2 changes: 1 addition & 1 deletion src/Crud.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ protected function getJsGridAction(Model\UserAction $action): ?JsExpressionable
case Model\UserAction::MODIFIER_DELETE:
// use deleted record ID to remove row, fallback to closest tr if ID is not available
$js = $this->deletedId
? $this->js(false, null, 'tr[data-id="' . $this->deletedId . '"]')
? $this->js(false, null, 'tr[data-id="' . $this->getApp()->uiPersistence->typecastSaveField($this->model->getField($this->model->idField), $this->deletedId) . '"]')
: (new Jquery())->closest('tr');
$js = $js->transition('fade left', new JsFunction([], [new JsExpression('this.remove()')]));

Expand Down
2 changes: 1 addition & 1 deletion src/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Form extends View
public $controlDisplaySelector = '.field';

/** @var array<string, mixed> Use this apiConfig variable to pass API settings to Fomantic-UI in .api(). */
public $apiConfig = [];
public array $apiConfig = [];

/** @var array<string, mixed> Use this formConfig variable to pass settings to Fomantic-UI in .from(). */
public $formConfig = [];
Expand Down
Loading

0 comments on commit 868fbb3

Please sign in to comment.