Avoid code duplication in PR #2891

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

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 = '';
 
@@ -34,6 +34,30 @@ class FileViewerController extends BaseController
         return $content;
     }
 
+    /**
+     * 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
      *
@@ -65,21 +89,7 @@ class FileViewerController extends BaseController
     public function image()
     {
         $file = $this->getFile();
-        $etag = md5($file['path']);
+        $this->renderFileWithCache($file, $this->helper->file->getImageMimeType($file['name']));
-        $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());
-            }
-        }
     }
 
     /**
@@ -90,21 +100,7 @@ class FileViewerController extends BaseController
     public function browser()
     {
         $file = $this->getFile();
-        $etag = md5($file['path']);
+        $this->renderFileWithCache($file, $this->helper->file->getBrowserViewType($file['name']));
-        $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());
-            }
-        }
     }
 
     /**

app/Helper/FileHelper.php

@@ -21,9 +21,7 @@ class FileHelper extends Base
      */
     public function icon($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
+        switch (get_file_extension($filename)) {
-
-        switch ($extension) {
             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 (get_file_extension($filename)) {
-
-        switch ($extension) {
             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 (get_file_extension($filename)) {
-
-        switch ($extension) {
             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 (get_file_extension($filename)) {
-
-        switch ($extension) {
             case 'pdf':
                 return 'application/pdf';
         }

app/Model/FileModel.php

@@ -210,9 +210,7 @@ abstract class FileModel extends Base
      */
     public function isImage($filename)
     {
-        $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
+        switch (get_file_extension($filename)) {
-
-        switch ($extension) {
             case 'jpeg':
             case 'jpg':
             case 'png':

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>

app/functions.php

@@ -145,6 +145,17 @@ function get_upload_max_size()
     return min(ini_get('upload_max_filesize'), ini_get('post_max_size'));
 }
 
+/**
+ * Get file extension
+ *
+ * @param  $filename
+ * @return string
+ */
+function get_file_extension($filename)
+{
+    return strtolower(pathinfo($filename, PATHINFO_EXTENSION));
+}
+
 /**
  * Translate a string
  *

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'));
+    }
 }