[fix] Model::duplicate

[georgehristov]

No description provided.

[mvorisek]

Entity approach must satisty several axioms. One of them is to never allow to assing different ID to the same instance.

So I think we should remove duplicate completely or clone before at the beginning of this method and return the cloned model then.

[georgehristov]

Removal of method is not good option as functionality is useful.
Cloning model and returning it is a good option but $entityId is private. how to clear it best?

[georgehristov]

I came up with following code but it does not work as even if model unloaded $entityId value remains saved. I think this is not the correct behavior

    public function duplicate($newId = null): self
    {
        $duplicate = clone $this;

        $duplicate->unload();

        $duplicate->setMulti($this->data);

        if ($duplicate->id_field) {
            $duplicate->setId($newId);
        }

        return $duplicate;
    }

[mvorisek]

simply set entityId to null after clone, + then set whateer needed (unload, reset dirty, do your research)

[georgehristov]

I can make it work but code is not consistent.
The problem I see is that clearing entityId should be done with unload. What is unload doing otherwise?

[mvorisek]

clone should not normally do these things

as a design rule, otherwise there is not a real issues

but to satisfy standards as well, we should do that in special method

now in duplciate() :)

[georgehristov]

I am still of the opinion that if we support unload then entityId should be cleared. All other values are cleared.

[mvorisek]

You got me for 1, 2 seconds. The answer is clear no.

Entity approach must satisty several axioms. One of them is to never allow to assing different ID to the same instance.

So I think we should remove duplicate completely or clone before at the beginning of this method and return the cloned model then.

because if we allow to unset entityId, it can be easily changed, and this axiom will be violated. This any code that clears entityId is dangerous (except you know what you are dooing or do that on a fresh clone)

[georgehristov]

Then we should remove unload method as it is not working properly at the moment or just use it to return a fresh object without entity loaded.

[mvorisek]

unload unloads data, but keep entity bound to a specific ID

I think there are usecases for it.

[DarkSide666]

If it works like this (also clears entityId) then it's fine with me.

BUT I think that probably we have to finish refactoring this Model / Entity situation and then quite many internal things will change anyway. Currently this Model/Entity approach looks wrong and hard to use and understand.

[mvorisek]

main issues (cloning, no $newId) resolved