-
Notifications
You must be signed in to change notification settings - Fork 96
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
ENH Display error messages in admin with an admin context #1317
Conversation
a66c668
to
25129b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much nicer having error messages in the CMS then going to the front-end
A few things
lang/en.yml
Outdated
ErrorDetail: 'Error detail: {detail}' | ||
ErrorMessage: '<p>Sorry, it seems there was a {errorcode} error.</p>' | ||
ErrorMessage403: '<p>Sorry, it seems the action you were trying to perform is forbidden.</p>' | ||
ErrorMessage404: '<p>Sorry, it seems you were trying to access a page that doesn''t exist.</p>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really a page that does not exist - it's a route that does not exist. Maybe a "section" is better language?
e.g. /admin/security/asdfasdfsadf
is will say this page does not exist, though you wouldn't call /admin/security
a page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I think "section" has the same problem as it may be for example a specific ID in an edit form that has no associated data object.
How about something like "You were trying to access a section or object that doesn't exist"? Or simply "The URL you were trying to access resulted in a 'Not Found' error"?
25129b3
to
1d56b55
Compare
0569c41
to
068f26e
Compare
068f26e
to
8d48969
Compare
@GuySartorelli looks like this has caused regressions - https://github.com/silverstripe/silverstripe-cms/runs/7571102762?check_suite_focus=true
|
#1331 fixes that. |
Currently if there is an error in any /admin route, there is no admin-specific way of handling the errors.
This means that if silverstripe/errorpage is installed the user is shown the frontend error page (if there is one) and if it isn't installed or doesn't have a page for the given error, a simple text message is displayed.
This PR renders an error message in the context of the admin, so that users can simply select an appropriate section in the left menu.
The error detail only appears in Dev mode.
In this example I tried to navigate to /admin/badonk and an appropriate 404 error is displayed
In this example I tried to navigate to /admin/pages/save and an appropriate forbidden error is displayed
Parent issues
Depends on