worked on suggestions

canyongbs · Dec 25, 2024 · cd32e84 · cd32e84
1 parent 80084ff
commit cd32e84
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 35 deletions.
diff --git a/app-modules/task/src/Policies/TaskPolicy.php b/app-modules/task/src/Policies/TaskPolicy.php
@@ -99,11 +99,14 @@ public function update(Authenticatable $authenticatable, Task $task): Response
             return Response::deny('You do not have permission to update this task.');
         }
 
-        if ($authenticatable->getKey() !== $task?->assigned_to && $authenticatable->getKey() !== $task?->created_by) {
+        if ($authenticatable->getKey() !== $task->assigned_to && $authenticatable->getKey() !== $task->created_by) {
             return Response::deny('You do not have permission to update this task.');
         }
 
-        return Response::allow();
+        return $authenticatable->canOrElse(
+            abilities: ['task.*.update'],
+            denyResponse: 'You do not have permission to update this task.'
+        );
     }
 
     public function delete(Authenticatable $authenticatable, Task $task): Response

diff --git a/app-modules/task/tests/EditTaskTest.php b/app-modules/task/tests/EditTaskTest.php
@@ -54,9 +54,7 @@
 test('EditTask is gated with proper access control', function () {
     $user = User::factory()->licensed(LicenseType::cases())->create();
 
-    $task = Task::factory()->state([
-        'created_by' => $user->getKey(),
-    ])->create();
+    $task = Task::factory()->create();
 
     actingAs($user)
         ->get(
@@ -71,6 +69,18 @@
         ->assertForbidden();
 
     $user->givePermissionTo('task.view-any');
+    $user->givePermissionTo('task.*.update');
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertForbidden();
+
+    $task->assignedTo()->associate($user)->save();
+
+    $task->refresh();
 
     actingAs($user)
         ->get(
@@ -90,4 +100,4 @@
         ->assertHasNoFormErrors();
 
     // TODO: Check for changes
-});
+})->only();
diff --git a/app-modules/task/tests/TaskAssignmentTest.php b/app-modules/task/tests/TaskAssignmentTest.php
@@ -34,44 +34,15 @@
 </COPYRIGHT>
 */
 
-use AdvisingApp\Authorization\Enums\LicenseType;
-use AdvisingApp\Task\Filament\Resources\TaskResource;
 use AdvisingApp\Task\Models\Task;
 use AdvisingApp\Task\Notifications\TaskAssignedToUserNotification;
 use App\Models\User;
 use Illuminate\Support\Facades\Notification;
 
-use function Pest\Laravel\actingAs;
-
 beforeEach(function () {
     Notification::fake();
 });
 
-it('gives the proper permission to the assigned User of a Task on update', function () {
-    /** @var Task $task */
-    $task = Task::factory()->assigned()->create();
-
-    $user = User::factory()->licensed(LicenseType::cases())->create();
-
-    actingAs($user)->get(TaskResource::getUrl('edit', [
-        'record' => $task,
-    ]))->assertForbidden();
-
-    $user->givePermissionTo('task.view-any');
-    $user->givePermissionTo('task.*.view');
-    $user->givePermissionTo('task.*.update');
-
-    $user->refresh();
-
-    $task->assignedTo()->associate($user)->save();
-
-    $task->refresh();
-
-    actingAs($user)->get(TaskResource::getUrl('edit', [
-        'record' => $task,
-    ]))->assertSuccessful();
-});
-
 it('sends the proper notification to the assigned User', function () {
     $task = Task::factory()->assigned()->create();