fix: NorthYorkshire - find HTML data dynamically in AJAX response

[InertiaUK]

The AJAX endpoint at /ajax returns a list of command objects. The HTML bin data was at index [1] but has shifted to index [2] — the response now has a CSS stylesheet object at [1] instead.

Changed the parser to search the list for the first item where data is a string containing HTML, instead of hardcoding the index. This is resilient to future index shifts.

Tested with UPRN 10093091235 in Selby.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced North Yorkshire council's bin collection data parsing to dynamically locate collection information in API responses instead of assuming a fixed response structure, with improved error handling for missing data.

[coderabbitai]

📝 Walkthrough

Walkthrough

The parse_data method in the North Yorkshire council module was updated to replace fixed JSON indexing with dynamic scanning. Instead of directly accessing bin_data[1]["data"], the code now iterates through response items to locate the first dictionary containing HTML string data in its "data" field, raising a ValueError if no HTML payload is found.

Changes

Cohort / File(s)	Summary
North Yorkshire Parser Update uk_bin_collection/uk_bin_collection/councils/NorthYorkshire.py	Modified parse_data method to dynamically scan for HTML content within JSON response items instead of assuming fixed structure at bin_data[1]["data"]. Added validation to raise ValueError when expected HTML payload is not found.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• dp247

Poem

🐰 Through JSON fields our rabbit hops,
No fixed paths where data stops,
Now scanning wide with twitching nose,
Each HTML treasure brightly shows! ✨

🚥 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 accurately describes the main change: updating the NorthYorkshire parser to dynamically find HTML data in the AJAX response instead of relying on a fixed index, which is the core fix addressed in the PR.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

uk_bin_collection/uk_bin_collection/councils/NorthYorkshire.py (1)

31-39: Tighten HTML payload detection to the expected container

"<div" is a broad match and may select the wrong command payload if multiple HTML fragments appear. Prefer matching the known marker (e.g., upcoming-collection) so failure mode stays deterministic.

Proposed refactor

-        html_data = None
-        for item in bin_data:
-            if isinstance(item, dict) and isinstance(item.get("data"), str) and "<div" in item["data"]:
-                html_data = item["data"]
-                break
-
-        if not html_data:
-            raise ValueError("No HTML bin data found in API response")
+        html_data = next(
+            (
+                item.get("data")
+                for item in bin_data
+                if isinstance(item, dict)
+                and isinstance(item.get("data"), str)
+                and "upcoming-collection" in item["data"]
+            ),
+            None,
+        )
+
+        if html_data is None:
+            raise ValueError("No upcoming-collection HTML found in API response")

🤖 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/NorthYorkshire.py` around lines
31 - 39, The current loop in NorthYorkshire.py that picks html_data from
bin_data uses a broad "<div" check and can select the wrong fragment; update the
detection logic in that loop (the block that assigns html_data) to look for the
specific marker string (e.g., "upcoming-collection") inside item.get("data")
instead of "<div" so the parser (BeautifulSoup(html_data, ...)) always receives
the expected container; keep the existing isinstance checks and the subsequent
ValueError if html_data is not found.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@uk_bin_collection/uk_bin_collection/councils/NorthYorkshire.py`:
- Around line 31-39: The current loop in NorthYorkshire.py that picks html_data
from bin_data uses a broad "<div" check and can select the wrong fragment;
update the detection logic in that loop (the block that assigns html_data) to
look for the specific marker string (e.g., "upcoming-collection") inside
item.get("data") instead of "<div" so the parser (BeautifulSoup(html_data, ...))
always receives the expected container; keep the existing isinstance checks and
the subsequent ValueError if html_data is not found.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c552d3b-ccb6-4f22-af02-80d67d532513

📥 Commits

Reviewing files that changed from the base of the PR and between 60bd3cc and 86b1b2c.

📒 Files selected for processing (1)

• uk_bin_collection/uk_bin_collection/councils/NorthYorkshire.py