Apply suggestions from code review

diff --git a/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.5_boxCondtions.php b/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.5_boxCondtions.php
index b1fcef9874a0675605d848787f9fe48c4d6d8700..8681cc79a2638ea95e425e1f83a0b7a5544887ae 100644 (file)
--- a/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.5_boxCondtions.php
+++ b/wcfsetup/install/files/acp/update_com.woltlab.wcf_5.5_boxCondtions.php
@@ -1,7 +1,7 @@
 <?php
 
 /**
- * Creates for all boxes own conditions.
+ * Create conditions for all boxes that use the legacy page filter.
  *
  * @author Joshua Ruesweg
  * @copyright 2001-2021 WoltLab GmbH

diff --git a/wcfsetup/install/files/lib/acp/form/BoxAddForm.class.php b/wcfsetup/install/files/lib/acp/form/BoxAddForm.class.php
index 466199712cbc2b3f90f95510473528946065cafd..667f3590b583792ef87e09ad7380f68da78b57cc 100644 (file)
--- a/wcfsetup/install/files/lib/acp/form/BoxAddForm.class.php
+++ b/wcfsetup/install/files/lib/acp/form/BoxAddForm.class.php
@@ -405,38 +405,18 @@ class BoxAddForm extends AbstractForm
         );
 
         $this->pageIDs = $this->visibleEverywhere = null;
-        foreach ($this->groupedConditionObjectTypes as $groupedObjectTypes) {
-            foreach ($groupedObjectTypes as $objectTypes) {
-                if (\is_array($objectTypes)) {
-                    foreach ($objectTypes as $objectType) {
-                        $objectType->getProcessor()->readFormParameters();
-                        if ($objectType->objectTypeID == $pageConditionObjectTypeID) {
-                            \assert($objectTypes->getProcessor() instanceof MultiPageCondition);
-
-                            $data = $objectType->getProcessor()->getData();
-                            if ($data !== null) {
-                                $this->pageIDs = $data['pageIDs'];
-                                $this->visibleEverywhere = $data['pageIDs_reverseLogic'] ? 1 : 0;
-                            } else {
-                                $this->pageIDs = [];
-                                $this->visibleEverywhere = 1;
-                            }
-                        }
-                    }
+        foreach ($this->toFlatList($this->groupedConditionObjectTypes) as $objectType) {
+            $objectType->getProcessor()->readFormParameters();
+            if ($objectType->objectTypeID == $pageConditionObjectTypeID) {
+                \assert($objectType->getProcessor() instanceof MultiPageCondition);
+
+                $data = $objectType->getProcessor()->getData();
+                if ($data !== null) {
+                    $this->pageIDs = $data['pageIDs'];
+                    $this->visibleEverywhere = $data['pageIDs_reverseLogic'] ? 1 : 0;
                 } else {
-                    $objectTypes->getProcessor()->readFormParameters();
-                    if ($objectTypes->objectTypeID == $pageConditionObjectTypeID) {
-                        \assert($objectTypes->getProcessor() instanceof MultiPageCondition);
-
-                        $data = $objectTypes->getProcessor()->getData();
-                        if ($data !== null) {
-                            $this->pageIDs = $data['pageIDs'];
-                            $this->visibleEverywhere = $data['pageIDs_reverseLogic'] ? 1 : 0;
-                        } else {
-                            $this->pageIDs = [];
-                            $this->visibleEverywhere = 1;
-                        }
-                    }
+                    $this->pageIDs = [];
+                    $this->visibleEverywhere = 1;
                 }
             }
         }
@@ -590,16 +570,8 @@ class BoxAddForm extends AbstractForm
             $this->pageIDs = $statement->fetchAll(\PDO::FETCH_COLUMN);
         }
 
-        foreach ($this->groupedConditionObjectTypes as $groupedObjectTypes) {
-            foreach ($groupedObjectTypes as $objectTypes) {
-                if (\is_array($objectTypes)) {
-                    foreach ($objectTypes as $objectType) {
-                        $objectType->getProcessor()->validate();
-                    }
-                } else {
-                    $objectTypes->getProcessor()->validate();
-                }
-            }
+        foreach ($this->toFlatList($this->groupedConditionObjectTypes) as $objectType) {
+            $objectType->getProcessor()->validate();
         }
     }
 
@@ -706,18 +678,10 @@ class BoxAddForm extends AbstractForm
         // save acl
         SimpleAclHandler::getInstance()->setValues('com.woltlab.wcf.box', $box->boxID, $this->aclValues);
 
-        // transform conditions array into one-dimensional array
-        $conditions = [];
-        foreach ($this->groupedConditionObjectTypes as $groupedObjectTypes) {
-            foreach ($groupedObjectTypes as $objectTypes) {
-                if (\is_array($objectTypes)) {
-                    $conditions = \array_merge($conditions, $objectTypes);
-                } else {
-                    $conditions[] = $objectTypes;
-                }
-            }
-        }
-        ConditionHandler::getInstance()->createConditions($box->boxID, $conditions);
+        ConditionHandler::getInstance()->createConditions(
+            $box->boxID,
+            $this->toFlatList($this->groupedConditionObjectTypes)
+        );
 
         // call saved event
         $this->saved();
@@ -742,7 +706,7 @@ class BoxAddForm extends AbstractForm
         $this->linkPageID = 0;
         $this->linkPageObjectID = 0;
 
-        foreach ($conditions as $condition) {
+        foreach ($this->toFlatList($this->groupedConditionObjectTypes) as $condition) {
             $condition->getProcessor()->reset();
         }
     }
@@ -826,6 +790,27 @@ class BoxAddForm extends AbstractForm
         }
     }
 
+    /**
+     * This is a helper method to convert groupedConditionObjectTypes to a flat list.
+     * This method should not be used for any other purpose!
+     *
+     * @since 5.5
+     */
+    protected function toFlatList(array $array): array
+    {
+        $returnList = [];
+
+        foreach ($array as $element) {
+            if (\is_array($element)) {
+                $returnList = \array_merge($returnList, $this->toFlatList($element));
+            } else {
+                $returnList[] = $element;
+            }
+        }
+
+        return $returnList;
+    }
+
     /**
      * @inheritDoc
      */

diff --git a/wcfsetup/install/files/lib/acp/form/BoxEditForm.class.php b/wcfsetup/install/files/lib/acp/form/BoxEditForm.class.php
index c5e6387671f174f507fbb777bf3851dac7792102..f7b8ed81575bea5d0e3e92a7e1e44480cae71e9c 100644 (file)
--- a/wcfsetup/install/files/lib/acp/form/BoxEditForm.class.php
+++ b/wcfsetup/install/files/lib/acp/form/BoxEditForm.class.php
@@ -166,22 +166,10 @@ class BoxEditForm extends BoxAddForm
 
         SimpleAclHandler::getInstance()->setValues('com.woltlab.wcf.box', $this->box->boxID, $this->aclValues);
 
-        // transform conditions array into one-dimensional array
-        $conditions = [];
-        foreach ($this->groupedConditionObjectTypes as $groupedObjectTypes) {
-            foreach ($groupedObjectTypes as $objectTypes) {
-                if (\is_array($objectTypes)) {
-                    $conditions = \array_merge($conditions, $objectTypes);
-                } else {
-                    $conditions[] = $objectTypes;
-                }
-            }
-        }
-
         ConditionHandler::getInstance()->updateConditions(
             $this->box->boxID,
             $this->box->getVisibilityConditions(),
-            $conditions
+            $this->toFlatList($this->groupedConditionObjectTypes)
         );
 
         // call saved event
@@ -264,17 +252,9 @@ class BoxEditForm extends BoxAddForm
                 $conditionsByObjectTypeID[$condition->objectTypeID] = $condition;
             }
 
-            foreach ($this->groupedConditionObjectTypes as $objectTypes1) {
-                foreach ($objectTypes1 as $objectTypes2) {
-                    if (\is_array($objectTypes2)) {
-                        foreach ($objectTypes2 as $objectType) {
-                            if (isset($conditionsByObjectTypeID[$objectType->objectTypeID])) {
-                                $conditionsByObjectTypeID[$objectType->objectTypeID]->getObjectType()->getProcessor()->setData($conditionsByObjectTypeID[$objectType->objectTypeID]);
-                            }
-                        }
-                    } elseif (isset($conditionsByObjectTypeID[$objectTypes2->objectTypeID])) {
-                        $conditionsByObjectTypeID[$objectTypes2->objectTypeID]->getObjectType()->getProcessor()->setData($conditionsByObjectTypeID[$objectTypes2->objectTypeID]);
-                    }
+            foreach ($this->toFlatList($this->groupedConditionObjectTypes) as $objectType) {
+                if (isset($conditionsByObjectTypeID[$objectType->objectTypeID])) {
+                    $conditionsByObjectTypeID[$objectType->objectTypeID]->getObjectType()->getProcessor()->setData($conditionsByObjectTypeID[$objectType->objectTypeID]);
                 }
             }
         }

diff --git a/wcfsetup/install/files/lib/data/box/Box.class.php b/wcfsetup/install/files/lib/data/box/Box.class.php
index 424d4e640fae2d792206e4153aaaf477b4384325..dce62c6b6f8c97bef22b0b4e9b27a50b64e29b4b 100644 (file)
--- a/wcfsetup/install/files/lib/data/box/Box.class.php
+++ b/wcfsetup/install/files/lib/data/box/Box.class.php
@@ -538,7 +538,7 @@ class Box extends DatabaseObject
      * Returns the conditions for the box controller.
      *
      * @return  Condition[]
-     * @deprecated since 5.5 - use self::getControllerConditions() instead
+     * @deprecated 5.5 - use self::getControllerConditions() instead
      */
     public function getConditions()
     {
@@ -552,7 +552,7 @@ class Box extends DatabaseObject
      * @return  Condition[]
      * @since   5.5
      */
-    public function getControllerConditions()
+    public function getControllerConditions(): array
     {
         /** @noinspection PhpUndefinedMethodInspection */
         if ($this->boxType === 'system' && $this->getController() instanceof IConditionBoxController && $this->getController()->getConditionDefinition()) {