Skip to content

Commit

Permalink
Only show the Google Map widget on Place pages for places contained w…
Browse files Browse the repository at this point in the history
…ithin the US (#4771)

The Geometry coordinates for detailed Google maps boundaries is only
available for places contained in the USA (not even the USA country), so
we only want to show that component for the places contained within the
USA.
  • Loading branch information
gmechali authored Dec 6, 2024
1 parent bdef654 commit 732f6da
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 27 deletions.
1 change: 1 addition & 0 deletions packages/client/src/data_commons_web_client_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,5 @@ export interface RelatedPlacesApiResponse {
nearbyPlaces: Place[];
place: Place;
similarPlaces: Place[];
parentPlaces: Place[];
}
14 changes: 7 additions & 7 deletions server/routes/dev_place/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ def related_places(place_dcid: str):
child_place_dcids = place_utils.fetch_child_place_dcids(place,
child_place_type,
locale=g.locale)
parent_places = place_utils.get_parent_places(place.dcid)

# Fetch all place objects in one request to reduce latency (includes name and typeOf)
all_place_dcids = [
Expand All @@ -163,11 +164,10 @@ def related_places(place_dcid: str):
similar_places = [all_place_by_dcid[dcid] for dcid in similar_place_dcids]
child_places = [all_place_by_dcid[dcid] for dcid in child_place_dcids]

response = RelatedPlacesApiResponse(
childPlaceType=child_place_type,
childPlaces=child_places,
nearbyPlaces=nearby_places,
place=place,
similarPlaces=similar_places,
)
response = RelatedPlacesApiResponse(childPlaceType=child_place_type,
childPlaces=child_places,
nearbyPlaces=nearby_places,
place=place,
similarPlaces=similar_places,
parentPlaces=parent_places)
return jsonify(response)
1 change: 1 addition & 0 deletions server/routes/dev_place/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,4 @@ class RelatedPlacesApiResponse:
nearbyPlaces: List[Place]
place: Place
similarPlaces: List[Place]
parentPlaces: List[Place] = None
34 changes: 26 additions & 8 deletions server/routes/dev_place/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,25 @@ def get_place_html_link(place_dcid: str, place_name: str) -> str:
return f'<a href="{url}">{place_name}</a>'


def get_parent_places(dcid: str) -> List[Place]:
"""Gets the parent places for a given DCID
Args:
dcid: dcid of the place to get parents for
Returns:
A list of places that are all the parents of the given DCID.
"""
parents_resp = place_api.parent_places([dcid], include_admin_areas=True).get(
dcid, [])
all_parents = []
for parent in parents_resp:
all_parents.append(
Place(dcid=parent['dcid'], name=parent['name'], types=parent['type']))

return all_parents


def get_place_type_with_parent_places_links(dcid: str) -> str:
"""Get '<place type> in <parent places>' with html links for a given DCID
Expand All @@ -68,28 +87,27 @@ def get_place_type_with_parent_places_links(dcid: str) -> str:
place_type_display_name = place_api.get_place_type_i18n_name(place_type)

# Get parent places
all_parents = place_api.parent_places([dcid],
include_admin_areas=True).get(dcid, [])
all_parents = get_parent_places(dcid)

# Filter parents to only the types desired
parents_to_include = [
parent for parent in all_parents
if parent['type'] in PARENT_PLACE_TYPES_TO_HIGHLIGHT
if parent.types in PARENT_PLACE_TYPES_TO_HIGHLIGHT
]

# Fetch the localized names of the parents
parent_dcids = [parent['dcid'] for parent in parents_to_include]
parent_dcids = [parent.dcid for parent in parents_to_include]
localized_names = place_api.get_i18n_name(parent_dcids)
places_with_names = [
parent for parent in parents_to_include
if parent['dcid'] in localized_names.keys()
if parent.dcid in localized_names.keys()
]

# Generate <a href=place page url> tag for each parent place
links = [
get_place_html_link(place_dcid=parent['dcid'],
place_name=localized_names.get(parent['dcid']))
if parent['type'] != 'Continent' else localized_names.get(parent['dcid'])
get_place_html_link(place_dcid=parent.dcid,
place_name=localized_names.get(parent.dcid))
if parent.types != 'Continent' else localized_names.get(parent.dcid)
for parent in places_with_names
]

Expand Down
32 changes: 30 additions & 2 deletions server/tests/routes/api/dev_place_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,13 @@ def test_dev_place_charts(self, mock_obs_point_within, mock_obs_point,
self.assertEqual(5, len(response_json["charts"][2]["denominator"]))
self.assertEqual(5, len(response_json["charts"][3]["denominator"]))

@patch('server.routes.shared_api.place.parent_places')
@patch('server.routes.dev_place.utils.fetch.raw_property_values')
@patch('server.routes.dev_place.utils.fetch.multiple_property_values')
@patch('server.routes.dev_place.utils.fetch.descendent_places')
def test_related_places(self, mock_descendent_places,
mock_multiple_property_values,
mock_raw_property_values):
mock_raw_property_values, mock_parent_places):
"""Test the /api/dev-place/related-places endpoint. Mocks fetch.* and dc.* calls."""

with app.app_context():
Expand Down Expand Up @@ -139,6 +140,18 @@ def mock_raw_property_values_side_effect(nodes, prop, out):

mock_raw_property_values.side_effect = mock_raw_property_values_side_effect

# Define side effects for mock_parent_places_
def mock_parent_places_side_effect(dcids, include_admin_areas):
return {
'dcid': 'geoId/06',
'parents': [{
'type': 'Country',
'dcid': 'country/USA'
}]
}

mock_parent_places.side_effect = mock_parent_places_side_effect

# Send a GET request to the related-places endpoint
response = app.test_client().get(
f'/api/dev-place/related-places/{place_dcid}')
Expand All @@ -155,6 +168,7 @@ def mock_raw_property_values_side_effect(nodes, prop, out):
self.assertIn('nearbyPlaces', response_json)
self.assertIn('place', response_json)
self.assertIn('similarPlaces', response_json)
self.assertIn('parentPlaces', response_json)

# Check the place field
self.assertEqual(response_json['place']['dcid'], place_dcid)
Expand Down Expand Up @@ -186,12 +200,14 @@ def mock_raw_property_values_side_effect(nodes, prop, out):
# Check the 'childPlaceType' field
self.assertEqual(response_json['childPlaceType'], "State")

@patch('server.routes.shared_api.place.parent_places')
@patch('server.routes.dev_place.utils.fetch.raw_property_values')
@patch('server.routes.dev_place.utils.fetch.multiple_property_values')
@patch('server.routes.dev_place.utils.fetch.descendent_places')
def test_related_places_es_locale(self, mock_descendent_places,
mock_multiple_property_values,
mock_raw_property_values):
mock_raw_property_values,
mock_parent_places):
"""Test the /api/dev-place/related-places endpoint with 'es' locale."""
with app.app_context():
# Sample place_dcid
Expand Down Expand Up @@ -267,6 +283,18 @@ def mock_raw_property_values_side_effect(nodes, prop, out):

mock_raw_property_values.side_effect = mock_raw_property_values_side_effect

# Define side effects for mock_parent_places_
def mock_parent_places_side_effect(dcids, include_admin_areas):
return {
'dcid': 'geoId/06',
'parents': [{
'type': 'Country',
'dcid': 'country/USA'
}]
}

mock_parent_places.side_effect = mock_parent_places_side_effect

mock_descendent_places.return_value = {
'country/USA': ['geoId/06', 'geoId/07']
}
Expand Down
24 changes: 19 additions & 5 deletions static/js/place/dev_place_main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
defaultDataCommonsClient,
defaultDataCommonsWebClient,
} from "../utils/data_commons_client";
import { isPlaceContainedInUsa } from "./util";

/**
* Returns the stat var key for a chart.
Expand Down Expand Up @@ -355,8 +356,12 @@ const PlaceOverviewTable = (props: { placeDcid: string }) => {
const PlaceOverview = (props: {
place: NamedTypedPlace;
placeSummary: string;
parentPlaces: NamedTypedPlace[];
}) => {
const { place, placeSummary } = props;
const { place, placeSummary, parentPlaces } = props;
const isInUsa = isPlaceContainedInUsa(
parentPlaces.map((place) => place.dcid)
);
return (
<div className="place-overview">
<div className="place-icon">
Expand All @@ -365,10 +370,13 @@ const PlaceOverview = (props: {
<div className="place-name">{place.name}</div>
<div className="place-summary">{placeSummary}</div>
<div className="row place-map">
<div className="col-md-3">
<GoogleMap dcid={place.dcid}></GoogleMap>
</div>
{isInUsa && (
<div className="col-md-3">
<GoogleMap dcid={place.dcid}></GoogleMap>
</div>
)}
<div className="col-md-9">
{!isInUsa && <br></br>}
<PlaceOverviewTable placeDcid={place.dcid} />
</div>
</div>
Expand Down Expand Up @@ -454,6 +462,7 @@ export const DevPlaceMain = () => {
// Derived place data
const [childPlaceType, setChildPlaceType] = useState<string>();
const [childPlaces, setChildPlaces] = useState<NamedTypedPlace[]>([]);
const [parentPlaces, setParentPlaces] = useState<NamedTypedPlace[]>([]);
const [pageConfig, setPageConfig] = useState<SubjectPageConfig>();

const urlParams = new URLSearchParams(window.location.search);
Expand Down Expand Up @@ -505,6 +514,7 @@ export const DevPlaceMain = () => {
);
setChildPlaceType(relatedPlacesApiResponse.childPlaceType);
setChildPlaces(relatedPlacesApiResponse.childPlaces);
setParentPlaces(relatedPlacesApiResponse.parentPlaces);
setPageConfig(pageConfig);
})();
}, [place]);
Expand All @@ -520,7 +530,11 @@ export const DevPlaceMain = () => {
placeSubheader={placeSubheader}
/>
<PlaceTopicTabs category={category} place={place} />
<PlaceOverview place={place} placeSummary={placeSummary} />
<PlaceOverview
place={place}
placeSummary={placeSummary}
parentPlaces={parentPlaces}
/>
<RelatedPlaces place={place} childPlaces={childPlaces} />
{place && pageConfig && (
<PlaceCharts
Expand Down
14 changes: 9 additions & 5 deletions static/js/place/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,23 @@ import { intl } from "../i18n/i18n";
import { USA_PLACE_DCID } from "../shared/constants";

/**
* Given a list of parent places, return true if the place is in USA.
* Given a list of parent places, return true if one of them is the USA country DCID.
*/
export function isPlaceInUsa(dcid: string, parentPlaces: string[]): boolean {
if (dcid === USA_PLACE_DCID) {
return true;
}
export function isPlaceContainedInUsa(parentPlaces: string[]): boolean {
for (const parent of parentPlaces) {
if (parent === USA_PLACE_DCID) {
return true;
}
}
return false;
}
/**
* Given a DCID and list of parent places, returns whether this dcid is the USA, or contained in the USA.
*/
export function isPlaceInUsa(dcid: string, parentPlaces: string[]): boolean {
return dcid === USA_PLACE_DCID || isPlaceContainedInUsa(parentPlaces);
}

/**
* A set of place types to render a choropleth for.
*/
Expand Down

0 comments on commit 732f6da

Please sign in to comment.