diff --git a/app/routes/thematic/geoencoder.py b/app/routes/thematic/geoencoder.py index 8dcb4dc8..4e5aa5ba 100644 --- a/app/routes/thematic/geoencoder.py +++ b/app/routes/thematic/geoencoder.py @@ -116,6 +116,9 @@ def sanitize_names( region: str | None, subregion: str | None, ) -> List[str | None]: + """Turn any empty strings into Nones, enforce the admin level hierarchy, + and optionally unaccent names. + """ names = [] if subregion and not region: @@ -137,6 +140,9 @@ def sanitize_names( def determine_admin_level( country: str | None, region: str | None, subregion: str | None ) -> str: + """Infer the native admin level of a request based on the presence of + non-empty fields + """ if subregion: return "2" elif region: @@ -182,7 +188,11 @@ async def version_is_valid( dataset: str, version: str, ) -> None: - """ """ + """Validate a version string for a given dataset.""" + # Note: At some point I intend to change the version validator to + # use messaging like this. However, that is out of scope for the + # current ticket, and I want to see what people think, so I'll + # keep it here for now. if re.match(VERSION_REGEX, version) is None: raise HTTPException( status_code=400, @@ -200,6 +210,7 @@ async def version_is_valid( status_code=400, detail=( "Version not found. Existing versions for this dataset " - f"include {await get_version_names(dataset)}" + f"include {[v[0] for v in await get_version_names(dataset)]}" + # FIXME: Maybe change get_version_names to do unpacking? ^ ), ) diff --git a/tests_v2/unit/app/routes/thematic/geoencoder/test_geoencoder.py b/tests_v2/unit/app/routes/thematic/geoencoder/test_geoencoder.py index 6a2ec249..14e30b99 100644 --- a/tests_v2/unit/app/routes/thematic/geoencoder/test_geoencoder.py +++ b/tests_v2/unit/app/routes/thematic/geoencoder/test_geoencoder.py @@ -4,6 +4,7 @@ from httpx import AsyncClient from app.models.pydantic.geostore import GeostoreCommon +from app.routes.datasets.versions import get_version from app.routes.thematic import geoencoder from app.routes.thematic.geoencoder import _admin_boundary_lookup_sql @@ -75,7 +76,7 @@ async def test_geoencoder_invalid_version_pattern(async_client: AsyncClient) -> resp = await async_client.get("/thematic/geoencode", params=params) - assert resp.json().get("message", {}).startswith("Invalid version") + assert resp.json().get("message", {}).startswith("Invalid version name") assert resp.status_code == 400 @@ -89,6 +90,22 @@ async def test_geoencoder_nonexistant_version(async_client: AsyncClient) -> None assert resp.status_code == 400 +@pytest.mark.asyncio +async def test_geoencoder_nonexistant_version_lists_existing( + async_client: AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + async def mock_get_version_names(dataset: str): + return [("fdkj",), ("dslj",), ("kfj",)] + + monkeypatch.setattr(geoencoder, "get_version_names", mock_get_version_names) + + params = {"country": "Canada", "admin_version": "v4.0"} + resp = await async_client.get("/thematic/geoencode", params=params) + + assert resp.json().get("message", {}).endswith("['fdkj', 'dslj', 'kfj']") + assert resp.status_code == 400 + + @pytest.mark.asyncio async def test_geoencoder_bad_boundary_source(async_client: AsyncClient) -> None: params = {