wrote attachment api controller, fixed bug caught by unit test in CardService

Signed-off-by: Ryan Fletcher <ryan.fletcher@codepassion.ca>

appinfo/routes.php

@@ -107,6 +107,13 @@ return [
 		['name' => 'label_api#update', 'url' => '/api/v1.0/boards/{boardId}/labels/{labelId}', 'verb' => 'PUT'],
 		['name' => 'label_api#delete', 'url' => '/api/v1.0/boards/{boardId}/labels/{labelId}', 'verb' => 'DELETE'],
 
+		['name' => 'attachment_api#getAll', 'url' => '/api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments', 'verb' => 'GET'],
+		['name' => 'attachment_api#display', 'url' => '/api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments/{attachmentId}', 'verb' => 'GET'],
+		['name' => 'attachment_api#create', 'url' => '/api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments', 'verb' => 'POST'],
+		['name' => 'attachment_api#update', 'url' => '/api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments/{attachmentId}', 'verb' => 'PUT'],
+		['name' => 'attachment_api#delete', 'url' => '/api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments/{attachmentId}', 'verb' => 'DELETE'],
+		['name' => 'attachment_api#restore', 'url' => '/api/v1.0/boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments/{attachmentId}/restore', 'verb' => 'PUT'],
+
 		['name' => 'board_api#preflighted_cors', 'url' => '/api/v1.0/{path}','verb' => 'OPTIONS', 'requirements' => ['path' => '.+']],
 	]
 ];

lib/Controller/AttachmentApiController.php

@@ -27,20 +27,80 @@ use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IRequest;
 
+use OCA\Deck\Service\AttachmentService;
+
 class AttachmentApiController extends ApiController {
-    public function __construct($appName, IRequest $request) {
+
+    private $attachmentService;
+
+    public function __construct($appName, IRequest $request, AttachmentService $attachmentService) {
         parent::__construct($appName, $request);
+        $this->attachmentService = $attachmentService;
     }
 
-    public function getAll() {}
+    /**
-
+	 * @NoAdminRequired
-    public function display() {}
+	 * @CORS
-
+	 * @NoCSRFRequired
-    public function create() {}
+	 *
-
+	 */
-    public function update() {}
+    public function getAll() {
-
+        $attachment = $this->attachmentService->findAll($this->request->getParam('cardId'));
-    public function delete() {}
+        return new DataResponse($attachment, HTTP::STATUS_OK);
-
+    }
-    public function restore() {}
+
+    /**
+	 * @NoAdminRequired
+	 * @CORS
+	 * @NoCSRFRequired
+	 *
+	 */
+    public function display() {
+        $attachment = $this->attachmentService->display($this->request->getParam('cardId'), $this->request->getParam('attachmentId'));
+        return new DataResponse($attachment, HTTP::STATUS_OK);
+    }
+
+    /**
+	 * @NoAdminRequired
+	 * @CORS
+	 * @NoCSRFRequired
+	 *
+	 */
+    public function create($type, $data) {
+        $attachment = $this->attachmentService->create($this->request->getParam('cardId'), $type, $data);
+        return new DataResponse($attachment, HTTP::STATUS_OK);
+    }
+
+    /**
+	 * @NoAdminRequired
+	 * @CORS
+	 * @NoCSRFRequired
+	 *
+	 */
+    public function update($data) {
+        $attachment = $this->attachmentService->update($this->request->getParam('cardId'), $this->request->getParam('attachmentId'), $data);
+        return new DataResponse($attachment, HTTP::STATUS_OK);
+    }
+
+    /**
+	 * @NoAdminRequired
+	 * @CORS
+	 * @NoCSRFRequired
+	 *
+	 */
+    public function delete() {
+        $attachment = $this->attachmentService->delete($this->request->getParam('cardId'), $this->request->getParam('attachmentId'));
+        return new DataResponse($attachment, HTTP::STATUS_OK);
+    }
+
+    /**
+	 * @NoAdminRequired
+	 * @CORS
+	 * @NoCSRFRequired
+	 *
+	 */
+    public function restore() {
+        $attachment = $this->attachmentService->restore($this->request->getParam('cardId'), $this->request->getParam('attachmentId'));
+        return new DataResponse($attachment, HTTP::STATUS_OK);
+    }
 }

lib/Service/CardService.php

@@ -356,6 +356,7 @@ class CardService {
 	 * @throws \OCA\Deck\NoPermissionException
 	 * @throws \OCP\AppFramework\Db\DoesNotExistException
 	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
+	 * @throws BadRequestException
 	 */
 	public function assignLabel($cardId, $labelId) {
 
@@ -421,8 +422,8 @@ class CardService {
 			throw new BadRequestException('card id must be a number');
 		}
 
-		if (is_numeric($userId) === false) {
+		if ($userId === false || $userId === null) {
-			throw new BadRequestException('user id must be a number');
+			throw new BadRequestException('user id must be provided');
 		}
 
 		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
@@ -461,8 +462,8 @@ class CardService {
 			throw new BadRequestException('card id must be a number');
 		}
 
-		if (is_numeric($userId) === false) {
+		if ($userId === false || $userId === null) {
-			throw new BadRequestException('user id must be a number');
+			throw new BadRequestException('user must be provided');
 		}
 
 		$assignments = $this->assignedUsersMapper->find($cardId);

tests/unit/controller/AttachmentApiControllerTest.php

@@ -47,7 +47,8 @@ class AttachmentApiControllerTest extends \Test\TestCase {
         $this->attachmentService = $this->createMock(AttachmentService::class);
         $this->controller = new AttachmentApiController(
             $this->appName,
-            $this->request
+            $this->request,
+            $this->attachmentService
         );
     }
 
@@ -64,7 +65,7 @@ class AttachmentApiControllerTest extends \Test\TestCase {
             ->with('cardId')
             ->willReturn($allAttachments);
 
-        $expected = new DataResponse($this->attachmentExample, HTTP::STATUS_OK);
+        $expected = new DataResponse($allAttachments, HTTP::STATUS_OK);
         $actual = $this->controller->getAll();
         $this->assertEquals($expected, $actual);
     }
@@ -91,6 +92,9 @@ class AttachmentApiControllerTest extends \Test\TestCase {
 
     public function testCreate() {
 
+        $type = 'not null';
+        $data = ['not null'];
+
         $this->attachmentService->expects($this->once())
             ->method('create')
             ->willReturn($this->attachmentExample);
@@ -101,12 +105,15 @@ class AttachmentApiControllerTest extends \Test\TestCase {
             ->willReturn($this->cardId);
 
         $expected = new DataResponse($this->attachmentExample, HTTP::STATUS_OK);
-        $actual = $this->controller->create();
+        $actual = $this->controller->create($type, $data);
         $this->assertEquals($expected, $actual);
     }
 
     public function testUpdate() {
 
+        // FIXME: what is data supposed to be in this context?        
+        $data = ['not empty data'];
+
         $this->attachmentService->expects($this->once())
             ->method('update')
             ->willReturn($this->attachmentExample);
@@ -121,7 +128,7 @@ class AttachmentApiControllerTest extends \Test\TestCase {
                 $this->attachmentExample->getId());
 
         $expected = new DataResponse($this->attachmentExample, HTTP::STATUS_OK);
-        $actual = $this->controller->update();
+        $actual = $this->controller->update($data);
         $this->assertEquals($expected, $actual);
     }