Add child places for all allowed place types and sort them (#4824)

Some places did not have a name which caused an error when we tried to
get the name property. See
https://autopush.datacommons.org/browser/nuts/LVZZ

Discussed whether to keep the restriction for related place charts on
only one child_place_type and we decided to drop the requirement. The
old place page didnt have it, and the new one would show significantly
changed child places. Some countries use EurostatNuts1 while others use
AA1. Now we show children of different type and dedupe them.

See Latvia:
Old Place Page: https://screenshot.googleplex.com/BjiMRDVEqQpXEZK
New Place Page before: https://screenshot.googleplex.com/3L6ot765jxM4AjT
New place page after: https://screenshot.googleplex.com/A4kBUEaGitVkum4

Sources tile no longer shown on load:
Before: https://screenshot.googleplex.com/8RGHgDKNTBQKoYD
After: https://screenshot.googleplex.com/9L6AA9XWVYgEgjW

server/routes/dev_place/api.py

@@ -82,7 +82,9 @@ def place_charts(place_dcid: str):
   place = place_utils.fetch_place(place_dcid, locale=g.locale)
 
   # Determine child place type
-  child_place_type = place_utils.get_child_place_type(place)
+  ordered_child_place_types = place_utils.get_child_place_types(place)
+  child_place_type = ordered_child_place_types[
+      0] if ordered_child_place_types else None
 
   # Retrieve available place page charts
   full_chart_config = copy.deepcopy(current_app.config['CHART_CONFIG'])
@@ -146,10 +148,23 @@ def related_places(place_dcid: str):
                                                             locale=g.locale)
   similar_place_dcids = place_utils.fetch_similar_place_dcids(place,
                                                               locale=g.locale)
-  child_place_type = place_utils.get_child_place_type(place)
-  child_place_dcids = place_utils.fetch_child_place_dcids(place,
-                                                          child_place_type,
-                                                          locale=g.locale)
+  ordered_child_place_types = place_utils.get_child_place_types(place)
+  primary_child_place_type = ordered_child_place_types[
+      0] if ordered_child_place_types else None
+
+  child_place_dcids = []
+  seen_dcids = set(
+  )  # Keep track of seen DCIDs to prevent dupes but keep ordering.
+
+  # TODO(gmechali): Refactor this into async calls.
+  for child_place_type in ordered_child_place_types:
+    for dcid in place_utils.fetch_child_place_dcids(place,
+                                                    child_place_type,
+                                                    locale=g.locale):
+      if dcid not in seen_dcids:
+        child_place_dcids.append(dcid)
+        seen_dcids.add(dcid)
+
   parent_places = place_utils.get_parent_places(place.dcid)
 
   # Fetch all place objects in one request to reduce latency (includes name and typeOf)
@@ -176,7 +191,7 @@ def related_places(place_dcid: str):
       if not all_place_by_dcid[dcid].dissolved
   ]
 
-  response = RelatedPlacesApiResponse(childPlaceType=child_place_type,
+  response = RelatedPlacesApiResponse(childPlaceType=primary_child_place_type,
                                       childPlaces=child_places,
                                       nearbyPlaces=nearby_places,
                                       place=place,

server/routes/dev_place/utils.py

@@ -66,8 +66,9 @@ def get_parent_places(dcid: str) -> List[Place]:
       dcid, [])
   all_parents = []
   for parent in parents_resp:
-    all_parents.append(
-        Place(dcid=parent['dcid'], name=parent['name'], types=parent['type']))
+    if 'type' in parent and 'name' in parent and 'type' in parent:
+      all_parents.append(
+          Place(dcid=parent['dcid'], name=parent['name'], types=parent['type']))
 
   return all_parents
 
@@ -366,24 +367,24 @@ def chart_config_to_overview_charts(chart_config, child_place_type: str):
 ])
 
 
-def get_child_place_type(place: Place) -> str | None:
+def get_child_place_types(place: Place) -> list[str]:
   """
-  Determines the primary child place type for a given place.
+  Determines the child place types for a given place.
 
   This function uses custom matching rules and fallback hierarchies to decide
-  the most relevant child place type for a given place.
+  the ordered child place types for a given place.
 
   Args:
       place (Place): The place object to analyze.
 
   Returns:
-      str | None: The primary child place type for the given place, or None if no match is found.
+      list[str]: The child place types for the given place, or empty list if no match is found.
   """
   # Attempt to directly match a child place type using the custom expressions.
   for f in PLACE_MATCH_EXPRESSIONS:
     matched_child_place_type = f(place)
     if matched_child_place_type:
-      return matched_child_place_type
+      return [matched_child_place_type]
 
   # Fetch all possible child places for the given place using its containment property.
   raw_property_values_response = fetch.raw_property_values(
@@ -401,13 +402,17 @@ def get_child_place_type(place: Place) -> str | None:
   for raw_property_value in raw_property_values_response.get(place.dcid, []):
     child_place_types.update(raw_property_value.get("types", []))
 
+  child_place_type_candidates = []
   # Return the first child place type that matches the ordered list of possible child types.
-  for primary_place_type_candidate in ordered_child_place_types:
-    if primary_place_type_candidate in child_place_types:
-      return primary_place_type_candidate
+  for place_type_candidate in ordered_child_place_types:
+    if place_type_candidate in child_place_types:
+      child_place_type_candidates.append(place_type_candidate)
 
-  # If no matching child place type is found, return None.
-  return None
+  if child_place_type_candidates:
+    return child_place_type_candidates
+
+  # If no matching child place type is found, return empty.
+  return []
 
 
 def fetch_child_place_dcids(place: Place,

server/tests/routes/api/dev_place_test.py

@@ -181,10 +181,16 @@ def mock_parent_places_side_effect(dcids, include_admin_areas):
       self.assertEqual(len(response_json['nearbyPlaces']), 2)
       self.assertEqual(len(response_json['similarPlaces']), 2)
 
+      # Sort the childPlaces list by dcid
+      response_json['childPlaces'].sort(key=lambda x: x['dcid'])
+
       # Check contents of childPlaces
       self.assertEqual(response_json['childPlaces'][0]['dcid'], 'geoId/06')
       self.assertEqual(response_json['childPlaces'][0]['name'], 'California')
       self.assertEqual(response_json['childPlaces'][0]['types'], ['State'])
+      self.assertEqual(response_json['childPlaces'][1]['dcid'], 'geoId/07')
+      self.assertEqual(response_json['childPlaces'][1]['name'], 'New York')
+      self.assertEqual(response_json['childPlaces'][1]['types'], ['State'])
 
       # Check contents of nearbyPlaces
       self.assertEqual(response_json['nearbyPlaces'][0]['dcid'], 'country/CAN')
@@ -328,6 +334,9 @@ def mock_parent_places_side_effect(dcids, include_admin_areas):
       self.assertEqual(len(response_json['nearbyPlaces']), 2)
       self.assertEqual(len(response_json['similarPlaces']), 2)
 
+      # Sort the list to ensure the right values.
+      response_json['childPlaces'].sort(key=lambda x: x['dcid'])
+
       # Check contents of childPlaces
       # First child place should have Spanish name
       self.assertEqual(response_json['childPlaces'][0]['dcid'], 'geoId/06')
@@ -365,7 +374,7 @@ def test_earth_continent(self, mock_raw_property_values):
     """Test for a 'Place' type returning 'Continent'."""
     mock_raw_property_values.return_value = {}
     place = Place(dcid="Earth", name="World", types=["Place"])
-    self.assertEqual(place_utils.get_child_place_type(place), "Continent")
+    self.assertEqual(place_utils.get_child_place_types(place), ["Continent"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_usa_state(self, mock_raw_property_values):
@@ -374,7 +383,7 @@ def test_usa_state(self, mock_raw_property_values):
     place = Place(dcid="country/USA",
                   name="United States of America",
                   types=["Country"])
-    self.assertEqual(place_utils.get_child_place_type(place), "State")
+    self.assertEqual(place_utils.get_child_place_types(place), ["State"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_canada_administrative_area(self, mock_raw_property_values):
@@ -392,8 +401,8 @@ def test_canada_administrative_area(self, mock_raw_property_values):
         ]
     }
     place = Place(dcid="country/CAN", name="Canada", types=["Country"])
-    self.assertEqual(place_utils.get_child_place_type(place),
-                     "AdministrativeArea1")
+    self.assertEqual(place_utils.get_child_place_types(place),
+                     ["AdministrativeArea1"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_un_geo_region_country(self, mock_raw_property_values):
@@ -402,7 +411,7 @@ def test_un_geo_region_country(self, mock_raw_property_values):
     place = Place(dcid="geoRegion/123",
                   name="UN Geo Region",
                   types=["UNGeoRegion"])
-    self.assertEqual(place_utils.get_child_place_type(place), "Country")
+    self.assertEqual(place_utils.get_child_place_types(place), ["Country"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_us_state_county(self, mock_raw_property_values):
@@ -411,7 +420,7 @@ def test_us_state_county(self, mock_raw_property_values):
     place = Place(dcid="geoId/06",
                   name="California",
                   types=["State", "AdministrativeArea1"])
-    self.assertEqual(place_utils.get_child_place_type(place), "County")
+    self.assertEqual(place_utils.get_child_place_types(place), ["County"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_county_with_children(self, mock_raw_property_values):
@@ -429,7 +438,7 @@ def test_county_with_children(self, mock_raw_property_values):
         ]
     }
     place = Place(dcid="geoId/06001", name="Alameda County", types=["County"])
-    self.assertEqual(place_utils.get_child_place_type(place), "City")
+    self.assertEqual(place_utils.get_child_place_types(place), ["City", "Town"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_county_with_ordered_children(self, mock_raw_property_values):
@@ -447,7 +456,8 @@ def test_county_with_ordered_children(self, mock_raw_property_values):
         ]
     }
     place = Place(dcid="geoId/06001", name="Alameda County", types=["County"])
-    self.assertEqual(place_utils.get_child_place_type(place), "Town")
+    self.assertEqual(place_utils.get_child_place_types(place),
+                     ["Town", "Village"])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_no_matching_child_type(self, mock_raw_property_values):
@@ -459,7 +469,7 @@ def test_no_matching_child_type(self, mock_raw_property_values):
         },]
     }
     place = Place(dcid="geoId/06001", name="Alameda County", types=["County"])
-    self.assertIsNone(place_utils.get_child_place_type(place))
+    self.assertEqual(place_utils.get_child_place_types(place), [])
 
   @patch('server.routes.dev_place.utils.fetch.raw_property_values')
   def test_default_child_type(self, mock_raw_property_values):
@@ -471,5 +481,5 @@ def test_default_child_type(self, mock_raw_property_values):
         },]
     }
     place = Place(dcid="geoId/06001", name="Alameda County", types=["County"])
-    self.assertEqual(place_utils.get_child_place_type(place),
-                     "CensusZipCodeTabulationArea")
+    self.assertEqual(place_utils.get_child_place_types(place),
+                     ["CensusZipCodeTabulationArea"])

static/js/utils/tile_utils.tsx

@@ -378,22 +378,26 @@ export function TileSources(props: {
     );
   });
   return (
-    <div className="sources" {...{ part: "source" }}>
-      {sourcesJsx.length > 1 ? "Sources" : "Source"}:{" "}
-      <span {...{ part: "source-links" }}>{sourcesJsx}</span>
-      {statVarSpecs && statVarSpecs.length > 0 && (
-        <>
-          <span {...{ part: "source-separator" }}> • </span>
-          <span {...{ part: "source-show-metadata-link" }}>
-            <TileMetadataModal
-              apiRoot={props.apiRoot}
-              containerRef={props.containerRef}
-              statVarSpecs={statVarSpecs}
-            ></TileMetadataModal>
-          </span>
-        </>
+    <>
+      {sourcesJsx.length > 0 && (
+        <div className="sources" {...{ part: "source" }}>
+          {sourcesJsx.length > 1 ? "Sources" : "Source"}:{" "}
+          <span {...{ part: "source-links" }}>{sourcesJsx}</span>
+          {statVarSpecs && statVarSpecs.length > 0 && (
+            <>
+              <span {...{ part: "source-separator" }}> • </span>
+              <span {...{ part: "source-show-metadata-link" }}>
+                <TileMetadataModal
+                  apiRoot={props.apiRoot}
+                  containerRef={props.containerRef}
+                  statVarSpecs={statVarSpecs}
+                ></TileMetadataModal>
+              </span>
+            </>
+          )}
+        </div>
       )}
-    </div>
+    </>
   );
 }