atk4 / data

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

Set fields to dirty when they are changed within hooks within a model #1154

Closed bedengler closed 9 months ago

bedengler commented 9 months ago

According to https://github.com/atk4/data/blob/e601c2cbdf04e49213d3ddf6473b26a41b470390/src/Model.php#L1667 the "HOOK_AFTER_SAVE" hook is fired only, when we have dirty fields, means, when a field of a model has been changed before saving the model.

When we set a field of the model within a hook (e.g. we call "$model->set('field', 'value');" in the "HOOK_BEFORE_SAVE" hook), the field isn't set to dirty and all following hooks relying on dirty fields won't fire.

This is misleading and should be changed, so that if a field of a model gets saved within a hook of this model, the field must be set to dirty.

According to the documentation it's said, that it's only possible to alternate $data of the model directly (see https://agile-data.readthedocs.io/en/develop/hooks.html?highlight=hook#insert-update-hooks).

Issues with that:

  1. The variable is set to private
  2. At the definition of $data we can read: "Avoid accessing $data directly, use set() / get() instead." - see https://github.com/atk4/data/blob/e0c08833c29eea41014a580d53e546d41a81e195/src/Model.php#L143

Therefore I suggest to set all fields dirty, that are set using set() within hooks before the model is saved.

Any field that's touched with "set()" within the following hooks should be set to dirty:


Use case: For the date selection within a form I'm using the atk4 datepicker with a date range (one field). But in the database I have separate fields for start- and end-date (two fields).

The datepicker returns a string (e.g. "10.09.2023 bis 10.10.2023" or "07.12.2023" if just 1 date is selected) which is saved temporarily in a non-persisting field when submitting the form.

In the "BEFORE_SAVE" hook I'm disassembling the date string from this non-persisting field and prepare and set the start and end ($model->set('startdate') and $model->set('enddate') ). These 2 fields are persisting!

The start and end field of the model don't get changed unless the HOOK_BEFORE_SAVE hook. And they are regular fields of the db table and they are changed before saving the model. Therefore they should be part of the dirty array.

The HOOK_AFTER_SAVE hook doesn't get fired unless another field has been changed beside the date field in this case...

mvorisek commented 9 months ago

please provide minimal self contained test case

https://github.com/atk4/data/blob/3.1.0/src/Model.php#L1588 hooks should still be able to set the data dirty

bedengler commented 9 months ago

Tried it again and it seems to work like expected - so whenever you set something in another hook, the fields get dirty...I have no explanation why it didn't work before...

mvorisek commented 9 months ago

Thank you for updating this issue!