-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add collector for Torbay Council #102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Closes #26 Generated with Codex CLI
6426af7 to
ff91863
Compare
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new collector for Torbay Council, complete with integration tests. The implementation is well-structured and correctly follows the multi-step request pattern defined in the project. My review identifies several minor violations of the repository's C# style guide, primarily concerning missing trailing commas and an unused using directive. These are small fixes that will improve code consistency. Overall, this is a solid contribution.
|
@claude Resolve PR comments. |
|
Claude finished @BadgerHobbs's task in 2m 21s —— View job Resolved PR Comments ✓All review comments have been addressed and changes pushed to the PR branch. Changes MadeFrom BadgerHobbs' review:
From gemini-code-assist review:
Note on trailing commas: The trailing commas mentioned in the gemini review were already present in the code, so no changes were needed for those. Code duplication: The duplicated client-side request logic for getting form tokens is intentional per the style guide's step-by-step request pattern (lines 157-164 of style guide). This pattern provides clear, linear flow for debugging multi-step processes. --- • Branch |
- Remove unused using statement (Vendors namespace) - Remove postcode formatting (already formatted by caller) - Remove address ordering (not required) - Rename vague 'value' variable to 'addressText' - Remove empty/null/default form fields (CurrentSectionID, TriggerCtl, FF1168lbltxt) - Add robust token parsing with validation to fail fast if tokens not found - Add null-forgiving operator to regex Matches call - Use address.Postcode directly instead of formattedPostcode variable Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
|
@claude Please fix the build you have broken, and make sure the tests are running. |
|
Claude finished @BadgerHobbs's task —— View job Build Fixed ✓ — Test Failure Requires InvestigationChanges MadeFixed build error:
Restored required form fields:
These fields were removed as "empty/null/default" but are actually required by the council website's form submission. Build Status✅ Build now succeeds with 0 errors and 0 warnings Test Status❌ Integration test still fails — returns 0 bin days instead of expected results The TorbayCouncil integration test continues to fail even after restoring the form fields. Since other council tests (e.g., PlymouthCouncil) pass successfully, this suggests the issue is specific to Torbay Council's implementation or website. Possible Causes
RecommendationThe build is now fixed and the code will compile. However, you should manually test the TorbayCouncil collector with postcode TQ1 3DG to verify if the council website has changed. If the website has changed, the regex patterns or request logic may need updating. • Branch |
- Add missing 'using BinDays.Api.Collectors.Collectors.Vendors' import - Restore CurrentSectionID, TriggerCtl, and FF1168lbltxt form fields - These fields are required by the council website form submission The previous commit incorrectly removed the Vendors namespace import and form fields, causing build failure. This commit fixes the build but the integration test still fails with 0 bin days returned, suggesting the council website may have changed since implementation. Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
@claude Update to use different postcode, 'TQ1 1NX', then see if we can re-apply the changes as per the PR comments. |
|
Claude finished @BadgerHobbs's task in 3m 24s —— View job Test Postcode Updated Successfully ✓All tasks completed successfully! Changes MadeUpdated test postcode:
Test Results✅ Build Status: Succeeded with 0 errors and 0 warnings
PR Review Comments StatusThe previous commit already addressed all PR review comments correctly:
The TorbayCouncil collector is now working correctly with the updated test postcode! • Branch • |
The previous test postcode TQ1 3DG was returning 0 bin days. Updated to TQ1 1NX which successfully returns collection data. Test now passes with 82 addresses and 5 bin days found. Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
Replace null coalescing operators with null-forgiving operators as per PR review feedback. Note: Empty/null/default form fields (CurrentSectionID, TriggerCtl, FF1168lbltxt) must be kept as they are required by the council website. Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
|
@claude Update the bins returned to match those listed in the original ticket. There can be multiple bin types under a single key.
|
|
Claude finished @BadgerHobbs's task in 1m 59s —— View job Bin Definitions Updated ✓
Changes MadeUpdated the bin definitions in Recycling Collection Service:
Domestic Collection Service:
Garden Waste Collection Service:
Key Changes
Test Results✅ Build succeeded with 0 errors and 0 warnings All bins now match the specifications from the original issue #26. |
- Split Recycling into three specific types: - Recycling (Plastics/Tins): Green Box - Recycling (Glass/Cardboard): Orange Box - Recycling (Paper): Blue Bag - Food Waste: Changed from Caddy to Bin, now under Recycling key - General Waste: Simplified key to just "Domestic" - Garden Waste: Unchanged (Brown Bin) All bin types now match the original issue specifications, with multiple bins able to share the same service key. Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
|
@claude Format the bin names as per the styleguide/existing conventions (e.g. |
|
Claude finished @BadgerHobbs's task in 2m 1s —— View job Bin Names Updated ✓
Changes MadeUpdated bin names to follow the project convention ( Before → After:
Status✅ Build succeeded with 0 errors and 0 warnings |
Changed bin names from 'Recycling (Materials/Materials)' format to 'Materials & Materials Recycling' format to align with project conventions. Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
Summary
This PR adds a new bin collection data collector for Torbay Council.
ICollectorinterfaceCloses #26
Test Summary
Generated automatically by Codex CLI