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

Add site redirects for recent chart redirects #4568

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

rakyi
Copy link
Contributor

@rakyi rakyi commented Feb 17, 2025

We take Grapher redirects into consideration only when a route returns 404 and Cloudflare caches assets up to a week in the CDN, so it's possible to still get the old page after a redirect is created for it.

Authors are confused why they don't see chart redirects working after they push changes. It also means different users can see different charts when clicking on the same link, depending on whether the CDN data center that handles their request had the old page cached. I tested this case with Pablo A.

We take a similar approach as for deleting articles and add redirects in the special _redirects file - that Cloudflare always takes into consideration - for Grapher redirects that are less than a week old.

We take Grapher redirects into consideration only when a route returns
404 and Cloudflare caches assets up to a week in the CDN, so it's
possible to still get the old page after a redirect is created for it.

Authors are confused why they don't see chart redirects working after
they push changes. It also means different users can see different
charts when clicking on the same link, depending on whether the CDN data
center that handles their request had the old page cached. I tested this
case with Pablo A.

We take a similar approach as for deleting articles and add redirects in
the special `_redirects` file - that Cloudflare always takes into
consideration - for Grapher redirects that are less than a week old.
@owidbot
Copy link
Contributor

owidbot commented Feb 17, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-temporary-site-redirects-for

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2025-02-17 16:14:43 UTC
Execution time: 1.18 seconds

Copy link
Member

@marcelgerber marcelgerber left a comment

Choose a reason for hiding this comment

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

Nice! Good idea, and I think this'll work well.

The only other consideration (which is harder, though) is about deleted Grapher pages. For gdocs, we're putting tombstones there, and are adding temporary redirects. Should we be doing a similar thing for grapher pages, too?

@rakyi rakyi merged commit 2a251cd into master Feb 18, 2025
40 checks passed
@rakyi rakyi deleted the temporary-site-redirects-for-charts branch February 18, 2025 08:35
@rakyi
Copy link
Contributor Author

rakyi commented Feb 18, 2025

Good point, although I'd like to know if it's a problem in practice. Do you have a sense of how often we delete a chart without creating a redirect for it?

If we are not sure, I'd wait until we encounter it in practice before doing anything about it.

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