Refactor attach, detach and sync models methods

santigarcor · Oct 2, 2017 · 7da556a · 7da556a
1 parent 7e1bc34
commit 7da556a
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 14 deletions.
diff --git a/src/Helper.php b/src/Helper.php
@@ -37,6 +37,17 @@ public static function getIdFor($object, $type)
         );
     }
 
+    /**
+     * Check if a string is a valid relationship name.
+     *
+     * @param string $relationship
+     * @return boolean
+     */
+    public static function isValidRelationship($relationship)
+    {
+        return in_array($relationship, ['roles', 'permissions']);
+    }
+
     /**
      * Returns the team's foreign key.
      *

diff --git a/src/Traits/LaratrustUserTrait.php b/src/Traits/LaratrustUserTrait.php
@@ -347,18 +347,18 @@ public function ability($roles, $permissions, $team = null, $options = [])
      * Alias to eloquent many-to-many relation's attach() method.
      *
      * @param  string  $relationship
-     * @param  string  $objectType
      * @param  mixed  $object
      * @param  mixed  $team
      * @return static
      */
-    private function attachModel($relationship, $objectType, $object, $team)
+    private function attachModel($relationship, $object, $team)
     {
-        if (!in_array($relationship, ['roles', 'permissions'])) {
+        if (!Helper::isValidRelationship($relationship)) {
             throw new InvalidArgumentException;
         }
 
         $attributes = [];
+        $objectType = Str::singular($relationship);
         $object = Helper::getIdFor($object, $objectType);
 
         if (Config::get('laratrust.use_teams')) {
@@ -390,17 +390,17 @@ private function attachModel($relationship, $objectType, $object, $team)
      * Alias to eloquent many-to-many relation's detach() method.
      *
      * @param  string  $relationship
-     * @param  string  $objectType
      * @param  mixed  $object
      * @param  mixed  $team
      * @return static
      */
-    private function detachModel($relationship, $objectType, $object, $team)
+    private function detachModel($relationship, $object, $team)
     {
-        if (!in_array($relationship, ['roles', 'permissions'])) {
+        if (!Helper::isValidRelationship($relationship)) {
             throw new InvalidArgumentException;
         }
 
+        $objectType = Str::singular($relationship);
         $relationshipQuery = $this->$relationship();
 
         if (Config::get('laratrust.use_teams')) {
@@ -419,8 +419,22 @@ private function detachModel($relationship, $objectType, $object, $team)
         return $this;
     }
 
-    private function syncModels($relationship, $objectType, $objects, $team, $detaching)
+    /**
+     * Alias to eloquent many-to-many relation's sync() method.
+     *
+     * @param  string  $relationship
+     * @param  mixed  $objects
+     * @param  mixed  $team
+     * @param  boolean  $detaching
+     * @return static
+     */
+    private function syncModels($relationship, $objects, $team, $detaching)
     {
+        if (!Helper::isValidRelationship($relationship)) {
+            throw new InvalidArgumentException;
+        }
+
+        $objectType = Str::singular($relationship);
         $mappedObjects = [];
         $useTeams = Config::get('laratrust.use_teams');
         $team = $useTeams ? Helper::getIdFor($team, 'team') : null;
@@ -456,7 +470,7 @@ private function syncModels($relationship, $objectType, $objects, $team, $detach
      */
     public function attachRole($role, $team = null)
     {
-        return $this->attachModel('roles', 'role', $role, $team);
+        return $this->attachModel('roles', $role, $team);
     }
 
     /**
@@ -468,7 +482,7 @@ public function attachRole($role, $team = null)
      */
     public function detachRole($role, $team = null)
     {
-        return $this->detachModel('roles', 'role', $role, $team);
+        return $this->detachModel('roles', $role, $team);
     }
 
     /**
@@ -517,7 +531,7 @@ public function detachRoles($roles = [], $team = null)
      */
     public function syncRoles($roles = [], $team = null, $detaching = true)
     {
-        return $this->syncModels('roles', 'role', $roles, $team, $detaching);
+        return $this->syncModels('roles', $roles, $team, $detaching);
     }
 
     /**
@@ -541,7 +555,7 @@ public function syncRolesWithoutDetaching($roles = [], $team = null)
      */
     public function attachPermission($permission, $team = null)
     {
-        return $this->attachModel('permissions', 'permission', $permission, $team);
+        return $this->attachModel('permissions', $permission, $team);
     }
 
     /**
@@ -553,7 +567,7 @@ public function attachPermission($permission, $team = null)
      */
     public function detachPermission($permission, $team = null)
     {
-        return $this->detachModel('permissions', 'permission', $permission, $team);
+        return $this->detachModel('permissions', $permission, $team);
     }
 
     /**
@@ -602,7 +616,7 @@ public function detachPermissions($permissions = [], $team = null)
      */
     public function syncPermissions($permissions = [], $team = null, $detaching = true)
     {
-        return $this->syncModels('permissions', 'permission', $permissions, $team, $detaching);
+        return $this->syncModels('permissions', $permissions, $team, $detaching);
     }
 
     /**
@@ -614,7 +628,7 @@ public function syncPermissions($permissions = [], $team = null, $detaching = tr
      */
     public function syncPermissionsWithoutDetaching($permissions = [], $team = null)
     {
-        return $this->syncModels('permissions', 'permission', $permissions, $team, false);
+        return $this->syncPermissions($permissions, $team, false);
     }
 
     /**