Fix x-ui-yaml-editor validations #69
Conversation
WalkthroughThe validation logic in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant check_conflicting_ui_properties
User->>check_conflicting_ui_properties: Call with spec_obj
check_conflicting_ui_properties->>check_conflicting_ui_properties: Check for patternProperties
alt patternProperties present and x-ui-yaml-editor: true
check_conflicting_ui_properties->>check_conflicting_ui_properties: Iterate over patternProperties
alt Any type == "object"
check_conflicting_ui_properties-->>User: Raise UsageError
else All types != "object"
check_conflicting_ui_properties-->>User: No error
end
else Other conditions
check_conflicting_ui_properties-->>User: No error or other checks
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ftf_cli/utils.py (1)
386-401: Implementation correctly validates object-type conflicts while allowing string types.The refined logic properly iterates through
patternPropertiesand only raises conflicts when any pattern hastype: "object"combined withx-ui-yaml-editor: true. This allows string-type patterns to coexist with YAML editor functionality, which aligns with the PR objectives.However, there's an unused loop variable that should be addressed:
- for pattern_key, pp_val in pp.items(): + for _, pp_val in pp.items():
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ftf_cli/utils.py(2 hunks)tests/test_utils_validation.py(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: vishnukv-facets
PR: Facets-cloud/module-development-cli#0
File: :0-0
Timestamp: 2025-05-26T13:13:07.039Z
Learning: All user-facing properties in spec must have both 'title' and 'description' fields. Properties with 'x-ui-override-disable': true are considered internal and skip this validation, while properties with 'x-ui-overrides-only': true are still user-facing and require validation. This is enforced by the check_properties_have_required_fields() function in ftf_cli/utils.py.
ftf_cli/utils.py (1)
Learnt from: vishnukv-facets
PR: Facets-cloud/module-development-cli#0
File: :0-0
Timestamp: 2025-05-26T13:13:07.039Z
Learning: All user-facing properties in spec must have both 'title' and 'description' fields. Properties with 'x-ui-override-disable': true are considered internal and skip this validation, while properties with 'x-ui-overrides-only': true are still user-facing and require validation. This is enforced by the check_properties_have_required_fields() function in ftf_cli/utils.py.
tests/test_utils_validation.py (1)
Learnt from: vishnukv-facets
PR: Facets-cloud/module-development-cli#0
File: :0-0
Timestamp: 2025-05-26T13:13:07.039Z
Learning: All user-facing properties in spec must have both 'title' and 'description' fields. Properties with 'x-ui-override-disable': true are considered internal and skip this validation, while properties with 'x-ui-overrides-only': true are still user-facing and require validation. This is enforced by the check_properties_have_required_fields() function in ftf_cli/utils.py.
🧬 Code Graph Analysis (1)
tests/test_utils_validation.py (1)
ftf_cli/utils.py (1)
check_conflicting_ui_properties(371-418)
🪛 Ruff (0.11.9)
ftf_cli/utils.py
393-393: Loop control variable pattern_key not used within loop body
Rename unused pattern_key to _pattern_key
(B007)
🔇 Additional comments (7)
ftf_cli/utils.py (1)
376-378: Documentation accurately reflects the refined validation logic.The updated docstring correctly specifies that only "patternProperties with object type" conflict with
x-ui-yaml-editor: true, reflecting the more nuanced validation behavior.tests/test_utils_validation.py (6)
66-66: Enhanced test coverage with valid string-type pattern scenario.The addition of
field6with string-typepatternPropertiesandx-ui-yaml-editor: trueappropriately tests the newly allowed combination, ensuring the refined validation logic works correctly.
72-73: Test function renamed to accurately reflect its scope.The rename from
test_pattern_properties_with_yaml_editor_raises()totest_pattern_properties_object_type_with_yaml_editor_raises()clearly indicates that this test specifically validates conflicts with object-type patterns, aligning with the refined validation logic.
84-84: Error message assertion updated to match refined validation.The assertion now correctly checks for "patternProperties of type 'object'" in the error message, which matches the updated error message from the refined validation logic.
124-135: Comprehensive test for allowed string-type pattern scenario.This new test case properly validates that
patternPropertieswith string type can coexist withx-ui-yaml-editor: true, covering the core behavioral change introduced by the refined validation logic.
214-231: Thorough testing of mixed-type edge case.This test case excellently covers the scenario where
patternPropertiescontains both string and object types withx-ui-yaml-editor: true. It correctly validates that the presence of any object-type pattern still triggers the conflict error, ensuring the validation logic works correctly for complex schemas.
233-246: Complete validation of multiple string-type patterns.This test case ensures that multiple
patternPropertiesentries, all with string type, can coexist withx-ui-yaml-editor: truewithout triggering validation errors. It provides excellent coverage for multi-pattern scenarios.
Summary by CodeRabbit
Bug Fixes
Tests