Bring test coverage back to 100% (PHP 7 lies)

src/api/controllers/Attachments.php

@@ -94,11 +94,6 @@ class Attachments extends BaseController {
             }
         } // @codeCoverageIgnore
 
-        if (!$this->checkBoardAccess($this->getBoardId($attachment->task_id),
-                $request)) {
-            return $this->jsonResponse($response, 403);
-        }
-
         if ($attachment->id !== $id) {
             $this->logger->addError('Remove Attachment: ', [$attachment]);
             $this->apiJson->addAlert('error', 'Error removing attachment. ' .
@@ -107,6 +102,11 @@ class Attachments extends BaseController {
             return $this->jsonResponse($response);
         }
 
+        if (!$this->checkBoardAccess($this->getBoardId($attachment->task_id),
+                $request)) {
+            return $this->jsonResponse($response, 403);
+        }
+
         $before = $attachment;
         $attachment->delete();
 
@@ -124,10 +124,6 @@ class Attachments extends BaseController {
     private function getBoardId($taskId) {
         $task = new Task($this->container, $taskId);
 
-        if ($task->id === 0) {
-            return 0;
-        }
-
         $column = new Column($this->container, $task->column_id);
 
         return $column->board_id;

src/api/controllers/Boards.php

@@ -139,10 +139,6 @@ class Boards extends BaseController {
         $id = (int)$args['id'];
         $board = new Board($this->container, $id);
 
-        if (!$this->checkBoardAccess($board->id, $request)) {
-            return $this->jsonResponse($response, 403);
-        }
-
         if ($board->id !== $id) {
             $this->logger->addError('Remove Board: ', [$board]);
             $this->apiJson->addAlert('error', 'Error removing board. ' .

test/api/controllers/AttachmentsTest.php

@@ -186,16 +186,39 @@ class AttachmentsTest extends PHPUnit_Framework_TestCase {
 
     }
 
+    public function testRemoveAttachmentForbidden() {
+        $this->createAttachment();
+
+        DataMock::createBoardAdminUser();
+
+        $args = [];
+        $args['id'] = 1;
+
+        $request = new RequestMock();
+        $request->header = [DataMock::getJwt(2)];
+
+        $this->attachments = new Attachments(new ContainerMock());
+
+        $actual = $this->attachments->removeAttachment($request,
+            new ResponseMock(), $args);
+        $this->assertEquals('Access restricted.',
+            $actual->alerts[0]['text']);
+    }
+
     public function testRemoveBadAttachment() {
+        $this->createAttachment();
+        $this->attachments = new Attachments(new ContainerMock());
+
         $args = [];
         $args['id'] = 5; // No such attachment
 
         $request = new RequestMock();
         $request->header = [DataMock::getJwt()];
 
-        $response = $this->attachments->removeAttachment($request,
+        $actual = $this->attachments->removeAttachment($request,
             new ResponseMock(), $args);
-        $this->assertEquals('failure', $response->status);
+        $this->assertEquals('Error removing attachment. ' .
+            'No attachment found for ID 5.', $actual->alerts[0]['text']);
     }
 
     private function createBoard() {