-
Notifications
You must be signed in to change notification settings - Fork 189
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
Remove Consultation presentation #3006
Conversation
@@ -1,18 +0,0 @@ | |||
class ConsultationsController < DocumentsController | |||
def index | |||
filter_params = params.except(:controller, :action, :format, :_) |
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.
Isn't this still needed? I think it's responsible for this redirect: https://www.gov.uk/government/consultations
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.
It is. Good spot.
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.
Should be fixed with 53ddfb7
3217ba6
to
53ddfb7
Compare
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.
👍
(Also good spot on the bug with final documents attachment published date which we'd missed before)
We can merge this once the 4 unpublishing routes that currently point to whitehall are addressed. |
@@ -3,16 +3,4 @@ def index | |||
filter_params = params.except(:controller, :action, :format, :_) | |||
redirect_to publications_path(filter_params.merge(publication_filter_option: 'consultations')) |
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.
It's a shame to have to leave this in. We could just add the redirect into the router. That might be a bit 'magic' though. Thoughts?
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'd prefer it to be in the router. That's where I'd go to look for redirects.
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.
Can we do merging of filter_params
in the router? Do we still need to?
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.
Could we use a prefix route? /government/consultations
. That might let the filter params pass through to Whitehall with the explicit routes taking the actual consultation requests to government frontend. Need to check that though as I'm not sure.
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.
Going to merge this and look to address the index action in a subsequent PR.
Consultations have been migrated and are now rendered by Government Frontend. This removes the frontend code that is no longer needed.
53ddfb7
to
c37c06e
Compare
Reverting #3006 has angered the linter.
Consultations have been migrated and are now rendered by Government Frontend. This removes the frontend code that is no longer needed.
Trello