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 help centre sitemap #1277

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Fix help centre sitemap #1277

merged 2 commits into from
Jan 24, 2024

Conversation

andrewHEguardian
Copy link
Contributor

@andrewHEguardian andrewHEguardian commented Dec 15, 2023

What does this change?

Fix the help centre sitemap by asserting the content type that it actually is in S3. Spotted because I was testing turning on eslint rule '@typescript-eslint/prefer-nullish-coalescing'

How to test

Go to https://manage.theguardian.com/sitemap.txt, see content instead of [] as before this change.

image

How can we measure success?

Have we considered potential risks?

Should we rather just change the content type of the file in S3 to text/plain (which seems to be more accurate?) - or fix whatever is creating that file

Images

Before
image

After
image

Accessibility

Sorry, something went wrong.

Copy link
Member

@kelvin-chappell kelvin-chappell left a comment

Choose a reason for hiding this comment

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

The change looks fine to me but if this script is still the way the sitemap is generated it would be better if the publishing or taking down of an article triggered a sitemap update. There are a few things in the Help Centre that were never quite finished off.

@graham228221
Copy link
Contributor

The change looks fine to me but if this script is still the way the sitemap is generated it would be better if the publishing or taking down of an article triggered a sitemap update. There are a few things in the Help Centre that were never quite finished off.

I think site map updating is something you managed to finish off @kelvin-chappell 😄 guardian/manage-help-content-publisher#114

@kelvin-chappell
Copy link
Member

The change looks fine to me but if this script is still the way the sitemap is generated it would be better if the publishing or taking down of an article triggered a sitemap update. There are a few things in the Help Centre that were never quite finished off.

I think site map updating is something you managed to finish off @kelvin-chappell 😄 guardian/manage-help-content-publisher#114

Thanks for doing a proper search, Graham! Looks like that script can go then. And should be possible to change the content type of the generated S3 file.

@andrewHEguardian andrewHEguardian merged commit f953988 into main Jan 24, 2024
8 checks passed
@andrewHEguardian andrewHEguardian deleted the ahe/help-centre-sitemap branch January 24, 2024 15:17
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @andrewHEguardian 9 minutes and 56 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants