Fix GOV.UK Chat promo on pages with multiple path segments #3440
Conversation
This file contains 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
We have an allow list of URLs on which we want to render the GOV.UK Chat promo, which we compare the content item's `base_path` against to determine whether to show the promo or not. This results in a bug in some guide pages where we only want to show the promo on certain steps of the guide. Let's consider the "/contracted-out" URL. This guide has 3 steps, and so 3 URLs 1. /contracted-out 2. /contracted-out/how-contracting-out-affects-your-amount 3. /contracted-out/check-if-you-were-contracted-out We only want to show the promo on URL 1 and 3 in the list above, but not the second one. With the current logic, we're showing the promo on all of these URLs. That's because we're looking at the `content_item.base_path` property, which is "/contracted-out" for all of those URLs. Instead we should do the comparison on the `requested_path` method, which is the full path. It also fixes a bug in a similar vein where we want to show the promo on only one of the sub-pages in a guide (e.g. URL 2 above), but because we're comparing against the `base_path`, we end up showing it on all pages of the guide.
jackbot
force-pushed
the
fix-govuk-chat-promo-link
branch
from
95e39a2 to
0b829ce
Compare
davidgisbey
approved these changes
Nov 26, 2024
kevindew
added a commit
that referenced
this pull request
Dec 3, 2024
In #3440 we changed the mechanism for rendering the GOV.UK Chat banner to utilise request path rather than content item base_path. A side effect of this was that for any existing guides that we were embedding the banner on lost the banner on any of their nested sections. This commit adds each individual path of guides that previously had the banner.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We have an allow list of URLs on which we want to render the GOV.UK Chat
promo, which we compare the content item's
base_pathagainst todetermine whether to show the promo or not.
This results in a bug in some guide pages where we only want to show the
promo on certain steps of the guide.
Let's consider the "/contracted-out" URL. This guide has 3 steps, and so
3 URLs
We only want to show the promo on URL 1 and 3 in the list above, but not
the second one.
With the current logic, we're showing the promo on all of these URLs.
That's because we're looking at the
content_item.base_pathproperty,which is "/contracted-out" for all of those URLs.
Instead we should do the comparison on the
requested_pathmethod,which is the full path.
It also fixes a bug in a similar vein where we want to show the promo on
only one of the sub-pages in a guide (e.g. URL 2 above), but because
we're comparing against the
base_path, we end up showing it on allpages of the guide.