Skip to content

fix: let belfast pick the address by house number - #2320

Open
InertiaUK wants to merge 1 commit into
robbrad:masterfrom
InertiaUK:fix/belfast-house-number-lookup
Open

InertiaUK wants to merge 1 commit into
robbrad:masterfrom
InertiaUK:fix/belfast-house-number-lookup

Conversation

@InertiaUK

Copy link
Copy Markdown
Contributor

Summary

Belfast already posts the postcode to Default.aspx, and the response contains the full address list (lstAddresses) with UPRNs as the option values, but the scraper still needed the UPRN passed in to choose one. When no UPRN is given it now picks the address from that list by house number, so postcode + house number works without looking up a UPRN anywhere else.

  • UPRN still takes priority when passed, so existing configs are unaffected
  • House number matched as a whole token; a number not in the postcode raises ValueError instead of choosing the first address
  • Added timeout=30 to the three requests
  • Wiki note updated; test data unchanged (postcode + UPRN)

Testing

  • Postcode + house number: BT5 5LY + 63 picks 185023941 (63 Martinez Avenue), 3 collections returned
  • Postcode + UPRN (backward compat, existing test data): BT10 0GY + 185086469, 4 collections returned
  • Run end to end through my own API as well

The scraper already searches by postcode but then needed the UPRN to
choose from the address list. When no UPRN is given it now picks the
address by house number from that same list. Adds request timeouts.
@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 58 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d45a56c1-75a3-48b9-8a5b-a3b3c76a1375
Commits

Reviewing files that changed from the base of the PR and between 0d1b26a and 20d818d.

Files selected for processing: 2
  • uk_bin_collection/tests/input.json
  • uk_bin_collection/uk_bin_collection/councils/BelfastCityCouncil.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.57%. Comparing base (0d1b26a) to head (20d818d).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2320   +/-   ##
=======================================
  Coverage   83.57%   83.57%           
=======================================
  Files          43       43           
  Lines        3380     3380           
=======================================
  Hits         2825     2825           
  Misses        555      555           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant