[SILO-723] feat: Updated custom properties api to support passing options while creating #12
Conversation
…ions while crating
|
Caution Review failedThe pull request is closed. WalkthroughAdded optional inline Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
![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)
plane/models/work_item_properties.py (1)
72-72: Consider validating options field based on property_type.While the inline options support is functional, consider extending the existing
validate_settings_and_relation_typevalidator (lines 82-112) to ensureoptionsare only provided forPropertyType.OPTIONproperties. This would provide clearer client-side validation and better developer experience.Example validation logic to add in the validator:
# RELATION properties require relation_type if prop_type == PropertyType.RELATION: if relation_type is None: raise ValueError("relation_type is required for RELATION properties") + + # OPTIONS should only be provided for OPTION properties + if self.options is not None and prop_type != PropertyType.OPTION: + raise ValueError("options can only be provided for OPTION properties") return self
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
plane/models/work_item_properties.py(2 hunks)tests/scripts/test_property_values.py(1 hunks)tests/scripts/test_work_item_types_and_properties.py(4 hunks)tests/unit/test_work_item_properties.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/scripts/test_property_values.py (3)
plane/models/work_item_properties.py (2)
CreateWorkItemProperty(55-112)CreateWorkItemPropertyOption(207-218)plane/models/enums.py (1)
PropertyType(53-64)tests/scripts/test_work_item_types_and_properties.py (2)
print_success(46-48)print_step(39-43)
tests/scripts/test_work_item_types_and_properties.py (3)
tests/scripts/test_property_values.py (2)
print_success(51-53)print_step(44-48)plane/models/work_item_properties.py (3)
CreateWorkItemProperty(55-112)CreateWorkItemPropertyOption(207-218)CreateWorkItemPropertyValue(260-288)plane/models/enums.py (1)
PropertyType(53-64)
tests/unit/test_work_item_properties.py (2)
plane/models/work_item_properties.py (3)
CreateWorkItemProperty(55-112)CreateWorkItemPropertyOption(207-218)UpdateWorkItemProperty(115-179)plane/models/enums.py (1)
PropertyType(53-64)
🪛 Ruff (0.14.6)
tests/scripts/test_work_item_types_and_properties.py
32-32: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
tests/unit/test_work_item_properties.py
112-113: try-except-pass detected, consider logging the exception
(S110)
112-112: Do not catch blind exception: Exception
(BLE001)
266-267: try-except-pass detected, consider logging the exception
(S110)
266-266: Do not catch blind exception: Exception
(BLE001)
613-614: try-except-pass detected, consider logging the exception
(S110)
613-613: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (15)
plane/models/work_item_properties.py (1)
44-44: LGTM! Inline options support added to response model.The optional
optionsfield allows work item properties to include their associated options in API responses, enabling inline option creation and retrieval.tests/unit/test_work_item_properties.py (4)
8-12: LGTM! Necessary imports for inline options testing.The additional imports enable the new test scenarios for creating properties with inline options.
61-113: LGTM! Comprehensive test for inline options in list responses.The test thoroughly validates that properties created with inline options include those options in the list API response, with appropriate cleanup logic.
220-267: LGTM! Validates inline options in retrieve responses.The test confirms that retrieving a property includes its inline options and validates both option count and names, including testing the
is_defaultparameter.
570-614: LGTM! Dedicated test for inline options creation.The test validates that OPTION properties can be created with inline options and that those options are immediately available in the creation response, covering 4 options with one marked as default.
tests/scripts/test_property_values.py (4)
218-240: LGTM! Status property now uses inline options.The Status property creation has been successfully updated to define 5 options inline, demonstrating the new feature. The success message appropriately logs the inline option count.
242-268: LGTM! Multi-value property with inline options.The Tags property demonstrates inline options working correctly with multi-value properties (
is_multi=True), creating 6 options inline.
270-282: LGTM! Verification of inline options.The verification step confirms that inline options are immediately accessible from the created property responses, eliminating the need for separate option creation API calls.
284-309: LGTM! Validates options in list and retrieve responses.The validation confirms that inline options are consistently included in both list and retrieve API responses, with correct option counts for both single-value (5 options) and multi-value (6 options) properties.
tests/scripts/test_work_item_types_and_properties.py (6)
32-34: Import added for TextAttributeSettings usage.The import is necessary for line 168 where TextAttributeSettings is used. Note: Static analysis detected an unused
noqadirective that could be removed for cleanup.
168-168: LGTM! TextAttributeSettings correctly applied to TEXT property.The settings parameter is properly configured for the TEXT property type, satisfying the model's validation requirements.
176-215: LGTM! Priority property with 4 inline options.The Priority property demonstrates inline option creation with proper structure, including setting Medium as the default option. The success message appropriately logs the inline option count.
245-274: LGTM! Comprehensive verification of inline options in list/retrieve.The verification steps thoroughly validate that inline options for the Priority property are accessible immediately after creation and are consistently included in list and retrieve responses.
294-320: LGTM! Simplified property value creation syntax.The property value creation has been streamlined to use
CreateWorkItemPropertyValue(value=...)directly, which is cleaner and more idiomatic. The response handling correctly accesses the.valueattribute.
327-348: LGTM! Individual property value retrieval for clearer verification.The updated approach retrieves and prints each property value individually, providing clearer validation and more informative test output.
Description
Type of Change
Note
Add inline option support to work item properties (create/response) and update tests to verify options appear in list/retrieve; bump version to 0.2.2.
optionstoWorkItemPropertyandCreateWorkItemPropertyto allow inline creation/return ofOPTIONchoices.optionsand assert options are present inlist/retrieveresponses.CreateWorkItemPropertyValue(value=...)and verify retrieval across types (including multi-value options).plane-sdkversion to0.2.2.Written by Cursor Bugbot for commit d0ce071. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.