search

atk4 / filestore

Implements integration between ATK UI and FlySystem
https://agiletoolkit.org/
MIT License
9 stars 8 forks source link

Subsequently adding new files in separate fields not working #26

Closed mkrecek234 closed 2 years ago

mkrecek234 commented 2 years ago

Steps to reproduce in the existing demo:

  1. Upload a File in the "File" field.
  2. Save the form
  3. Then, upload a second file in the "File 2" field.
  4. Save the form.

What happens: it sets the token of the first "File" field to null, and fills the second "File 2" field with the new filename.

Expected behaviour: It should keep the "File" token, and only add another token to the "File 2" field.

mvorisek commented 2 years ago

I confirm the token for a file is unset.

To repro, save 1 file in the demo. Refresh the page, open a modal for this upload and click save. No change of any field even needed.

Then notice the file token (in DB/CRUD view) is gone for the edited row.

mvorisek commented 2 years ago

The issue is comming from https://github.com/atk4/filestore/blob/b711e5e1897d02c2778d76257428b1999e04613b/src/Field/FileField.php#L84

then the ID is unset due WRONG referencing thru url field:

image

mvorisek commented 2 years ago

Thank you for posting this issue. There are actually two issues, one in data, one in ui.

The issue in atk4/data seems to be not new, but it is triggered as an ui value is saved as '' (empty string) instead of null. This causes a field to became dirty.

The atk4/data problem is in https://github.com/atk4/data/blob/ff302f5fe5f7cd2b00928f017284010b17805993/src/Reference/HasOneSql.php#L162. The hook is intended to allow to set the reference ID by a value from the referenced table. This is nice, but broken. Also the value is set by SQL expression and not by value, which prevents save hooks to trigger.

Once the reference ID is set (!== null), then this hook should do nothing. Or better check if the (dirty/changed) "value from the referenced table" points to the same record. If not, throw.

So I will fix the atk4/data issue with tests and then let's see what can we do with the ui. We do not distinqush null/empty string there, but nullable field should save empty string as null.