diff --git a/code/controllers/DocumentationViewer.php b/code/controllers/DocumentationViewer.php index 53b51a2..5161616 100755 --- a/code/controllers/DocumentationViewer.php +++ b/code/controllers/DocumentationViewer.php @@ -588,10 +588,26 @@ class DocumentationViewer extends Controller implements PermissionProvider */ public function getCanonicalUrl() { - if (!$this->getPage()) { + $page = $this->getPage(); + + if (!$page) { return ''; } - return $this->getPage()->getCanonicalUrl(); + + // Build the basic canoncial URL, relative to link_base as is mapped in the manifest + $url = Controller::join_links( + $page->getEntity()->getLanguage(), + $page->getRelativeLink() + ); + + // If the canoncial URL is, in fact, a redirect, post that directly - redirecting canoncials are a bad thing. + // See https://github.com/silverstripe/doc.silverstripe.org/issues/181 for discussion + if ($potentialRedirect = $this->manifest->getRedirect($url)) { + $url = $potentialRedirect; + } + + // Turn into an absolute URL + return Director::absoluteUrl(Controller::join_links(self::getDocumentationBaseHref(), $url)); } /** diff --git a/code/models/DocumentationPage.php b/code/models/DocumentationPage.php index 2537d89..49b0623 100755 --- a/code/models/DocumentationPage.php +++ b/code/models/DocumentationPage.php @@ -376,6 +376,8 @@ class DocumentationPage extends ViewableData */ public function setCanonicalUrl($canonicalUrl) { + user_error("DocumentationPage::setCanonicalUrl() is deprecated; it won't affect DocumentationViewer", E_USER_WARNING); + $this->canonicalUrl = $canonicalUrl; return $this; } @@ -388,6 +390,11 @@ class DocumentationPage extends ViewableData */ public function getCanonicalUrl() { + user_error( + "DocumentationPage::getCanonicalUrl() is deprecated; use DocumentationViewer::getCanonicalUrl() instead.", + E_USER_WARNING + ); + if (!$this->canonicalUrl) { $this->populateCanonicalUrl(); } diff --git a/tests/DocumentationPageTest.php b/tests/DocumentationPageTest.php index e4d90c4..dff072d 100755 --- a/tests/DocumentationPageTest.php +++ b/tests/DocumentationPageTest.php @@ -90,22 +90,4 @@ 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'); - } }