Fix system condition handling
fixes #662
Specific method in Model is not a good idea.
- huge BC break
- does not solve issue with cloning Model (and scope)
Feel free to make you own PR. I will not change this one as it fixes the issue quite nicely.
huge BC break
wanted, in refs, we clone before and we can fix easily
does not solve issue with cloning Model (and scope)
please describe
as it fixes the issue quite nicely.
the issue is that we should never set system/default by default if addCondition is added by an user! and I think this PR does NOT solve it
I will not support having a special method in Model for it. It should go along with Model::addCondition and Model::scope()->addCondition
The system indicator should be kept if scope cloned and used for another Model. But removed a when scope cloned and added to another scope
please describe
does not solve issue with cloning Model (and scope)
I am not strictly againts a flag in a Condition, but only like allowSystemIfAlwaysTrue and default to false - in refs, we can then set to true and there will be no implicit/unwanted behaviour
Will try to assign this already when references are being set and remove it completely from Condition
It is not about references but liting the scope. So what is bothering you - the method name?
setSystem -> allowSystemIfAlwaysTrue (setSystemFieldIfDefinite)
currently, we assume and change behaviour of Field implicitly
please reread the whole discussion - I would remove it completely, but if this makes sense for refs and possibly some usecases, then fine, but user must explicitly enable this feature per given condition
Unrequested mutation of a model is forbidden by design. Definitive fields should be resolved at runtime - they must not be editable (Field::isEditable()) and the default field value must be implied dynamically as well.
fixes https://github.com/atk4/data/issues/662
this PR does not fix #662 issue, this PR adds a concept of system scope which might be wanted, but the requirements are mainly to be able to loosen/remove "system" scope for soft-delete purposes and possibly cleanup "non-system" scope added during traversing
as not addressed by this PR, no tests, no feedback, closing in favor of #1054