From: Dan Brown <redacted>
Date: Sun, 9 Oct 2022 16:14:11 +0000 (+0100)
Subject: Centralised handling of permission form data to own class
X-Git-Tag: v22.10~1^2~11^2^2~9
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/ffd6a1002e8ed40ba7b651391ee39c9ff6b2ea1f

Centralised handling of permission form data to own class

Also updates show roles on permission view to just those with
permissions applied.
Fixes rounded borders for lone permission rows.
Moves "Everyone Else" handling from role to new class.
---

diff --git a/app/Auth/Permissions/EntityPermission.php b/app/Auth/Permissions/EntityPermission.php
index 8af5f480a..32ebc440d 100644
--- a/app/Auth/Permissions/EntityPermission.php
+++ b/app/Auth/Permissions/EntityPermission.php
@@ -2,7 +2,9 @@
 
 namespace BookStack\Auth\Permissions;
 
+use BookStack\Auth\Role;
 use BookStack\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**
@@ -29,4 +31,12 @@ class EntityPermission extends Model
     {
         return $this->morphTo('restrictable');
     }
+
+    /**
+     * Get the role assigned to this entity permission.
+     */
+    public function role(): BelongsTo
+    {
+        return $this->belongsTo(Role::class);
+    }
 }
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
new file mode 100644
index 000000000..07c95c534
--- /dev/null
+++ b/app/Auth/Permissions/PermissionFormData.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace BookStack\Auth\Permissions;
+
+use BookStack\Auth\Role;
+use BookStack\Entities\Models\Entity;
+
+class PermissionFormData
+{
+    protected Entity $entity;
+
+    public function __construct(Entity $entity)
+    {
+        $this->entity = $entity;
+    }
+
+    /**
+     * Get the roles with permissions assigned.
+     */
+    public function rolesWithPermissions(): array
+    {
+        return $this->entity->permissions()
+            ->with('role')
+            ->where('role_id', '!=', 0)
+            ->get(['id', 'role_id'])
+            ->pluck('role')
+            ->sortBy('display_name')
+            ->all();
+    }
+
+    /**
+     * Get the roles that don't yet have specific permissions for the
+     * entity we're managing permissions for.
+     */
+    public function rolesNotAssigned(): array
+    {
+        $assigned = $this->entity->permissions()->pluck('role_id');
+        return Role::query()
+            ->where('system_name', '!=', 'admin')
+            ->whereNotIn('id', $assigned)
+            ->orderBy('display_name', 'asc')
+            ->get()
+            ->all();
+    }
+
+    /**
+     * Get the "Everyone Else" role entry.
+     */
+    public function everyoneElseRole(): Role
+    {
+        return (new Role())->forceFill([
+            'id' => 0,
+            'display_name' => 'Everyone Else',
+            'description' => 'Set permissions for all roles not specifically overridden.'
+        ]);
+    }
+}
diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index d5ce5cab7..17a4edcc0 100644
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -118,30 +118,6 @@ class Role extends Model implements Loggable
         return static::query()->where('hidden', '=', false)->orderBy('name')->get();
     }
 
-    /**
-     * Get the roles that can be restricted.
-     */
-    public static function restrictable(): Collection
-    {
-        return static::query()
-            ->where('system_name', '!=', 'admin')
-            ->orderBy('display_name', 'asc')
-            ->get();
-    }
-
-    /**
-     * Get a role to represent the case of 'Everyone else' in the system.
-     * Used within the interface since the default-fallback for permissions uses role_id=0.
-     */
-    public static function getEveryoneElseRole(): self
-    {
-        return (new static())->forceFill([
-            'id' => 0,
-            'display_name' => 'Everyone Else',
-            'description'  => 'Set permissions for all roles not specifically overridden.'
-        ]);
-    }
-
     /**
      * {@inheritdoc}
      */
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
index 92f994b00..d8dca9825 100644
--- a/app/Http/Controllers/PermissionsController.php
+++ b/app/Http/Controllers/PermissionsController.php
@@ -2,6 +2,7 @@
 
 namespace BookStack\Http\Controllers;
 
+use BookStack\Auth\Permissions\PermissionFormData;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Bookshelf;
 use BookStack\Entities\Models\Chapter;
@@ -28,6 +29,7 @@ class PermissionsController extends Controller
 
         return view('pages.permissions', [
             'page' => $page,
+            'data' => new PermissionFormData($page),
         ]);
     }
 
@@ -56,6 +58,7 @@ class PermissionsController extends Controller
 
         return view('chapters.permissions', [
             'chapter' => $chapter,
+            'data' => new PermissionFormData($chapter),
         ]);
     }
 
@@ -84,6 +87,7 @@ class PermissionsController extends Controller
 
         return view('books.permissions', [
             'book' => $book,
+            'data' => new PermissionFormData($book),
         ]);
     }
 
@@ -112,6 +116,7 @@ class PermissionsController extends Controller
 
         return view('shelves.permissions', [
             'shelf' => $shelf,
+            'data' => new PermissionFormData($shelf),
         ]);
     }
 
diff --git a/resources/sass/_components.scss b/resources/sass/_components.scss
index d0aadce6e..42477982a 100644
--- a/resources/sass/_components.scss
+++ b/resources/sass/_components.scss
@@ -818,6 +818,9 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
   border-radius: 0 0 4px 4px;
   border-bottom-width: 1.5px;
 }
+.content-permissions-row:first-child:last-child {
+  border-radius: 4px;
+}
 .content-permissions-row-toggle-all {
   visibility: hidden;
 }
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 408414b76..2fd0a4a43 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -19,13 +19,13 @@
     @endif
 
     <div class="content-permissions mt-m mb-xl">
-        @foreach(\BookStack\Auth\Role::restrictable() as $role)
+        @foreach($data->rolesWithPermissions() as $role)
             @include('form.entity-permissions-row', ['role' => $role, 'model' => $model])
         @endforeach
     </div>
 
     <div class="content-permissions mt-m mb-xl">
-        @include('form.entity-permissions-row', ['role' => \BookStack\Auth\Role::getEveryoneElseRole(), 'model' => $model])
+        @include('form.entity-permissions-row', ['role' => $data->everyoneElseRole(), 'model' => $model])
     </div>
 
     <div class="text-right">