fix: TewkesburyBoroughCouncil - handle new API response format#1971
fix: TewkesburyBoroughCouncil - handle new API response format#1971InertiaUK wants to merge 1 commit into
Conversation
The api-2.tewkesbury.gov.uk /incab/rounds/{uprn}/next-collection
endpoint now returns a per-bin-type dict (food, garden, recycling,
refuse) with nextCollectionDate ISO strings, instead of the legacy
{status:'OK', body:[{collectionType, NextCollection}]} array.
Parse both formats so we remain compatible if the council flips back,
and keep the timeout bound on requests.get.
📝 WalkthroughWalkthroughThe Tewkesbury Borough Council module now supports an additional JSON response format alongside the legacy format. The network request includes a 30-second timeout. JSON parsing handles both legacy responses and new responses where bin types are top-level keys. Date extraction and sorting logic has been updated to parse ISO timestamps and sort by datetime values rather than lexicographically. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
uk_bin_collection/uk_bin_collection/councils/TewkesburyBoroughCouncil.py (1)
21-58:⚠️ Potential issue | 🟠 MajorFail explicitly on unknown or malformed response formats.
elif isinstance(json_data, dict)accepts every dict, so legacy error envelopes or future API shapes can silently return no bins. The new branch also drops malformed bin entries withcontinue, masking upstream format changes.Proposed explicit-format handling
- elif isinstance(json_data, dict): + elif isinstance(json_data, dict) and any( + key in json_data for key in ("refuse", "recycling", "garden", "food") + ): type_map = { "refuse": "Refuse", "recycling": "Recycling", "garden": "Garden", "food": "Food", } for key, display_name in type_map.items(): - entry = json_data.get(key) - if not isinstance(entry, dict): + if key not in json_data: continue + entry = json_data[key] + if not isinstance(entry, dict): + raise ValueError(f"Unexpected Tewkesbury response: {key} entry is not an object") date_str = entry.get("nextCollectionDate") if not date_str: - continue + raise ValueError(f"Unexpected Tewkesbury response: {key} missing nextCollectionDate") try: # "2026-04-23T01:00:00.000Z" collection_date = datetime.strptime(date_str[:10], "%Y-%m-%d") data["bins"].append({ "type": display_name, "collectionDate": collection_date.strftime(date_format) }) - except ValueError: - continue + except ValueError as exc: + raise ValueError( + f"Unexpected Tewkesbury response: invalid nextCollectionDate for {key}: {date_str}" + ) from exc + else: + raise ValueError("Unexpected Tewkesbury response format")Based on learnings: In
uk_bin_collection/**/*.py, when parsing council bin collection data, prefer explicit failures over silent defaults or swallowed errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@uk_bin_collection/uk_bin_collection/councils/TewkesburyBoroughCouncil.py` around lines 21 - 58, The parsing currently treats any dict as the "current format" and silently skips malformed entries; update the logic in TewkesburyBoroughCouncil.py so that after checking for the legacy envelope (json_data with "status" == "OK" and "body"), you only accept the current format when the dict contains at least one of the expected top-level keys from type_map (refuse, recycling, garden, food) and fail explicitly otherwise (raise a ValueError or log and raise) referencing json_data and type_map; also change the per-entry handling so that malformed dates or missing nextCollectionDate on an expected entry do not silently continue but surface an error indicating the specific bin key and offending date_str (use the existing date parsing via datetime.strptime and data["bins"] and the display_name mapping in your error messages).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@uk_bin_collection/uk_bin_collection/councils/TewkesburyBoroughCouncil.py`:
- Around line 21-58: The parsing currently treats any dict as the "current
format" and silently skips malformed entries; update the logic in
TewkesburyBoroughCouncil.py so that after checking for the legacy envelope
(json_data with "status" == "OK" and "body"), you only accept the current format
when the dict contains at least one of the expected top-level keys from type_map
(refuse, recycling, garden, food) and fail explicitly otherwise (raise a
ValueError or log and raise) referencing json_data and type_map; also change the
per-entry handling so that malformed dates or missing nextCollectionDate on an
expected entry do not silently continue but surface an error indicating the
specific bin key and offending date_str (use the existing date parsing via
datetime.strptime and data["bins"] and the display_name mapping in your error
messages).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b93e995-73fe-47cf-a1dc-7412caa9aa21
📒 Files selected for processing (1)
uk_bin_collection/uk_bin_collection/councils/TewkesburyBoroughCouncil.py
|
Included in May 2026 Release PR #1992. Closing. |
Problem
api-2.tewkesbury.gov.uk/incab/rounds/{uprn}/next-collectionnow returns a dict keyed by bin type (food,garden,recycling,refuse) withnextCollectionDateISO strings, not the legacy{\"status\":\"OK\",\"body\":[{\"collectionType\":..., \"NextCollection\":...}]}array. The scraper returned no bins against live data.Live response today:
```json
{
"food": {"nextCollectionDate": "2026-04-23T01:00:00.000Z", ...},
"garden": {"nextCollectionDate": "2026-04-23T01:00:00.000Z", ...},
"recycling": {"nextCollectionDate": "2026-04-30T01:00:00.000Z", ...},
"refuse": {"nextCollectionDate": "2026-04-23T01:00:00.000Z", ...}
}
```
Fix
Parse both shapes. If the legacy envelope is present, use it. Otherwise map the four known keys to display names and parse the ISO date prefix. Also bound
requests.getwith a 30s timeout (was unbounded).Verification
Fixture UPRN
10067626314(fromtests/input.json) now returns 4 bins:```
Refuse 23/04/2026
Garden 23/04/2026
Food 23/04/2026
Recycling 30/04/2026
```
Summary by CodeRabbit