atk4 / data

Data Access PHP Framework for SQL & high-latency databases
https://atk4-data.readthedocs.io
MIT License
273 stars 46 forks source link

HasOneSql is not merging imported field seed properly #1046

Closed mkrecek234 closed 2 years ago

mkrecek234 commented 2 years ago

Steps to reproduce:

  1. You have a model and addExpression which by nature is non-editable.
  2. You for example relate to that model with a hasOne relation, and import that expression as a field

Error TypeError: Atk4\Data\Field::isEditable(): Return value must be of type bool, array returned in Model:730

The issue is in line 734, as isEditable is not existing as a property for SqlExpressionField.

mvorisek commented 2 years ago

@mkrecek234 please do not add MAJOR to (every) bug, MAJOR is something conceptually wrong or with major impact on most users.

Also, please copy/screenshot full backtrace in case you want to refer to it. Refering to a line number without knowing the file is almost useless. The same for the exception message, as the reason might depend on a context which is shown in exceptions params (added by Exception::addMoreInfo()).

mkrecek234 commented 2 years ago

image

mvorisek commented 2 years ago

The issue is in line 734, as isEditable is not existing as a property for SqlExpressionField.

There is no isEditable property in atk4/data. There is only Field::isEditable() method, which by inheritance available in SqlExpressionField. Can you please post a minimalistic code to reproduce or is it possible to reproduce it on atk4/ui demos?

mkrecek234 commented 2 years ago

Correct - it relates to the missing isEditable() property of SqlExpressionField. In Model::getFields() the following code is not compatible with SqlExpressionField:

                    ($f === 'system' && $field->system)
                    || ($f === 'not system' && !$field->system)
                    || ($f === 'editable' && $field->isEditable())
                    || ($f === 'visible' && $field->isVisible())
                ) {

As I cannot create minimalistic code on demo today, here already the info to reproduce:

  1. Add an expression to any model. $model->addExpression('expressionfield', ['expr' => "1"]);

In a child model create a hasOne and import that expression: $childmodel->hasOne('model_id', ['model' => [Model::class]])->addFields(['expressionfield']);

Then simple check it in a crud: $crud->setModel($childmodel);

mkrecek234 commented 2 years ago

I checked further, the issue is that $this->ui['editable']in Field:457 returns an array which includes true and false, rather than just a boolean value.

mvorisek commented 2 years ago

Thank you for the code. I am looking into it.

it relates to the missing isEditable() property of SqlExpressionField

really a property? I seached ->(is)?editable(?!\(), no such property is accessed in the whole project

I belive the issue is caused by ui['editable'] beiing an array - https://github.com/atk4/data/blob/0892a701f955b5c560bb3dd324645788bedd7594/src/Field.php#L457

mkrecek234 commented 2 years ago

image

mvorisek commented 2 years ago

yes, doesn't it come from your code?

mkrecek234 commented 2 years ago

No, I haven't touched anything there, just standard model definitions

mvorisek commented 2 years ago

issue is here: https://github.com/atk4/data/blob/0892a701f955b5c560bb3dd324645788bedd7594/src/Reference/HasOneSql.php#L87

array_merge_recursive is not merging the seed properly - https://3v4l.org/4Tgo8