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

Make /government/get-involved an exact route #9908

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

KludgeKML
Copy link
Contributor

  • The prefix route seems to be a hangover from when whitehall was rendering the Take Part items. These all now have their own content item, so anything under get-involved is either the get involved item or has its own route.
  • Both of the SpecialRoute instances are now exact path, so make that the default in rake task and test, and remove it from the instances.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

- The prefix route seems to be a hangover from when whitehall was rendering the Take Part items. These all now have their own content item, so anything under get-involved is either the get involved item or has its own route.
- Both of the SpecialRoute instances are now exact path, so make that the default in rake task and test, and remove it from the instances.
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Code looks good to me, so assuming you've validated that there aren't risks in this changing this looks good to me.

Good luck with fixing the test though that looked a little confusing

@KludgeKML
Copy link
Contributor Author

The only routes under /government/get-involved are the take-part pages (all linked to from the main get-involved page), which all have their own content items and routes (https://github.com/alphagov/frontend/blob/main/config/routes.rb#L82-L85), so I'm fairly confident that this won't have any knock-on effects.

@KludgeKML
Copy link
Contributor Author

I think that test might be a known flaky one (it passed on re-run), but I might double-check with the Whitehall team to confim.

@KludgeKML
Copy link
Contributor Author

Have confirmed all relevant paths still work on integration after republishing this route.

@KludgeKML KludgeKML merged commit bd7e2bf into main Feb 6, 2025
20 checks passed
@KludgeKML KludgeKML deleted the make-get-involved-exact-route branch February 6, 2025 15:33
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.

2 participants