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

Remove code related to world location news article #2529

Merged
merged 2 commits into from
Sep 22, 2022

Conversation

BeckaL
Copy link
Contributor

@BeckaL BeckaL commented Aug 25, 2022

This content type is no longer in use, having been replaced by the 'news article' schema. For example, if you look at some of the urls that are removed here in config/govuk_examples.yml, you can see that all of the world_location_news_article examples now redirect to a .../news/... url, and have a schema of news_article.

Whitehall does not publish anything with this schema anymore, and the few articles that were left behind with this schema were 'orphaned' content items which should have been previously redirected (but we suspect a bulk redirect task probably failed in a few cases, hence leaving behind 5 content items with this schema). This redirect has now been completed for all content items, meaning we have no remaining content items with this schema.

This means that code to render it can now be removed. This will also enable us to clean up the schema from govuk-content-schemas - PR here (failing because of tests removed in the current PR here).

Query on non draft content store showing no remaining items:

criteria = ContentItem.where(schema_name: "world_location_news_article")
criteria.map{ | doc | doc.id }

=> []

There are two content items in the draft content store - these also look like similar cases to the above, as they look 'orphaned' and the editions in whitehall link, and publish, to the version that does not use the world-location-news-article schema. Since these are both old content items, and on the draft stack, and are duplicates of the correct content items with the in-use schema (so if they are ever published, which is unlikely because they're from 2017, they will get published with the new schema and to the correct url), we've made the decision to leave these since we don't think they'll cause issues.

Trello

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

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2529 August 25, 2022 16:30 Inactive
@BeckaL BeckaL force-pushed the remove-world-location-news-article branch from b5f1be1 to e444dd1 Compare September 21, 2022 15:00
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2529 September 21, 2022 15:01 Inactive
@BeckaL BeckaL force-pushed the remove-world-location-news-article branch from e444dd1 to 8349714 Compare September 21, 2022 15:41
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2529 September 21, 2022 15:42 Inactive
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2529 September 21, 2022 15:48 Inactive
@BeckaL BeckaL changed the title Remove code related to world location news Remove code related to world location news article Sep 22, 2022
@BeckaL BeckaL marked this pull request as ready for review September 22, 2022 09:15
Copy link
Contributor

@AgaDufrat AgaDufrat left a comment

Choose a reason for hiding this comment

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

Thank you for the PR with a good description, which clearly sets out the context for this work as well as all the preliminary investigations you did and decisions made. As I understand there will be no need to render/preview those 2 'orphaned' draft editions.


I also appreciate that you included the translation deletion in a separate commit as it’s much easier to review the changes in the multiple files 🏅

I checked the redirect. Looks like it deletes the relevant code and the existing news_article is sufficiently tested.

Could we also remove the entry from the list of schemas in the README, the erb file and the frontend'ey bits?

@BeckaL
Copy link
Contributor Author

BeckaL commented Sep 22, 2022

Thanks so much Aga, really appreciate you having a look! And good spot on those remaining references 🕵️‍♀️, I'll remove those now!

@BeckaL BeckaL force-pushed the remove-world-location-news-article branch from 9d74e0b to 22d9508 Compare September 22, 2022 13:11
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2529 September 22, 2022 13:11 Inactive
This content type is no longer in use, and by the time this is merged we will have migrated all content items away from this type.
This content type is no longer in use.
@BeckaL BeckaL force-pushed the remove-world-location-news-article branch from 22d9508 to 2728400 Compare September 22, 2022 13:59
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2529 September 22, 2022 13:59 Inactive
@BeckaL BeckaL merged commit 796061a into main Sep 22, 2022
@BeckaL BeckaL deleted the remove-world-location-news-article branch September 22, 2022 14: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.

3 participants