Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH Display error messages in admin with an admin context #1317

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions _config/forms.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@ SilverStripe\Forms\Form:
SilverStripe\Forms\FormField:
extensions:
- SilverStripe\Forms\FormMessageBootstrapExtension

SilverStripe\Control\RequestHandler:
extensions:
- SilverStripe\Admin\AdminErrorExtension
34 changes: 34 additions & 0 deletions code/AdminErrorExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace SilverStripe\Admin;

use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Core\Extension;

class AdminErrorExtension extends Extension
{
/**
* Used by {@see RequestHandler::httpError}
*/
public function onBeforeHTTPError($statusCode, HTTPRequest $request, $errorMessage = null)
{
$controller = $this->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;
}
}
50 changes: 50 additions & 0 deletions code/LeftAndMain.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
}
6 changes: 6 additions & 0 deletions lang/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
24 changes: 24 additions & 0 deletions templates/SilverStripe/Admin/Includes/LeftAndMain_Error.ss
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<div class="cms-content flexbox-area-grow $BaseCSSClasses" data-layout-type="border" data-pjax-fragment="Content">

<div class="cms-content-header north">
<div class="cms-content-header-info vertical-align-items flexbox-area-grow">
<div class="breadcrumbs-wrapper">
<span class="cms-panel-link crumb last">
$ErrorType
</span>
</div>
</div>
</div>

$Tools

<div class="panel panel--padded">
<p>$Message</p>
<% if $isDev && $HttpErrorMessage %>
<p>
<strong><%t SilverStripe\Admin\LeftAndMain.ErrorDetail "Error detail: {detail}" detail=$HttpErrorMessage %></strong>
</p>
<% end_if %>
</div>

</div>
39 changes: 39 additions & 0 deletions tests/behat/features/notfound.feature
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions tests/behat/src/AdminContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down