fix(schemas): make 26 declared settings reachable in the config form - #299
Merged
Conversation
The web UI's config form iterates x-propertyOrder and nothing else:
{% set property_order = schema['x-propertyOrder']
if 'x-propertyOrder' in schema
else schema.properties.keys()|list %}
{% for key in property_order %}
{% if key in schema.properties %}
A property the schema declares but that list omits is therefore never
rendered. No field, no error, no hint the setting exists. The value still
validates on save and the plugin still reads it, so the only way to set one
was to hand-edit config.json on the device.
Twenty-six settings across seven plugins were in that state:
ledmatrix-flights flightaware_api_key + 7 more
basketball scroll_card, background_service, both idle intervals
afl / nrl / soccer scroll_card, both idle intervals
masters-tournament four duration/sizing settings
f1-scoreboard customization.auto_scale
Two of those deserve calling out. ledmatrix-flights' flightaware_api_key is
marked x-secret: true -- someone set up masking for a field that could not be
typed into. And the idle-poll intervals are the ones whose plumbing was fixed
in #295 so they would finally take effect; they still could not be set.
These are omissions, not deliberate hiding. Twenty-two of the twenty-six
already carry x-advanced: true, and nobody flags a field "advanced" meaning
"invisible" -- x-advanced is the supported way to de-emphasise one, and it
puts the field in a collapsed Advanced Settings section. There is no
supported way to hide a property and no schema in the repo attempts it.
What hid this for so long is an asymmetry between the two renderers: the
client-side one in app-shell.js sorts unlisted properties into an
unorderedEntries list and still shows them, while the server-rendered form
drops them. The same schema looks fine in one and is unreachable in the
other.
Order-only change: verified per plugin that the schemas are byte-identical
once x-propertyOrder is stripped, so nothing was added, renamed or retyped.
scripts/test_property_order_coverage.py guards it, and does not merely assert
the rule -- it lifts the ordering loop out of the shipped template, renders it
with a two-property schema listing only one, and shows the other never reaches
the form. Mutation-checked: dropping flightaware_api_key back out fails the
check by name. All 52 test suites across the seven plugins exit 0.
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 23 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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.
Found while auditing for broken functionality and schema defects. This one is invisible by construction, which is why it lasted.
The mechanism
The web UI's config form iterates
x-propertyOrderand nothing else:A property the schema declares but that list omits is never rendered. No field, no error, no hint the setting exists. The value still validates on save and the plugin still reads it — so the only way to set one was to hand-edit
config.jsonon the device.What was unreachable
26 settings across 7 plugins:
ledmatrix-flightsflightaware_api_key,flight_plan_enabled,max_api_calls_per_hour,daily_api_budget,flight_plan_cache_ttl_hours,min_callsign_length,airline_callsign_prefixes,background_servicebasketball-scoreboardscroll_card,background_service, both idle intervalsafl/nrl/soccerscroll_card, both idle intervalsmasters-tournamentplayer_card_duration,hole_display_duration,page_display_duration,scroll_card_widthf1-scoreboardcustomization.auto_scaleTwo worth calling out:
flightaware_api_keyis markedx-secret: true. Someone set up masking for a field that could not be typed into.These are omissions, not deliberate hiding
22 of the 26 already carry
x-advanced: true— nobody flags a field "advanced" meaning "invisible".x-advancedis the supported way to de-emphasise a field, and it puts it in a collapsed Advanced Settings section. There is no supported way to hide a property, and no schema in the repo attempts one.Why it went unnoticed
The two renderers disagree.
app-shell.jssorts unlisted properties into anunorderedEntrieslist and still shows them; the server-rendered form drops them. The same schema looks fine in one and is unreachable in the other.Confirmed on a live rig before changing anything:
soccer-scoreboard's installed schema has 20 properties and 19 order entries, the odd one out isscroll_card, andscroll_cardis absent from the 989 KB rendered page whiledisplay_durationappears four times.Verification
Order-only change — verified per plugin that each schema is identical once
x-propertyOrderis stripped, so nothing was added, renamed or retyped.scripts/test_property_order_coverage.pyguards it, and doesn't merely restate the rule: it lifts the ordering loop out of the shipped template, renders it against a two-property schema listing only one, and shows the other never reaches the form. If that loop is ever changed to append unlisted keys, the test notices.Mutation-checked — dropping
flightaware_api_keyback out fails the check by name. All 52 test suites across the seven plugins exit 0.Also checked, and clean
youtube-statsrequiresapi_key/channel_idwith no defaults, which is the normal credentials pattern, not a defect.🤖 Generated with Claude Code
https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW