From 98b79adca72dfd095617a0e7666fcee4fec8405b Mon Sep 17 00:00:00 2001 From: James Barnsley Date: Thu, 24 Nov 2016 16:28:25 +1300 Subject: [PATCH 1/4] Optional field that allows users to force PDF downloads, otherwise allow browsers to open in new tab --- README.md | 9 ++++++++ _config.php | 2 -- _config/dmsdocument.yml | 4 +++- code/model/DMSDocument.php | 43 ++++++++++++++++++++++++++++++++++---- 4 files changed, 51 insertions(+), 7 deletions(-) mode change 100644 => 100755 README.md mode change 100644 => 100755 _config.php mode change 100644 => 100755 _config/dmsdocument.yml mode change 100644 => 100755 code/model/DMSDocument.php diff --git a/README.md b/README.md old mode 100644 new mode 100755 index b8d2a5a..1847344 --- a/README.md +++ b/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 diff --git a/_config.php b/_config.php old mode 100644 new mode 100755 index a1eaa70..b56df74 --- a/_config.php +++ b/_config.php @@ -1,7 +1,6 @@ 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')); } - diff --git a/_config/dmsdocument.yml b/_config/dmsdocument.yml old mode 100644 new mode 100755 index 47acaf0..9aa977f --- a/_config/dmsdocument.yml +++ b/_config/dmsdocument.yml @@ -10,4 +10,6 @@ SiteTree: - DMSSiteTreeExtension HtmlEditorField_Toolbar: extensions: - - DocumentHtmlEditorFieldToolbar \ No newline at end of file + - DocumentHtmlEditorFieldToolbar +DMSDocument_versions: + enable_versions: true diff --git a/code/model/DMSDocument.php b/code/model/DMSDocument.php old mode 100644 new mode 100755 index f4a3989..37e757d --- a/code/model/DMSDocument.php +++ b/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. Open in browser 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); @@ -1341,6 +1368,14 @@ 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(); @@ -1348,7 +1383,7 @@ class DMSDocument_Controller extends Controller header('Content-Type: ' . $mime); header('Content-Length: ' . filesize($path), null); if (!empty($mime) && $mime != "text/html") { - header('Content-Disposition: attachment; filename="'.$doc->getFilenameWithoutID().'"'); + header('Content-Disposition: '.$disposition.'; filename="'.$doc->getFilenameWithoutID().'"'); } header('Content-transfer-encoding: 8bit'); header('Expires: 0'); From 41533deb41ecdd5f44a9a5ce60012bf8e323a925 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 20 Dec 2016 11:02:11 +0000 Subject: [PATCH 2/4] Reverting file permission changes --- README.md | 0 _config.php | 0 _config/dmsdocument.yml | 0 code/model/DMSDocument.php | 0 4 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 README.md mode change 100755 => 100644 _config.php mode change 100755 => 100644 _config/dmsdocument.yml mode change 100755 => 100644 code/model/DMSDocument.php diff --git a/README.md b/README.md old mode 100755 new mode 100644 diff --git a/_config.php b/_config.php old mode 100755 new mode 100644 diff --git a/_config/dmsdocument.yml b/_config/dmsdocument.yml old mode 100755 new mode 100644 diff --git a/code/model/DMSDocument.php b/code/model/DMSDocument.php old mode 100755 new mode 100644 From f7f5f115ee055472260591ea904e4ee30ffce5a9 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 20 Dec 2016 14:49:23 +0000 Subject: [PATCH 3/4] Adding 3.5 to test suite --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d3b62f2..3db9042 100644 --- a/.travis.yml +++ b/.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 From 97ec75df59d8cf69768cc38a3aafaa223055caa7 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 20 Dec 2016 11:57:19 +0000 Subject: [PATCH 4/4] Added test --- code/model/DMSDocument.php | 36 ++++++++++++------- tests/DMSDocumentControllerTest.php | 54 +++++++++++++++++++++++++++++ tests/DMSDocumentTest.php | 16 +++++++-- tests/DMSEmbargoTest.php | 4 ++- tests/dmstest.yml | 2 +- 5 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 tests/DMSDocumentControllerTest.php diff --git a/code/model/DMSDocument.php b/code/model/DMSDocument.php index 37e757d..b47e573 100644 --- a/code/model/DMSDocument.php +++ b/code/model/DMSDocument.php @@ -1278,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) { @@ -1380,18 +1381,8 @@ class DMSDocument_Controller extends Controller //its ViewCount should be increased by 1 just before the browser sending the file to front. $doc->trackView(); - header('Content-Type: ' . $mime); - header('Content-Length: ' . filesize($path), null); - if (!empty($mime) && $mime != "text/html") { - header('Content-Disposition: '.$disposition.'; filename="'.$doc->getFilenameWithoutID().'"'); - } - header('Content-transfer-encoding: 8bit'); - header('Expires: 0'); - header('Pragma: cache'); - header('Cache-Control: private'); - flush(); - readfile($path); - exit; + $this->sendFile($path, $mime, $doc->getFilenameWithoutID(), $disposition); + return; } } } @@ -1401,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; + } } diff --git a/tests/DMSDocumentControllerTest.php b/tests/DMSDocumentControllerTest.php new file mode 100644 index 0000000..5365bed --- /dev/null +++ b/tests/DMSDocumentControllerTest.php @@ -0,0 +1,54 @@ +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); + } + +} diff --git a/tests/DMSDocumentTest.php b/tests/DMSDocumentTest.php index 3ba43e7..f8f8f33 100644 --- a/tests/DMSDocumentTest.php +++ b/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()); + } } diff --git a/tests/DMSEmbargoTest.php b/tests/DMSEmbargoTest.php index 4eac0aa..2c30c1d 100644 --- a/tests/DMSEmbargoTest.php +++ b/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() diff --git a/tests/dmstest.yml b/tests/dmstest.yml index d476c2d..e727c01 100644 --- a/tests/dmstest.yml +++ b/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 \ No newline at end of file + Pages: =>SiteTree.s5, =>SiteTree.s6