Fix unchecked boolean checkboxes not saving as false#216
Conversation
HTML checkboxes don't submit values when unchecked. The plugin config save endpoint starts from existing config (for partial updates), so an unchecked checkbox's old `true` value persists. Additionally, merge_with_defaults fills in schema defaults for missing fields, causing booleans with `"default": true` to always re-enable. This affected the odds-ticker plugin where NFL/NBA leagues (default: true) could not be disabled via the checkbox UI, while NHL (default: false) appeared to work by coincidence. Changes: - Add _set_missing_booleans_to_false() that walks the schema after form processing and sets any boolean field absent from form data to false - Add value="true" to boolean checkboxes so checked state sends "true" instead of "on" (proper boolean parsing) - Handle "on"/"off" strings in _parse_form_value_with_schema for backwards compatibility with checkboxes lacking value="true" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds logic to ensure unchecked boolean form fields become False in saved plugin configs, normalizes boolean-like form values ("on"/"off"/"true"/"false") to booleans, and updates the checkbox template to submit "true" when checked. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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
🤖 Fix all issues with AI agents
In `@web_interface/blueprints/api_v3.py`:
- Around line 3091-3095: The boolean coercion currently treats any "on"/"off"
strings as booleans; modify the block that checks stripped.lower() in
('true','on') and ('false','off') so that "on"/"off" are only coerced when the
field's schema type indicates a boolean (e.g., field_schema.get('type') ==
'boolean' or similar), while still allowing unconditional parsing of
"true"/"false"; in practice, keep the existing checks for "true"/"false" as-is
and wrap the "on"/"off" branches with a guard that inspects the schema/type for
the target field before returning True/False.
- Around line 3197-3231: The _set_missing_booleans_to_false function currently
only recurses object schema properties and misses boolean fields inside arrays
of objects; update it to also handle prop_type == 'array' where
prop_schema.get('items') is an object with 'properties' by filtering form_keys
for keys that belong to this array (keys that start with f"{full_path}.") and
then recurse into the items' properties (items['properties']) using those
filtered keys and the same full_path as prefix so missing boolean fields inside
array items get set to False via _set_nested_value just like nested objects.
🧹 Nitpick comments (1)
web_interface/templates/v3/partials/plugin_config.html (1)
754-759: No-schema boolean fallback path may not benefit from this fix.This fallback path (for plugins without schema definitions) still lacks
value="true"and cannot benefit from the backend's_set_missing_booleans_to_false()helper since there's no schema to walk. Unchecked checkboxes here would still not submit any value.Consider adding
value="true"for consistency, or using the hidden input fallback pattern (as seen in lines 402-407 and 469-475) to ensure the field is always submitted.♻️ Option 1: Add value="true" (minimal change)
{% if value is sameas true or value is sameas false %} <label class="flex items-center cursor-pointer"> - <input type="checkbox" name="{{ key }}" {% if value %}checked{% endif %} + <input type="checkbox" name="{{ key }}" value="true" {% if value %}checked{% endif %} class="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded"> <span class="ml-2 text-sm text-gray-600">Enabled</span> </label>
- Only coerce "on"/"off" strings to booleans when the schema type is boolean; "true"/"false" remain unconditional - Extend _set_missing_booleans_to_false to recurse into arrays of objects (e.g. custom_feeds.0.enabled) by discovering item indices from submitted form keys and recursing per-index Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web_interface/blueprints/api_v3.py`:
- Around line 3203-3262: The function _set_missing_booleans_to_false currently
sets missing booleans by calling _set_nested_value with dotted paths (e.g.
"feeds.custom_feeds.0.enabled"), but _set_nested_value only handles dicts and
will replace lists with {} thereby corrupting array-of-object configs; change
the logic to avoid calling _set_nested_value for paths that contain numeric
array indices and instead walk/ensure the corresponding nested structure
in-place: when recursing into an array item (inside
_set_missing_booleans_to_false where you compute item_prefix and indices),
locate the list at the parent key in the actual config, ensure the item at that
index exists and is a dict (create a dict but never replace the list), then set
the boolean directly on that item dict (e.g.
config['feeds']['custom_feeds'][0]['enabled'] = False); keep using
_set_nested_value only for pure-dict paths and preserve lists by descending into
them when handling array item indices.
_set_nested_value uses dict-style access for all path segments, which corrupts lists when paths contain numeric array indices (e.g. "feeds.custom_feeds.0.enabled"). Refactored _set_missing_booleans_to_false to: - Accept an optional config_node parameter for direct array item access - When inside an array item, set booleans directly on the item dict - Navigate to array lists manually, preserving their list type - Ensure array items exist as dicts before recursing This prevents array-of-object configs (like custom_feeds) from being converted to nested dicts with numeric string keys. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
falsestatetrue) persists when starting from existing config for partial form updatesmerge_with_defaults()fills in schema defaults for missing fields, causing booleans with"default": trueto always re-enableThis directly fixes the reported issue where NFL/NBA leagues in the odds-ticker plugin could not be disabled via the UI, while NHL (which defaults to
false) appeared to work.Changes
api_v3.py): Add_set_missing_booleans_to_false()helper that walks the schema after form processing and sets any boolean field absent from form data tofalseapi_v3.py): Handle"on"/"off"strings in_parse_form_value_with_schemafor backwards compatibilityplugin_config.html): Addvalue="true"to boolean checkboxes so checked state sends"true"instead of"on"Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.