Clean up the world location base path class #1035
Merged
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
added 2 commits
August 8, 2018 16:24
I have no idea what this did, but it didn't affect the related links as I'd hoped, as it turns out that the code for them is in the govuk_publishing_components gem. This reverts commit 84cc5f8.
As far as I can work out, this is never used. The only document type that uses the metadata, which is I think where this is possibly used is travel advice. I checked if any travel advice content is tagged to world locations, and it isn't. This is very confusing, as the exceptional cases aren't exceptional. Looking at the code, the behaviour looks to be the same with or without them. I've also removed the comment, as the situation with the migration of "world locations to a taxonomy based model" is pretty uncertian, as the Topic Taxonomy doesn't have anything to do with country classification (it's about subject, not geography), and this "temporary solution" both as far as I can work out, doesn't do anything, and has been here for over a year.
boffbowsh
approved these changes
Aug 9, 2018
|
Thanks for taking a look Paul :) |
thomasleese
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Feb 3, 2020
The slug for the world location 'UK mission in the European Union' doesn't match exactly the title of the world location. Instead the slug shortens 'European Union' to 'EU'. This means that links to this world location are currently broken (for example https://www.gov.uk/government/speeches/triggering-the-jcpoa-dispute-resolution-mechanism-foreign-secretarys-commons-statement (see section at the bottom of the page). This is similar to the special cases that we used to have in government-frontend before they were removed and moved into govuk_publishing_components: alphagov/government-frontend#1035
thomasleese
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Feb 3, 2020
The slug for the world location 'UK mission in the European Union' doesn't match exactly the title of the world location. Instead the slug shortens 'European Union' to 'EU'. This means that links to this world location are currently broken (for example https://www.gov.uk/government/speeches/triggering-the-jcpoa-dispute-resolution-mechanism-foreign-secretarys-commons-statement (see section at the bottom of the page). This is similar to the special cases that we used to have in government-frontend before they were removed and moved into govuk_publishing_components: alphagov/government-frontend#1035
thomasleese
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Feb 3, 2020
The slug for the world location 'UK mission in the European Union' doesn't match exactly the title of the world location. Instead the slug shortens 'European Union' to 'EU'. This means that links to this world location are currently broken (for example https://www.gov.uk/government/speeches/triggering-the-jcpoa-dispute-resolution-mechanism-foreign-secretarys-commons-statement (see section at the bottom of the page). This is similar to the special cases that we used to have in government-frontend before they were removed and moved into govuk_publishing_components: alphagov/government-frontend#1035
thomasleese
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Feb 3, 2020
The slug for the world location 'UK mission in the European Union' doesn't match exactly the title of the world location. Instead the slug shortens 'European Union' to 'EU'. This means that links to this world location are currently broken (for example https://www.gov.uk/government/speeches/triggering-the-jcpoa-dispute-resolution-mechanism-foreign-secretarys-commons-statement (see section at the bottom of the page). This is similar to the special cases that we used to have in government-frontend before they were removed and moved into govuk_publishing_components: alphagov/government-frontend#1035
thomasleese
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Feb 3, 2020
The slug for the world location 'UK mission in the European Union' doesn't match exactly the title of the world location. Instead the slug shortens 'European Union' to 'EU'. This means that links to this world location are currently broken (for example https://www.gov.uk/government/speeches/triggering-the-jcpoa-dispute-resolution-mechanism-foreign-secretarys-commons-statement (see section at the bottom of the page). This is similar to the special cases that we used to have in government-frontend before they were removed and moved into govuk_publishing_components: alphagov/government-frontend#1035
|
Looks like this is still necessary e.g. for St Pierre & Miquelon (link is at the bottom of the page). |
I don't believe the changes here are directly involved, since the special case removed here doesn't help, and just leads to a non-existent page: Using the slug |
|
Sure, the actual page is https://www.gov.uk/world/st-pierre-and-miquelon/news. I should clarify my intent here: this was mainly to document the continued need for the original code, which provided for special cases like this one. However, I don't think it's worth investing effort in fixing it right now, as that's a distracting from the underlying problem - no base paths 😨. |
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.
First, revert a ineffective change I made, then attempt to clean this WorldLocationBasePath class up.
As far as I can work out, this is never used. The only document type
that uses the metadata, which is I think where this is possibly used
is travel advice. I checked if any travel advice content is tagged to
world locations, and it isn't.
This is very confusing, as the exceptional cases aren't
exceptional. Looking at the code, the behaviour looks to be the same
with or without them.
I've also removed the comment, as the situation with the migration of
"world locations to a taxonomy based model" is pretty uncertian, as
the Topic Taxonomy doesn't have anything to do with country
classification (it's about subject, not geography), and this
"temporary solution" both as far as I can work out, doesn't do
anything, and has been here for over a year.