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

Refactor page redirect path after 'destroy', 'create', 'update' #1995

Conversation

edimossilva
Copy link
Contributor

@edimossilva edimossilva commented Jun 3, 2021

@pablobm , please review

This is a follow-up of PR #1973

Should I squash the commits?

hound is complaining about something but I think it is not relevant, and maybe this rule should be disabled => rubocop/rubocop#3675

@edimossilva edimossilva force-pushed the refactor-page-redirect-path-after-destroy-create-edit branch from ef3bf08 to f606d8d Compare June 3, 2021 16:40
@edimossilva edimossilva force-pushed the refactor-page-redirect-path-after-destroy-create-edit branch 2 times, most recently from 93f12b8 to 554e6af Compare June 3, 2021 16:46
@edimossilva edimossilva force-pushed the refactor-page-redirect-path-after-destroy-create-edit branch from 554e6af to cfade80 Compare June 3, 2021 16:56
@edimossilva edimossilva marked this pull request as draft June 3, 2021 16:56
@edimossilva edimossilva marked this pull request as ready for review June 3, 2021 17:07
Copy link
Collaborator

@pablobm pablobm left a comment

Choose a reason for hiding this comment

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

Thank you for taking over this one! I agree that those Hound comments are not important. I also get them and I ignore them happily.

I have left a couple of comments. After that we should be good to go.

docs/customizing_controller_actions.md Outdated Show resolved Hide resolved
spec/controllers/admin/application_controller_spec.rb Outdated Show resolved Hide resolved
spec/controllers/admin/application_controller_spec.rb Outdated Show resolved Hide resolved
@edimossilva edimossilva force-pushed the refactor-page-redirect-path-after-destroy-create-edit branch from 3390b33 to ce3433f Compare June 10, 2021 17:20
this will make us easier to override redirect path

fix syntax

fix hound

restore unchanged lines

fix hound review

Update application_controller.rb

fix hound review

Add specs and doc for override redirect feature

Update docs/customizing_controller_actions.md

Co-authored-by: Pablo Brasero <36066+pablobm@users.noreply.github.com>

use anonimous controller for custom redirect
@edimossilva edimossilva force-pushed the refactor-page-redirect-path-after-destroy-create-edit branch from ce3433f to c3c0568 Compare June 10, 2021 17:22
@edimossilva edimossilva requested a review from pablobm June 10, 2021 17:32
@edimossilva
Copy link
Contributor Author

Thank you for taking over this one! I agree that those Hound comments are not important. I also get them and I ignore them happily.

I have left a couple of comments. After that we should be good to go.

I've done the suggested changes, please review again @pablobm

@pablobm pablobm merged commit 9071a7e into thoughtbot:main Jun 17, 2021
@pablobm
Copy link
Collaborator

pablobm commented Jun 17, 2021

Thank you @edimossilva! Incidentally, I found what was wrong with those Hound errors on get, post, etc. It was because of a shim in our code. I'm fixing that on a different PR: #2001 I'll rebase on top of your code to also fix the ones that you introduce here.

@edimossilva edimossilva deleted the refactor-page-redirect-path-after-destroy-create-edit branch June 17, 2021 18:19
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