Skip to content

Commit

Permalink
Add docstrings, add a test, and slightly improve error message on non…
Browse files Browse the repository at this point in the history
…existing version
  • Loading branch information
dmannarino committed Dec 27, 2024
1 parent 2d979a2 commit 0c6d541
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
15 changes: 13 additions & 2 deletions app/routes/thematic/geoencoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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,
Expand All @@ -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? ^
),
)
19 changes: 18 additions & 1 deletion tests_v2/unit/app/routes/thematic/geoencoder/test_geoencoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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


Expand All @@ -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 = {
Expand Down

0 comments on commit 0c6d541

Please sign in to comment.