-
Notifications
You must be signed in to change notification settings - Fork 14
Fyst 2082 redirect all existing ty 24 fyst ur ls to fileyourstatetaxes org #6104
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
Fyst 2082 redirect all existing ty 24 fyst ur ls to fileyourstatetaxes org #6104
Conversation
…r-ls-to-fileyourstatetaxes-org
|
Heroku app: https://gyr-review-app-6104-4916c0a62b34.herokuapp.com/ |
powersurge360
left a comment
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.
Looks good
mrotondo
left a comment
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.
One non-blocking question about how this interacts with before_actions from other concerns.
| module Questions | ||
| class QuestionsController < ::Questions::QuestionsController | ||
| include StateFile::StateFileIntakeConcern | ||
| include StateFile::FystSunsetRedirectConcern |
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.
(I'm not sure if this how multiple before_actions from different concerns work, but)
Should this go before StateFileIntakeConcern, so that this redirect applies even if they try and load a question page with no logged-in intake (i.e. if require_state_file_intake_login fails)
Note that this should be very uncommon, and so I don't think this is a blocking question one way or another.
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.
This is a good call. I'm not sure if it's first in, first out or last in, first out in setting up before_actions. Looks like it's first in, first out for before_action and last in, first out for after_action. Seems like it's worth putting first in inheritance just for maximum safety.
https://reganchan.ca/blog/ordering-of-filters-in-rails-controllers/
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.
Good call it is first in first out and it was causing a problem with the redirect
https://codeforamerica.atlassian.net/browse/FYST-2082
Yes - don't merge until JIRA issue is accepted!
What was done?
How to test?