[SILO-723] feat: add support to set options in create option property api #21
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Caution Review failedThe pull request is closed. WalkthroughAdded partial-update (PATCH) and delete (DELETE) endpoints for WorkItem property values, changed the property value model to a flat Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 0
🧹 Nitpick comments (2)
tests/unit/work-item-types/properties-values.test.ts (1)
29-61: Add test coverage for the newupdateanddeletemethods.The test only covers
create,retrieve, andlistoperations. The Values API now exposesupdateanddeletemethods (added insrc/api/WorkItemProperties/Values.ts), but this workflow test doesn't verify them.Consider extending the test to include:
expect(workItemPropertyValue).toBeDefined(); // Retrieve the work item property value const retrievedWorkItemPropertyValue = await client.workItemProperties.values.retrieve( workspaceSlug, projectId, workItemId, propertyId ); expect(retrievedWorkItemPropertyValue).toBeDefined(); + + // Update the work item property value + const updatedValue = randomizeName("Updated WI Property Value"); + const updatedWorkItemPropertyValue = await client.workItemProperties.values.update( + workspaceSlug, + projectId, + workItemId, + propertyId, + { + value: updatedValue, + } + ); + + expect(updatedWorkItemPropertyValue).toBeDefined(); // List all work item property values for the work item const workItemPropertyValues = await client.workItemProperties.values.list(workspaceSlug, projectId, workItemId, { limit: 10, offset: 0, }); expect(workItemPropertyValues).toBeDefined(); + + // Delete the work item property value + await client.workItemProperties.values.delete( + workspaceSlug, + projectId, + workItemId, + propertyId + ); });src/api/WorkItemProperties/Values.ts (1)
1-7: Minor: Inconsistent import formatting.The imports mix single and double quotes. While this doesn't affect functionality, consistent quote usage improves readability.
-import { BaseResource } from "../BaseResource"; -import { Configuration } from "../../Configuration"; +import { BaseResource } from '../BaseResource'; +import { Configuration } from '../../Configuration'; import { UpdateWorkItemPropertyValue, ListWorkItemPropertyValuesParams, WorkItemPropertyValues, -} from "../../models/WorkItemProperty"; +} from '../../models/WorkItemProperty';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/api/WorkItemProperties/Values.ts(4 hunks)src/models/WorkItemProperty.ts(2 hunks)tests/unit/work-item-types/properties-options.test.ts(2 hunks)tests/unit/work-item-types/properties-values.test.ts(1 hunks)
🔇 Additional comments (11)
tests/unit/work-item-types/properties-values.test.ts (1)
38-38: LGTM! Payload structure aligns with the updated model.The simplified
valuefield correctly reflects the newUpdateWorkItemPropertyValuestructure defined insrc/models/WorkItemProperty.ts.tests/unit/work-item-types/properties-options.test.ts (5)
1-5: LGTM! Import cleanup improves clarity.Removing unused
WorkItemPropertyOptionimport and simplifying thedescribeIfalias usage makes the imports cleaner.
28-86: LGTM! Comprehensive TEXT property lifecycle test.The test thoroughly covers the full CRUD workflow (create → retrieve → update → list → delete) with proper assertions on IDs, types, and list results.
88-182: LGTM! Well-structured OPTION property tests with proper cleanup.The test suite properly uses
beforeEach/afterEachhooks to manage test isolation and cleanup. The tests verify:
- Property creation with predefined options
- Option metadata (name, description, is_default, is_active)
- Property retrieval, update, and listing
184-254: LGTM! Property options CRUD workflow is comprehensive.The test properly validates the full lifecycle of property options (create → retrieve → update → delete) with appropriate cleanup using
beforeEach/afterEachhooks.
91-108: Thedescriptionfield is fully supported by the Plane API.Based on verification through Plane's official API documentation, work item property options include a
descriptionfield across all supported endpoints (POST/GET/PATCH/DELETE/api/v1/.../issue-properties/:property_id/options/). The test code correctly implements this field, and the backend accepts and persists it. No changes are required.src/models/WorkItemProperty.ts (2)
105-105: LGTM! Useful addition for option metadata.Adding the optional
descriptionfield toWorkItemPropertyOptionallows for more descriptive options and aligns with the test data inproperties-options.test.ts.
141-165: LGTM! Well-documented breaking change with clear multi-value support.The restructured
UpdateWorkItemPropertyValuetype:
- Simplifies the structure from nested
valuesarray to a singlevaluefield- Supports multi-value properties via
string[]type- Adds external metadata fields (
external_id,external_source)- Includes comprehensive documentation for different property types
The breaking change is properly reflected in the test updates.
src/api/WorkItemProperties/Values.ts (3)
48-70: LGTM! Clarified documentation for create behavior.The updated documentation clearly explains the difference between single-value (upsert) and multi-value (sync) behavior, which is important for API consumers to understand.
72-96: LGTM! Newupdatemethod provides partial update capability.The method:
- Uses PATCH for partial updates (as opposed to POST's upsert behavior)
- Properly documents single-value vs multi-value behavior
- Documents error handling (404 when value doesn't exist)
- Aligns with the updated
UpdateWorkItemPropertyValuetype
98-113: LGTM! Newdeletemethod enables value removal.The method:
- Uses
httpDeletefor proper HTTP DELETE semantics- Documents behavior for both single and multi-value properties
- Documents error handling (404 when value doesn't exist)
- Follows consistent parameter patterns with other methods
Description
Type of Change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.