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

Resolve errors page fix #1439

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fellipepcs
Copy link

@fellipepcs fellipepcs commented Dec 14, 2022

Description

  • Added customizable error pages for Rails 404,422 and 500 statuses.
  • Created ErrorsController Class for use in different places where you want to implement error pages, just calling the method.

Complement the issue: #3

Created by @DafneM and @fellipepcs

Co-Authored-by: fellipepcs <[email protected]>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Rubocop found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Member

@wwahammy wwahammy left a comment

Choose a reason for hiding this comment

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

@DafneM and @fellipepcs thank you so much for the contribution. I particularly appreciate the initiative you took to realize that we should use exceptions_app. This is a really great start.

I've made a number of comments on necessary improvements before merging; most of them are minor. I look forward to your future changes. Please re-request my review when you're ready.

Thanks so much again.

config/routes.rb Outdated Show resolved Hide resolved
app/controllers/errors_controller.rb Outdated Show resolved Hide resolved
app/controllers/errors_controller.rb Outdated Show resolved Hide resolved
app/controllers/errors_controller.rb Outdated Show resolved Hide resolved
app/views/errors/internal_server.html.erb Show resolved Hide resolved
app/views/errors/not_found.html.erb Outdated Show resolved Hide resolved
config/routes.rb Show resolved Hide resolved
app/controllers/errors_controller.rb Outdated Show resolved Hide resolved
app/controllers/errors_controller.rb Outdated Show resolved Hide resolved
public/404.html Show resolved Hide resolved
@fellipepcs
Copy link
Author

Made changes to requests requested above.

@DafneM
Copy link

DafneM commented Feb 1, 2023

Hey @wwahammy is there anything we can do here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants