From 9d38df9043d2e4334c8e380c6a62c6ede0512f70 Mon Sep 17 00:00:00 2001
From: Matthew Ross <kiswa.com@gmail.com>
Date: Thu, 16 Jun 2016 11:49:15 -0400
Subject: [PATCH] Bring test coverage back to 100% (PHP 7 lies)

---
 src/api/controllers/Attachments.php      | 14 +++++-------
 src/api/controllers/Boards.php           |  4 ----
 test/api/controllers/AttachmentsTest.php | 27 ++++++++++++++++++++++--
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/src/api/controllers/Attachments.php b/src/api/controllers/Attachments.php
index da9c432..c70d452 100644
--- a/src/api/controllers/Attachments.php
+++ b/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;
diff --git a/src/api/controllers/Boards.php b/src/api/controllers/Boards.php
index 0282091..ee3fbaa 100644
--- a/src/api/controllers/Boards.php
+++ b/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. ' .
diff --git a/test/api/controllers/AttachmentsTest.php b/test/api/controllers/AttachmentsTest.php
index dda93e3..9b27b5e 100644
--- a/test/api/controllers/AttachmentsTest.php
+++ b/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() {