From 030b5e063d14ddc65d4ed7de4fbddfc5532fd562 Mon Sep 17 00:00:00 2001 From: Georgi Hristov Date: Sat, 8 Aug 2020 09:26:42 +0200 Subject: [PATCH 1/7] [update] introduce and use Scope::create --- src/Model/Scope.php | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Model/Scope.php b/src/Model/Scope.php index 42c28f68c..38ee2c688 100644 --- a/src/Model/Scope.php +++ b/src/Model/Scope.php @@ -70,15 +70,11 @@ public function __clone() */ public function addCondition($field, $operator = null, $value = null) { - if (func_num_args() === 1 && $field instanceof Scope\AbstractScope) { - $condition = $field; - } elseif (func_num_args() === 1 && is_array($field)) { - $condition = static::createAnd(func_get_args()); - } else { - $condition = new Scope\Condition(...func_get_args()); - } + $this->add(static::create(...func_get_args())); + + return $this; + } - $this->add($condition); return $this; } @@ -188,6 +184,28 @@ public function toWords(Model $model = null): string return implode($glue, $parts); } + /** + * Creates the simplest scope object based on arguments. + * + * @param Scope\AbstractScope|string|array|\atk4\data\Field $field + * @param string $operator + * @param mixed $value + * + * @return Scope\AbstractScope + */ + public static function create($field, $operator = null, $value = null) + { + if (func_num_args() === 1 && $field instanceof Scope\AbstractScope) { + $scope = $field; + } elseif (func_num_args() === 1 && is_array($field)) { + $scope = static::createAnd(func_get_args()); + } else { + $scope = new Scope\Condition(...func_get_args()); + } + + return $scope; + } + /** * @return static */ From e9e6dfb4788ad43d289ca8c2246773f041e992cc Mon Sep 17 00:00:00 2001 From: Georgi Hristov Date: Sat, 8 Aug 2020 10:03:07 +0200 Subject: [PATCH 2/7] [update] introduce Condition::isTraversing --- src/Model/Scope/Condition.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Model/Scope/Condition.php b/src/Model/Scope/Condition.php index 776cceab6..321de5e0d 100644 --- a/src/Model/Scope/Condition.php +++ b/src/Model/Scope/Condition.php @@ -146,7 +146,7 @@ protected function onChangeModel(): void // @todo: consider this when condition is part of OR scope if ($this->operator === self::OPERATOR_EQUALS && !is_object($this->value) && !is_array($this->value)) { // key containing '/' means chained references and it is handled in toQueryArguments method - if (is_string($field = $this->key) && !str_contains($field, '/')) { + if (is_string($field = $this->key) && !$this->isTraversing()) { $field = $model->getField($field); } @@ -158,6 +158,11 @@ protected function onChangeModel(): void } } + protected function isTraversing() + { + return is_string($this->key) && str_contains($this->key, '/'); + } + public function toQueryArguments(): array { if ($this->isEmpty()) { @@ -172,7 +177,7 @@ public function toQueryArguments(): array if (is_string($field)) { // shorthand for adding conditions on references // use chained reference names separated by "/" - if (str_contains($field, '/')) { + if ($this->isTraversing()) { $references = explode('/', $field); $field = array_pop($references); @@ -263,7 +268,7 @@ protected function keyToWords(Model $model): string $words = []; if (is_string($field = $this->key)) { - if (str_contains($field, '/')) { + if ($this->isTraversing()) { $references = explode('/', $field); $words[] = $model->getModelCaption(); @@ -332,7 +337,7 @@ protected function valueToWords(Model $model, $value): string // handling of scope on references if (is_string($field = $this->key)) { - if (str_contains($field, '/')) { + if ($this->isTraversing()) { $references = explode('/', $field); $field = array_pop($references); From 0cc4be9684e41b704be9ab71fce753460dc66cb5 Mon Sep 17 00:00:00 2001 From: Georgi Hristov Date: Sat, 8 Aug 2020 10:21:37 +0200 Subject: [PATCH 3/7] [fix] introduce AbstractScope::setSystem --- src/Model/Scope.php | 5 +++++ src/Model/Scope/AbstractScope.php | 5 +++++ src/Model/Scope/Condition.php | 19 ++++++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/Model/Scope.php b/src/Model/Scope.php index 38ee2c688..0f2509ad4 100644 --- a/src/Model/Scope.php +++ b/src/Model/Scope.php @@ -75,6 +75,11 @@ public function addCondition($field, $operator = null, $value = null) return $this; } + protected function setSystem($system = true) + { + foreach ($this->elements as $nestedCondition) { + $nestedCondition->setSystem($system && $this->isAnd()); + } return $this; } diff --git a/src/Model/Scope/AbstractScope.php b/src/Model/Scope/AbstractScope.php index 9268314ba..70d3de4c1 100644 --- a/src/Model/Scope/AbstractScope.php +++ b/src/Model/Scope/AbstractScope.php @@ -31,11 +31,16 @@ public function init(): void $this->_init(); + // always set system flag if condition added to another condition + $this->setSystem($this->owner instanceof RootScope); + $this->onChangeModel(); } abstract protected function onChangeModel(); + abstract protected function setSystem($system = true); + /** * Get the model this condition is associated with. */ diff --git a/src/Model/Scope/Condition.php b/src/Model/Scope/Condition.php index 321de5e0d..008e0f039 100644 --- a/src/Model/Scope/Condition.php +++ b/src/Model/Scope/Condition.php @@ -34,6 +34,8 @@ class Condition extends AbstractScope */ public $value; + protected $system = false; + public const OPERATOR_EQUALS = '='; public const OPERATOR_DOESNOT_EQUAL = '!='; public const OPERATOR_GREATER = '>'; @@ -137,6 +139,13 @@ public function __construct($key, $operator = null, $value = null) } } + protected function setSystem($system = true) + { + $this->system = $system; + + return $this; + } + protected function onChangeModel(): void { if ($model = $this->getModel()) { @@ -144,7 +153,7 @@ protected function onChangeModel(): void // sets it as default value for field and locks it // new records will automatically get this value assigned for the field // @todo: consider this when condition is part of OR scope - if ($this->operator === self::OPERATOR_EQUALS && !is_object($this->value) && !is_array($this->value)) { + if ($this->system && $this->setsDefiniteValue()) { // key containing '/' means chained references and it is handled in toQueryArguments method if (is_string($field = $this->key) && !$this->isTraversing()) { $field = $model->getField($field); @@ -229,6 +238,14 @@ public function isEmpty(): bool return array_filter([$this->key, $this->operator, $this->value]) ? false : true; } + /** + * Checks if condition sets a definitive scalar value for a field. + */ + protected function setsDefiniteValue(): bool + { + return $this->operator === self::OPERATOR_EQUALS && !is_object($this->value) && !is_array($this->value); + } + public function clear() { $this->key = $this->operator = $this->value = null; From a332d86c7776145c2f05ac7ec42c03535d9149a0 Mon Sep 17 00:00:00 2001 From: Georgi Hristov Date: Sat, 8 Aug 2020 10:21:57 +0200 Subject: [PATCH 4/7] [fix] use correct operator --- tests/Model/Smbo/Transfer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Model/Smbo/Transfer.php b/tests/Model/Smbo/Transfer.php index 4da5a4eba..39dcad6dd 100644 --- a/tests/Model/Smbo/Transfer.php +++ b/tests/Model/Smbo/Transfer.php @@ -18,7 +18,7 @@ public function init(): void // only used to create / destroy trasfer legs if (!$this->detached) { - $this->addCondition('transfer_document_id', 'not', null); + $this->addCondition('transfer_document_id', 'is not', null); } $this->addField('destination_account_id', ['never_persist' => true]); From 7db95b1df7ea66647e6d88bf48d4fbe6b2bc493f Mon Sep 17 00:00:00 2001 From: Georgi Hristov Date: Sat, 8 Aug 2020 10:03:07 +0200 Subject: [PATCH 5/7] Revert "[update] introduce Condition::isTraversing" This reverts commit e9e6dfb4788ad43d289ca8c2246773f041e992cc. --- src/Model/Scope/Condition.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Model/Scope/Condition.php b/src/Model/Scope/Condition.php index 008e0f039..af4db2f86 100644 --- a/src/Model/Scope/Condition.php +++ b/src/Model/Scope/Condition.php @@ -155,7 +155,7 @@ protected function onChangeModel(): void // @todo: consider this when condition is part of OR scope if ($this->system && $this->setsDefiniteValue()) { // key containing '/' means chained references and it is handled in toQueryArguments method - if (is_string($field = $this->key) && !$this->isTraversing()) { + if (is_string($field = $this->key) && !str_contains($field, '/')) { $field = $model->getField($field); } @@ -167,11 +167,6 @@ protected function onChangeModel(): void } } - protected function isTraversing() - { - return is_string($this->key) && str_contains($this->key, '/'); - } - public function toQueryArguments(): array { if ($this->isEmpty()) { @@ -186,7 +181,7 @@ public function toQueryArguments(): array if (is_string($field)) { // shorthand for adding conditions on references // use chained reference names separated by "/" - if ($this->isTraversing()) { + if (str_contains($field, '/')) { $references = explode('/', $field); $field = array_pop($references); @@ -285,7 +280,7 @@ protected function keyToWords(Model $model): string $words = []; if (is_string($field = $this->key)) { - if ($this->isTraversing()) { + if (str_contains($field, '/')) { $references = explode('/', $field); $words[] = $model->getModelCaption(); @@ -354,7 +349,7 @@ protected function valueToWords(Model $model, $value): string // handling of scope on references if (is_string($field = $this->key)) { - if ($this->isTraversing()) { + if (str_contains($field, '/')) { $references = explode('/', $field); $field = array_pop($references); From d12f1cc3b79920a4bb7d7e4d6c72274f50690377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 8 Aug 2020 10:36:36 +0200 Subject: [PATCH 6/7] Revert "[update] introduce and use Scope::create" This reverts commit 030b5e063d14ddc65d4ed7de4fbddfc5532fd562. --- src/Model/Scope.php | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/src/Model/Scope.php b/src/Model/Scope.php index 0f2509ad4..277b12377 100644 --- a/src/Model/Scope.php +++ b/src/Model/Scope.php @@ -70,7 +70,15 @@ public function __clone() */ public function addCondition($field, $operator = null, $value = null) { - $this->add(static::create(...func_get_args())); + if (func_num_args() === 1 && $field instanceof Scope\AbstractScope) { + $condition = $field; + } elseif (func_num_args() === 1 && is_array($field)) { + $condition = static::createAnd(func_get_args()); + } else { + $condition = new Scope\Condition(...func_get_args()); + } + + $this->add($condition); return $this; } @@ -189,28 +197,6 @@ public function toWords(Model $model = null): string return implode($glue, $parts); } - /** - * Creates the simplest scope object based on arguments. - * - * @param Scope\AbstractScope|string|array|\atk4\data\Field $field - * @param string $operator - * @param mixed $value - * - * @return Scope\AbstractScope - */ - public static function create($field, $operator = null, $value = null) - { - if (func_num_args() === 1 && $field instanceof Scope\AbstractScope) { - $scope = $field; - } elseif (func_num_args() === 1 && is_array($field)) { - $scope = static::createAnd(func_get_args()); - } else { - $scope = new Scope\Condition(...func_get_args()); - } - - return $scope; - } - /** * @return static */ From 50c8d750edace39798b3578920ea8ee102b39b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Fri, 1 Sep 2023 11:32:19 +0200 Subject: [PATCH 7/7] fix merge --- src/Model/Scope.php | 2 +- src/Model/Scope/AbstractScope.php | 7 +++++-- src/Model/Scope/Condition.php | 12 +++++++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Model/Scope.php b/src/Model/Scope.php index a3db0a4fa..3a13f751b 100644 --- a/src/Model/Scope.php +++ b/src/Model/Scope.php @@ -89,7 +89,7 @@ public function addCondition($field, $operator = null, $value = null) return $this; } - protected function setSystem($system = true) + protected function setSystem(bool $system = true) { foreach ($this->elements as $nestedCondition) { $nestedCondition->setSystem($system && $this->isAnd()); diff --git a/src/Model/Scope/AbstractScope.php b/src/Model/Scope/AbstractScope.php index badfa0f7c..2257e2ce5 100644 --- a/src/Model/Scope/AbstractScope.php +++ b/src/Model/Scope/AbstractScope.php @@ -34,14 +34,17 @@ protected function init(): void $this->_init(); // always set system flag if condition added to another condition - $this->setSystem($this->owner instanceof RootScope); + $this->setSystem($owner instanceof RootScope); $this->onChangeModel(); } abstract protected function onChangeModel(): void; - abstract protected function setSystem($system = true); + /** + * @return $this + */ + abstract protected function setSystem(bool $system = true); /** * Get the model this condition is associated with. diff --git a/src/Model/Scope/Condition.php b/src/Model/Scope/Condition.php index 2c1a56325..dfeaf6ffe 100644 --- a/src/Model/Scope/Condition.php +++ b/src/Model/Scope/Condition.php @@ -26,7 +26,7 @@ class Condition extends AbstractScope /** @var mixed */ public $value; - protected $system = false; + protected bool $system = false; public const OPERATOR_EQUALS = '='; public const OPERATOR_DOESNOT_EQUAL = '!='; @@ -151,7 +151,7 @@ public function __construct($key, $operator = null, $value = null) } } - protected function setSystem($system = true) + protected function setSystem(bool $system = true) { $this->system = $system; @@ -165,7 +165,7 @@ protected function onChangeModel(): void // if we have a definitive equal condition set the value as default value for field // new records will automatically get this value assigned for the field // TODO: fix when condition is part of OR scope - if ($this->system && $this->setsDefiniteValue()) { + if ($this->system && $this->isDefiniteValue()) { // key containing '/' means chained references and it is handled in toQueryArguments method $field = $this->key; if (is_string($field) && !str_contains($field, '/')) { @@ -271,9 +271,11 @@ public function isEmpty(): bool /** * Checks if condition sets a definitive scalar value for a field. */ - protected function setsDefiniteValue(): bool + protected function isDefiniteValue(): bool { - return $this->operator === self::OPERATOR_EQUALS && !is_object($this->value) && !is_array($this->value); + return $this->operator === self::OPERATOR_EQUALS && !is_array($this->value) + && !$this->value instanceof Expressionable + && !$this->value instanceof Persistence\Array_\Action; // needed to pass hintable tests } public function clear()