From 5800c262b472e949f25707c6bdfc5fab212d6e26 Mon Sep 17 00:00:00 2001 From: Dan Noble Date: Tue, 23 Jan 2024 17:06:01 -0500 Subject: [PATCH] removed trailing slash from place page URL (#3900) Verified these flows work: ``` * GET http://localhost:8080/place/geoId/06 returns 200 * GET http://localhost:8080/place/geoId/06/ returns 301, redirecting to http://localhost:8080/place/geoId/06 * GET http://localhost:8080/place/geoId/06?category=Housing returns 201 * GET http://localhost:8080/place/geoId/06/?category=Housing returns 301, redirecting to http://localhost:8080/place/geoId/06?category=Housing ``` --- server/routes/place/html.py | 13 +++++++++++-- server/tests/routes/place_test.py | 3 +++ server/webdriver/tests/place_landing_test.py | 4 ++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/server/routes/place/html.py b/server/routes/place/html.py index eb5b95eaef..eeb1ebaaf9 100644 --- a/server/routes/place/html.py +++ b/server/routes/place/html.py @@ -30,15 +30,23 @@ @bp.route('', strict_slashes=False) -@bp.route('//', strict_slashes=False) +@bp.route('/') def place(place_dcid=None): redirect_args = dict(flask.request.args) + + # Strip trailing slashes from place dcids should_redirect = False + if place_dcid and place_dcid.endswith('/'): + place_dcid = place_dcid.rstrip('/') + should_redirect = True + + # Rename legacy "topic" request argument to "category" if 'topic' in flask.request.args: redirect_args['category'] = flask.request.args.get('topic', '') del redirect_args['topic'] should_redirect = True + # Rename legacy category request arguments category = redirect_args.get('category', None) if category in CATEGORY_REDIRECTS: redirect_args['category'] = CATEGORY_REDIRECTS[category] @@ -46,7 +54,8 @@ def place(place_dcid=None): if should_redirect: redirect_args['place_dcid'] = place_dcid - return flask.redirect(flask.url_for('place.place', **redirect_args)) + return flask.redirect(flask.url_for('place.place', **redirect_args), + code=301) dcid = flask.request.args.get('dcid', None) if dcid: diff --git a/server/tests/routes/place_test.py b/server/tests/routes/place_test.py index 5bde418080..69cbd29052 100644 --- a/server/tests/routes/place_test.py +++ b/server/tests/routes/place_test.py @@ -92,6 +92,9 @@ def test_place(self, mock_get_place_type, mock_get_i18n_name): assert b"California" in response.data response = app.test_client().get('/place/geoId/06/', follow_redirects=False) + assert response.status_code == 301 + + response = app.test_client().get('/place/geoId/06/', follow_redirects=True) assert response.status_code == 200 assert b"<title>California" in response.data diff --git a/server/webdriver/tests/place_landing_test.py b/server/webdriver/tests/place_landing_test.py index 5f35f90527..54c0666ed5 100644 --- a/server/webdriver/tests/place_landing_test.py +++ b/server/webdriver/tests/place_landing_test.py @@ -43,7 +43,7 @@ def test_place_landing_en(self): '//*[@id="body"]/ul[1]/li[3]/a[4]') self.assertEqual(kentucky.text, 'Kentucky') self.assertEqual(kentucky.get_attribute('href'), - self.url_ + '/place/geoId/21/') + self.url_ + '/place/geoId/21') median_income = self.driver.find_element( By.XPATH, '//*[@id="body"]/ul[2]/li[1]/strong') @@ -83,7 +83,7 @@ def test_place_landing_ru(self): '//*[@id="body"]/ul[1]/li[3]/a[4]') self.assertEqual(kentucky.text, 'Кентукки') self.assertEqual(kentucky.get_attribute('href'), - self.url_ + '/place/geoId/21/?hl=ru') + self.url_ + '/place/geoId/21?hl=ru') median_income = self.driver.find_element( By.XPATH, '//*[@id="body"]/ul[2]/li[1]/strong')