fix(MertonCouncil): add postcode search for property ID resolution#2034
fix(MertonCouncil): add postcode search for property ID resolution#2034InertiaUK wants to merge 1 commit into
Conversation
Merton uses internal SocietyWorks IDs, not ONSUD UPRNs. Old scraper required manual UPRN lookup. Added postcode search: POST to /waste with postcode, parse address dropdown, match by house number. Falls back to direct UPRN if provided and valid. Same fix pattern as LondonBoroughSutton (same SocietyWorks platform). Updated input.json with postcode + house_number test params.
📝 WalkthroughWalkthroughThe Merton Council bin collection scraper is refactored to resolve property identifiers via postcode and house number lookup against the FixMyStreet endpoint as a fallback when numeric UPRN is unavailable, updates polling and date extraction logic, and adjusts test input to reflect the new input format. ChangesMerton Council Scraper Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2034 +/- ##
=======================================
Coverage 86.67% 86.67%
=======================================
Files 9 9
Lines 1141 1141
=======================================
Hits 989 989
Misses 152 152 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py`:
- Around line 21-33: The current PAON selection in MertonCouncil (paon_lower,
select.find_all("option"), text.startswith(paon_lower), best) silently falls
back to the first non-empty option; change this to require an exact or uniquely
identifying match: collect all option values where the visible text equals or
matches the PAON more strictly (e.g., exact match or normalized equality), do
not use the loose startswith heuristic, and if zero or more than one match is
found raise a descriptive exception (e.g., ValueError) instead of returning
best; remove/stop using the fallback variable best and ensure calling code can
handle the raised error.
- Around line 42-43: The parser currently only reads paon from kwargs so house
numbers from the new input contract are ignored; update the code in
MertonCouncil.py to also read kwargs.get("house_number") and, if present,
normalize/assign it into the paon variable (or prefer house_number over paon)
before any postcode lookup logic (same change where kwargs.get("paon") is read
around the other occurrence at the block that currently reads paon on lines
~60-61); ensure you handle None and empty values so existing fallback behavior
remains when no house_number is provided.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34fafc45-e199-4471-a1b8-016990e1393c
📒 Files selected for processing (2)
uk_bin_collection/tests/input.jsonuk_bin_collection/uk_bin_collection/councils/MertonCouncil.py
| paon_lower = (paon or "").strip().lower() | ||
| best = None | ||
| for opt in select.find_all("option"): | ||
| val = opt.get("value", "") | ||
| if not val or val == "missing": | ||
| continue | ||
| text = opt.get_text(strip=True).lower() | ||
| if paon_lower and text.startswith(paon_lower): | ||
| return val | ||
| if not best and val: | ||
| best = val | ||
|
|
||
| Required Parameters: | ||
| uprn (str): Unique Property Reference Number (numeric only) | ||
| return best |
There was a problem hiding this comment.
Fail instead of guessing the property from postcode results.
text.startswith(paon_lower) will match prefixes like 1 against 10 ..., and if nothing matches this returns the first non-empty option anyway. On any postcode with multiple addresses, that can silently resolve the wrong property and return another household's collection dates. Please make this path require a unique match and raise when the PAON cannot be matched exactly enough.
Based on learnings: "prefer explicit failures (raise exceptions on unexpected formats) over silent defaults or swallowed errors."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py` around lines
21 - 33, The current PAON selection in MertonCouncil (paon_lower,
select.find_all("option"), text.startswith(paon_lower), best) silently falls
back to the first non-empty option; change this to require an exact or uniquely
identifying match: collect all option values where the visible text equals or
matches the PAON more strictly (e.g., exact match or normalized equality), do
not use the loose startswith heuristic, and if zero or more than one match is
found raise a descriptive exception (e.g., ValueError) instead of returning
best; remove/stop using the fallback variable best and ensure calling code can
handle the raised error.
| postcode = kwargs.get("postcode") | ||
| paon = kwargs.get("paon") |
There was a problem hiding this comment.
Read house_number here too.
The PR updates input.json and the wiki note to use house_number, but this parser only reads paon. With the current code, postcode lookups ignore the supplied house number and rely on the ambiguous fallback above. A local normalization here would keep the new input contract working:
Suggested fix
- paon = kwargs.get("paon")
+ paon = kwargs.get("paon") or kwargs.get("house_number")Also applies to: 60-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py` around lines
42 - 43, The parser currently only reads paon from kwargs so house numbers from
the new input contract are ignored; update the code in MertonCouncil.py to also
read kwargs.get("house_number") and, if present, normalize/assign it into the
paon variable (or prefer house_number over paon) before any postcode lookup
logic (same change where kwargs.get("paon") is read around the other occurrence
at the block that currently reads paon on lines ~60-61); ensure you handle None
and empty values so existing fallback behavior remains when no house_number is
provided.
1 participant
Summary
/wastewith postcode, parse address dropdown, match by house numberinput.jsonwithpostcode+house_numbertest paramsSummary by CodeRabbit
New Features
Bug Fixes