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

Cleanup: Remove explorer url migrations #3924

Open
marcelgerber opened this issue Sep 4, 2024 · 1 comment
Open

Cleanup: Remove explorer url migrations #3924

marcelgerber opened this issue Sep 4, 2024 · 1 comment

Comments

@marcelgerber
Copy link
Member

marcelgerber commented Sep 4, 2024

We have a bunch of complex logic that solely exists to rewrite explorer URLs, living in the https://github.com/owid/owid-grapher/tree/db6e1333ed1b96ab82d94bf0ff17df6f049561a5/explorer/urlMigrations directory and https://github.com/owid/owid-grapher/blob/398ac2b99017c9596b2da95b9720288e41d6b321/baker/replaceExplorerRedirects.ts.

The main purpose of these is to place HTML files that enable redirecting old (pre-2021) explorer URLs like /coronavirus-data-explorer?casesMetric=true to the corresponding views in the new explorer - and in these redirects also capturing the client-side query params and migrating them too (this is why these migrations are running on the client side).

As stated above, these migrations have been introduced in early 2021.
We usually want to take great to not break old URLs, and in this we especially also want to take care of the large number of embeds that we did get in news articles in 2020.
However, it has been more than 3 years now, and it seems like a maintenance burden to me.

We broke these recently

Also, we broke these redirects recently, when we moved from /explorers/coronavirus-data-explorer to /explorers/covid.
We set up a large number of redirects, including /explorers/coronavirus-data-explorer -> /explorers/covid and /coronavirus-data-explorer -> /explorers/covid.
The two sources here both had special redirects handling that we have disabled as such.

This means that, right now, https://ourworldindata.org/coronavirus-data-explorer?casesMetric=true redirects to the Covid explorer, but not to the correct view.

Obviously, one option is to fix these, and it probably wouldn't be too hard.
I feel like we can soft-break old embeds (they'll still show something, just not the correct view, which may be very confusing). But I guess this depends on how much we want to guarantee an URL to continue working into eternity.

Additional stuff

I also found https://github.com/owid/owid-grapher/blob/22ef530668c83ccb57f22a13659974a40b0210de/explorerAdminServer/ExplorerRedirects.ts.
These we should probably all convert to "normal" redirects in the redirects table, if they're not already present.

Example:

One example I could find is a news article embed here, from May 2020: https://www.eldiario.es/internacional/coronavirus-suecia_1_5963547.html

They wanted to embed total deaths in the Nordics as a chart, but instead they get a map of excess deaths (our default view).

@marcelgerber
Copy link
Member Author

We talked a bit about this just now on the data viz call, and decided that it's worth removing this code now.

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

No branches or pull requests

1 participant