Move the off option to the top of the manual override dropdowns - #5108
Open
springfall2008 wants to merge 1 commit into
Open
springfall2008 wants to merge 1 commit into
springfall2008 wants to merge 1 commit into
Conversation
Owner
Author
|
Automated review failed to complete for this PR - see the triage bot's logs for details. The run exited cleanly but posted nothing, so the review step itself did not complete. Not retrying automatically; remove |
springfall2008
marked this pull request as ready for review
September 16, 2026 07:52
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are covered by documentation and regression tests with no unresolved blocking issues.
Pull request overview
Moves the off option to the top of manual override dropdowns for easier cancellation.
Changes:
- Reorders options in all three dropdown builders.
- Updates customization documentation.
- Adds regression tests for ordering and selection behavior.
File summaries
| File | Description |
|---|---|
docs/customisation.md |
Documents the new option position. |
apps/predbat/userinterface.py |
Places off first in manual dropdown options. |
apps/predbat/tests/test_manual_select.py |
Tests ordering and cancellation behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated draft PR generated from issue #5105 — a maintainer should review it before merging.
Fixes #5105
Summary
offwas appended last in all three manual-override dropdown builders, so on a 48-hour list (~576 entries at 5-minute granularity) cancelling every override meant scrolling to the very bottom. It is now the first entry inmanual_times(),manual_rates()andapi_select_update()(apps/predbat/userinterface.py), which covers everyselect.predbat_manual_xxxcontrol and the same lists as rendered in the web config page. The docs line that describedoffas being "at the bottom of the list" is updated to match.Selection handling is unaffected:
manual_select()/api_select()match on the value"off", never on its position, and nothing indexesitem["options"]positionally.Testing
./run_pre_commitfromcoverage/— all hooks passed, and the quick suite it runs afterwards passed in full (4 slow tests skipped).tools/triage_test.sh manual_selectwith theuserinterface.pychange stashed — FAILS, flagging all three builders (ERROR: T10 Expected off to be the first option of manual_charge, got Tue 12:00and the same formanual_import_ratesandmanual_api, plus T13 after a slot is selected).tools/triage_test.sh manual_selectwith the change applied — PASSES.The new
test_off_option_first()inapps/predbat/tests/test_manual_select.pyexercises one config item per builder, checksoffis first and appears exactly once, and then checks it stays first once a slot is selected and still clears the selection.Notes
impact()reports HIGH risk for all three methods, because they sit oninitialize/update_time_loop/run_time_loop. That reflects where they live rather than this change: the return values and the state written byexpose_config()are untouched, only the order of the presentation listitem["options"]moves.detect_changes()shows the three methods and the test as the only changed symbols.if values not in time_values / item["options"] = / expose_configtail. That duplication predates this change and I left it alone rather than widening the diff into a refactor of code on the main loop.valuesentry appended when nothing is selected — it renders as "None" in the web config page). That is pre-existing and outside what this ticket asked for, so it is untouched; with this change it now sits second in the list formanual_api.run_test_manual_select()was extracted intofirst_future_option()so the new test could reuse it rather than copy it — behaviour is unchanged (prefer a non-today day label, fall back to any selectable option).