From 4830e09bad340e30192032d6e20ff94ffb3cba08 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Fri, 1 Jul 2011 14:37:55 +1200 Subject: [PATCH] ENHANCEMENT: moved 404 page out to a separate template and altered DocumentationViewer to throw 404s on pages which do not resolve to anything. Fixes: #6616 --- code/DocumentationService.php | 15 ++- code/controllers/DocumentationViewer.php | 118 ++++++++++-------- templates/DocFolderListing.ss | 16 +-- templates/Includes/DocNotFound.ss | 24 ---- templates/Layout/DocumentationViewer.ss | 6 +- templates/Layout/DocumentationViewer_error.ss | 13 ++ tests/DocumentationViewerTest.php | 10 +- 7 files changed, 109 insertions(+), 93 deletions(-) delete mode 100755 templates/Includes/DocNotFound.ss create mode 100644 templates/Layout/DocumentationViewer_error.ss diff --git a/code/DocumentationService.php b/code/DocumentationService.php index 718c40a..6badd78 100755 --- a/code/DocumentationService.php +++ b/code/DocumentationService.php @@ -335,9 +335,12 @@ class DocumentationService { if($modules) { foreach($modules as $key => $module) { - if(is_dir(BASE_PATH .'/'. $module) && !in_array($module, self::get_ignored_files(), true)) { + $dir = is_dir(Controller::join_links(BASE_PATH, $module)); + $ignored = in_array($module, self::get_ignored_files(), true); + + if($dir && !$ignored) { // check to see if it has docs - $docs = BASE_PATH .'/'. $module .'/docs/'; + $docs = Controller::join_links($dir, 'docs'); if(is_dir($docs)) { self::register($module, $docs, '', $module, true); @@ -355,7 +358,13 @@ class DocumentationService { * @param String $code code */ public static function get_language_title($lang) { - return (isset(self::$language_mapping[$lang])) ? _t("DOCUMENTATIONSERVICE.LANG-$lang", self::$language_mapping[$lang]) : $lang; + $map = self::$language_mapping; + + if(isset($map[$lang])) { + return _t("DOCUMENTATIONSERVICE.LANG-$lang", $map[$lang]); + } + + return $lang; } diff --git a/code/controllers/DocumentationViewer.php b/code/controllers/DocumentationViewer.php index 6b81bb3..b124c21 100755 --- a/code/controllers/DocumentationViewer.php +++ b/code/controllers/DocumentationViewer.php @@ -48,7 +48,7 @@ class DocumentationViewer extends Controller { protected static $link_base = 'dev/docs/'; /** - * @var String|array Optional permssion check + * @var String|array Optional permission check */ static $check_permission = 'ADMIN'; @@ -59,12 +59,8 @@ class DocumentationViewer extends Controller { // javascript Requirements::javascript(THIRDPARTY_DIR .'/jquery/jquery.js'); - Requirements::javascript('sapphiredocs/thirdparty/syntaxhighlighter/scripts/shCore.js'); - Requirements::javascript('sapphiredocs/thirdparty/syntaxhighlighter/scripts/shBrushJScript.js'); - Requirements::javascript('sapphiredocs/thirdparty/syntaxhighlighter/scripts/shBrushPhp.js'); - Requirements::javascript('sapphiredocs/thirdparty/syntaxhighlighter/scripts/shBrushXml.js'); - Requirements::javascript('sapphiredocs/thirdparty/syntaxhighlighter/scripts/shBrushCss.js'); Requirements::javascript('sapphiredocs/javascript/shBrushSS.js'); + Requirements::combine_files( 'syntaxhighlighter.js', array( @@ -97,11 +93,11 @@ class DocumentationViewer extends Controller { } /** - * Overloaded to avoid "action doesnt exist" errors - all URL parts in this + * Overloaded to avoid "action doesn't exist" errors - all URL parts in this * controller are virtual and handled through handleRequest(), not controller methods. */ public function handleAction($request) { - try{ + try { $response = parent::handleAction($request); } catch(SS_HTTPResponse_Exception $e) { if(strpos($e->getMessage(), 'does not exist') !== FALSE) { @@ -141,6 +137,7 @@ class DocumentationViewer extends Controller { // allow assets if($firstParam == "assets") return parent::handleRequest($request); + // check for permalinks if($link = DocumentationPermalinks::map($firstParam)) { // the first param is a shortcode for a page so redirect the user to // the short code. @@ -151,8 +148,14 @@ class DocumentationViewer extends Controller { } + // check to see if the module is a valid module. If it isn't, then we + // need to throw a 404. + if(!DocumentationService::is_registered_module($firstParam)) { + return $this->throw404(); + } + $this->module = $firstParam; - $this->lang = $secondParam; + $this->language = $secondParam; if(isset($thirdParam) && (is_numeric($thirdParam) || in_array($thirdParam, array('master', 'trunk')))) { $this->version = $thirdParam; @@ -166,28 +169,44 @@ class DocumentationViewer extends Controller { } // 'current' version mapping - $module = DocumentationService::is_registered_module($this->module, null, $this->getLang()); - - if($module) { - $current = $module->getLatestVersion(); + $entity = DocumentationService::is_registered_module($this->module, null, $this->getLang()); + + if($entity) { + $current = $entity->getLatestVersion(); $version = $this->getVersion(); - if(!$version || $version == '') { + if(!$version) { $this->version = $current; } // Check if page exists, otherwise return 404 if(!$this->locationExists()) { - $body = $this->renderWith(get_class($this)); - $this->response = new SS_HTTPResponse($body, 404); - - return $this->response; + return $this->throw404(); } + + + return parent::handleRequest($request); } - - return parent::handleRequest($request); + + return $this->throw404(); } + + /** + * Helper function for throwing a 404 error from the {@link handleRequest} + * method. + * + * @return HttpResponse + */ + function throw404() { + $class = get_class($this); + + $body = $this->renderWith(array("{$class}_error", $class)); + $this->response = new SS_HTTPResponse($body, 404); + + return $this->response; + } + /** * Custom templates for each of the sections. */ @@ -404,6 +423,7 @@ class DocumentationViewer extends Controller { $module = $this->getModule(); if($module) { + $has_dir = is_dir(Controller::join_links( $module->getPath($this->getVersion(), $this->getLang()), implode('/', $this->Remaining) @@ -567,37 +587,35 @@ class DocumentationViewer extends Controller { if($page) { return DBField::create("HTMLText", $page->getHTML($this->getVersion(), $this->getLang())); } - else { - // If no page found then we may want to get the listing of the folder. - // In case no folder exists, show a "not found" page. - $module = $this->getModule(); - $url = $this->Remaining; - - if($url && $module) { - $pages = DocumentationService::get_pages_from_folder( - $module, - implode('/', $url), - false, - $this->getVersion(), - $this->getLang() - ); - - // If no pages are found, the 404 is handled in the same template - return $this->customise(array( - 'Title' => DocumentationService::clean_page_name(array_pop($url)), - 'Pages' => $pages - ))->renderWith('DocFolderListing'); - } - else { - // get all available modules and show a table of contents. - - return $this->customise(array( - 'Title' => _t('DocumentationViewer.MODULES', 'Modules'), - 'Pages' => $this->getModules() - ))->renderWith('DocFolderListing'); - } - } + // If no page found then we may want to get the listing of the folder. + // In case no folder exists, show a "not found" page. + $module = $this->getModule(); + $url = $this->Remaining; + + if($url && $module) { + $pages = DocumentationService::get_pages_from_folder( + $module, + implode('/', $url), + false, + $this->getVersion(), + $this->getLang() + ); + + return $this->customise(array( + 'Content' => false, + 'Title' => DocumentationService::clean_page_name(array_pop($url)), + 'Pages' => $pages + ))->renderWith('DocFolderListing'); + } + else { + return $this->customise(array( + 'Content' => false, + 'Title' => _t('DocumentationViewer.MODULES', 'Modules'), + 'Pages' => $this->getModules() + ))->renderWith('DocFolderListing'); + } + return false; } diff --git a/templates/DocFolderListing.ss b/templates/DocFolderListing.ss index b9abd97..7eb1f2c 100755 --- a/templates/DocFolderListing.ss +++ b/templates/DocFolderListing.ss @@ -1,13 +1,13 @@ -<% if Pages %> -
-

$Title

- +
+

$Title

+ + <% if Pages %>
    <% control Pages %>
  • $Title
    • <% end_control %>
-
-<% else %> - <% include DocNotFound %> -<% end_if %> \ No newline at end of file + <% else %> +

No documentation pages found for $Title. If you need help writing documentation please consult the README.

+ <% end_if %> +
\ No newline at end of file diff --git a/templates/Includes/DocNotFound.ss b/templates/Includes/DocNotFound.ss deleted file mode 100755 index e981b70..0000000 --- a/templates/Includes/DocNotFound.ss +++ /dev/null @@ -1,24 +0,0 @@ -
-
-

We're sorry…

-

The page you are looking for does not exist, or might have moved. -

Perhaps you could…
-

Return to the homepage or try searching for the page (see input box on the top right).

- - -
- -
\ No newline at end of file diff --git a/templates/Layout/DocumentationViewer.ss b/templates/Layout/DocumentationViewer.ss index e494842..fb88b48 100755 --- a/templates/Layout/DocumentationViewer.ss +++ b/templates/Layout/DocumentationViewer.ss @@ -1,10 +1,6 @@
- <% if Content %> - $Content - <% else %> - <% include DocNotFound %> - <% end_if %> + $Content
<% if Content %> diff --git a/templates/Layout/DocumentationViewer_error.ss b/templates/Layout/DocumentationViewer_error.ss new file mode 100644 index 0000000..652acba --- /dev/null +++ b/templates/Layout/DocumentationViewer_error.ss @@ -0,0 +1,13 @@ +
+
+
+

We're sorry…

+

The page you are looking for does not exist, or might have moved. +

Perhaps you could…
+
    +
  • Return to the homepage
    • +
  • Try searching for the page (see input box on the top right).
    • +
+
+
+
\ No newline at end of file diff --git a/tests/DocumentationViewerTest.php b/tests/DocumentationViewerTest.php index b7a3eeb..dad2c46 100755 --- a/tests/DocumentationViewerTest.php +++ b/tests/DocumentationViewerTest.php @@ -50,8 +50,7 @@ class DocumentationViewerTest extends FunctionalTest { $this->assertEquals($response->getStatusCode(), 200, 'Existing base folder'); $response = $this->get('dev/docs/DocumentationViewerTests/en/2.4/'); - $this->assertEquals($response->getStatusCode(), 200, 'Existing base folder'); - + $this->assertEquals($response->getStatusCode(), 200, 'Existing base folder'); $response = $this->get('dev/docs/DocumentationViewerTests/en/2.3/nonexistant-subfolder'); $this->assertEquals($response->getStatusCode(), 404, 'Nonexistant subfolder'); @@ -79,9 +78,14 @@ class DocumentationViewerTest extends FunctionalTest { $response = $this->get('dev/docs/DocumentationViewerTests/en/3.0/test/'); $this->assertEquals($response->getStatusCode(), 404, 'Missing page'); + + $response = $this->get('dev/docs/en'); + $this->assertEquals($response->getStatusCode(), 404, 'Must include a module'); + + $response = $this->get('dev/docs/DocumentationViewerTests/dk/');; + $this->assertEquals($response->getStatusCode(), 404, 'Access a language that doesn\'t exist'); } - function testRouting() { $response = $this->get('dev/docs/DocumentationViewerTests/en/2.4');