Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model::dirty not available in Model::HOOK_AFTER_SAVE spot #1130

Closed
PhilippGrashoff opened this issue Sep 13, 2023 · 1 comment
Closed

Model::dirty not available in Model::HOOK_AFTER_SAVE spot #1130

PhilippGrashoff opened this issue Sep 13, 2023 · 1 comment

Comments

@PhilippGrashoff
Copy link
Contributor

PhilippGrashoff commented Sep 13, 2023

The current decription of Model::dirty indicates that the dirty information is available in all hooks:
* The $dirty data will be reset after you save() the data but it is still available to all before/after save handlers.

However, that is not the case. Model::dirty is reset before the Model::HOOK_AFTER_SAVE spot is executed (taken from current develop):

            $dirtyRef = &$this->getDirtyRef();
            $dirtyRef = [];

            if ($this->idField && $this->reloadAfterSave) {
                $this->reload();
            }

            $this->hook(self::HOOK_AFTER_SAVE, [$isUpdate]);

It is not enough to move the reset after the hook, as Model::reload() also resets dirty. This is where is gets complicated: reloaded model information could be important in the after save hook, as does the dirty information does.

The only fix I can think of is to clone $dirtyRef, assign in to $this right before the hook is executed and then reset it afterwards. Something like:

            $dirtyRef = &$this->getDirtyRef();
            $dirtyRefCache = clone dirtyRef;

            if ($this->idField && $this->reloadAfterSave) {
                $this->reload();
            }

            $dirtyRef = $dirtyRefCache;
            $this->hook(self::HOOK_AFTER_SAVE, [$isUpdate]);
            $dirtyRef = [];

WDYT about this? If you are fine with this, I will create a PR with a change like this and add a test.

@mvorisek
Copy link
Member

You can capture the original dirty data in "before update" hook. The dirtiness is logical, before update, the data are dirty as they are. Once they are saved, they are no longer dirty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants