[MAGNOLIA-4043] page editor, rendering: Previewing a page excluded on a given channel should display a nicer message instead of 404 servlet container page Created: 02/Mar/12 Updated: 28/Apr/14 Resolved: 25/Apr/14 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | bundle, page editor |
| Affects Version/s: | None |
| Fix Version/s: | 4.5.19 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Federico Grilli | Assignee: | Christoph Meier |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | support, ux | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||
| Template: |
|
||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||
| Description |
|
Steps to reproduce the issue on stk demo-project (author):
The servlet container default 404 error page is shown which is fine if you're on a public instance but on the author looks more like a bug and force editors to use the browser's back button to go back to Magnolia. |
| Comments |
| Comment by Federico Grilli [ 02/Mar/12 ] |
|
Adding a nice 404 error page to web.xml in magnolia-empty-webapp could be a solution, something like <error-page> <error-code>404</error-code> <location>/docroot/404.html</location> </error-page> |
| Comment by Federico Grilli [ 02/Mar/12 ] |
|
attached an 404 html page (and a screenshot of it) I could come up with my limited design skills. |
| Comment by Andreas Weder [ 05/Mar/12 ] |
|
You've asked me for a quick review - I think this is fine, the way it is and looks (except maybe for the missing comma between "browser" and "but" I'd prefer a solution, which would distinguish between a 404 and a page not being available due to it not being published on a particular channel. This would allow us to show a precise error message and possibly additional information along with it (e.g. "the page is only available on channels A and C"). But this works for now. |
| Comment by Christoph Meier [ 23/Apr/14 ] |
|
In agreement with Andreas i added a simple error-handler for 404 to the web.xml and a static page to docroot (as proposed by Federico). |
| Comment by Zdenek Skodik [ 23/Apr/14 ] |
|
404 pages are pretty common subject of change, could we perhaps also make the rendering filter to log something along by sending the error code? This should help with narrowing down issues caused by misconfigured channels. |
| Comment by Christoph Meier [ 24/Apr/14 ] |
|
I changed RenderingFilter#doFilter according to Zdenek's input; e.g. it will log now: |
| Comment by Roman Kovařík [ 25/Apr/14 ] |
log.info("The requested resource ('"+aggregationState.getCurrentURI()+"') is not available on channel '{}'", aggregationState.getChannel().getName());
|
| Comment by Christoph Meier [ 25/Apr/14 ] |
|
Added a unit-test (to ensure it's logged if a resource is not available in the requested channel); ensured proper formatting. |
| Comment by Roman Kovařík [ 25/Apr/14 ] |
|
Sorry for reopening again
- assertTrue(stringWriter.toString().indexOf(logForChannelExclude404) > -1);
+ assertThat(stringWriter.toString(), containsString("is not available on channel"))
The reason is that you get more descriptive error message when the test fails. Also we don't need to match whole sentence so we don't need to change tests when a small unimportant part of a message is changed. |
| Comment by Christoph Meier [ 25/Apr/14 ] |
|
This ticket should be ported to 5.2x and 5.3, too. |