Skip to content

Commit

Permalink
Merge pull request #17 from biigle/can-reviw-user
Browse files Browse the repository at this point in the history
Support reviewer users to approve and reject storage requests
  • Loading branch information
mzur authored Jun 2, 2023
2 parents 35739df + 6ef3e64 commit 583946d
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/Http/Controllers/Api/StorageRequestFileController.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public function store(StoreStorageRequestFile $request)
*/
public function show(Request $request, $id)
{
if (!$request->user()->can('sudo')) {
if (!$request->user()->can('review')) {
abort(Response::HTTP_NOT_FOUND);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Http/Requests/RejectStorageRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function authorize()
$this->storageRequest = StorageRequest::whereNull('expires_at')
->findOrFail($this->route('id'));

return $this->user()->can('destroy', $this->storageRequest);
return $this->user()->can('reject', $this->storageRequest);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Http/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
$router->delete('storage-requests/{id}/directories', 'StorageRequestDirectoryController@destroy');

$router->group([
'middleware' => ['can:sudo'],
'middleware' => ['can:review'],
], function ($router) {
$router->post('storage-requests/{id}/approve', 'StorageRequestController@approve');
$router->post('storage-requests/{id}/reject', 'StorageRequestController@reject');
Expand Down
19 changes: 16 additions & 3 deletions src/Policies/StorageRequestPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function update(User $user, StorageRequest $request)
}

/**
* Determine if the given user can update the expiration date of the storage request.
* Determine if the given user can approve the storage request.
*
* @param User $user
* @param StorageRequest $request
Expand All @@ -83,8 +83,21 @@ public function update(User $user, StorageRequest $request)
*/
public function approve(User $user, StorageRequest $request)
{
// Only global admins can do this.
return false;
// Only global admins and reviewers can do this.
return $user->canReview;
}

/**
* Determine if the given user can reject the storage request.
*
* @param User $user
* @param StorageRequest $request
*
* @return bool
*/
public function reject(User $user, StorageRequest $request)
{
return $this->approve($user, $request);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions tests/Http/Controllers/Api/StorageRequestControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public function testApprove()
$this->be($request->user);
$this->postJson("/api/v1/storage-requests/{$id}/approve")->assertStatus(403);

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->postJson("/api/v1/storage-requests/{$id}/approve")->assertStatus(200);

$this->assertNotNull($request->fresh()->expires_at);
Expand All @@ -208,7 +208,7 @@ public function testApproveEmpty()
$request = StorageRequest::factory()->create();
$id = $request->id;

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->postJson("/api/v1/storage-requests/{$id}/approve")->assertStatus(422);
}

Expand All @@ -223,7 +223,7 @@ public function testApproveAlreadyApproved()
]);
$id = $request->id;

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->postJson("/api/v1/storage-requests/{$id}/approve")->assertStatus(404);
}

Expand All @@ -244,7 +244,7 @@ public function testReject()
$this->be($request->user);
$this->postJson("/api/v1/storage-requests/{$id}/reject")->assertStatus(403);

$this->beGlobalAdmin();
$this->beGlobalReviewer();
// Needs a reason
$this->postJson("/api/v1/storage-requests/{$id}/reject")->assertStatus(422);

Expand All @@ -271,7 +271,7 @@ public function testRejectAlreadyApproved()
]);
$id = $request->id;

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->postJson("/api/v1/storage-requests/{$id}/reject")->assertStatus(404);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ public function testShow() {
$this->be($request->user);
$this->get("/api/v1/storage-request-files/{$file->id}")->assertStatus(404);

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->get("/api/v1/storage-request-files/{$file->id}")->assertStatus(200);
$this->get("/api/v1/storage-request-files/{$file2->id}")->assertStatus(404);
}
Expand All @@ -728,7 +728,7 @@ public function testShowApproved() {
});
$disk->put("user-{$request->user_id}/a.jpg", 'abc');

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->get("/api/v1/storage-request-files/{$file->id}")->assertStatus(200);
}

Expand All @@ -741,7 +741,7 @@ public function testShowPublic() {
'path' => 'a.jpg',
]);

$this->beGlobalAdmin();
$this->beGlobalReviewer();
$this->get("/api/v1/storage-request-files/{$file->id}")
->assertRedirect('myurl');
}
Expand Down
19 changes: 19 additions & 0 deletions tests/Policies/StorageRequestPolicyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public function testCreate()
$this->assertTrue($this->editor()->can('create', StorageRequest::class));
$this->assertTrue($this->expert()->can('create', StorageRequest::class));
$this->assertTrue($this->admin()->can('create', StorageRequest::class));
$this->assertTrue($this->globalReviewer()->can('create', StorageRequest::class));
$this->assertTrue($this->globalAdmin()->can('create', StorageRequest::class));
}

Expand All @@ -34,6 +35,7 @@ public function testCreateMaintenanceMode()
$this->assertFalse($this->editor()->can('create', StorageRequest::class));
$this->assertFalse($this->expert()->can('create', StorageRequest::class));
$this->assertFalse($this->admin()->can('create', StorageRequest::class));
$this->assertFalse($this->globalReviewer()->can('create', StorageRequest::class));
$this->assertTrue($this->globalAdmin()->can('create', StorageRequest::class));
}

Expand All @@ -45,6 +47,7 @@ public function testAccess()
$this->assertTrue($this->editor()->can('access', $this->request));
$this->assertFalse($this->expert()->can('access', $this->request));
$this->assertFalse($this->admin()->can('access', $this->request));
$this->assertFalse($this->globalReviewer()->can('access', $this->request));
$this->assertTrue($this->globalAdmin()->can('access', $this->request));
}

Expand All @@ -56,6 +59,7 @@ public function testUpdate()
$this->assertTrue($this->editor()->can('update', $this->request));
$this->assertFalse($this->expert()->can('update', $this->request));
$this->assertFalse($this->admin()->can('update', $this->request));
$this->assertFalse($this->globalReviewer()->can('update', $this->request));
$this->assertFalse($this->globalAdmin()->can('update', $this->request));
}

Expand All @@ -68,6 +72,7 @@ public function testUpdateMaintenanceMode()
$this->assertFalse($this->editor()->can('update', $this->request));
$this->assertFalse($this->expert()->can('update', $this->request));
$this->assertFalse($this->admin()->can('update', $this->request));
$this->assertFalse($this->globalReviewer()->can('update', $this->request));
$this->assertFalse($this->globalAdmin()->can('update', $this->request));
}

Expand All @@ -86,9 +91,22 @@ public function testApprove()
$this->assertFalse($this->editor()->can('approve', $this->request));
$this->assertFalse($this->expert()->can('approve', $this->request));
$this->assertFalse($this->admin()->can('approve', $this->request));
$this->assertTrue($this->globalReviewer()->can('approve', $this->request));
$this->assertTrue($this->globalAdmin()->can('approve', $this->request));
}

public function testReject()
{
$this->assertFalse($this->globalGuest()->can('reject', $this->request));
$this->assertFalse($this->user()->can('reject', $this->request));
$this->assertFalse($this->guest()->can('reject', $this->request));
$this->assertFalse($this->editor()->can('reject', $this->request));
$this->assertFalse($this->expert()->can('reject', $this->request));
$this->assertFalse($this->admin()->can('reject', $this->request));
$this->assertTrue($this->globalReviewer()->can('reject', $this->request));
$this->assertTrue($this->globalAdmin()->can('reject', $this->request));
}

public function testDestroy()
{
$this->assertFalse($this->globalGuest()->can('destroy', $this->request));
Expand All @@ -97,6 +115,7 @@ public function testDestroy()
$this->assertTrue($this->editor()->can('destroy', $this->request));
$this->assertFalse($this->expert()->can('destroy', $this->request));
$this->assertFalse($this->admin()->can('destroy', $this->request));
$this->assertFalse($this->globalReviewer()->can('destroy', $this->request));
$this->assertTrue($this->globalAdmin()->can('destroy', $this->request));
}
}

0 comments on commit 583946d

Please sign in to comment.