Skip to content

Combine Themyleaf error pages #1122

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 17, 2019
Merged

Combine Themyleaf error pages #1122

merged 1 commit into from
Sep 17, 2019

Conversation

mukeshk
Copy link
Contributor

@mukeshk mukeshk commented Sep 13, 2019 •
edited by php-coder
Loading

Fix #1119

@mukeshk mukeshk requested a review from php-coder as a code owner September 13, 2019 13:58
@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch 2 times, most recently from 245ee16 to 627b667 Compare September 13, 2019 14:07
@mukeshk
Copy link
Contributor Author

mukeshk commented Sep 13, 2019

I have an IntelliJ inspect error on the error/status-code.html - duplicate id.
line 63-74 on status-code.html.

I verified for 404.
How do I verify for other 403 and 500?

@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch from 627b667 to 7b39ef8 Compare September 13, 2019 14:10
@mystamps-bot
Copy link

mystamps-bot commented Sep 13, 2019 •
edited
Loading

1 Warning
⚠️ danger check: pull request description doesn’t contain a link to original issue.
Consider adding a comment in the following format: Addressed to #XXX where XXX is an issue number

Generated by 🚫 Danger

@php-coder php-coder changed the title [wip] combine theme leaf error pages [wip] combine Themyleaf error pages Sep 13, 2019
php-coder
php-coder requested changes Sep 13, 2019
View reviewed changes
@php-coder
Copy link
Owner

How do I verify for other 403 and 500?

Testing for 403 is easy, for example:

mystamps/src/test/robotframework/category/access.robot

Lines 10 to 13 in 2b7bd1b

Anonymous user cannot create category
Go To ${SITE_URL}/category/add
Element Text Should Be id=error-code 403
Element Text Should Be id=error-msg Forbidden

Reproducing 500 isn't easy but we have a bug #986 that causes an exception, so you might use it :) In order to reproduce it you should login as a user, open the same series in 2 tabs and add it it collection from the first tab and then try to add it again from the second tab.

@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch from 7b39ef8 to bcba306 Compare September 14, 2019 07:22
php-coder
php-coder reviewed Sep 14, 2019
View reviewed changes
php-coder
php-coder reviewed Sep 14, 2019
View reviewed changes
php-coder
php-coder reviewed Sep 14, 2019
View reviewed changes
@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch 2 times, most recently from 66a0e49 to a995e1e Compare September 15, 2019 08:06
@php-coder php-coder changed the title [wip] combine Themyleaf error pages Combine Themyleaf error pages Sep 15, 2019
php-coder
php-coder reviewed Sep 15, 2019
View reviewed changes
@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch from a995e1e to 9f6d6cd Compare September 16, 2019 07:03
php-coder
php-coder requested changes Sep 16, 2019
View reviewed changes
@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch from 9f6d6cd to e0083de Compare September 16, 2019 17:09
php-coder
php-coder requested changes Sep 16, 2019
View reviewed changes
Copy link
Owner

@php-coder php-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments and also, please, amend the commit title and modify the first line:

refactor: combine Thymeleaf error pages into one.

@mukeshk mukeshk force-pushed the gh1119_combine_theme_leaf_error_page branch from e0083de to 27d3658 Compare September 17, 2019 05:16
php-coder
php-coder approved these changes Sep 17, 2019
View reviewed changes
Copy link
Owner

@php-coder php-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@php-coder php-coder merged commit c844ac1 into php-coder:master Sep 17, 2019
@mukeshk mukeshk deleted the gh1119_combine_theme_leaf_error_page branch September 18, 2019 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@php-coder php-coder php-coder approved these changes

Assignees
No one assigned
Labels
resolution/merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Combine Thymeleaf error pages
3 participants
@mukeshk @mystamps-bot @php-coder