Merged
Conversation
thomasiles
force-pushed
the
add-sitemap
branch
from
521a4be to
d29e2c1
Compare
DavidBiddle
reviewed
Feb 11, 2026
DavidBiddle
approved these changes
Feb 11, 2026
Contributor
DavidBiddle
left a comment
DavidBiddle
left a comment
There was a problem hiding this comment.
I've pushed a couple of changes:
- I've removed the MOU link for non-super admins based on a conversation on the Trello card
- I've some extra text that I don't think was included intentionally
Looks like a sensible approach overall, and all works for me locally.
Contributor
Author
Amazing, thank you! |
The Group model uses the attribute `external_id` to generate links instead of the id. The external_id is set using a before_create callback. The factory doesn't set the external_id. The callback is only triggered when the create strategy is used. Tests which call code that contains helpers like `link_to` need to use `create`. If they use `build_stubbed`, which is faster, the external_id will be not be set and an error will be raised. Adding external_id to the factory lets tests use `build_stubbed` when they don't need to test the database. This will make the tests run faster.
We link our accessibility statement, cookies, privacy, and terms of use pages to the product pages in the footer. These links are currently hard-coded in the application layout. To use these links in other places, such as the sitemap, we define them in the routes file. This makes them more obvious and easier to change. They currently all link directly to the production site. We could change them to use a setting which takes the URL of the product site. That requires changes to the deploy repo, so I think it's best to do it in another PR.
Some pages, such as the Users page, only have one link to access them. This was identified as an issue in the accessibility audit. This commit adds the view for a sitemap, which links to each page in the app.
We add the route and controller for the sitemap view.
Add a link to the sitemap in the footer of the application.
Initially this was set to show an MOU link to any user who aren't able to see the list of MOU signatures (i.e. standard users and org admins). We have decided not to show them this link - the MOU page includes a form to sign the MOU, and we're concerned that linking to this directly from the sitemap would result in unwanted MOU signatures. Additionally, since we don't currently have another way for non super admins to access this page, adding a link here would likely violate WCAG SC 2.4.5: Multiple Ways. Instead, we won't show any MOU information to non super admins. In future we might make a non-signable version of the MOU page visible to users on this page, but that will be a separate bit of work.
Some error text had been included in the view, presumably copied and pasted by accident. This commit removes it.
thomasiles
force-pushed
the
add-sitemap
branch
from
6670474 to
2fb9740
Compare
|
|
🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2544.admin.review.forms.service.gov.uk/ It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready For the sign in details and more information, see the review apps wiki page. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a sitemap with links to each page
Trello card: https://trello.com/c/sdUWJCNl/2739-users-list-page-can-only-be-accessed-from-the-header-navigation
Add a sitemap, linked from the footer with links to each significant page.
I've used headers because we needed some to be links. Lists have extra spacing to make them easier to use with touch.