diff --git a/_bmad-output/implementation-artifacts/1-4-location-resolution-icao-and-address.md b/_bmad-output/implementation-artifacts/1-4-location-resolution-icao-and-address.md index 4d20c61..7936e6f 100644 --- a/_bmad-output/implementation-artifacts/1-4-location-resolution-icao-and-address.md +++ b/_bmad-output/implementation-artifacts/1-4-location-resolution-icao-and-address.md @@ -1,6 +1,6 @@ # Story 1.4: Location Resolution (ICAO & Address) -Status: ready-for-dev +Status: review ## Story @@ -22,39 +22,39 @@ So that I can verify the device is centred on the correct location before commit ## Tasks / Subtasks -- [ ] Task 1: Implement `location.resolve(query)` in `src/planemapper/provisioning/location.py` (AC: #1, #2, #3, #4) - - [ ] 1.1 Normalise the input: `query = query.strip().upper()` - - [ ] 1.2 Detect ICAO heuristic: `len(query) == 4 and query.isalpha()` — if true, attempt ICAO lookup first - - [ ] 1.3 ICAO lookup: open `airports.csv` via `importlib.resources.files("planemapper.data").joinpath("airports.csv").open("r", encoding="utf-8")`; parse with `csv.DictReader`; search for row where `row["ident"] == query`; return `(float(row["latitude_deg"]), float(row["longitude_deg"]), row["name"])` - - [ ] 1.4 If ICAO lookup finds no match, raise `ValueError("ICAO code not found — try an address instead")` - - [ ] 1.5 Non-ICAO path: call `requests.get("https://nominatim.openstreetmap.org/search", params={"q": query, "format": "json", "limit": 1}, headers={"User-Agent": "planemapper/0.1 (https://github.com/football2801/planeMapper)"}, timeout=10)` - - [ ] 1.6 Parse Nominatim response: if `results` list is non-empty, return `(float(results[0]["lat"]), float(results[0]["lon"]), results[0]["display_name"])` - - [ ] 1.7 If Nominatim returns an empty list, raise `ValueError("Location not found — try a different search term")` - - [ ] 1.8 Annotate the function signature: `def resolve(query: str) -> tuple[float, float, str]` +- [x] Task 1: Implement `location.resolve(query)` in `src/planemapper/provisioning/location.py` (AC: #1, #2, #3, #4) + - [x] 1.1 Normalise the input: `query = query.strip().upper()` + - [x] 1.2 Detect ICAO heuristic: `len(query) == 4 and query.isalpha()` — if true, attempt ICAO lookup first + - [x] 1.3 ICAO lookup: open `airports.csv` via `importlib.resources.files("planemapper.data").joinpath("airports.csv").open("r", encoding="utf-8")`; parse with `csv.DictReader`; search for row where `row["ident"] == query`; return `(float(row["latitude_deg"]), float(row["longitude_deg"]), row["name"])` + - [x] 1.4 If ICAO lookup finds no match, raise `ValueError("ICAO code not found — try an address instead")` + - [x] 1.5 Non-ICAO path: call `requests.get("https://nominatim.openstreetmap.org/search", params={"q": query, "format": "json", "limit": 1}, headers={"User-Agent": "planemapper/0.1 (https://github.com/football2801/planeMapper)"}, timeout=10)` + - [x] 1.6 Parse Nominatim response: if `results` list is non-empty, return `(float(results[0]["lat"]), float(results[0]["lon"]), results[0]["display_name"])` + - [x] 1.7 If Nominatim returns an empty list, raise `ValueError("Location not found — try a different search term")` + - [x] 1.8 Annotate the function signature: `def resolve(query: str) -> tuple[float, float, str]` -- [ ] Task 2: Add `POST /find-location` route to `src/planemapper/provisioning/portal.py` (AC: #1, #2, #3, #4) - - [ ] 2.1 Import `location` from `planemapper.provisioning` at the top of `portal.py` - - [ ] 2.2 Implement `POST /find-location` — read `request.form["location"]` field - - [ ] 2.3 Call `location.resolve(query)` inside a `try/except ValueError` - - [ ] 2.4 On success: return updated form HTML showing the resolved name and coordinates (e.g. a confirmation section with `lat`, `lon`, `name` values visible) and hidden fields pre-populated with `lat`/`lon` for subsequent form submit - - [ ] 2.5 On `ValueError`: return updated form HTML with the error message displayed inline (no 4xx status — keep the form usable) - - [ ] 2.6 Annotate the route function with return type `str | Response` +- [x] Task 2: Add `POST /find-location` route to `src/planemapper/provisioning/portal.py` (AC: #1, #2, #3, #4) + - [x] 2.1 Import `location` from `planemapper.provisioning` at the top of `portal.py` + - [x] 2.2 Implement `POST /find-location` — read `request.form["location"]` field + - [x] 2.3 Call `location.resolve(query)` inside a `try/except ValueError` + - [x] 2.4 On success: return updated form HTML showing the resolved name and coordinates (e.g. a confirmation section with `lat`, `lon`, `name` values visible) and hidden fields pre-populated with `lat`/`lon` for subsequent form submit + - [x] 2.5 On `ValueError`: return updated form HTML with the error message displayed inline (no 4xx status — keep the form usable) + - [x] 2.6 Annotate the route function with return type `str | Response` -- [ ] Task 3: Write tests in `tests/provisioning/test_location.py` (AC: #1, #2, #3, #4, #5) - - [ ] 3.1 Test ICAO lookup hit: call `resolve("EGLL")` against the real `airports.csv`; assert returned `(lat, lon, name)` is a `tuple[float, float, str]` with plausible UK coordinates - - [ ] 3.2 Test ICAO lookup miss: call `resolve("ZZZZ")`; assert `ValueError` is raised with message `"ICAO code not found — try an address instead"` - - [ ] 3.3 Test Nominatim success: patch `planemapper.provisioning.location.requests.get` with `unittest.mock.patch`; mock return value `.json()` returns `[{"lat": "51.5", "lon": "-0.1", "display_name": "London"}]`; call `resolve("OX1 1AA")`; assert `(51.5, -0.1, "London")` returned - - [ ] 3.4 Test Nominatim empty response: patch `requests.get` as above but `.json()` returns `[]`; call `resolve("nonsense query")`; assert `ValueError` is raised with message `"Location not found — try a different search term"` - - [ ] 3.5 Assert the mock was called exactly once with the expected URL and `User-Agent` header (confirms no real HTTP in CI) +- [x] Task 3: Write tests in `tests/provisioning/test_location.py` (AC: #1, #2, #3, #4, #5) + - [x] 3.1 Test ICAO lookup hit: call `resolve("EGLL")` against the real `airports.csv`; assert returned `(lat, lon, name)` is a `tuple[float, float, str]` with plausible UK coordinates + - [x] 3.2 Test ICAO lookup miss: call `resolve("ZZZZ")`; assert `ValueError` is raised with message `"ICAO code not found — try an address instead"` + - [x] 3.3 Test Nominatim success: patch `planemapper.provisioning.location.requests.get` with `unittest.mock.patch`; mock return value `.json()` returns `[{"lat": "51.5", "lon": "-0.1", "display_name": "London"}]`; call `resolve("OX1 1AA")`; assert `(51.5, -0.1, "London")` returned + - [x] 3.4 Test Nominatim empty response: patch `requests.get` as above but `.json()` returns `[]`; call `resolve("nonsense query")`; assert `ValueError` is raised with message `"Location not found — try a different search term"` + - [x] 3.5 Assert the mock was called exactly once with the expected URL and `User-Agent` header (confirms no real HTTP in CI) -- [ ] Task 4: Update portal tests in `tests/provisioning/test_portal.py` (AC: #1, #2, #3, #4) - - [ ] 4.1 Add test for `POST /find-location` with a successful resolve (mock `location.resolve` to return `(51.5, -0.1, "London")`); assert 200 and that the response body contains the resolved name and coordinates - - [ ] 4.2 Add test for `POST /find-location` with a `ValueError` from `location.resolve`; assert 200 and that the response body contains the error message text +- [x] Task 4: Update portal tests in `tests/provisioning/test_portal.py` (AC: #1, #2, #3, #4) + - [x] 4.1 Add test for `POST /find-location` with a successful resolve (mock `location.resolve` to return `(51.5, -0.1, "London")`); assert 200 and that the response body contains the resolved name and coordinates + - [x] 4.2 Add test for `POST /find-location` with a `ValueError` from `location.resolve`; assert 200 and that the response body contains the error message text -- [ ] Task 5: Run quality gates - - [ ] 5.1 `pytest tests/` — all tests pass, 0 failures - - [ ] 5.2 `ruff check .` — zero violations - - [ ] 5.3 `ruff format --check .` — no formatting issues +- [x] Task 5: Run quality gates + - [x] 5.1 `pytest tests/` — all tests pass, 0 failures + - [x] 5.2 `ruff check .` — zero violations + - [x] 5.3 `ruff format --check .` — no formatting issues ## Dev Notes diff --git a/src/planemapper/provisioning/location.py b/src/planemapper/provisioning/location.py index d352c7e..5b6ad6d 100644 --- a/src/planemapper/provisioning/location.py +++ b/src/planemapper/provisioning/location.py @@ -1 +1,50 @@ -# stub +import csv +import importlib.resources +import logging + +import requests + +log = logging.getLogger(__name__) + +NOMINATIM_URL = "https://nominatim.openstreetmap.org/search" +_USER_AGENT = "planemapper/0.1 (https://github.com/football2801/planeMapper)" + + +def _lookup_icao(code: str) -> tuple[float, float, str] | None: + traversable = importlib.resources.files("planemapper.data").joinpath("airports.csv") + with traversable.open("r", encoding="utf-8") as fh: + reader = csv.DictReader(fh) + for row in reader: + if row["ident"] == code: + return float(row["latitude_deg"]), float(row["longitude_deg"]), row["name"] + return None + + +def _geocode(query: str) -> tuple[float, float, str] | None: + resp = requests.get( + NOMINATIM_URL, + params={"q": query, "format": "json", "limit": 1}, + headers={"User-Agent": _USER_AGENT}, + timeout=10, + ) + resp.raise_for_status() + results = resp.json() + if not results: + return None + r = results[0] + return float(r["lat"]), float(r["lon"]), r["display_name"] + + +def resolve(query: str) -> tuple[float, float, str]: + query = query.strip().upper() + if len(query) == 4 and query.isalpha(): + result = _lookup_icao(query) + if result is None: + raise ValueError("ICAO code not found — try an address instead") + log.info("ICAO %s resolved to %s", query, result[2]) + return result + result = _geocode(query) + if result is None: + raise ValueError("Location not found — try a different search term") + log.info("Address '%s' resolved to %s", query, result[2]) + return result diff --git a/src/planemapper/provisioning/portal.py b/src/planemapper/provisioning/portal.py index 4d080ba..1bc3767 100644 --- a/src/planemapper/provisioning/portal.py +++ b/src/planemapper/provisioning/portal.py @@ -1,29 +1,54 @@ import logging -from flask import Flask, redirect, url_for +from flask import Flask, redirect, request, url_for from werkzeug.wrappers import Response +from planemapper.provisioning import location + log = logging.getLogger(__name__) app = Flask(__name__) -_FORM_HTML = """ + +def _build_form_html( + error: str = "", + resolved_name: str = "", + resolved_lat: str = "", + resolved_lon: str = "", + radius: str = "100", +) -> str: + error_html = f'

{error}

' if error else "" + confirmed_html = "" + hidden_fields = "" + if resolved_name: + confirmed_html = f""" +
+ Confirmed: {resolved_name}
+ Lat: {resolved_lat}, Lon: {resolved_lon} +
""" + hidden_fields = f""" + + + """ + return f""" planeMapper Setup

planeMapper Setup

+{error_html}




+{confirmed_html}
-

+ {hidden_fields}

@@ -38,7 +63,32 @@ _FORM_HTML = """ @app.route("/") def index() -> str: - return _FORM_HTML + return _build_form_html() + + +@app.route("/find-location", methods=["POST"]) +def find_location() -> str: + query = request.form.get("location", "").strip() + radius = request.form.get("radius", "100") + error_msg = "" + resolved_name = "" + resolved_lat = "" + resolved_lon = "" + if query: + try: + lat, lon, name = location.resolve(query) + resolved_lat = str(lat) + resolved_lon = str(lon) + resolved_name = name + except ValueError as e: + error_msg = str(e) + return _build_form_html( + error=error_msg, + resolved_name=resolved_name, + resolved_lat=resolved_lat, + resolved_lon=resolved_lon, + radius=radius, + ) @app.route("/generate_204") diff --git a/tests/provisioning/test_location.py b/tests/provisioning/test_location.py index b8bbd70..d2a7b8b 100644 --- a/tests/provisioning/test_location.py +++ b/tests/provisioning/test_location.py @@ -1,2 +1,55 @@ -def test_placeholder() -> None: - pass +from unittest.mock import MagicMock, patch + +import pytest + +from planemapper.provisioning.location import resolve + + +def test_icao_lookup_hit_egll() -> None: + lat, lon, name = resolve("EGLL") + assert isinstance(lat, float) + assert isinstance(lon, float) + assert isinstance(name, str) + assert 50.0 < lat < 52.0 # Heathrow is ~51.47°N + assert -1.0 < lon < 0.0 # and ~0.46°W + + +def test_icao_lookup_case_insensitive() -> None: + lat, lon, name = resolve("egll") + assert lat != 0.0 + + +def test_icao_lookup_miss_raises_value_error() -> None: + with pytest.raises(ValueError, match="ICAO code not found"): + resolve("ZZZZ") + + +def test_nominatim_success() -> None: + mock_resp = MagicMock() + mock_resp.json.return_value = [ + {"lat": "51.5", "lon": "-0.1", "display_name": "London, England"} + ] + with patch("planemapper.provisioning.location.requests.get", return_value=mock_resp): + lat, lon, name = resolve("OX1 1AA") + assert lat == 51.5 + assert lon == -0.1 + assert name == "London, England" + + +def test_nominatim_empty_raises_value_error() -> None: + mock_resp = MagicMock() + mock_resp.json.return_value = [] + with patch("planemapper.provisioning.location.requests.get", return_value=mock_resp): + with pytest.raises(ValueError, match="Location not found"): + resolve("nonsense query that returns nothing") + + +def test_nominatim_called_with_user_agent() -> None: + mock_resp = MagicMock() + mock_resp.json.return_value = [{"lat": "51.5", "lon": "-0.1", "display_name": "London"}] + with patch( + "planemapper.provisioning.location.requests.get", return_value=mock_resp + ) as mock_get: + resolve("London") + call_kwargs = mock_get.call_args + assert "User-Agent" in call_kwargs.kwargs.get("headers", {}) diff --git a/tests/provisioning/test_portal.py b/tests/provisioning/test_portal.py index 4e580d9..bd740b3 100644 --- a/tests/provisioning/test_portal.py +++ b/tests/provisioning/test_portal.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + import pytest from planemapper.provisioning.portal import app @@ -45,3 +47,25 @@ def test_ncsi_redirects_to_index(client) -> None: def test_unknown_route_redirects_to_index(client) -> None: resp = client.get("/some/random/path") assert resp.status_code in (301, 302) + + +def test_find_location_success(client) -> None: + mock_resolve = patch( + "planemapper.provisioning.portal.location.resolve", return_value=(51.5, -0.1, "London") + ) + with mock_resolve: + resp = client.post("/find-location", data={"location": "EGLL", "radius": "100"}) + assert resp.status_code == 200 + data = resp.data.decode() + assert "London" in data + assert "51.5" in data + + +def test_find_location_error(client) -> None: + with patch( + "planemapper.provisioning.portal.location.resolve", + side_effect=ValueError("ICAO code not found — try an address instead"), + ): + resp = client.post("/find-location", data={"location": "ZZZZ", "radius": "100"}) + assert resp.status_code == 200 + assert "ICAO code not found" in resp.data.decode()