[V5] WherePivot instead of only Where on team relation pivot, better …

…readability (#1944) * WherePivot instead of only Where on team relation pivot * Fix detach, sync after wherePivot on relations Co-authored-by: Erik Niebla <[email protected]>
spatie · Dec 17, 2021 · 79f7dbf · 79f7dbf
1 parent 3f3d874
commit 79f7dbf
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 90 deletions.
diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php
@@ -44,18 +44,19 @@ public function getPermissionClass()
      */
     public function permissions(): BelongsToMany
     {
-        return $this->morphToMany(
+        $relation = $this->morphToMany(
             config('permission.models.permission'),
             'model',
             config('permission.table_names.model_has_permissions'),
             config('permission.column_names.model_morph_key'),
             PermissionRegistrar::$pivotPermission
-        )
-        ->where(function ($q) {
-            $q->when(PermissionRegistrar::$teams, function ($q) {
-                $q->where(PermissionRegistrar::$teamsKey, app(PermissionRegistrar::class)->getPermissionsTeamId());
-            });
-        });
+        );
+
+        if (! PermissionRegistrar::$teams) {
+            return $relation;
+        }
+
+        return $relation->wherePivot(PermissionRegistrar::$teamsKey, getPermissionsTeamId());
     }
 
     /**
@@ -314,21 +315,6 @@ public function getAllPermissions(): Collection
         return $permissions->sort()->values();
     }
 
-    /**
-     * Add teams pivot if teams are enabled
-     *
-     * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany
-     */
-    protected function getPermissionsRelation()
-    {
-        $relation = $this->permissions();
-        if (PermissionRegistrar::$teams && ! is_a($this, Role::class)) {
-            $relation->wherePivot(PermissionRegistrar::$teamsKey, app(PermissionRegistrar::class)->getPermissionsTeamId());
-        }
-
-        return $relation;
-    }
-
     /**
      * Grant the given permission(s) to a role.
      *
@@ -338,33 +324,30 @@ protected function getPermissionsRelation()
      */
     public function givePermissionTo(...$permissions)
     {
-        $permissionClass = $this->getPermissionClass();
         $permissions = collect($permissions)
             ->flatten()
-            ->map(function ($permission) {
+            ->reduce(function ($array, $permission) {
                 if (empty($permission)) {
-                    return false;
+                    return $array;
+                }
+
+                $permission = $this->getStoredPermission($permission);
+                if (! $permission instanceof Permission) {
+                    return $array;
                 }
 
-                return $this->getStoredPermission($permission);
-            })
-            ->filter(function ($permission) {
-                return $permission instanceof Permission;
-            })
-            ->each(function ($permission) {
                 $this->ensureModelSharesGuard($permission);
-            })
-            ->map(function ($permission) {
-                return [$permission->getKeyName() => $permission->getKey(), 'values' => PermissionRegistrar::$teams && ! is_a($this, Role::class) ?
-                    [PermissionRegistrar::$teamsKey => app(PermissionRegistrar::class)->getPermissionsTeamId()] : [],
-                ];
-            })
-            ->pluck('values', (new $permissionClass())->getKeyName())->toArray();
+
+                $array[$permission->getKey()] = PermissionRegistrar::$teams && ! is_a($this, Role::class) ?
+                    [PermissionRegistrar::$teamsKey => getPermissionsTeamId()] : [];
+
+                return $array;
+            }, []);
 
         $model = $this->getModel();
 
         if ($model->exists) {
-            $this->getPermissionsRelation()->sync($permissions, false);
+            $this->permissions()->sync($permissions, false);
             $model->load('permissions');
         } else {
             $class = \get_class($model);
@@ -396,7 +379,7 @@ function ($object) use ($permissions, $model) {
      */
     public function syncPermissions(...$permissions)
     {
-        $this->getPermissionsRelation()->detach();
+        $this->permissions()->detach();
 
         return $this->givePermissionTo($permissions);
     }
@@ -410,7 +393,7 @@ public function syncPermissions(...$permissions)
      */
     public function revokePermissionTo($permission)
     {
-        $this->getPermissionsRelation()->detach($this->getStoredPermission($permission));
+        $this->permissions()->detach($this->getStoredPermission($permission));
 
         if (is_a($this, get_class(app(PermissionRegistrar::class)->getRoleClass()))) {
             $this->forgetCachedPermissions();

diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php
@@ -41,25 +41,23 @@ public function getRoleClass()
      */
     public function roles(): BelongsToMany
     {
-        $model_has_roles = config('permission.table_names.model_has_roles');
-
-        return $this->morphToMany(
+        $relation = $this->morphToMany(
             config('permission.models.role'),
             'model',
-            $model_has_roles,
+            config('permission.table_names.model_has_roles'),
             config('permission.column_names.model_morph_key'),
             PermissionRegistrar::$pivotRole
-        )
-        ->where(function ($q) use ($model_has_roles) {
-            $q->when(PermissionRegistrar::$teams, function ($q) use ($model_has_roles) {
-                $teamId = app(PermissionRegistrar::class)->getPermissionsTeamId();
-                $q->where($model_has_roles.'.'.PermissionRegistrar::$teamsKey, $teamId)
-                    ->where(function ($q) use ($teamId) {
-                        $teamField = config('permission.table_names.roles').'.'.PermissionRegistrar::$teamsKey;
-                        $q->whereNull($teamField)->orWhere($teamField, $teamId);
-                    });
+        );
+
+        if (! PermissionRegistrar::$teams) {
+            return $relation;
+        }
+
+        return $relation->wherePivot(PermissionRegistrar::$teamsKey, getPermissionsTeamId())
+            ->where(function ($q) {
+                $teamField = config('permission.table_names.roles').'.'.PermissionRegistrar::$teamsKey;
+                $q->whereNull($teamField)->orWhere($teamField, getPermissionsTeamId());
             });
-        });
     }
 
     /**
@@ -98,21 +96,6 @@ public function scopeRole(Builder $query, $roles, $guard = null): Builder
         });
     }
 
-    /**
-     * Add teams pivot if teams are enabled
-     *
-     * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany
-     */
-    protected function getRolesRelation()
-    {
-        $relation = $this->roles();
-        if (PermissionRegistrar::$teams && ! is_a($this, Permission::class)) {
-            $relation->wherePivot(PermissionRegistrar::$teamsKey, app(PermissionRegistrar::class)->getPermissionsTeamId());
-        }
-
-        return $relation;
-    }
-
     /**
      * Assign the given role to the model.
      *
@@ -122,33 +105,30 @@ protected function getRolesRelation()
      */
     public function assignRole(...$roles)
     {
-        $roleClass = $this->getRoleClass();
         $roles = collect($roles)
             ->flatten()
-            ->map(function ($role) {
+            ->reduce(function ($array, $role) {
                 if (empty($role)) {
-                    return false;
+                    return $array;
+                }
+
+                $role = $this->getStoredRole($role);
+                if (! $role instanceof Role) {
+                    return $array;
                 }
 
-                return $this->getStoredRole($role);
-            })
-            ->filter(function ($role) {
-                return $role instanceof Role;
-            })
-            ->each(function ($role) {
                 $this->ensureModelSharesGuard($role);
-            })
-            ->map(function ($role) {
-                return [$role->getKeyName() => $role->getKey(), 'values' => PermissionRegistrar::$teams && ! is_a($this, Permission::class) ?
-                    [PermissionRegistrar::$teamsKey => app(PermissionRegistrar::class)->getPermissionsTeamId()] : [],
-                ];
-            })
-            ->pluck('values', (new $roleClass())->getKeyName())->toArray();
+
+                $array[$role->getKey()] = PermissionRegistrar::$teams && ! is_a($this, Permission::class) ?
+                    [PermissionRegistrar::$teamsKey => getPermissionsTeamId()] : [];
+
+                return $array;
+            }, []);
 
         $model = $this->getModel();
 
         if ($model->exists) {
-            $this->getRolesRelation()->sync($roles, false);
+            $this->roles()->sync($roles, false);
             $model->load('roles');
         } else {
             $class = \get_class($model);
@@ -178,7 +158,7 @@ function ($object) use ($roles, $model) {
      */
     public function removeRole($role)
     {
-        $this->getRolesRelation()->detach($this->getStoredRole($role));
+        $this->roles()->detach($this->getStoredRole($role));
 
         $this->load('roles');
 
@@ -198,7 +178,7 @@ public function removeRole($role)
      */
     public function syncRoles(...$roles)
     {
-        $this->getRolesRelation()->detach();
+        $this->roles()->detach();
 
         return $this->assignRole($roles);
     }

diff --git a/src/helpers.php b/src/helpers.php
@@ -36,6 +36,6 @@ function setPermissionsTeamId($id)
      */
     function getPermissionsTeamId()
     {
-        app(\Spatie\Permission\PermissionRegistrar::class)->getPermissionsTeamId();
+        return app(\Spatie\Permission\PermissionRegistrar::class)->getPermissionsTeamId();
     }
 }