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

Fix duplicate page titles in places user journey #4072

Merged
merged 4 commits into from
May 21, 2024

Conversation

hannalaakso
Copy link
Member

@hannalaakso hannalaakso commented May 20, 2024

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

What

De-duplicate places page titles by making the search result and select address page titles more descriptive.

Why

This issue with duplicated page titles was identified on a recent DAC audit of similar journeys. In the audit, screen reader users who carried out a postcode search had to explore the other pages in the journey to find out that the search result pages were different from the search index page since the page titles across the postcode finder journey were duplicated.

Trello card

How

  • Set custom page titles for places pages to differentiate between search result pages and pages requesting an address to be selected from a drop down in the case of multiple authorities,
  • Fix a bug that would result in a location error when a search returned multiple authorities. You could see the bug on live eg. on the DSA provider search when multiple authorities were returned which had the page title Error: Find a Disabled Students' Allowance assessment provider - GOV.UK. The check added in this PR stops the location error from happening when there have been addresses returned. This is the same as the check on line 17 of the places controller which checks if the multiple authorities template should be rendered (that line also checks if postcode is provided, but we already check for that on line 55).
  • Add automated tests for correct places search result and multiple authorities page titles

Screenshots

Find local family hub search result

Before

Screenshot 2024-05-21 at 11 36 22

After

Screenshot 2024-05-21 at 11 35 58

Find local family hub, select from multiple authorities

Before

Screenshot 2024-05-21 at 11 42 06

After

Screenshot 2024-05-21 at 11 42 31

Report child abuse to local council search result

Before

Screenshot 2024-05-21 at 11 44 12

After

Screenshot 2024-05-21 at 11 44 45

Report child abuse to local council, select from multiple authorities

Before

Screenshot 2024-05-21 at 11 50 19

After

Screenshot 2024-05-21 at 11 45 17

Find a Disabled Students' Allowance assessment provider search result

Before

Screenshot 2024-05-21 at 11 45 49

After

Screenshot 2024-05-21 at 11 46 20

Find a Disabled Students' Allowance assessment provider, select from multiple authorities

Before

Screenshot 2024-05-21 at 11 46 40

After

Screenshot 2024-05-21 at 11 46 56

Find a register office search result

Before

Screenshot 2024-05-21 at 11 47 16

After

Screenshot 2024-05-21 at 11 47 42

Find a register office, select from multiple authorities

Before

Screenshot 2024-05-21 at 11 47 58

After

Screenshot 2024-05-21 at 11 48 15

This line was creating a location error when the search result contained multiple authorities. You could see this on live on the multiple authorities search result page page title eg. "Error: Find a Disabled Students' Allowance assessment provider - GOV.UK"

This PR already adds the correct page title for search result pages, but the location error would still get appended to that eg. "Find a Disabled Students' Allowance assessment provider: select an address - GOV.UKError: Find a Disabled Students' Allowance assessment provider - GOV.UK"

The check added in this commit is the same as the check on line 17 to check if the multiple authorities template should be rendered (that line also checks if postcode is provided, but we already check for that on line 55 here).

However, I'm not sure if my "fix" is the best way to handle this or if bypassing multiple authorities in this way could be introducing some other issue. Maybe we should why the places_not_found method from the model doesn't recognise multiple authorities as a valid result. I've tried to superficially compare how this is handled for the other postcode finders like local transaction but those use the location_api model instead.
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4072 May 20, 2024 17:11 Inactive
@hannalaakso hannalaakso changed the title Deduplicate places page titles Fix duplicate page titles in places user journey May 21, 2024
@hannalaakso hannalaakso marked this pull request as ready for review May 21, 2024 11:10
@@ -55,7 +55,7 @@ def location_error
return LocationError.new(INVALID_POSTCODE) unless postcode_provided?
return LocationError.new(INVALID_POSTCODE) if places_manager_response.invalid_postcode?

LocationError.new(NO_LOCATION) if places_manager_response.places_not_found?
LocationError.new(NO_LOCATION) if places_manager_response.places_not_found? && !places_manager_response.addresses_returned?
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks okay as a fix.

Copy link
Contributor

@MartinJJones MartinJJones left a comment

Choose a reason for hiding this comment

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

Nice work on this and fixing the existing bug 👍

@hannalaakso hannalaakso merged commit 3295334 into main May 21, 2024
13 checks passed
@hannalaakso hannalaakso deleted the deduplicate-places-page-titles-2 branch May 21, 2024 13:16
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.

4 participants