From 8d4896941a0a05cc65202e446e125e890ac0ff9a Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 4 Jul 2022 21:47:46 +1200 Subject: [PATCH] ENH Display error messages in admin with an admin context --- _config/forms.yml | 4 ++ code/AdminErrorExtension.php | 34 +++++++++++++ code/LeftAndMain.php | 50 +++++++++++++++++++ lang/en.yml | 6 +++ .../Admin/Includes/LeftAndMain_Error.ss | 24 +++++++++ tests/behat/features/notfound.feature | 39 +++++++++++++++ tests/behat/src/AdminContext.php | 18 +++++++ 7 files changed, 175 insertions(+) create mode 100644 code/AdminErrorExtension.php create mode 100644 templates/SilverStripe/Admin/Includes/LeftAndMain_Error.ss create mode 100644 tests/behat/features/notfound.feature diff --git a/_config/forms.yml b/_config/forms.yml index a6d1b214b..4cbbe25d5 100644 --- a/_config/forms.yml +++ b/_config/forms.yml @@ -16,3 +16,7 @@ SilverStripe\Forms\Form: SilverStripe\Forms\FormField: extensions: - SilverStripe\Forms\FormMessageBootstrapExtension + +SilverStripe\Control\RequestHandler: + extensions: + - SilverStripe\Admin\AdminErrorExtension diff --git a/code/AdminErrorExtension.php b/code/AdminErrorExtension.php new file mode 100644 index 000000000..b8c73e158 --- /dev/null +++ b/code/AdminErrorExtension.php @@ -0,0 +1,34 @@ +getAdminController(); + if (!$controller || Director::is_ajax($request)) { + return; + } + $controller->setHttpErrorMessage($errorMessage); + } + + private function getAdminController(): ?Controller + { + if ($this->owner instanceof LeftAndMain) { + return $this->owner; + } + if (Controller::has_curr() && (Controller::curr() instanceof LeftAndMain)) { + return Controller::curr(); + } + return null; + } +} diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index af700f103..a4b0dbbe4 100644 --- a/code/LeftAndMain.php +++ b/code/LeftAndMain.php @@ -175,6 +175,11 @@ class LeftAndMain extends Controller implements PermissionProvider */ protected $pageID = null; + /** + * Set by {@link LeftAndMainErrorExtension} if an http error occurs + */ + private string $httpErrorMessage; + /** * Assign themes to use for cms * @@ -773,6 +778,32 @@ protected function init() Versioned::set_default_reading_mode(Versioned::get_reading_mode()); } + public function afterHandleRequest() + { + if ($this->response->isError()) { + $this->init(); + $errorCode = $this->response->getStatusCode(); + $errorType = $this->response->getStatusDescription(); + $defaultMessage = _t( + self::class . '.ErrorMessage', + 'Sorry, it seems there was a {errorcode} error.', + ['errorcode' => $errorCode] + ); + $this->response = HTTPResponse::create($this->render([ + 'Title' => $this->getApplicationName() . ' - ' . $errorType, + 'Content' => $this->renderWith($this->getTemplatesWithSuffix('_Error'), [ + 'ErrorCode' => $errorCode, + 'ErrorType' => $errorType, + 'Message' => DBField::create_field( + 'HTMLFragment', + _t(self::class . '.ErrorMessage' . $errorCode, $defaultMessage) + ), + ]), + ]), $errorCode, $errorType); + } + parent::afterHandleRequest(); + } + public function handleRequest(HTTPRequest $request) { try { @@ -1998,4 +2029,23 @@ public function getVersionProvider() { return $this->versionProvider; } + + /** + * Get the HTTP error message if one has occurred during HandleRequest. + */ + public function getHttpErrorMessage(): string + { + return $this->httpErrorMessage; + } + + /** + * Set the HTTP error message when one occurs during HandleRequest. + * Called by {@link LeftAndMainErrorExtension} + * @internal + */ + public function setHttpErrorMessage(string $message): self + { + $this->httpErrorMessage = $message; + return $this; + } } diff --git a/lang/en.yml b/lang/en.yml index f133800c4..dda68f893 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -33,6 +33,12 @@ en: CollapsePanel: 'Collapse panel' DELETED: Deleted. DropdownBatchActionsDefault: 'Choose an action...' + ErrorDetail: 'Error detail: {detail}' + ErrorMessage: 'Sorry, it seems there was a {errorcode} error.' + ErrorMessage403: 'Sorry, it seems the action you were trying to perform is forbidden.' + ErrorMessage404: 'Sorry, it seems you were trying to access a section or object that doesn''t exist.' + ErrorMessage500: 'Sorry, it seems there was an internal server error.' + ErrorMessage503: 'Sorry, it seems the service is temporarily unavailable.' ExpandPanel: 'Expand panel' HelpMenu: 'Help menu' LOGOUT: 'Log out' diff --git a/templates/SilverStripe/Admin/Includes/LeftAndMain_Error.ss b/templates/SilverStripe/Admin/Includes/LeftAndMain_Error.ss new file mode 100644 index 000000000..ede271ae1 --- /dev/null +++ b/templates/SilverStripe/Admin/Includes/LeftAndMain_Error.ss @@ -0,0 +1,24 @@ +
+ +
+
+
+ + $ErrorType + +
+
+
+ + $Tools + +
+

$Message

+ <% if $isDev && $HttpErrorMessage %> +

+ <%t SilverStripe\Admin\LeftAndMain.ErrorDetail "Error detail: {detail}" detail=$HttpErrorMessage %> +

+ <% end_if %> +
+ +
diff --git a/tests/behat/features/notfound.feature b/tests/behat/features/notfound.feature new file mode 100644 index 000000000..5c2ba314c --- /dev/null +++ b/tests/behat/features/notfound.feature @@ -0,0 +1,39 @@ +@gsat +Feature: Not found + As a site owner + I want error messages to be displayed in the context of the admin section + + Background: + Given I am logged in with "ADMIN" permissions + + Scenario: Errors are displayed in the admin context + Given I go to "/admin/nothing" + Then I should see "Not Found" + And I should see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu + Given I go to "/admin/pages/nothing" + Then I should see "Not Found" + And I should see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu + Given I go to "/admin/security/EditForm/nothing" + Then I should see "Not Found" + And I should see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu + Given I go to "/admin/security/EditForm/field/Members/nothing" + Then I should see "Not Found" + And I should see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu + Given I go to "/admin/settings/nothing" + Then I should see "Not Found" + And I should see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu + + Scenario: Valid routes do not display the error + Given I go to "/admin/settings" + Then I should not see "Not Found" + And I should not see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu + Given I go to "/admin/security/EditForm/field/Members/item/new" + Then I should not see "Not Found" + And I should not see "Sorry, it seems you were trying to access a section or object that doesn't exist." + And I should see the admin menu diff --git a/tests/behat/src/AdminContext.php b/tests/behat/src/AdminContext.php index 626e69c5f..85bb810af 100644 --- a/tests/behat/src/AdminContext.php +++ b/tests/behat/src/AdminContext.php @@ -37,6 +37,24 @@ public function iShouldSeeAnInvalidTabIcon(string $not, string $tabLabel) } } + /** + * @Then /^I should (not |)see the admin menu/ + * @param string $not + * @param string $tabLabel + */ + public function iShouldSeeTheAdminMenu(string $not) + { + $selector = '#cms-menu'; + $page = $this->getMainContext()->getSession()->getPage(); + if ($not) { + $element = $page->find('css', $selector); + Assert::assertNull($element, 'The admin menu is visible when it should not be'); + } else { + $element = $page->find('css', $selector); + Assert::assertNotNull($element, 'The admin menu was not found'); + } + } + /** * @When /^I can (not |)see the form validation error message$/ * @param $not