Ticket attachments can be deleted

api/ApplicationContext.php

@@ -22,6 +22,7 @@ use BusinessLogic\Tickets\TrackingIdGenerator;
 use BusinessLogic\Tickets\VerifiedEmailChecker;
 use DataAccess\Attachments\AttachmentGateway;
 use DataAccess\Categories\CategoryGateway;
+use DataAccess\Files\FileDeleter;
 use DataAccess\Files\FileReader;
 use DataAccess\Files\FileWriter;
 use DataAccess\Logging\LoggingGateway;
@@ -102,11 +103,13 @@ class ApplicationContext {
         $this->get[UserToTicketChecker::class] = new UserToTicketChecker($this->get[UserGateway::class]);
         $this->get[FileWriter::class] = new FileWriter();
         $this->get[FileReader::class] = new FileReader();
+        $this->get[FileDeleter::class] = new FileDeleter();
         $this->get[AttachmentGateway::class] = new AttachmentGateway();
         $this->get[AttachmentHandler::class] = new AttachmentHandler($this->get[TicketGateway::class],
             $this->get[AttachmentGateway::class],
             $this->get[FileWriter::class],
-            $this->get[UserToTicketChecker::class]);
+            $this->get[UserToTicketChecker::class],
+            $this->get[FileDeleter::class]);
         $this->get[AttachmentRetriever::class] = new AttachmentRetriever($this->get[AttachmentGateway::class],
             $this->get[FileReader::class],
             $this->get[TicketGateway::class],

api/BusinessLogic/Attachments/AttachmentHandler.php

@@ -3,6 +3,7 @@
 namespace BusinessLogic\Attachments;
 
 
+use BusinessLogic\Exceptions\ApiFriendlyException;
 use BusinessLogic\Exceptions\ValidationException;
 use BusinessLogic\Security\UserContext;
 use BusinessLogic\Security\UserToTicketChecker;
@@ -10,6 +11,7 @@ use BusinessLogic\Tickets\Attachment;
 use BusinessLogic\Tickets\Ticket;
 use BusinessLogic\ValidationModel;
 use DataAccess\Attachments\AttachmentGateway;
+use DataAccess\Files\FileDeleter;
 use DataAccess\Files\FileWriter;
 use DataAccess\Tickets\TicketGateway;
 
@@ -23,14 +25,18 @@ class AttachmentHandler {
     /* @var $fileWriter FileWriter */
     private $fileWriter;
 
+    /* @var $fileDeleter FileDeleter */
+    private $fileDeleter;
+
     /* @var  $userToTicketChecker UserToTicketChecker */
     private $userToTicketChecker;
 
-    function __construct($ticketGateway, $attachmentGateway, $fileWriter, $userToTicketChecker) {
+    function __construct($ticketGateway, $attachmentGateway, $fileWriter, $userToTicketChecker, $fileDeleter) {
         $this->ticketGateway = $ticketGateway;
         $this->attachmentGateway = $attachmentGateway;
         $this->fileWriter = $fileWriter;
         $this->userToTicketChecker = $userToTicketChecker;
+        $this->fileDeleter = $fileDeleter;
     }
 
 
@@ -75,6 +81,31 @@ class AttachmentHandler {
         return $ticketAttachment;
     }
 
+    function deleteAttachmentFromTicket($ticketId, $attachmentId, $userContext, $heskSettings) {
+        $ticket = $this->ticketGateway->getTicketById($ticketId, $heskSettings);
+
+        if (!$this->userToTicketChecker->isTicketWritableToUser($userContext, $ticket, true, $heskSettings)) {
+            throw new \Exception("User does not have access to ticket {$ticketId} being created / edited!");
+        }
+
+        $indexToRemove = -1;
+        for ($i = 0; $i < count($ticket->attachments); $i++) {
+            $attachment = $ticket->attachments[$i];
+            if ($attachment->id === $attachmentId) {
+                $indexToRemove = $i;
+                $this->fileDeleter->deleteFile($attachment->savedName, $heskSettings['attach_dir']);
+            }
+        }
+
+        if ($indexToRemove === -1) {
+            throw new ApiFriendlyException("Attachment not found for ticket!", "Attachment not found", 404);
+        }
+
+        $attachments = $ticket->attachments;
+        unset($attachments[$indexToRemove]);
+        $this->ticketGateway->updateAttachmentsForTicket($ticketId, $attachments, $heskSettings);
+    }
+
     /**
      * @param $createAttachmentModel CreateAttachmentForTicketModel
      * @param $heskSettings array

api/BusinessLogic/Security/UserToTicketChecker.php

@@ -30,7 +30,9 @@ class UserToTicketChecker {
             $categoryManagerId = $this->userGateway->getManagerForCategory($ticket->categoryId, $heskSettings);
 
             $hasAccess = $hasAccess &&
-                (in_array(UserPrivilege::CAN_EDIT_TICKETS, $user->permissions) || $categoryManagerId == $user->id);
+                ($user->admin === true
+                    || in_array(UserPrivilege::CAN_EDIT_TICKETS, $user->permissions)
+                    || $categoryManagerId == $user->id);
         }
 
         return $hasAccess;

api/Controllers/Attachments/StaffTicketAttachmentsController.php

@@ -55,4 +55,15 @@ class StaffTicketAttachmentsController {
 
         return $model;
     }
+
+    function delete($ticketId, $attachmentId) {
+        global $applicationContext, $hesk_settings, $userContext;
+
+        /* @var $attachmentHandler AttachmentHandler */
+        $attachmentHandler = $applicationContext->get[AttachmentHandler::class];
+
+        $attachmentHandler->deleteAttachmentFromTicket($ticketId, $attachmentId, $userContext, $hesk_settings);
+
+        return http_response_code(204);
+    }
 }

api/DataAccess/Files/FileDeleter.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace DataAccess\Files;
+
+
+use BusinessLogic\Exceptions\ApiFriendlyException;
+
+class FileDeleter {
+    function deleteFile($name, $folder) {
+        $path = __DIR__ . "/../../../{$folder}/{$name}";
+        if (!file_exists($path)) {
+            return;
+        }
+
+        @unlink($path);
+    }
+}

api/Tests/BusinessLogic/Attachments/AttachmentHandlerTest.php

@@ -9,6 +9,7 @@ use BusinessLogic\Security\UserContext;
 use BusinessLogic\Security\UserToTicketChecker;
 use BusinessLogic\Tickets\Ticket;
 use DataAccess\Attachments\AttachmentGateway;
+use DataAccess\Files\FileDeleter;
 use DataAccess\Files\FileWriter;
 use DataAccess\Tickets\TicketGateway;
 use PHPUnit\Framework\TestCase;
@@ -33,6 +34,9 @@ class AttachmentHandlerTest extends TestCase {
     /* @var $fileWriter \PHPUnit_Framework_MockObject_MockObject */
     private $fileWriter;
 
+    /* @var $fileDeleter \PHPUnit_Framework_MockObject_MockObject */
+    private $fileDeleter;
+
     /* @var $userContext UserContext */
     private $userContext;
 
@@ -43,8 +47,8 @@ class AttachmentHandlerTest extends TestCase {
         $this->ticketGateway = $this->createMock(TicketGateway::class);
         $this->attachmentGateway = $this->createMock(AttachmentGateway::class);
         $this->fileWriter = $this->createMock(FileWriter::class);
+        $this->fileDeleter = $this->createMock(FileDeleter::class);
         $this->userToTicketChecker = $this->createMock(UserToTicketChecker::class);
-        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->heskSettings = array(
             'attach_dir' => 'attachments',
             'attachments' => array(
@@ -56,7 +60,8 @@ class AttachmentHandlerTest extends TestCase {
         $this->attachmentHandler = new AttachmentHandler($this->ticketGateway,
             $this->attachmentGateway,
             $this->fileWriter,
-            $this->userToTicketChecker);
+            $this->userToTicketChecker,
+            $this->fileDeleter);
         $this->createAttachmentForTicketModel = new CreateAttachmentForTicketModel();
         $this->createAttachmentForTicketModel->attachmentContents = base64_encode('string');
         $this->createAttachmentForTicketModel->displayName = 'DisplayName.txt';
@@ -67,6 +72,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsNull() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->attachmentContents = null;
 
         //-- Assert
@@ -79,6 +85,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsEmpty() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->attachmentContents = '';
 
         //-- Assert
@@ -91,6 +98,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheAttachmentBodyIsInvalidBase64() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->attachmentContents = 'invalid base 64';
 
         //-- Assert
@@ -103,6 +111,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsNull() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->displayName = null;
 
         //-- Assert
@@ -115,6 +124,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheDisplayNameIsEmpty() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->displayName = '';
 
         //-- Assert
@@ -127,6 +137,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheTicketIdIsNull() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->ticketId = null;
 
         //-- Assert
@@ -139,6 +150,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheTicketIdIsANonPositiveInteger() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->ticketId = 0;
 
         //-- Assert
@@ -151,6 +163,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheFileExtensionIsNotPermitted() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->heskSettings['attachments']['allowed_types'] = array('.gif');
         $this->createAttachmentForTicketModel->ticketId = 0;
 
@@ -164,6 +177,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testThatValidateThrowsAnExceptionWhenTheFileSizeIsLargerThanMaxPermitted() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->attachmentContents = base64_encode("msg");
         $this->heskSettings['attachments']['max_size'] = 1;
 
@@ -177,6 +191,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testItSavesATicketWithTheProperProperties() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->ticketId = 1;
         $ticket = new Ticket();
         $ticket->trackingId = 'ABC-DEF-1234';
@@ -205,6 +220,7 @@ class AttachmentHandlerTest extends TestCase {
 
     function testItSavesTheFileToTheFileSystem() {
         //-- Arrange
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
         $this->createAttachmentForTicketModel->ticketId = 1;
         $ticket = new Ticket();
         $ticket->trackingId = 'ABC-DEF-1234';
@@ -229,4 +245,60 @@ class AttachmentHandlerTest extends TestCase {
     }
 
     //-- TODO Test UserToTicketChecker
+
+    function testDeleteThrowsAnExceptionWhenTheUserDoesNotHaveAccessToTheTicket() {
+        //-- Arrange
+        $ticketId = 1;
+        $ticket = new Ticket();
+        $this->ticketGateway->method('getTicketById')
+            ->with($ticketId, $this->heskSettings)->willReturn($ticket);
+        $this->userToTicketChecker->method('isTicketWritableToUser')
+            ->with($this->userContext, $ticket, true, $this->heskSettings)
+            ->willReturn(false);
+
+        //-- Assert
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage("User does not have access to ticket {$ticketId} being created / edited!");
+
+        //-- Act
+        $this->attachmentHandler->deleteAttachmentFromTicket($ticketId, 1, $this->userContext, $this->heskSettings);
+    }
+
+    function testDeleteActuallyDeletesTheFile() {
+        //-- Arrange
+        $ticketId = 1;
+        $ticket = new Ticket();
+        $attachment = new Attachment();
+        $attachment->id = 5;
+        $attachment->savedName = 'foobar.txt';
+        $this->heskSettings['attach_dir'] = 'attach-dir';
+        $ticket->attachments = array($attachment);
+        $this->ticketGateway->method('getTicketById')->willReturn($ticket);
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
+
+        //-- Assert
+        $this->fileDeleter->expects($this->once())->method('deleteFile')->with('foobar.txt', 'attach-dir');
+
+        //-- Act
+        $this->attachmentHandler->deleteAttachmentFromTicket($ticketId, 5, $this->userContext, $this->heskSettings);
+    }
+
+    function testDeleteUpdatesTheTicketItselfAndSavesIt() {
+        //-- Arrange
+        $ticketId = 1;
+        $ticket = new Ticket();
+        $attachment = new Attachment();
+        $attachment->id = 5;
+        $attachment->savedName = 'foobar.txt';
+        $this->heskSettings['attach_dir'] = 'attach-dir';
+        $ticket->attachments = array($attachment);
+        $this->ticketGateway->method('getTicketById')->willReturn($ticket);
+        $this->userToTicketChecker->method('isTicketWritableToUser')->willReturn(true);
+
+        //-- Assert
+        $this->ticketGateway->expects($this->once())->method('updateAttachmentsForTicket');
+
+        //-- Act
+        $this->attachmentHandler->deleteAttachmentFromTicket($ticketId, 5, $this->userContext, $this->heskSettings);
+    }
 }