From 80e36c3350145e9060e24eceedc445f2a5aee9c6 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 24 May 2017 11:43:23 +1200 Subject: [PATCH] NEW Add friendly URL segments for DMS documents --- code/model/DMSDocument.php | 13 +++++---- code/model/DMSDocument_Controller.php | 23 ++++++++++++--- tests/DMSDocumentControllerTest.php | 41 ++++++++++++++++++--------- tests/DMSDocumentTest.php | 14 +++++++++ tests/DMSShortcodeTest.php | 5 +++- 5 files changed, 71 insertions(+), 25 deletions(-) diff --git a/code/model/DMSDocument.php b/code/model/DMSDocument.php index d4e503b..445ced3 100644 --- a/code/model/DMSDocument.php +++ b/code/model/DMSDocument.php @@ -260,7 +260,8 @@ class DMSDocument extends DataObject implements DMSDocumentInterface */ public function getLink() { - $result = Controller::join_links(Director::baseURL(), 'dmsdocument/' . $this->ID); + $urlSegment = sprintf('%d-%s', $this->ID, URLSegmentFilter::create()->filter($this->getTitle())); + $result = Controller::join_links(Director::baseURL(), 'dmsdocument/' . $urlSegment); if (!$this->canView()) { $result = sprintf("javascript:alert('%s')", $this->getPermissionDeniedReason()); } @@ -787,12 +788,12 @@ class DMSDocument extends DataObject implements DMSDocumentInterface $gridFieldConfig->getComponentByType('GridFieldDataColumns') ->setDisplayFields(array( - 'Title'=>'Title', - 'ClassName'=>'Page Type', - 'ID'=>'Page ID' + 'Title' => 'Title', + 'ClassName' => 'Page Type', + 'ID' => 'Page ID' )) ->setFieldFormatting(array( - 'Title'=>sprintf( + 'Title' => sprintf( '$Title', singleton('CMSPageEditController')->Link('show') ) @@ -823,7 +824,7 @@ class DMSDocument extends DataObject implements DMSDocumentInterface ->setDisplayFields(Config::inst()->get('DMSDocument_versions', 'display_fields')) ->setFieldFormatting( array( - 'FilenameWithoutID' => '' + 'FilenameWithoutID' => '' . '$FilenameWithoutID' ) ); diff --git a/code/model/DMSDocument_Controller.php b/code/model/DMSDocument_Controller.php index 7fa4f0b..6aa4912 100644 --- a/code/model/DMSDocument_Controller.php +++ b/code/model/DMSDocument_Controller.php @@ -31,21 +31,37 @@ class DMSDocument_Controller extends Controller $doc = null; $id = Convert::raw2sql($request->param('ID')); - if (strpos($id, 'version') === 0) { // Versioned document - $id = str_replace('version', '', $id); + $id = $this->getDocumentIdFromSlug(str_replace('version', '', $id)); $doc = DataObject::get_by_id('DMSDocument_versions', $id); $this->extend('updateVersionFromID', $doc, $request); } else { // Normal document - $doc = DataObject::get_by_id('DMSDocument', $id); + $doc = DataObject::get_by_id('DMSDocument', $this->getDocumentIdFromSlug($id)); $this->extend('updateDocumentFromID', $doc, $request); } return $doc; } + /** + * Get a document's ID from a "friendly" URL slug containing a numeric ID and slugged title + * + * @param string $slug + * @return int + * @throws InvalidArgumentException if an invalid format is provided + */ + protected function getDocumentIdFromSlug($slug) + { + $parts = (array) sscanf($slug, '%d-%s'); + $id = array_shift($parts); + if (is_numeric($id)) { + return (int) $id; + } + throw new InvalidArgumentException($slug . ' is not a valid DMSDocument URL'); + } + /** * Access the file download without redirecting user, so we can block direct * access to documents. @@ -54,7 +70,6 @@ class DMSDocument_Controller extends Controller { $doc = $this->getDocumentFromID($request); - if (!empty($doc)) { $canView = $doc->canView(); diff --git a/tests/DMSDocumentControllerTest.php b/tests/DMSDocumentControllerTest.php index 82440ea..1f4bb97 100644 --- a/tests/DMSDocumentControllerTest.php +++ b/tests/DMSDocumentControllerTest.php @@ -7,6 +7,29 @@ class DMSDocumentControllerTest extends SapphireTest { protected static $fixture_file = 'dmstest.yml'; + /** + * @var DMSDocument_Controller + */ + protected $controller; + + public function setUp() + { + parent::setUp(); + + Config::inst()->update('DMS', 'folder_name', 'assets/_unit-test-123'); + $this->logInWithPermission('ADMIN'); + + $this->controller = $this->getMockBuilder('DMSDocument_Controller') + ->setMethods(array('sendFile')) + ->getMock(); + } + + public function tearDown() + { + DMSFilesystemTestHelper::delete('assets/_unit-test-123'); + parent::tearDown(); + } + /** * Test that the download behaviour is either "open" or "download" * @@ -16,16 +39,8 @@ class DMSDocumentControllerTest extends SapphireTest */ public function testDownloadBehaviourOpen($behaviour, $expectedDisposition) { - Config::inst()->update('DMS', 'folder_name', 'assets/_unit-test-123'); - - $this->logInWithPermission('ADMIN'); - - /** @var DMSDocument_Controller $controller */ - $controller = $this->getMockBuilder('DMSDocument_Controller') - ->setMethods(array('sendFile'))->getMock(); - $self = $this; - $controller->expects($this->once()) + $this->controller->expects($this->once()) ->method('sendFile') ->will( $this->returnCallback(function ($path, $mime, $name, $disposition) use ($self, $expectedDisposition) { @@ -38,11 +53,9 @@ class DMSDocumentControllerTest extends SapphireTest $openDoc->clearEmbargo(false); $openDoc->write(); - $request = new SS_HTTPRequest('GET', 'index/' . $openDoc->ID); - $request->match('index/$ID'); - $controller->index($request); - - DMSFilesystemTestHelper::delete('assets/_unit-test-123'); + $request = new SS_HTTPRequest('GET', $openDoc->Link()); + $request->match('dmsdocument/$ID'); + $this->controller->index($request); } /** diff --git a/tests/DMSDocumentTest.php b/tests/DMSDocumentTest.php index 53be3d7..0e24e09 100644 --- a/tests/DMSDocumentTest.php +++ b/tests/DMSDocumentTest.php @@ -264,6 +264,20 @@ class DMSDocumentTest extends SapphireTest DMSFilesystemTestHelper::delete('assets/_unit-tests'); } + /** + * Test that the link contains an ID and URL slug + */ + public function testGetLink() + { + Config::inst()->update('DMS', 'folder_name', 'assets/_unit-tests'); + + $document = DMS::inst()->storeDocument('dms/tests/DMS-test-lorum-file.pdf'); + + $expected = '/dmsdocument/' . $document->ID . '-dms-test-lorum-file-pdf'; + $this->assertSame($expected, $document->Link()); + $this->assertSame($expected, $document->getLink()); + } + /** * Ensure that the description can be returned in HTML format */ diff --git a/tests/DMSShortcodeTest.php b/tests/DMSShortcodeTest.php index 1564401..ee7c56b 100644 --- a/tests/DMSShortcodeTest.php +++ b/tests/DMSShortcodeTest.php @@ -22,7 +22,10 @@ class DMSShortcodeTest extends SapphireTest $value = Injector::inst()->create('HTMLValue', $result); $link = $value->query('//a')->item(0); - $this->assertStringEndsWith("/dmsdocument/$document->ID", $link->getAttribute('href')); + $this->assertStringEndsWith( + '/dmsdocument/' . $document->ID . '-dms-test-lorum-file-pdf', + $link->getAttribute('href') + ); $this->assertEquals($document->getExtension(), $link->getAttribute('data-ext')); $this->assertEquals($document->getFileSizeFormatted(), $link->getAttribute('data-size'));