fix(web): preserve dotted schema keys when saving plugin config#370
Conversation
The plugin config form posts form-data with dot-notation paths (e.g. "leagues.fifa.world.enabled"). _get_schema_property and _set_nested_value split those paths on every dot, so a schema key that itself contains a dot (soccer league keys like "fifa.world", "eng.1") was mistaken for nested "fifa" -> "world" objects. Per-league edits (enable, favorite_teams, nested booleans) were written to a fabricated "leagues.fifa.world" branch while the real league object was never updated, so saves silently dropped the change and produced a byte-identical config. Both helpers now greedily match the longest path segment that exists in the schema (_get_schema_property) or the config being updated (_set_nested_value), mirroring the frontend's dotted-key handling. Adds regression tests covering schema lookup, value typing, and writes under dotted league keys, plus a guard that plain nested paths still work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 25 minutes and 4 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
Up to standards ✅🟢 Issues
|
Problem
The plugin config form posts form-data with dot-notation paths (e.g.
leagues.fifa.world.enabled). The two helpers that resolve those paths —_get_schema_propertyand_set_nested_valueinweb_interface/blueprints/api_v3.py— split on every dot. When a schema key itself contains a dot (soccer league keys likefifa.world,eng.1,usa.1), the dotted key was mistaken for nestedfifa→worldobjects:_get_schema_property("leagues.fifa.world.favorite_teams")returnedNone, so the field wasn't recognized as an array and the value was dropped/mistyped._set_nested_value(cfg, "leagues.fifa.world.enabled", True)created a fabricatedleagues["fifa"]["world"]branch and left the realleagues["fifa.world"]untouched.Net effect: every per-league edit (enable, favorite teams, nested booleans) was silently discarded. The save wrote the unchanged config and the file came out byte-identical, so users saw no error but no change either. This affects every league, since they all use dotted keys.
The frontend already solved this for its JSON path (greedy longest-match in
getSchemaProperty/dotToNested), but the server-rendered form posts form-data to this backend path, which was never fixed.Fix
Both helpers now greedily match the longest path segment that exists in the schema (
_get_schema_property) or in the config being updated (_set_nested_value), soleagues.fifa.world.*resolves to the realleagues["fifa.world"]object. This mirrors the existing frontend behavior. Plain nested paths (no dotted keys) are unaffected.Tests
Adds
test/web_interface/test_dotted_league_keys.py(6 cases): schema lookup for dotted keys and nested objects beneath them, value typing ("USA"→["USA"]), writes landing in the real league (no fabricated branch), writes into a not-yet-present leaf, plus a guard that plain nested paths still work. Verified the dotted-key cases fail before this change and pass after; the plain-nesting guard passes both ways.🤖 Generated with Claude Code