Add weak conditions/save support

[mkrecek234]

related initial issue #967

[mvorisek]

Usecases? What is "weak" and when? Sometimes traversing should be weak, sometimes not, we should probably start with all possible usecases and then find a well understandable solution.

[mvorisek]

What are the usecases? If a record is expected to be possibly deleted, you can always check with tryLoad or handle reload exception.

[mkrecek234]

The use case is as follows - consider a Crud which displays unread messages with addWeakCondition('unread', true) . You click on edit, and change the unread flag to false. You click save.
The crud then runs a normal ->save function. Since save allows that - in case a field is changed that moves it outside of a weak condition, the save command should only try to reload. If it is successful, the crud will be updated, if it is null, then the entity left the scope, and thus the same animation as in a delete function will be run.

[mvorisek]

This is legit usecase 👍

[mvorisek]

no try - we want strict behaviour as much as possible, not to silently ignore possible issues

[mkrecek234]

This is not a possible issue but a designed situation - if you do not have any weak condition then we could think about throwing an exception and not null. I think we should not be too dogmatic, especially if we destroy then logics which are present in Ui.

[mvorisek]

I do not understand this, please provide a testcase for it.

[mkrecek234]

I placed the tryReload for the usecase above (the crud remove unread message from scope on save) - so when a model saves something, the associated view gets a feedback if the entity is still there (in scope, so not null) or if it is gone - so save function only tries to reload, and if it is not successful it should return null.

[mvorisek]

I understand, but we cannot remove this check because of some need.

I have no solution is my head, maybe we need to add some support to ui for it.

In atk4/data, we have currently BEFORE_SAVE and AFTER_SAVE hooks, maybe we need some concept of try/finally SAVE hook.

In the past we discussed an immutable models. I am still not fully decided if a condition removal should be supported at all. I have no plan to disallow it in the near future, but to support the conditions removal fully, we need to implement #662 first.

If you want to pursue this feature, please start with a real Crud test in atk4/ui. If you need one or two atk4/data classes modified there, you can extend the atk4/data classes in atk4/ui. Please keep in mind, this feature should work also for other ui components like Multiline.

[mkrecek234]

Why not rund a proper reload if no weak condition exists - only if a weak condition exists, you only try to reload and return null of it left the scope.

[mvorisek]

Because we need to be able to rely on reload. If the record became out of scope, it can be some error as well, with tryReload, we can be left with old data.

[mvorisek]

please make the CS/stan CI happy and unit testing to finish at least

this is good practice so we can focus on the real issues

[mvorisek]

see

data/tests/LookupSqlTest.php

Line 112 in aacc247

// $c->insert([

I belive a weak approach is bad - any condition (added by an user) will still break the (currently commented) hook code. Maybe a solution can be to backup the conditions after init is invoked and allow it to be used like in #1054 invokeCallbackWithoutUndeletedCondition.

[mvorisek]

closing in favor of #1054