Avoid code duplication in PR #2891

7 files changed, 60 insertions, 50 deletions

diff --git a/ChangeLog b/ChangeLog
index 1a42aab8..628a183c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -8,6 +8,7 @@ New features:
 
 Improvements:
 
+* Open PDF attachments in browser tab (preview)
 * Handle username with dots in user mentions
 * Replace Chosen jQuery plugin by custom UI component
 * Rewrite UI component that change user/group roles
diff --git a/app/Controller/FileViewerController.php b/app/Controller/FileViewerController.php
index 3e0182ba..49568912 100644
--- a/app/Controller/FileViewerController.php
+++ b/app/Controller/FileViewerController.php
@@ -15,11 +15,11 @@ class FileViewerController extends BaseController
     /**
      * Get file content from object storage
      *
-     * @access private
+     * @access protected
      * @param  array $file
      * @return string
      */
-    private function getFileContent(array $file)
+    protected function getFileContent(array $file)
     {
         $content = '';
 
@@ -35,6 +35,30 @@ class FileViewerController extends BaseController
     }
 
     /**
+     * Output file with cache
+     *
+     * @param array $file
+     * @param $mimetype
+     */
+    protected function renderFileWithCache(array $file, $mimetype)
+    {
+        $etag = md5($file['path']);
+
+        if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') {
+            $this->response->status(304);
+        } else {
+            try {
+                $this->response->withContentType($mimetype);
+                $this->response->withCache(5 * 86400, $etag);
+                $this->response->send();
+                $this->objectStorage->output($file['path']);
+            } catch (ObjectStorageException $e) {
+                $this->logger->error($e->getMessage());
+            }
+        }
+    }
+
+    /**
      * Show file content in a popover
      *
      * @access public
@@ -65,21 +89,7 @@ class FileViewerController extends BaseController
     public function image()
     {
         $file = $this->getFile();
-        $etag = md5($file['path']);
-        $this->response->withContentType($this->helper->file->getImageMimeType($file['name']));
-        $this->response->withCache(5 * 86400, $etag);
-
-        if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') {
-            $this->response->status(304);
-        } else {
-
-            try {
-                $this->response->send();
-                $this->objectStorage->output($file['path']);
-            } catch (ObjectStorageException $e) {
-                $this->logger->error($e->getMessage());
-            }
-        }
+        $this->renderFileWithCache($file, $this->helper->file->getImageMimeType($file['name']));
     }
 
     /**
@@ -90,21 +100,7 @@ class FileViewerController extends BaseController
     public function browser()
     {
         $file = $this->getFile();
-        $etag = md5($file['path']);
-        $this->response->withContentType($this->helper->file->getBrowserViewType($file['name']));
-        $this->response->withCache(5 * 86400, $etag);
-
-        if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') {
-            $this->response->status(304);
-        } else {
-
-            try {
-                $this->response->send();
-                $this->objectStorage->output($file['path']);
-            } catch (ObjectStorageException $e) {
-                $this->logger->error($e->getMessage());
-            }
-        }
+        $this->renderFileWithCache($file, $this->helper->file->getBrowserViewType($file['name']));
     }
 
     /**
diff --git a/app/Helper/FileHelper.php b/app/Helper/FileHelper.php
index 82b444d0..06589124 100644
--- a/app/Helper/FileHelper.php
+++ b/app/Helper/FileHelper.php
@@ -21,9 +21,7 @@ class FileHelper extends Base
      */
     public function icon($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
-
-        switch ($extension) {
+        switch (get_file_extension($filename)) {
             case 'jpeg':
             case 'jpg':
             case 'png':
@@ -70,9 +68,7 @@ class FileHelper extends Base
      */
     public function getImageMimeType($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
-
-        switch ($extension) {
+        switch (get_file_extension($filename)) {
             case 'jpeg':
             case 'jpg':
                 return 'image/jpeg';
@@ -94,9 +90,7 @@ class FileHelper extends Base
      */
     public function getPreviewType($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
-
-        switch ($extension) {
+        switch (get_file_extension($filename)) {
             case 'md':
             case 'markdown':
                 return 'markdown';
@@ -108,17 +102,15 @@ class FileHelper extends Base
     }
 
     /**
-     * Return the browser view mimetype based on the file extension.
+     * Return the browser view mime-type based on the file extension.
      *
+     * @access public
      * @param  $filename
-     *
      * @return string
      */
     public function getBrowserViewType($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
-
-        switch ($extension) {
+        switch (get_file_extension($filename)) {
             case 'pdf':
                 return 'application/pdf';
         }
diff --git a/app/Model/FileModel.php b/app/Model/FileModel.php
index 98032f9d..b5852b08 100644
--- a/app/Model/FileModel.php
+++ b/app/Model/FileModel.php
@@ -210,9 +210,7 @@ abstract class FileModel extends Base
      */
     public function isImage($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
-
-        switch ($extension) {
+        switch (get_file_extension($filename)) {
             case 'jpeg':
             case 'jpg':
             case 'png':
diff --git a/app/Template/task_file/files.php b/app/Template/task_file/files.php
index 94c26f73..32bebdcb 100644
--- a/app/Template/task_file/files.php
+++ b/app/Template/task_file/files.php
@@ -18,6 +18,11 @@
                                     <i class="fa fa-eye fa-fw"></i>
                                     <?= $this->url->link(t('View file'), 'FileViewerController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'file_id' => $file['id']), false, 'popover') ?>
                                 </li>
+                            <?php elseif ($this->file->getBrowserViewType($file['name']) !== null): ?>
+                                <li>
+                                    <i class="fa fa-eye fa-fw"></i>
+                                    <?= $this->url->link(t('View file'), 'FileViewerController', 'browser', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'file_id' => $file['id']), false, '', '', true) ?>
+                                </li>
                             <?php endif ?>
                             <li>
                                 <i class="fa fa-download fa-fw"></i>
diff --git a/app/functions.php b/app/functions.php
index 9dd054fb..7cd59c41 100644
--- a/app/functions.php
+++ b/app/functions.php
@@ -146,6 +146,17 @@ function get_upload_max_size()
 }
 
 /**
+ * Get file extension
+ *
+ * @param  $filename
+ * @return string
+ */
+function get_file_extension($filename)
+{
+    return strtolower(pathinfo($filename, PATHINFO_EXTENSION));
+}
+
+/**
  * Translate a string
  *
  * @return string
diff --git a/tests/units/Helper/FileHelperText.php b/tests/units/Helper/FileHelperTest.php
index 215b024b..7db43460 100644
--- a/tests/units/Helper/FileHelperText.php
+++ b/tests/units/Helper/FileHelperTest.php
@@ -30,7 +30,14 @@ class FileHelperTest extends Base
         $helper = new FileHelper($this->container);
         $this->assertEquals('text', $helper->getPreviewType('test.txt'));
         $this->assertEquals('markdown', $helper->getPreviewType('test.markdown'));
-        $this->assertEquals('md', $helper->getPreviewType('test.md'));
+        $this->assertEquals('markdown', $helper->getPreviewType('test.md'));
         $this->assertEquals(null, $helper->getPreviewType('test.doc'));
     }
+
+    public function testGetBrowserViewType()
+    {
+        $fileHelper = new FileHelper($this->container);
+        $this->assertSame('application/pdf', $fileHelper->getBrowserViewType('SomeFile.PDF'));
+        $this->assertSame(null, $fileHelper->getBrowserViewType('SomeFile.doc'));
+    }
 }