TE-9680: Fixed Acl architecture issue (#8798)

TE-9680 Fixed ACL architecture issue: FacadeNoLogicRule
spryker · Oct 18, 2021 · fcb06b6 · fcb06b6
1 parent 240a575
commit fcb06b6
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 41 deletions.
diff --git a/architecture-baseline.json b/architecture-baseline.json
@@ -5,26 +5,5 @@
         "rule": "FacadeNoLogicRule",
         "ruleset": "Spryker Zed",
         "priority": "1"
-    },
-    {
-        "fileName": "/vendor/spryker/spryker/Bundles/Acl/src/Spryker/Zed/Acl/Business/AclFacade.php",
-        "description": "\n      Zed Business Layer - Facade: The method Spryker\\Zed\\Acl\\Business\\AclFacade::updateGroup() contains a \"if\" statement which violates the rule \"A Facade must not contain logic and only delegate.\"\n    ",
-        "rule": "FacadeNoLogicRule",
-        "ruleset": "Spryker Zed",
-        "priority": "1"
-    },
-    {
-        "fileName": "/vendor/spryker/spryker/Bundles/Acl/src/Spryker/Zed/Acl/Business/AclFacade.php",
-        "description": "\n      Zed Business Layer - Facade: The method Spryker\\Zed\\Acl\\Business\\AclFacade::addRolesToGroup() contains a \"foreach\" statement which violates the rule \"A Facade must not contain logic and only delegate.\"\n    ",
-        "rule": "FacadeNoLogicRule",
-        "ruleset": "Spryker Zed",
-        "priority": "1"
-    },
-    {
-        "fileName": "/vendor/spryker/spryker/Bundles/Acl/src/Spryker/Zed/Acl/Business/AclFacade.php",
-        "description": "\n      Zed Business Layer - Facade: The method Spryker\\Zed\\Acl\\Business\\AclFacade::addRolesToGroup() contains a \"if\" statement which violates the rule \"A Facade must not contain logic and only delegate.\"\n    ",
-        "rule": "FacadeNoLogicRule",
-        "ruleset": "Spryker Zed",
-        "priority": "1"
     }
 ]
diff --git a/src/Spryker/Zed/Acl/Business/AclFacade.php b/src/Spryker/Zed/Acl/Business/AclFacade.php
@@ -90,15 +90,7 @@ public function createGroup(GroupTransfer $groupTransfer, RolesTransfer $rolesTr
      */
     public function updateGroup(GroupTransfer $transfer, RolesTransfer $rolesTransfer)
     {
-        $groupTransfer = $this->getFactory()
-            ->createGroupModel()
-            ->updateGroup($transfer);
-
-        if (!empty($rolesTransfer)) {
-            $this->addRolesToGroup($groupTransfer, $rolesTransfer);
-        }
-
-        return $groupTransfer;
+        return $this->getFactory()->createGroupModel()->updateGroup($transfer, $rolesTransfer);
     }
 
     /**
@@ -567,14 +559,7 @@ public function addRoleToGroup($idRole, $idGroup)
      */
     public function addRolesToGroup(GroupTransfer $groupTransfer, RolesTransfer $rolesTransfer)
     {
-        $groupModel = $this->getFactory()->createGroupModel();
-        $groupModel->removeRolesFromGroup($groupTransfer->getIdAclGroup());
-
-        foreach ($rolesTransfer->getRoles() as $roleTransfer) {
-            if ($roleTransfer->getIdAclRole() > 0) {
-                $groupModel->addRoleToGroup($roleTransfer->getIdAclRole(), $groupTransfer->getIdAclGroup());
-            }
-        }
+        $this->getFactory()->createGroupModel()->addRolesToGroup($groupTransfer, $rolesTransfer);
     }
 
     /**

diff --git a/src/Spryker/Zed/Acl/Business/Model/Group.php b/src/Spryker/Zed/Acl/Business/Model/Group.php
@@ -54,18 +54,25 @@ public function addGroup($name)
 
     /**
      * @param \Generated\Shared\Transfer\GroupTransfer $group
+     * @param \Generated\Shared\Transfer\RolesTransfer|null $rolesTransfer
      *
      * @return \Generated\Shared\Transfer\GroupTransfer
      */
-    public function updateGroup(GroupTransfer $group)
+    public function updateGroup(GroupTransfer $group, ?RolesTransfer $rolesTransfer)
     {
         $original = $this->getGroupById($group->getIdAclGroup());
 
         if ($group->getName() !== $original->getName()) {
             $this->assertGroupHasName($group);
         }
 
-        return $this->save($group);
+        $groupTransfer = $this->save($group);
+
+        if ($rolesTransfer !== null) {
+            $this->addRolesToGroup($groupTransfer, $rolesTransfer);
+        }
+
+        return $groupTransfer;
     }
 
     /**
@@ -203,6 +210,23 @@ public function addRoleToGroup($idRole, $idGroup)
         return $entity->save();
     }
 
+    /**
+     * @param \Generated\Shared\Transfer\GroupTransfer $groupTransfer
+     * @param \Generated\Shared\Transfer\RolesTransfer $rolesTransfer
+     *
+     * @return void
+     */
+    public function addRolesToGroup(GroupTransfer $groupTransfer, RolesTransfer $rolesTransfer): void
+    {
+        $this->removeRolesFromGroup($groupTransfer->getIdAclGroup());
+
+        foreach ($rolesTransfer->getRoles() as $roleTransfer) {
+            if ($roleTransfer->getIdAclRole() > 0) {
+                $this->addRoleToGroup($roleTransfer->getIdAclRole(), $groupTransfer->getIdAclGroup());
+            }
+        }
+    }
+
     /**
      * @param int $idAclGroup
      *

diff --git a/src/Spryker/Zed/Acl/Business/Model/GroupInterface.php b/src/Spryker/Zed/Acl/Business/Model/GroupInterface.php
@@ -8,6 +8,7 @@
 namespace Spryker\Zed\Acl\Business\Model;
 
 use Generated\Shared\Transfer\GroupTransfer;
+use Generated\Shared\Transfer\RolesTransfer;
 
 interface GroupInterface
 {
@@ -22,10 +23,11 @@ public function addGroup($name);
 
     /**
      * @param \Generated\Shared\Transfer\GroupTransfer $group
+     * @param \Generated\Shared\Transfer\RolesTransfer $rolesTransfer
      *
      * @return \Generated\Shared\Transfer\GroupTransfer
      */
-    public function updateGroup(GroupTransfer $group);
+    public function updateGroup(GroupTransfer $group, RolesTransfer $rolesTransfer);
 
     /**
      * @param \Generated\Shared\Transfer\GroupTransfer $group
@@ -124,6 +126,14 @@ public function getUserGroups($idUser);
      */
     public function addRoleToGroup($idRole, $idGroup);
 
+    /**
+     * @param \Generated\Shared\Transfer\GroupTransfer $groupTransfer
+     * @param \Generated\Shared\Transfer\RolesTransfer $rolesTransfer
+     *
+     * @return void
+     */
+    public function addRolesToGroup(GroupTransfer $groupTransfer, RolesTransfer $rolesTransfer): void;
+
     /**
      * @param int $idAclGroup
      *