fix: invert refresh checkbox to a positive "Enable automatic data refresh" option - #2198
fix: invert refresh checkbox to a positive "Enable automatic data refresh" option#2198pacso wants to merge 5 commits into
Conversation
The manual_refresh_only checkbox was counter-intuitive: it was labelled "Automatically refresh the sensor" but ticking it DISABLED polling, and it defaulted to True, so new installs silently got no periodic updates (issue robbrad#2193). Invert the control so ticking enables polling. Introduce a positive config field auto_refresh_enabled (default True = poll), relabel the checkbox "Enable automatic data refresh", and decide the update interval from it in async_setup_entry. Existing installs are handled by a lazy fallback (auto_refresh_enabled = not manual_refresh_only) with no config version bump; the reconfigure/options flows drop the legacy key on save. Translation values already said "refresh automatically", which is now correct under the inverted semantics, so only the key is renamed (plus the improved English label). Tests cover the new key in both states and retain the legacy-fallback paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove comments that merely restated the self-documenting field name; keep the legacy-fallback and legacy-key-drop notes that explain non-obvious behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe integration replaces ChangesAutomatic refresh configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 #2198 +/- ##
==========================================
+ Coverage 80.04% 83.05% +3.01%
==========================================
Files 12 12
Lines 1393 1387 -6
==========================================
+ Hits 1115 1152 +37
+ Misses 278 235 -43 ☔ View full report in Codecov by Harness. |
![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 `@custom_components/uk_bin_collection/config_flow.py`:
- Around line 544-552: Update the flow around the auto_refresh_enabled handling
and data merge so disabling auto-refresh does not overwrite update_interval with
None. Preserve the last valid numeric interval in the config entry data, and
disable polling through the runtime behavior instead, ensuring options and
reconfigure defaults continue satisfying cv.positive_int and Range(min=1).
In `@custom_components/uk_bin_collection/strings.json`:
- Line 45: Add auto_refresh_enabled to the options-step translation structure at
options.step.init.data in strings.json, then add the same label to each
corresponding locale file. Preserve the existing config-flow translation and use
the established label text so the Options UI resolves the translation correctly.
🪄 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 Plus
Run ID: c7c9ad8c-0f65-4f20-825b-2bb4a76d8602
📒 Files selected for processing (11)
custom_components/uk_bin_collection/__init__.pycustom_components/uk_bin_collection/config_flow.pycustom_components/uk_bin_collection/const.pycustom_components/uk_bin_collection/strings.jsoncustom_components/uk_bin_collection/tests/test_config_flow.pycustom_components/uk_bin_collection/tests/test_init.pycustom_components/uk_bin_collection/translations/cy.jsoncustom_components/uk_bin_collection/translations/en.jsoncustom_components/uk_bin_collection/translations/ga.jsoncustom_components/uk_bin_collection/translations/gd.jsoncustom_components/uk_bin_collection/translations/pt.json
Address review feedback on the auto_refresh_enabled change: - Options flow no longer overwrites update_interval with None when auto refresh is disabled. Polling is already disabled at runtime via auto_refresh_enabled, so the numeric interval is kept valid for the schema defaults (cv.positive_int / Range(min=1)) and remembered if polling is re-enabled. - Add options.step.init.data.auto_refresh_enabled to strings.json and every locale so the Options UI shows a proper label instead of a humanised raw key (reusing each locale's established label text). - Add config-flow tests covering the reconfigure and options save paths (legacy manual_refresh_only dropped, auto_refresh_enabled stored, update_interval preserved), closing the patch-coverage gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/uk_bin_collection/config_flow.py (1)
319-364: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
auto_refresh_enableddefault-resolution logic.
build_reconfigure_schema(lines 328-334) andbuild_options_schema(lines 611-617) compute the same fallback expression (existing_data.get("auto_refresh_enabled", not existing_data.get("manual_refresh_only", False))) independently. Extracting a small helper avoids the two flows silently diverging if one copy is edited later.♻️ Proposed refactor
+ `@staticmethod` + def resolve_auto_refresh_default(existing_data: Dict[str, Any]) -> bool: + """Resolve the auto_refresh_enabled default, falling back to the legacy key.""" + return existing_data.get( + "auto_refresh_enabled", + not existing_data.get("manual_refresh_only", False), + )Then reuse it in both schema builders:
vol.Optional( "auto_refresh_enabled", - default=existing_data.get( - "auto_refresh_enabled", - not existing_data.get("manual_refresh_only", False), - ), + default=self.resolve_auto_refresh_default(existing_data), ): bool,Also applies to: 596-634
🤖 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 `@custom_components/uk_bin_collection/config_flow.py` around lines 319 - 364, Extract the shared auto-refresh default resolution from build_reconfigure_schema and build_options_schema into a small helper, preserving the fallback of the inverse manual_refresh_only value when auto_refresh_enabled is absent. Replace both inline expressions with calls to that helper so the two schema builders remain consistent.
🤖 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.
Nitpick comments:
In `@custom_components/uk_bin_collection/config_flow.py`:
- Around line 319-364: Extract the shared auto-refresh default resolution from
build_reconfigure_schema and build_options_schema into a small helper,
preserving the fallback of the inverse manual_refresh_only value when
auto_refresh_enabled is absent. Replace both inline expressions with calls to
that helper so the two schema builders remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 847f2f2e-cd0a-4472-a305-7baff0ed1de3
📒 Files selected for processing (8)
custom_components/uk_bin_collection/config_flow.pycustom_components/uk_bin_collection/strings.jsoncustom_components/uk_bin_collection/tests/test_config_flow.pycustom_components/uk_bin_collection/translations/cy.jsoncustom_components/uk_bin_collection/translations/en.jsoncustom_components/uk_bin_collection/translations/ga.jsoncustom_components/uk_bin_collection/translations/gd.jsoncustom_components/uk_bin_collection/translations/pt.json
🚧 Files skipped from review as they are similar to previous changes (6)
- custom_components/uk_bin_collection/translations/ga.json
- custom_components/uk_bin_collection/translations/en.json
- custom_components/uk_bin_collection/strings.json
- custom_components/uk_bin_collection/translations/cy.json
- custom_components/uk_bin_collection/translations/gd.json
- custom_components/uk_bin_collection/translations/pt.json
build_reconfigure_schema and build_options_schema duplicated the same default-resolution expression (auto_refresh_enabled, falling back to the inverse of legacy manual_refresh_only). Extract it into a module-level resolve_auto_refresh_default helper and call it from both builders so they stay consistent. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@custom_components/uk_bin_collection/config_flow.py`:
- Around line 20-29: Update the migration logic that inserts synthetic
manual_refresh_only values so entries lacking both keys retain the new
auto-refresh default of True; either remove that insertion or migrate directly
to auto_refresh_enabled using the inverse of an existing manual_refresh_only
value. Align the config version handling and related tests with the chosen
migration path, including the other affected migration locations.
🪄 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 Plus
Run ID: 8fc13072-d40a-47c7-b6ce-55bd9a495fcf
📒 Files selected for processing (1)
custom_components/uk_bin_collection/config_flow.py
UkBinCollectionConfigFlow.async_migrate_entry inserted a synthetic manual_refresh_only=True for entries lacking the key. That is dead code (Home Assistant's migration hook is the module-level async_migrate_entry in __init__.py; ConfigFlow has no migration hook) and, under the inverted semantics, would have forced auto refresh OFF — contradicting the new default of True. Remove the method. Entries lacking both keys now retain the True default via the runtime fallback (auto_refresh_enabled = not manual_refresh_only, defaulting to False -> True). The live module-level migration does not touch this key, so no version or test changes are needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
The
manual_refresh_onlycheckbox is counter-intuitive: it is labelled "Automatically refresh the sensor" but ticking it actually disables polling, and it defaults toTrue— so new installs silently get no periodic updates while showing a ticked "automatically refresh" box. This is the likely root cause of the recurring "entities only update on restart" reports.Fixes #2193
This PR inverts the control so the checkbox reads the way it behaves: ticking it enables automatic refresh.
What changes
auto_refresh_enabled(defaultTrue= poll onupdate_interval).async_setup_entrydecides the coordinator's update interval from it.Backward compatibility
auto_refresh_enabled = not manual_refresh_only.manual_refresh_onlykey when the entry is next saved.Translations (minimal)
The existing translated values already said "refresh automatically" — which is now correct under the inverted semantics — so only the key is renamed (
manual_refresh_only→auto_refresh_enabled), plus the improved English label. No value churn in cy/ga/gd/pt.Tests
auto_refresh_enabledin both states (polls whenTrue, no polling whenFalse).manual_refresh_only).test_init.pyandtest_config_flow.pypass locally (pre-existingevent_loop-fixture errors and thefreezegun-dependent sensor/calendar tests are unrelated to this change).Note for reviewers
The non-English strings are the project's existing translations, just re-keyed — a native-speaker glance is welcome but nothing was newly translated.
🤖 Generated with Claude Code
Summary by CodeRabbit