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

Update some US edition navlinks so that they don't redirect #27062

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

ioannakok
Copy link
Contributor

@ioannakok ioannakok commented Apr 18, 2024

Closes guardian/dotcom-rendering#11192

What does this change

These URLs in the US subnav --

https://www.theguardian.com/business
https://www.theguardian.com/environment
https://www.theguardian.com/soccer
https://www.theguardian.com/technology
https://www.theguardian.com/wellness

Updated to their /us/ versions --

https://www.theguardian.com/us/business
https://www.theguardian.com/us/environment
https://www.theguardian.com/us/soccer
https://www.theguardian.com/us/technology
https://www.theguardian.com/us/wellness

Why?

This is a temporary change requested by the SEO team in the US. It's part of a larger work to improve SEO. In the future we would like to make changes to the navbar so that it doesn't contain links that redirect:

For more details see SEO analysis here

Screenshots

Previously we were redirecting:

image

image

After the change we won't:

image

Checklist

  • Tested locally, and on CODE if necessary

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This is a temporary change requested by the SEO team in the US. It's part of a larger work to redesign the navbar so that it doesn't contain links that redirect: guardian/dotcom-rendering#11193. Until we complete this the team has asked us to update only the following links:

These URLs in the US subnav --

https://www.theguardian.com/business
https://www.theguardian.com/environment
https://www.theguardian.com/soccer
https://www.theguardian.com/technology
https://www.theguardian.com/wellness

Updated to their /us/ versions --

https://www.theguardian.com/us/business
https://www.theguardian.com/us/environment
https://www.theguardian.com/us/soccer
https://www.theguardian.com/us/technology
https://www.theguardian.com/us/wellness
@ioannakok ioannakok marked this pull request as ready for review April 18, 2024 14:33
@ioannakok ioannakok requested a review from a team as a code owner April 18, 2024 14:33
Co-authored-by: Ioanna Kokkini <[email protected]>
Copy link
Member

@arelra arelra left a comment

Choose a reason for hiding this comment

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

🔗

@arelra arelra merged commit 79ee005 into main Apr 22, 2024
2 checks passed
@arelra arelra deleted the update-us-fronts-to-not-redirect branch April 22, 2024 14:24
@prout-bot
Copy link
Collaborator

Seen on FRONTS-PROD, ADMIN-PROD (created by @ioannakok and merged by @arelra 16 minutes and 11 seconds ago)

@mxdvl
Copy link
Contributor

mxdvl commented Apr 22, 2024

Would it be worth adding a script to ensure that this always remains the case?

const links = [...document.querySelectorAll("nav a")].map(({ href }) => href);
for(const link of links) {
  const { redirected } = await fetch(link, { header: { "Cookie": "GU_EDITION=US" } });
  if(redirected) console.warn("The following link was redirected:", link)
}
Example of warning on redirects

@arelra
Copy link
Member

arelra commented Apr 22, 2024

Would it be worth adding a script to ensure that this always remains the case?

const links = [...document.querySelectorAll("nav a")].map(({ href }) => href);
for(const link of links) {
  const { redirected } = await fetch(link, { header: { "Cookie": "GU_EDITION=US" } });
  if(redirected) console.warn("The following link was redirected:", link)
}
Example of warning on redirects

Yes its a good idea to enforce this behaviour. Currently the nav is generated server side by Frontend and the existing tests check that the nav links are as expected from the implementation logic.

A complexity is that not all nav links redirect to their 'editionalised' pages (eg. https://www.theguardian.com/science), which would mean a script would have to know which links need to be redirected. I assume by script you mean an integration/e2e test?

@arelra
Copy link
Member

arelra commented Apr 22, 2024

Would it be worth adding a script to ensure that this always remains the case?

const links = [...document.querySelectorAll("nav a")].map(({ href }) => href);
for(const link of links) {
  const { redirected } = await fetch(link, { header: { "Cookie": "GU_EDITION=US" } });
  if(redirected) console.warn("The following link was redirected:", link)
}

Ah yes I see now yes you're right. We can warn/fail if there is a redirect as we want to move to a situation where there aren't any for a particular edition.

@mxdvl
Copy link
Contributor

mxdvl commented Apr 22, 2024

Yes, from my testing the trick is to pass the GU_EDITION cookie.

await fetch('https://www.theguardian.com/business', { headers: { "Cookie": "GU_EDITION=US" } })
  .then(r => r.url) // "https://www.theguardian.com/us/business"

await fetch('https://www.theguardian.com/business', { headers: { "Cookie": "GU_EDITION=UK" } })
  .then(r => r.url) // "https://www.theguardian.com/uk/business"

await fetch('https://www.theguardian.com/business', { headers: { "Cookie": "GU_EDITION=AU" } })
  .then(r => r.url) // "https://www.theguardian.com/au/business"

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.

Update some URLs at the US navbar so that they don't redirect
4 participants