Skip to content

Commit

Permalink
removed trailing slash from place page URL (#3900)
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
dwnoble authored and juliawu committed Jan 25, 2024
1 parent 48fb61a commit 5800c26
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
13 changes: 11 additions & 2 deletions server/routes/place/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,32 @@


@bp.route('', strict_slashes=False)
@bp.route('/<path:place_dcid>/', strict_slashes=False)
@bp.route('/<path:place_dcid>')
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]
should_redirect = True

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:
Expand Down
3 changes: 3 additions & 0 deletions server/tests/routes/place_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ def test_place(self, mock_get_place_type, mock_get_i18n_name):
assert b"<title>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

Expand Down
4 changes: 2 additions & 2 deletions server/webdriver/tests/place_landing_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 5800c26

Please sign in to comment.