From 4aad4e728dd220ca2b2144c9aeeb92534a8ceeb5 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 8 Aug 2017 13:37:46 +1200 Subject: [PATCH] NEW Add ability to handle canonical URLs --- code/controllers/DocumentationViewer.php | 13 +++++++ code/models/DocumentationPage.php | 49 +++++++++++++++++++++++- templates/Includes/DocumentationHead.ss | 3 ++ tests/DocumentationPageTest.php | 38 ++++++++++++------ tests/DocumentationViewerTest.php | 48 +++++++++++++++-------- 5 files changed, 122 insertions(+), 29 deletions(-) diff --git a/code/controllers/DocumentationViewer.php b/code/controllers/DocumentationViewer.php index d438fa7..3a48735 100755 --- a/code/controllers/DocumentationViewer.php +++ b/code/controllers/DocumentationViewer.php @@ -568,6 +568,19 @@ class DocumentationViewer extends Controller implements PermissionProvider return $link; } + /** + * Return the canonical URL from the page + * + * @return string + */ + public function getCanonicalUrl() + { + if (!$this->getPage()) { + return ''; + } + return $this->getPage()->getCanonicalUrl(); + } + /** * Generate a list of all the pages in the documentation grouped by the * first letter of the page. diff --git a/code/models/DocumentationPage.php b/code/models/DocumentationPage.php index 26019c6..1506f9e 100755 --- a/code/models/DocumentationPage.php +++ b/code/models/DocumentationPage.php @@ -38,6 +38,11 @@ class DocumentationPage extends ViewableData protected $read = false; + /** + * @var string + */ + protected $canonicalUrl; + /** * @param DocumentationEntity $entity * @param string $filename @@ -260,10 +265,25 @@ class DocumentationPage extends ViewableData Controller::join_links( $this->entity->Link($short), $this->getRelativeLink() - ), '/' + ), + '/' ); } + /** + * Determine and set the canonical URL for the given record, for example: dev/docs/en/Path/To/Document + */ + public function populateCanonicalUrl() + { + $url = Director::absoluteURL(Controller::join_links( + Config::inst()->get('DocumentationViewer', 'link_base'), + $this->getEntity()->getLanguage(), + $this->getRelativeLink() + )); + + $this->setCanonicalUrl($url); + } + /** * Return metadata from the first html block in the page, then remove the * block on request @@ -349,5 +369,30 @@ class DocumentationPage extends ViewableData { return sprintf(get_class($this) .': %s)', $this->getPath()); } -} + /** + * Set the canonical URL to use for this page + * + * @param string $canonicalUrl + * @return $this + */ + public function setCanonicalUrl($canonicalUrl) + { + $this->canonicalUrl = $canonicalUrl; + return $this; + } + + /** + * Get the canonical URL to use for this page. Will trigger discovery + * via {@link DocumentationPage::populateCanonicalUrl()} if none is already set. + * + * @return string + */ + public function getCanonicalUrl() + { + if (!$this->canonicalUrl) { + $this->populateCanonicalUrl(); + } + return $this->canonicalUrl; + } +} diff --git a/templates/Includes/DocumentationHead.ss b/templates/Includes/DocumentationHead.ss index 11d12d1..ac33f84 100644 --- a/templates/Includes/DocumentationHead.ss +++ b/templates/Includes/DocumentationHead.ss @@ -2,6 +2,9 @@ <% base_tag %> + <% if $CanonicalUrl %> + + <% end_if %> <% if Title %>$Title – <% end_if %>$DocumentationTitle diff --git a/tests/DocumentationPageTest.php b/tests/DocumentationPageTest.php index 331b4d3..7f9349d 100755 --- a/tests/DocumentationPageTest.php +++ b/tests/DocumentationPageTest.php @@ -19,10 +19,8 @@ class DocumentationPageTest extends SapphireTest Config::nest(); - // explicitly use dev/docs. Custom paths should be tested separately - Config::inst()->update( - 'DocumentationViewer', 'link_base', 'dev/docs/' - ); + // explicitly use dev/docs. Custom paths should be tested separately + Config::inst()->update('DocumentationViewer', 'link_base', 'dev/docs/'); $manifest = new DocumentationManifest(true); } @@ -41,7 +39,7 @@ class DocumentationPageTest extends SapphireTest 'test.md', DOCSVIEWER_PATH . '/tests/docs/en/test.md' ); - + // single layer $this->assertEquals( 'dev/docs/en/doctest/2.4/test/', $page->Link(), @@ -53,18 +51,18 @@ class DocumentationPageTest extends SapphireTest 'sort', DOCSVIEWER_PATH . '/tests/docs/en/sort/' ); - + $this->assertEquals('dev/docs/en/doctest/2.4/sort/', $page->Link()); - + $page = new DocumentationFolder( $this->entity, '1-basic.md', DOCSVIEWER_PATH . '/tests/docs/en/sort/1-basic.md' ); - + $this->assertEquals('dev/docs/en/doctest/2.4/sort/basic/', $page->Link()); } - + public function testGetBreadcrumbTitle() { $page = new DocumentationPage( @@ -74,13 +72,13 @@ class DocumentationPageTest extends SapphireTest ); $this->assertEquals("Test - Doctest", $page->getBreadcrumbTitle()); - + $page = new DocumentationFolder( $this->entity, '1-basic.md', DOCSVIEWER_PATH . '/tests/docs/en/sort/1-basic.md' ); - + $this->assertEquals('Basic - Sort - Doctest', $page->getBreadcrumbTitle()); $page = new DocumentationFolder( @@ -91,4 +89,22 @@ class DocumentationPageTest extends SapphireTest $this->assertEquals('Sort - Doctest', $page->getBreadcrumbTitle()); } + + public function testGetCanonicalUrl() + { + $page = new DocumentationPage( + $this->entity, + 'file.md', + DOCSVIEWER_PATH . '/tests/docs/en/test/file.md' + ); + + $this->assertContains( + 'dev/docs/en/test/file/', + $page->getCanonicalUrl(), + 'Canonical URL is determined, set and returned' + ); + + $page->setCanonicalUrl('some-other-url'); + $this->assertSame('some-other-url', $page->getCanonicalUrl(), 'Canonical URL can be adjusted via public API'); + } } diff --git a/tests/DocumentationViewerTest.php b/tests/DocumentationViewerTest.php index ebde27f..bc18fad 100755 --- a/tests/DocumentationViewerTest.php +++ b/tests/DocumentationViewerTest.php @@ -11,7 +11,7 @@ class DocumentationViewerTest extends FunctionalTest { protected $autoFollowRedirection = false; - + protected $manifest; public function setUp() @@ -20,7 +20,7 @@ class DocumentationViewerTest extends FunctionalTest Config::nest(); - // explicitly use dev/docs. Custom paths should be tested separately + // explicitly use dev/docs. Custom paths should be tested separately Config::inst()->update( 'DocumentationViewer', 'link_base', 'dev/docs/' ); @@ -65,11 +65,11 @@ class DocumentationViewerTest extends FunctionalTest $this->manifest = new DocumentationManifest(true); } - + public function tearDown() { parent::tearDown(); - + Config::unnest(); } @@ -98,47 +98,47 @@ class DocumentationViewerTest extends FunctionalTest $response = $this->get('dev/docs/en/doc_test/3.0/empty.md'); $this->assertEquals(301, $response->getStatusCode(), 'Direct markdown links also work. They should redirect to /empty/'); - + // 2.4 is the stable release. Not in the URL $response = $this->get('dev/docs/en/doc_test/2.4'); $this->assertEquals($response->getStatusCode(), 200, 'Existing base folder'); $this->assertContains('english test', $response->getBody(), 'Toplevel content page'); - + // accessing base redirects to the version with the version number. $response = $this->get('dev/docs/en/doc_test/'); $this->assertEquals($response->getStatusCode(), 301, 'Existing base folder redirects to with version'); $response = $this->get('dev/docs/en/doc_test/3.0/'); $this->assertEquals($response->getStatusCode(), 200, 'Existing base folder'); - + $response = $this->get('dev/docs/en/doc_test/2.3/nonexistant-subfolder'); $this->assertEquals($response->getStatusCode(), 404, 'Nonexistant subfolder'); - + $response = $this->get('dev/docs/en/doc_test/2.3/nonexistant-file.txt'); $this->assertEquals($response->getStatusCode(), 301, 'Nonexistant file'); $response = $this->get('dev/docs/en/doc_test/2.3/nonexistant-file/'); $this->assertEquals($response->getStatusCode(), 404, 'Nonexistant file'); - + $response = $this->get('dev/docs/en/doc_test/2.3/test'); $this->assertEquals($response->getStatusCode(), 200, 'Existing file'); - + $response = $this->get('dev/docs/en/doc_test/3.0/empty?foo'); $this->assertEquals(200, $response->getStatusCode(), 'Existing page'); - + $response = $this->get('dev/docs/en/doc_test/3.0/empty/'); $this->assertEquals($response->getStatusCode(), 200, 'Existing page'); - + $response = $this->get('dev/docs/en/doc_test/3.0/test'); $this->assertEquals($response->getStatusCode(), 404, 'Missing page'); - + $response = $this->get('dev/docs/en/doc_test/3.0/test.md'); $this->assertEquals($response->getStatusCode(), 301, 'Missing page'); - + $response = $this->get('dev/docs/en/doc_test/3.0/test/'); $this->assertEquals($response->getStatusCode(), 404, 'Missing page'); - + $response = $this->get('dev/docs/dk/'); $this->assertEquals($response->getStatusCode(), 404, 'Access a language that doesn\'t exist'); } @@ -188,7 +188,7 @@ class DocumentationViewerTest extends FunctionalTest $response = $v->handleRequest(new SS_HTTPRequest('GET', 'en/doc_test/2.3/subfolder/subsubfolder/subsubpage/'), DataModel::inst()); $this->assertEquals('en', $v->getLanguage()); } - + public function testAccessingAll() { @@ -230,4 +230,20 @@ class DocumentationViewerTest extends FunctionalTest // redirect should have been to the absolute url minus the .md extension $this->assertEquals(Director::absoluteURL('dev/docs/en/doc_test/3.0/tutorials/'), $response->getHeader('Location')); } + + public function testCanonicalUrlIsIncludedInLayout() + { + $response = $this->get('dev/docs/en/doc_test/2.3/subfolder/subsubfolder/subsubpage'); + + $this->assertEquals(200, $response->getStatusCode()); + + $expectedUrl = Director::absoluteURL('dev/docs/en/subfolder/subsubfolder/subsubpage/'); + $this->assertContains('', (string) $response->getBody()); + } + + public function testCanonicalUrlIsEmptyWhenNoPageExists() + { + $viewer = new DocumentationViewer; + $this->assertSame('', $viewer->getCanonicalUrl()); + } }