Skip to content

fix: address selection in Broxbourne Council#1873

Open
teofanis wants to merge 1 commit intorobbrad:masterfrom
teofanis:fix-broxbourne
Open

fix: address selection in Broxbourne Council#1873
teofanis wants to merge 1 commit intorobbrad:masterfrom
teofanis:fix-broxbourne

Conversation

@teofanis
Copy link
Contributor

@teofanis teofanis commented Mar 3, 2026

image

The addition <select> elements shown above leak into our lookup and cause Cannot locate option with value ... errors when we try to select the UPRN. This change makes the selector target the Choose address drop-down explicitly.

Summary by CodeRabbit

  • Bug Fixes
    • Improved address selection reliability for Broxbourne Council lookups, reducing the risk of selecting incorrect addresses during postcode searches.
    • Enhanced postcode input handling to ensure proper data entry when searching for collection details.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

Updates the BroxbourneCouncil web scraping module to use a more specific XPath for address selection and adds explicit clearing of the postcode input field before entering a new value, improving robustness of the address lookup flow.

Changes

Cohort / File(s) Summary
Council Module Address Selection Fix
uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py
Replaces address selection locator with a more specific XPath targeting the select element following the "Choose address" label. Adds explicit clear() call before postcode input to ensure field is empty before typing new values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • dp247

Poem

🐰 A hop and a click through Broxbourne's way,
XPaths refined, selectors at play,
Clear fields and addresses, now in their place,
No more confusion in form-filling's race!
With precision we bound, through the bins we collect,
A scraper so sturdy, what do we expect?

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: address selection in Broxbourne Council' directly and clearly describes the main change: fixing address selection functionality for Broxbourne Council, which aligns perfectly with the PR's objective of resolving address selection issues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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 and usage tips.

@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.67%. Comparing base (f6cbad7) to head (3db89c8).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1873   +/-   ##
=======================================
  Coverage   86.67%   86.67%           
=======================================
  Files           9        9           
  Lines        1141     1141           
=======================================
  Hits          989      989           
  Misses        152      152           

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py`:
- Around line 68-76: The code waits only for the <select> element before calling
Select(...).select_by_value(user_uprn), which can fail if options load
asynchronously; update BroxbourneCouncil.py to first wait for the option with
the desired UPRN to appear (e.g., use WebDriverWait +
EC.presence_of_element_located with an XPath like
"//label[normalize-space()='Choose
address']/following::select[1]/option[`@value`='{user_uprn}']"), then call
Select(address_select).select_by_value(user_uprn) so selection only occurs after
the option is present.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6cbad7 and 3db89c8.

📒 Files selected for processing (1)
  • uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py

Comment on lines 68 to 76
address_select = WebDriverWait(driver, 15).until(
EC.presence_of_element_located((By.XPATH, "//select"))
EC.presence_of_element_located(
(
By.XPATH,
"//label[normalize-space()='Choose address']/following::select[1]",
)
)
)
Select(address_select).select_by_value(user_uprn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "BroxbourneCouncil.py" | head -5

Repository: robbrad/UKBinCollectionData

Length of output: 138


🏁 Script executed:

git ls-files | grep -i broxbourne

Repository: robbrad/UKBinCollectionData

Length of output: 136


🏁 Script executed:

sed -n '60,85p' uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py

Repository: robbrad/UKBinCollectionData

Length of output: 970


🏁 Script executed:

sed -n '40,100p' uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py

Repository: robbrad/UKBinCollectionData

Length of output: 2411


🏁 Script executed:

grep -n "try\|except\|select_by_value" uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py | head -20

Repository: robbrad/UKBinCollectionData

Length of output: 317


🏁 Script executed:

grep -r "select_by_value" uk_bin_collection/uk_bin_collection/councils/ | head -10

Repository: robbrad/UKBinCollectionData

Length of output: 1373


🏁 Script executed:

grep -B5 -A5 "select_by_value" uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py

Repository: robbrad/UKBinCollectionData

Length of output: 520


🏁 Script executed:

grep -n "Cannot locate option" uk_bin_collection/uk_bin_collection/councils/*.py

Repository: robbrad/UKBinCollectionData

Length of output: 53


🏁 Script executed:

grep -B10 "select_by_value" uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py | grep -E "try|except|wait"

Repository: robbrad/UKBinCollectionData

Length of output: 53


🏁 Script executed:

sed -n '1,30p' uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py

Repository: robbrad/UKBinCollectionData

Length of output: 1274


🏁 Script executed:

grep -n "import\|from" uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py | head -15

Repository: robbrad/UKBinCollectionData

Length of output: 525


🏁 Script executed:

sed -n '28,120p' uk_bin_collection/uk_bin_collection/councils/BroxbourneCouncil.py | tail -30

Repository: robbrad/UKBinCollectionData

Length of output: 1529


🏁 Script executed:

python3 << 'EOF'
# Verify the proposed fix syntax would work with Selenium
# Check if lambda with Select.options would work

from selenium.webdriver.support.ui import Select

# The proposed fix uses:
# WebDriverWait(driver, 15).until(
#     lambda d: any(
#         opt.get_attribute("value") == user_uprn
#         for opt in Select(address_select).options
#     )
# )

# This syntax is correct for Selenium - Select.options returns list of WebElements
# and get_attribute is a valid WebElement method
print("Proposed fix syntax is valid for Selenium")
print("Lambda will check if any option's value matches user_uprn")
print("This prevents 'Cannot locate option' errors by ensuring option exists before select_by_value")
EOF

Repository: robbrad/UKBinCollectionData

Length of output: 191


Wait for the UPRN option to exist before calling select_by_value.

At line 76, select_by_value is called immediately after waiting only for the <select> element's presence in the DOM. This doesn't guarantee that options are populated if they load asynchronously. Add an explicit wait for the option value to appear in the dropdown before attempting selection to prevent "Cannot locate option with value ..." failures.

Proposed fix
             # Select address
             address_select = WebDriverWait(driver, 15).until(
                 EC.presence_of_element_located(
                     (
                         By.XPATH,
                         "//label[normalize-space()='Choose address']/following::select[1]",
                     )
                 )
             )
+            WebDriverWait(driver, 15).until(
+                lambda d: any(
+                    opt.get_attribute("value") == user_uprn
+                    for opt in Select(address_select).options
+                )
+            )
             Select(address_select).select_by_value(user_uprn)
🤖 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/BroxbourneCouncil.py` around
lines 68 - 76, The code waits only for the <select> element before calling
Select(...).select_by_value(user_uprn), which can fail if options load
asynchronously; update BroxbourneCouncil.py to first wait for the option with
the desired UPRN to appear (e.g., use WebDriverWait +
EC.presence_of_element_located with an XPath like
"//label[normalize-space()='Choose
address']/following::select[1]/option[`@value`='{user_uprn}']"), then call
Select(address_select).select_by_value(user_uprn) so selection only occurs after
the option is present.

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