Repository navigation
fix(api): return 422 for an unknown concelho on v2 incident endpoints (#27) - #49
Merged
Merged
Conversation
…#27) /v2/incidents/active, /v2/incidents/active/kml and /v2/incidents/search returned 200 with an empty list for any concelho, even "???". The value is now checked (case-insensitively) against the known municipalities (Location level 2, cached for a day) and an unknown or non-string value returns 422 {"success": false, "error": "invalid_concelho"}. Matching also accepts the casing the ingestion jobs store ("Vila Nova De Gaia"), so "Vila Nova de Gaia" now finds fires too while exact values keep working. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #27
Problem
?concelho=on the v2 incident endpoints returned200 {"success": true, "data": []}for any value, even???, so clients cannot tell a typo from "no fires".Change
App\Support\Concelho: known municipalities =Locationlevel 2 names (plusEspanha), lower-cased and cached for 24h. If thelocationscollection is empty, validation is skipped (fail open).IncidentController::concelhoFilter()used byGET /v2/incidents/active,/v2/incidents/active/kmland/v2/incidents/search:concelho[]=…) → 422{"success": false, "error": "invalid_concelho", "message": "Unknown concelho: ???"};Vila Nova De Gaia), soVila Nova de Gaia/vila nova de gaianow find fires and existing exact values keep working (whereIn, still uses the index).concelho-v2.yamlparameter (optional, case-insensitive, 422 on unknown) and422response on the three paths.v2-searchpreviously documentedconcelhoas required, which it isn't.Tests
tests/Feature/Controllers/IncidentController/ConcelhoFilterTest.php(5 tests): 422 on???(active + search), 422 on array, case-insensitive match, empty list for known concelho, no filter when absent/empty. 3 of them fail onmain.Deploy
Deploy as usual. No env changes, no migrations. The concelho list is cached for 24h under
locations.concelhos.v1(php artisan cache:forget locations.concelhos.v1after changinglocations).concelhonow get 422 instead of an empty 200.🤖 Generated with Claude Code