diff --git a/api/views.py b/api/views.py index 1f4cbffc..b1541345 100644 --- a/api/views.py +++ b/api/views.py @@ -100,14 +100,13 @@ def handle_exception(self, exc): if isinstance(exc, InvalidFiltersException): return Response(exc.errors_list, status=400) else: - if isinstance(exc, Http404): - redirect_url = DataUrlRedirect.redirect_from(self.request) - if redirect_url: - return redirect(redirect_url) return super().handle_exception(exc) @check_api_version_redirect def get(self, *args, **kwargs): + redirect_url = DataUrlRedirect.redirect_from(self.request) + if redirect_url: + return redirect(redirect_url) return super().get(*args, **kwargs) diff --git a/core/models.py b/core/models.py index 54e311ea..38b7ed3f 100644 --- a/core/models.py +++ b/core/models.py @@ -2,8 +2,9 @@ import random import string from collections import OrderedDict, namedtuple +from copy import deepcopy from textwrap import dedent -from urllib.parse import urlparse +from urllib.parse import urlencode, urlparse import django.contrib.postgres.indexes as pg_indexes import django.db.models.indexes as django_indexes @@ -724,12 +725,42 @@ def redirect_map(self): def redirect_from(cls, request): path = request.path redirects = {} + fields_map = {} - for data_url_redirect in cls.objects.all().iterator(): - redirects.update(**data_url_redirect.redirect_map) + for r in cls.objects.all().iterator(): + redirects.update(**r.redirect_map) + + if r.field_prev != r.field_dest: + key = (r.dataset_dest, r.tablename_dest, r.field_prev) + fields_map[key] = r.field_dest # Order prefixes begining by the most complex ones + redirect_url = "" for url_prefix in sorted(redirects, reverse=True): if path.startswith(url_prefix): redirect_url_prefix = redirects[url_prefix] - return path.replace(url_prefix, redirect_url_prefix) + redirect_url = path.replace(url_prefix, redirect_url_prefix) + break + + qs = "" + query_params = getattr(request, "GET", {}) + redirect_qs = deepcopy(query_params) + if query_params: + url = redirect_url or path + for key in fields_map: + dataset, tablename, field_prev = key + has_field_update = all([f"/{dataset}/" in url, f"/{tablename}/" in url, field_prev in redirect_qs]) + if has_field_update: + value = redirect_qs[field_prev] + redirect_qs.pop(field_prev) + redirect_qs[fields_map[key]] = value + + qs = urlencode(redirect_qs) + + if not redirect_url and qs and set(query_params.keys()) != set(redirect_qs.keys()): + redirect_url = path + + if not redirect_url: + return "" + + return redirect_url + (f"?{qs}" if qs else "") diff --git a/core/tests/test_views.py b/core/tests/test_views.py index 0df2a59c..0c681589 100644 --- a/core/tests/test_views.py +++ b/core/tests/test_views.py @@ -255,3 +255,80 @@ def test_avoid_infinite_redirect_if_same_dataset_config_in_table(self): response = self.client.get(url) assert 404 == response.status_code + + def test_redirect_dataset_plus_field_name_redirect(self): + ds_redirect = self.dataset_redirects[0] + ds_redirect.tablename_prev = "caso2020" + ds_redirect.tablename_dest = "caso2020" + ds_redirect.field_prev = "query1" + ds_redirect.field_dest = "newquery" + ds_redirect.save() + + url = reverse("api-v1:dataset-table-data", args=[ds_redirect.dataset_prev, "caso2020"]) + redirect_url = ( + reverse("api-v1:dataset-table-data", args=[ds_redirect.dataset_dest, "caso2020"]) + + "?newquery=foo&query2=bar" + ) + response = self.client.get(url, data={"query1": "foo", "query2": "bar"}) + + self.assertRedirects(response, redirect_url, fetch_redirect_response=False) + + def test_redirect_dataset_plus_table_pluse_field_name_redirect(self): + ds_redirect = self.dataset_redirects[0] + ds_redirect.tablename_prev = "caso2019" + ds_redirect.tablename_dest = "caso2020" + ds_redirect.field_prev = "query1" + ds_redirect.field_dest = "newquery" + ds_redirect.save() + + url = reverse("api-v1:dataset-table-data", args=[ds_redirect.dataset_prev, "caso2019"]) + redirect_url = ( + reverse("api-v1:dataset-table-data", args=[ds_redirect.dataset_dest, "caso2020"]) + + "?newquery=foo&query2=bar" + ) + response = self.client.get(url, data={"query1": "foo", "query2": "bar"}) + + self.assertRedirects(response, redirect_url, fetch_redirect_response=False) + + def test_redirect_only_querystring_using_new_fieldname(self): + ds_redirect = self.dataset_redirects[0] + ds_redirect.dataset_dest = "dataset" + ds_redirect.dataset_prev = "dataset" + ds_redirect.tablename_prev = "caso2020" + ds_redirect.tablename_dest = "caso2020" + ds_redirect.field_prev = "query1" + ds_redirect.field_dest = "newquery" + ds_redirect.save() + baker.make("core.Table", dataset__slug="dataset", name="caso2020", dataset__show=True) + + # API endpoint + url = reverse("api-v1:dataset-table-data", args=["dataset", "caso2020"]) + redirect_url = reverse("api-v1:dataset-table-data", args=["dataset", "caso2020"]) + "?newquery=foo&query2=bar" + response = self.client.get(url, data={"query1": "foo", "query2": "bar"}) + self.assertRedirects(response, redirect_url, fetch_redirect_response=False) + + # Dataset table route + url = reverse("core:dataset-table-detail", args=["dataset", "caso2020"]) + redirect_url = reverse("core:dataset-table-detail", args=["dataset", "caso2020"]) + "?newquery=foo&query2=bar" + response = self.client.get(url, data={"query1": "foo", "query2": "bar"}) + self.assertRedirects(response, redirect_url, fetch_redirect_response=False) + + def test_avoid_infinite_redirect_if_same_fieldname_config(self): + ds_redirect = self.dataset_redirects[0] + ds_redirect.dataset_dest = "dataset" + ds_redirect.dataset_prev = "dataset" + ds_redirect.tablename_prev = "caso2020" + ds_redirect.tablename_dest = "caso2020" + ds_redirect.field_prev = "query1" + ds_redirect.field_dest = "query1" + ds_redirect.save() + + # API url + url = reverse("api-v1:dataset-table-data", args=[ds_redirect.dataset_prev, ds_redirect.tablename_prev]) + response = self.client.get(url, data={"query1": "foo", "query2": "bar"}) + self.assertEqual(404, response.status_code) + + # Dataset table route + url = reverse("core:dataset-table-detail", args=[ds_redirect.dataset_prev, ds_redirect.tablename_prev]) + response = self.client.get(url, data={"query1": "foo", "query2": "bar"}) + self.assertEqual(404, response.status_code) diff --git a/core/views.py b/core/views.py index 55a9c636..cd085334 100644 --- a/core/views.py +++ b/core/views.py @@ -137,6 +137,11 @@ def dataset_detail(request, slug, tablename=""): pass return render(request, "404.html", context, status=404) + # check for querystring fields that should be redirected + redirect_url = DataUrlRedirect.redirect_from(request) + if redirect_url: + return redirect(redirect_url) + querystring = request.GET.copy() page_number = querystring.pop("page", ["1"])[0].strip() or "1" items_per_page = querystring.pop("items", [str(settings.ROWS_PER_PAGE)])[0].strip() or str(settings.ROWS_PER_PAGE)