Skip to content

Commit

Permalink
Added dev place page experiment setup (datacommonsorg#4773)
Browse files Browse the repository at this point in the history
Added experiment setup for dev place page

* Moved the dev place landing page from `/dev-place/<dcid>` to
`/place/<dcid>`
* Enable the experiment for a select list of 5 US states and 24
countries.
* Added optional URL parameter to force rendering the dev place page for
any place page. Enable by setting `?force_dev_places=true` (e.g.,
http://localhost:8080/place/geoId/06?force_dev_places=true)
* Added optional URL param to force rendering of the classic page page
(for places in the experiment group). Enable by setting
`?disable_dev_places=true`
* TODO: Add a sample of world cities to experiment group


To generate the experiment groups, I ran:

```
# >>> import random
# >>> DEV_PLACE_ALL_COUNTRY_DCIDS = ["country/ABW","country/AFG","country/AGO","country/ALB","country/AND","country/ANT","country/ARE","country/ARG","country/ARM","country/ASM","country/ATF","country/ATG","country/AUS","country/AUT","country/AZE","country/BDI","country/BEL","country/BEN","country/BES","country/BFA","country/BGD","country/BGR","country/BHR","country/BHS","country/BIH","country/BLM","country/BLR","country/BLZ","country/BMU","country/BOL","country/BRA","country/BRB","country/BRN","country/BTN","country/BWA","country/CAF","country/CAN","country/CCK","country/CHE","country/CHL","country/CHN","country/CIV","country/CMR","country/COD","country/COG","country/COK","country/COL","country/COM","country/CPV","country/CRI","country/CUB","country/CUW","country/CXR","country/CYM","country/CYP","country/CZE","country/DEU","country/DJI","country/DMA","country/DNK","country/DOM","country/DZA","country/ECU","country/EGY","country/ERI","country/ESH","country/ESP","country/EST","country/ETH","country/FIN","country/FJI","country/FLK","country/FRA","country/FRO","country/FSM","country/FXX","country/GAB","country/GBR","country/GEO","country/GGY","country/GHA","country/GIB","country/GIN","country/GLP","country/GMB","country/GNB","country/GNQ","country/GRC","country/GRD","country/GRL","country/GTM","country/GUF","country/GUM","country/GUY","country/HKG","country/HMD","country/HND","country/HRV","country/HTI","country/HUN","country/IDN","country/IMN","country/IND","country/IRL","country/IRN","country/IRQ","country/ISL","country/ISR","country/ITA","country/JAM","country/JEY","country/JOR","country/JPN","country/KAZ","country/KEN","country/KGZ","country/KHM","country/KIR","country/KNA","country/KOR","country/KWT","country/LAO","country/LBN","country/LBR","country/LBY","country/LCA","country/LIE","country/LKA","country/LSO","country/LTU","country/LUX","country/LVA","country/MAC","country/MAF","country/MAR","country/MCO","country/MDA","country/MDG","country/MDV","country/MEX","country/MHL","country/MKD","country/MLI","country/MLT","country/MMR","country/MNE","country/MNG","country/MNP","country/MOZ","country/MRT","country/MSR","country/MTQ","country/MUS","country/MWI","country/MYS","country/MYT","country/NAM","country/NCL","country/NER","country/NFK","country/NGA","country/NIC","country/NIU","country/NLD","country/NOR","country/NPL","country/NRU","country/NZL","country/OMN","country/PAK","country/PAN","country/PCN","country/PER","country/PHL","country/PLW","country/PNG","country/POL","country/PRI","country/PRK","country/PRT","country/PRY","country/PSE","country/PYF","country/QAT","country/REU","country/ROU","country/RUS","country/RWA","country/SAU","country/SDN","country/SEN","country/SGP","country/SHN","country/SLB","country/SLE","country/SLV","country/SMR","country/SOM","country/SPM","country/SRB","country/SSD","country/STP","country/SUR","country/SVK","country/SVN","country/SWE","country/SWZ","country/SXM","country/SYC","country/SYR","country/TCA","country/TCD","country/TGO","country/THA","country/TJK","country/TKL","country/TKM","country/TLS","country/TON","country/TTO","country/TUN","country/TUR","country/TUV","country/TWN","country/TZA","country/UGA","country/UKR","country/UMI","country/URY","country/USA","country/UZB","country/VAT","country/VCT","country/VEN","country/VGB","country/VIR","country/VNM","country/VUT","country/WLF","country/WSM","country/XKS","country/YEM","country/YUG","country/ZAF","country/ZMB","country/ZWE"]
# >>> DEV_PLACE_ALL_US_STATE_DCIDS = ["geoId/01","geoId/02","geoId/04","geoId/05","geoId/06","geoId/08","geoId/09","geoId/10","geoId/11","geoId/12","geoId/13","geoId/15","geoId/16","geoId/17","geoId/18","geoId/19","geoId/20","geoId/21","geoId/22","geoId/23","geoId/24","geoId/25","geoId/26","geoId/27","geoId/28","geoId/29","geoId/30","geoId/31","geoId/32","geoId/33","geoId/34","geoId/35","geoId/36","geoId/37","geoId/38","geoId/39","geoId/40","geoId/41","geoId/42","geoId/44","geoId/45","geoId/46","geoId/47","geoId/48","geoId/49","geoId/50","geoId/51","geoId/53","geoId/54","geoId/55","geoId/56","geoId/72"]
# >>> DEV_PLACE_EXPERIMENT_COUNTRY_DCIDS = random.sample(DEV_PLACE_ALL_COUNTRY_DCIDS, int(len(DEV_PLACE_ALL_COUNTRY_DCIDS) * 0.1))
# >>> DEV_PLACE_EXPERIMENT_US_STATE_DCIDS = random.sample(DEV_PLACE_ALL_US_STATE_DCIDS, int(len(DEV_PLACE_ALL_US_STATE_DCIDS) * 0.1))
```
  • Loading branch information
dwnoble authored Dec 6, 2024
1 parent 1124975 commit a38ca26
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 105 deletions.
3 changes: 0 additions & 3 deletions server/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ def register_routes_common(app):
from server.routes.dev_place import api as dev_place_api
app.register_blueprint(dev_place_api.bp)

from server.routes.dev_place import html as dev_place_html
app.register_blueprint(dev_place_html.bp)

from server.routes.ranking import html as ranking_html
app.register_blueprint(ranking_html.bp)

Expand Down
56 changes: 0 additions & 56 deletions server/routes/dev_place/html.py

This file was deleted.

2 changes: 1 addition & 1 deletion server/routes/dev_place/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def get_place_html_link(place_dcid: str, place_name: str) -> str:
Returns:
An html anchor tag linking to a place page.
"""
url = flask.url_for('dev_place.dev_place', place_dcid=place_dcid)
url = flask.url_for('place.place', place_dcid=place_dcid)
return f'<a href="{url}">{place_name}</a>'


Expand Down
68 changes: 68 additions & 0 deletions server/routes/place/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,19 @@
import os
import re
import time
from typing import List, Set

import flask
from flask import current_app
from flask import g
from flask_babel import gettext
from multidict import MultiDict

from server.lib.cache import cache
from server.lib.config import GLOBAL_CONFIG_BUCKET
from server.lib.i18n import AVAILABLE_LANGUAGES
from server.lib.i18n import DEFAULT_LOCALE
import server.routes.dev_place.utils as utils
import server.routes.shared_api.place as place_api
import shared.lib.gcs as gcs
from shared.lib.place_summaries import get_shard_filename_by_dcid
Expand Down Expand Up @@ -262,10 +266,52 @@ def is_seo_experiment_enabled(place_dcid: str, category: str,
return False


# Dev place page experiment groups for countries and US states
# Calculated offline using instructions here:
# https://github.com/datacommonsorg/website/pull/4773
DEV_PLACE_EXPERIMENT_COUNTRY_DCIDS: List[str] = [
'country/TLS', 'country/HUN', 'country/VEN', 'country/JAM', 'country/RWA',
'country/GGY', 'country/NGA', 'country/COD', 'country/COG', 'country/SVN',
'country/LSO', 'country/LBN', 'country/LCA', 'country/NFK', 'country/TTO',
'country/SGP', 'country/PYF', 'country/PRK', 'country/RUS', 'country/LVA',
'country/SUR', 'country/PRY', 'country/IND', 'country/MDV'
]
DEV_PLACE_EXPERIMENT_US_STATE_DCIDS: List[str] = [
'geoId/56', 'geoId/04', 'geoId/41', 'geoId/20', 'geoId/37'
]
DEV_PLACE_EXPERIMENT_DCIDS: Set[str] = set(DEV_PLACE_EXPERIMENT_COUNTRY_DCIDS +
DEV_PLACE_EXPERIMENT_US_STATE_DCIDS)


def is_dev_place_experiment_enabled(place_dcid: str, locale: str,
request_args: MultiDict[str, str]) -> bool:
"""Determine if dev place experiment should be enabled for the page"""
# Disable dev place experiment for non-dev environments
# TODO(dwnoble): Remove this before prod release
if os.environ.get('FLASK_ENV') not in [
'local', 'autopush', 'dev', 'webdriver'
] or not place_dcid:
return False

# Force dev place experiment for testing
if request_args.get("force_dev_places") == "true":
return True
# Disable dev place experiment for testing
if request_args.get("disable_dev_places") == "true":
return False

# Experiment is enabled for English pages for countries and US states in the experiment group
if locale == 'en' and place_dcid in DEV_PLACE_EXPERIMENT_DCIDS:
return True
return False


@bp.route('', strict_slashes=False)
@bp.route('/<path:place_dcid>')
@cache.cached(query_string=True)
def place(place_dcid=None):
if is_dev_place_experiment_enabled(place_dcid, g.locale, flask.request.args):
return dev_place(place_dcid=place_dcid)
redirect_args = dict(flask.request.args)

# Strip trailing slashes from place dcids
Expand Down Expand Up @@ -418,3 +464,25 @@ def place_landing(error_msg=''):
error_msg=error_msg,
place_names=place_names,
maps_api_key=current_app.config['MAPS_API_KEY'])


# Dev place experiment route
def dev_place(place_dcid=None):
place_type_with_parent_places_links = utils.get_place_type_with_parent_places_links(
place_dcid)
place_names = place_api.get_i18n_name([place_dcid]) or {}
place_name = place_names.get(place_dcid, place_dcid)
# Place summaries are currently only supported in English
if g.locale == DEFAULT_LOCALE:
place_summary = get_place_summaries(place_dcid).get(place_dcid,
{}).get("summary", "")
else:
place_summary = ""

return flask.render_template(
'dev_place.html',
maps_api_key=current_app.config['MAPS_API_KEY'],
place_dcid=place_dcid,
place_name=place_name,
place_type_with_parent_places_links=place_type_with_parent_places_links,
place_summary=place_summary)
14 changes: 13 additions & 1 deletion server/webdriver/shared_tests/place_explorer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,4 +330,16 @@ def test_ranking_chart_redirect_link(self):

# Check the title text
page_title = self.driver.find_element(By.ID, 'place-name').text
self.assertEqual(page_title, place_name_text)
self.assertEqual(page_title, place_name_text)

def test_dev_place_overview_california(self):
"""Ensure experimental dev place page content loads"""
self.driver.get(self.url_ + '/place/geoId/06?force_dev_places=true')

# For the dev place page, the related places callout is under the
# .related-places-callout div.
related_places_callout_el_present = EC.presence_of_element_located(
(By.CLASS_NAME, 'related-places-callout'))
related_places_callout_el = WebDriverWait(
self.driver, self.TIMEOUT_SEC).until(related_places_callout_el_present)
self.assertEqual(related_places_callout_el.text, 'Places in California')
34 changes: 0 additions & 34 deletions server/webdriver/tests/dev_place_test.py

This file was deleted.

2 changes: 1 addition & 1 deletion static/css/place/dev_place_page.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

/** Styles for /dev-place/<dcid> pages */
/** Styles for dev /place/<dcid> pages */

@use "../base.scss";
@use "../explore";
Expand Down
39 changes: 30 additions & 9 deletions static/js/place/dev_place_main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,11 @@ const PlaceHeader = (props: {
* @returns Navigation component with topic tabs
*/
const PlaceTopicTabs = ({
forceDevPlaces,
category,
place,
}: {
forceDevPlaces: boolean;
category: string;
place: NamedTypedPlace;
}) => {
Expand All @@ -209,7 +211,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Overview" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}`}
href={`/place/${place.dcid}${
forceDevPlaces ? "?force_dev_places=true" : ""
}`}
>
Overview
</a>
Expand All @@ -219,7 +223,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Economics" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}?category=Economics`}
href={`/place/${place.dcid}?category=Economics${
forceDevPlaces ? "&force_dev_places=true" : ""
}`}
>
Economics
</a>
Expand All @@ -229,7 +235,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Health" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}?category=Health`}
href={`/place/${place.dcid}?category=Health${
forceDevPlaces ? "&force_dev_places=true" : ""
}`}
>
Health
</a>
Expand All @@ -239,7 +247,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Equity" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}?category=Equity`}
href={`/place/${place.dcid}?category=Equity${
forceDevPlaces ? "&force_dev_places=true" : ""
}`}
>
Equity
</a>
Expand All @@ -249,7 +259,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Demographics" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}?category=Demographics`}
href={`/place/${place.dcid}?category=Demographics${
forceDevPlaces ? "&force_dev_places=true" : ""
}`}
>
Demographics
</a>
Expand All @@ -259,7 +271,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Environment" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}?category=Environment`}
href={`/place/${place.dcid}?category=Environment${
forceDevPlaces ? "&force_dev_places=true" : ""
}`}
>
Environment
</a>
Expand All @@ -269,7 +283,9 @@ const PlaceTopicTabs = ({
className={`item-list-text ${
category === "Energy" ? "selected" : ""
}`}
href={`/dev-place/${place.dcid}?category=Energy`}
href={`/place/${place.dcid}?category=Energy${
forceDevPlaces ? "&force_dev_places=true" : ""
}`}
>
Energy
</a>
Expand Down Expand Up @@ -406,7 +422,7 @@ const RelatedPlaces = (props: {
<div className="item-list-inner">
{childPlaces.map((place) => (
<div key={place.dcid} className="item-list-item">
<a className="item-list-text" href={`/dev-place/${place.dcid}`}>
<a className="item-list-text" href={`/place/${place.dcid}`}>
{place.name}
</a>
</div>
Expand Down Expand Up @@ -467,6 +483,7 @@ export const DevPlaceMain = () => {

const urlParams = new URLSearchParams(window.location.search);
const category = urlParams.get("category") || "Overview";
const forceDevPlaces = urlParams.get("force_dev_places") === "true";

/**
* On initial load, get place metadata from the page's metadata element
Expand Down Expand Up @@ -529,7 +546,11 @@ export const DevPlaceMain = () => {
place={place}
placeSubheader={placeSubheader}
/>
<PlaceTopicTabs category={category} place={place} />
<PlaceTopicTabs
category={category}
place={place}
forceDevPlaces={forceDevPlaces}
/>
<PlaceOverview
place={place}
placeSummary={placeSummary}
Expand Down

0 comments on commit a38ca26

Please sign in to comment.