Merge pull request #90 from jaedb/41-allow-browser-pdfs

Default to allow PDFs to load in-browser

.travis.yml

@@ -23,6 +23,8 @@ matrix:
       env: DB=MYSQL CORE_RELEASE=3.3
     - php: 5.6
       env: DB=MYSQL CORE_RELEASE=3.4
+    - php: 5.6
+      env: DB=MYSQL CORE_RELEASE=3.5
 
 before_script:
   - composer self-update || true
@@ -32,4 +34,4 @@ before_script:
   - composer install
 
 script:
-  - vendor/bin/phpunit dms/tests  
+  - vendor/bin/phpunit dms/tests

README.md

@@ -90,6 +90,15 @@ Note: Both operations copy the existing file.
 	$docs = $dms->getByTag('priority', 'important')->First();
 	$link = $doc->getLink();
 
+	// Set default download behavior ('open' or 'download'). 'download' is the system default
+	// Attempt to open the file in the browser
+	Config::inst()->update('DMSDocument', 'default_download_behaviour', 'open');
+	
+Or in you config.yml:
+
+    DMSDocument:
+      default_download_behaviour: open
+
 #### Manage Page Relations
 
 	// Find documents by page

_config.php

@@ -1,7 +1,6 @@
 <?php
 
 $config = Config::inst();
-$config->update('DMSDocument_versions', 'enable_versions', true);
 
 DMSSiteTreeExtension::show_documents_tab(); //show the Documents tab on all pages
 DMSSiteTreeExtension::no_documents_tab();   //and don't exclude it from any pages
@@ -21,4 +20,3 @@ if ($config->get('DMSDocument_versions', 'enable_versions')) {
 	//using the same db relations for the versioned documents, as for the actual documents
 	$config->update('DMSDocument_versions', 'db', $config->get('DMSDocument', 'db'));
 }
-

_config/dmsdocument.yml

@@ -10,4 +10,6 @@ SiteTree:
     - DMSSiteTreeExtension
 HtmlEditorField_Toolbar:
   extensions:
-    - DocumentHtmlEditorFieldToolbar
+    - DocumentHtmlEditorFieldToolbar
+DMSDocument_versions:
+  enable_versions: true

code/model/DMSDocument.php

@@ -17,7 +17,8 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         "EmbargoedIndefinitely" => 'Boolean(false)',
         "EmbargoedUntilPublished" => 'Boolean(false)',
         "EmbargoedUntilDate" => 'SS_DateTime',
-        "ExpireAtDate" => 'SS_DateTime'
+        "ExpireAtDate" => 'SS_DateTime',
+        "DownloadBehavior" => 'Enum(array("open","download"), "download")',
     );
 
     private static $many_many = array(
@@ -52,6 +53,12 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         'LastChanged'
     );
 
+    /**
+     * @var string download|open
+     * @config
+     */
+    private static $default_download_behaviour = 'download';
+
     /**
      * @param Member $member
      *
@@ -147,7 +154,6 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
 
 
 
-
     /**
      * Associates this document with a Page. This method does nothing if the
      * association already exists.
@@ -902,7 +908,6 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         $fields = new FieldList();  //don't use the automatic scaffolding, it is slow and unnecessary here
 
         $extraTasks = '';   //additional text to inject into the list of tasks at the bottom of a DMSDocument CMSfield
-        $extraFields = FormField::create('Empty');
 
         //get list of shortcode page relations
         $relationFinder = new ShortCodeRelationFinder();
@@ -914,6 +919,28 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         $fields->add(new TextField('Title', 'Title'));
         $fields->add(new TextareaField('Description', 'Description'));
 
+        $downloadBehaviorSource = array(
+            'open' => _t('DMSDocument.OPENINBROWSER', 'Open in browser'),
+            'download' => _t('DMSDocument.FORCEDOWNLOAD', 'Force download'),
+        );
+        $defaultDownloadBehaviour = Config::inst()->get('DMSDocument', 'default_download_behaviour');
+        if (!isset($downloadBehaviorSource[$defaultDownloadBehaviour])) {
+            user_error('Default download behaviour "' . $defaultDownloadBehaviour . '" not supported.', E_USER_WARNING);
+        }
+        else {
+            $downloadBehaviorSource[$defaultDownloadBehaviour] .= ' (' . _t('DMSDocument.DEFAULT', 'default') . ')';
+        }
+
+        $fields->add(
+            OptionsetField::create(
+                'DownloadBehavior',
+                _t('DMSDocument.DOWNLOADBEHAVIOUR', 'Download behavior'),
+                $downloadBehaviorSource,
+                $defaultDownloadBehaviour
+            )
+            ->setDescription('How the visitor will view this file. <strong>Open in browser</strong> allows files to be opened in a new tab.')
+        );
+
         //create upload field to replace document
         $uploadField = new DMSUploadField('ReplaceFile', 'Replace file');
         $uploadField->setConfig('allowedMaxFileNumber', 1);
@@ -1251,6 +1278,7 @@ class DMSDocument_Controller extends Controller
     /**
      * Returns the document object from the request object's ID parameter.
      * Returns null, if no document found
+     * @return DMSDocument|null
      */
     protected function getDocumentFromID($request)
     {
@@ -1341,22 +1369,20 @@ class DMSDocument_Controller extends Controller
                         return $path;
                     }
 
+                    // set fallback if no config nor file-specific value
+                    $disposition = 'attachment';
+
+                    // file-specific setting
+                    if ($doc->DownloadBehavior == 'open') {
+                        $disposition = 'inline';
+                    }
+
                     //if a DMSDocument can be downloaded and all the permissions/privileges has passed,
                     //its ViewCount should be increased by 1 just before the browser sending the file to front.
                     $doc->trackView();
 
-                    header('Content-Type: ' . $mime);
+                    $this->sendFile($path, $mime, $doc->getFilenameWithoutID(), $disposition);
-                    header('Content-Length: ' . filesize($path), null);
+                    return;
-                    if (!empty($mime) && $mime != "text/html") {
-                        header('Content-Disposition: attachment; filename="'.$doc->getFilenameWithoutID().'"');
-                    }
-                    header('Content-transfer-encoding: 8bit');
-                    header('Expires: 0');
-                    header('Pragma: cache');
-                    header('Cache-Control: private');
-                    flush();
-                    readfile($path);
-                    exit;
                 }
             }
         }
@@ -1366,4 +1392,25 @@ class DMSDocument_Controller extends Controller
         }
         $this->httpError(404, 'This asset does not exist.');
     }
+
+    /**
+     * @param string $path File path
+     * @param string $mime File mime type
+     * @param string $name File name
+     * @param string $disposition Content dispositon
+     */
+    protected function sendFile($path, $mime, $name, $disposition) {
+        header('Content-Type: ' . $mime);
+        header('Content-Length: ' . filesize($path), null);
+        if (!empty($mime) && $mime != "text/html") {
+            header('Content-Disposition: '.$disposition.'; filename="'.addslashes($name).'"');
+        }
+        header('Content-transfer-encoding: 8bit');
+        header('Expires: 0');
+        header('Pragma: cache');
+        header('Cache-Control: private');
+        flush();
+        readfile($path);
+        exit;
+    }
 }

tests/DMSDocumentControllerTest.php

@@ -0,0 +1,54 @@
+<?php
+
+/**
+ * Class DMSDocumentControllerTest
+ */
+class DMSDocumentControllerTest extends SapphireTest {
+
+    protected static $fixture_file = "dmstest.yml";
+
+    public function testDownloadBehaviourOpen() {
+        DMS::$dmsFolder = DMS_DIR;    //sneakily setting the DMS folder to the folder where the test file lives
+
+        $this->logInWithPermission('ADMIN');
+        /** @var DMSDocument_Controller $controller */
+        $controller = $this->getMockBuilder('DMSDocument_Controller')
+            ->setMethods(array('sendFile'))->getMock();
+        $self = $this;
+        $controller->expects($this->once())->method('sendFile')->will($this->returnCallback(function($path, $mime, $name, $disposition) use($self) {
+            $self->assertEquals('inline', $disposition);
+        }));
+        $openDoc = new DMSDocument();
+        $openDoc->Filename = "DMS-test-lorum-file.pdf";
+        $openDoc->Folder = "tests";
+        $openDoc->DownloadBehavior = 'open';
+        $openDoc->clearEmbargo(false);
+        $openDoc->write();
+        $request = new SS_HTTPRequest('GET', 'index/' . $openDoc->ID);
+        $request->match('index/$ID');
+        $controller->index($request);
+    }
+
+    public function testDownloadBehaviourDownload() {
+        DMS::$dmsFolder = DMS_DIR;    //sneakily setting the DMS folder to the folder where the test file lives
+
+        $this->logInWithPermission('ADMIN');
+        /** @var DMSDocument_Controller $controller */
+        $controller = $this->getMockBuilder('DMSDocument_Controller')
+            ->setMethods(array('sendFile'))->getMock();
+        $self = $this;
+        $controller->expects($this->once())->method('sendFile')->will($this->returnCallback(function($path, $mime, $name, $disposition) use($self) {
+            $self->assertEquals('attachment', $disposition);
+        }));
+        $openDoc = new DMSDocument();
+        $openDoc->Filename = "DMS-test-lorum-file.pdf";
+        $openDoc->Folder = "tests";
+        $openDoc->DownloadBehavior = 'download';
+        $openDoc->clearEmbargo(false);
+        $openDoc->write();
+        $request = new SS_HTTPRequest('GET', 'index/' . $openDoc->ID);
+        $request->match('index/$ID');
+        $controller->index($request);
+    }
+
+}

tests/DMSDocumentTest.php

@@ -2,12 +2,12 @@
 class DMSDocumentTest extends SapphireTest
 {
 
-    public static $fixture_file = "dms/tests/dmstest.yml";
+    protected static $fixture_file = "dms/tests/dmstest.yml";
 
     public function tearDownOnce()
     {
         self::$is_running_test = true;
-        
+
         $d = DataObject::get("DMSDocument");
         foreach ($d as $d1) {
             $d1->delete();
@@ -157,4 +157,16 @@ class DMSDocumentTest extends SapphireTest
              ."associated with causes that document to be deleted as well"
         );
     }
+
+    public function testDefaultDownloadBehabiourCMSFields() {
+        $document = singleton('DMSDocument');
+        Config::inst()->update('DMSDocument', 'default_download_behaviour', 'open');
+        $cmsFields = $document->getCMSFields();
+        $this->assertEquals('open', $cmsFields->dataFieldByName('DownloadBehavior')->Value());
+
+
+        Config::inst()->update('DMSDocument', 'default_download_behaviour', 'download');
+        $cmsFields = $document->getCMSFields();
+        $this->assertEquals('download', $cmsFields->dataFieldByName('DownloadBehavior')->Value());
+    }
 }

tests/DMSEmbargoTest.php

@@ -7,7 +7,7 @@ class DMSEmbargoTest extends SapphireTest
     public function tearDownOnce()
     {
         self::$is_running_test = true;
-        
+
         $d = DataObject::get("DMSDocument");
         foreach ($d as $d1) {
             $d1->delete();
@@ -30,6 +30,7 @@ class DMSEmbargoTest extends SapphireTest
     public function testBasicEmbargo()
     {
         $oldDMSFolder = DMS::$dmsFolder;
+        $oldTestMode = DMSDocument_Controller::$testMode;
         DMS::$dmsFolder = DMS_DIR;    //sneakily setting the DMS folder to the folder where the test file lives
 
         $doc = new DMSDocument();
@@ -54,6 +55,7 @@ class DMSEmbargoTest extends SapphireTest
         $this->assertNotEquals($doc->getFullPath(), $result, "File no longer returned (in test mode) when switching to other user group");
 
         DMS::$dmsFolder = $oldDMSFolder;
+        DMSDocument_Controller::$testMode = $oldTestMode;
     }
 
     public function testEmbargoIndefinitely()

tests/dmstest.yml

@@ -46,4 +46,4 @@ DMSDocument:
     Filename: test-file-file-doesnt-exist
     Folder: 5
     Tags: =>DMSTag.t5, =>DMSTag.t6
-    Pages: =>SiteTree.s5, =>SiteTree.s6
+    Pages: =>SiteTree.s5, =>SiteTree.s6