-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add weak conditions/save support #1058
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1002,6 +1002,30 @@ public function addCondition($field, $operator = null, $value = null) | |
return $this; | ||
} | ||
|
||
/** | ||
* Same as addCondition but condition will be dropped on save event and not be enforced. | ||
* Therefore, it is valid to save an entity where the saved entity will not comply to the | ||
* weak condition anymore. | ||
* | ||
* Example: | ||
* | ||
* $messsageModel->addWeakCondition('unread', true); | ||
* $messageModel->save(['unread' => false]); | ||
* | ||
* @param mixed $field | ||
* @param mixed $operator | ||
* @param mixed $value | ||
* | ||
* @return $this | ||
*/ | ||
public function addWeakCondition($field, $operator = null, $value = null) | ||
{ | ||
|
||
$this->scope()->addCondition(...func_get_args(), true); | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* Adds WITH/CTE model. | ||
* | ||
|
@@ -1340,6 +1364,21 @@ public function reload() | |
return $this; | ||
} | ||
|
||
/** | ||
* Try to reload model by taking its current ID, otherwise return null. | ||
* | ||
* @return $this | ||
*/ | ||
public function tryReload() | ||
{ | ||
$id = $this->getId(); | ||
$this->unload(); | ||
|
||
$res = $this->_load(true, true, $id); | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* Keeps the model data, but wipes out the ID so | ||
* when you save it next time, it ends up as a new | ||
|
@@ -1384,6 +1423,36 @@ public function saveAndUnload(array $data = []) | |
return $this; | ||
} | ||
|
||
/** | ||
* Store the data into database, but will never attempt to | ||
* reload the data and permits that entity leaves scope after save. | ||
* Additionally, any data will be unloaded. removeConditionFields will | ||
* eliminate all conditions on those fields, set null to ignore all conditions. | ||
* | ||
* @return $this | ||
*/ | ||
public function saveWithoutScope(array $data = [], array $removedConditionFields = null) | ||
{ | ||
$scopeElementsOrig = $this->getModel()->scope()->elements; | ||
try { | ||
if ($removedConditionFields) { | ||
foreach ($this->getModel()->scope()->elements as $k => $v) { | ||
if ($v instanceof Model\Scope\Condition && (!$removedConditionFields || in_array($v->key, $removedConditionFields))) { | ||
unset($this->getModel()->scope()->elements[$k]); | ||
} | ||
} | ||
} else { $this->getModel()->scope()->elements = array(); } | ||
|
||
$this->saveAndUnload($data); | ||
|
||
} finally { | ||
$this->getModel()->scope()->elements = $scopeElementsOrig; | ||
} | ||
|
||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* Create new model from the same base class as $this. | ||
* | ||
|
@@ -1465,10 +1534,30 @@ public function tryLoadBy(string $fieldName, $value) | |
return $this->_loadBy(true, $fieldName, $value); | ||
} | ||
|
||
protected function invokeCallbackWithoutWeakConditions(Model $model, \Closure $callback) | ||
{ | ||
$scopeElementsOrig = $model->scope()->elements; | ||
try { | ||
foreach ($model->scope()->elements as $k => $v) { | ||
if ($v instanceof Model\Scope\Condition && $v->weak) { | ||
unset($model->scope()->elements[$k]); | ||
} | ||
} | ||
|
||
return $callback(); | ||
} finally { | ||
$model->scope()->elements = $scopeElementsOrig; | ||
} | ||
} | ||
|
||
protected function validateEntityScope(): void | ||
{ | ||
if (!$this->getModel()->scope()->isEmpty()) { | ||
$this->getPersistence()->load($this->getModel(), $this->getId()); | ||
|
||
$this->invokeCallbackWithoutWeakConditions($entity->getModel(), function () use ($entity): void { | ||
$this->getPersistence()->load($this->getModel(), $this->getId()); | ||
}); | ||
|
||
} | ||
} | ||
|
||
|
@@ -1562,7 +1651,7 @@ public function save(array $data = []) | |
if ($this->idField && $this->reloadAfterSave) { | ||
$d = $dirtyRef; | ||
$dirtyRef = []; | ||
$this->reload(); | ||
$this->tryReload(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no try - we want strict behaviour as much as possible, not to silently ignore possible issues There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not understand this, please provide a testcase for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
$this->dirtyAfterReload = $dirtyRef; | ||
$dirtyRef = $d; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the usecases? If a record is expected to be possibly deleted, you can always check with
tryLoad
or handlereload
exception.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is legit usecase 👍