-
Notifications
You must be signed in to change notification settings - Fork 1
[SILO-437] fix: types for apis #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- add IssueSearchItemSerializer type for issues search endpoint - add a new IssueDetailSerializer extending IssueSerializer for detail api - exclude slug field from State API response - add many=True for serializer responses with arrays - add IssuePropertyValueAPIDetailSerializer for issue property values
WalkthroughThis update introduces new data models and adjusts API response handling to align with detailed entity representations. It updates the Node SDK publishing workflow to automate versioned GitHub releases, increments the SDK version, and updates documentation. Several model and API files are refactored to use new, more detailed types, with obsolete types removed and exports revised accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node SDK Publish Workflow
participant npm
participant GitHub Release
User->>Node SDK Publish Workflow: Trigger workflow
Node SDK Publish Workflow->>npm: Publish package
npm-->>Node SDK Publish Workflow: Publish complete
Node SDK Publish Workflow->>Node SDK Publish Workflow: Extract version from package.json
Node SDK Publish Workflow->>GitHub Release: Create release with tag "v<version>"
GitHub Release-->>Node SDK Publish Workflow: Release created
sequenceDiagram
participant Client
participant WorkItemsApi
participant WorkItemPropertiesApi
participant Models
Client->>WorkItemsApi: retrieveWorkItem(...)
WorkItemsApi->>Models: IssueDetailFromJSON(...)
Models-->>WorkItemsApi: IssueDetail
WorkItemsApi-->>Client: IssueDetail
Client->>WorkItemPropertiesApi: listIssueProperties(...)
WorkItemPropertiesApi->>Models: IssuePropertyAPIFromJSON(...)
Models-->>WorkItemPropertiesApi: Array<IssuePropertyAPI>
WorkItemPropertiesApi-->>Client: Array<IssuePropertyAPI>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.github/workflows/publish-node-sdk.yml(1 hunks).openapi-generator/FILES(2 hunks)README.md(2 hunks)package.json(1 hunks)src/apis/WorkItemPropertiesApi.ts(8 hunks)src/apis/WorkItemsApi.ts(4 hunks)src/models/IssueDetail.ts(1 hunks)src/models/IssueExpand.ts(2 hunks)src/models/IssuePropertyValueAPIDetail.ts(1 hunks)src/models/IssueSearch.ts(1 hunks)src/models/LabelLite.ts(0 hunks)src/models/index.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/models/LabelLite.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/apis/WorkItemsApi.ts (2)
src/runtime.ts (2)
InitOverrideFunction(297-297)ApiResponse(392-395)src/models/IssueDetail.ts (2)
IssueDetail(45-226)IssueDetailFromJSON(240-242)
src/models/IssueDetail.ts (3)
src/models/UserLite.ts (3)
UserLite(23-66)UserLiteFromJSON(75-77)UserLiteToJSON(95-97)src/models/Label.ts (3)
Label(23-114)LabelFromJSON(124-126)LabelToJSON(152-154)src/models/PriorityEnum.ts (2)
PriorityEnumFromJSON(44-46)PriorityEnumToJSON(52-54)
src/apis/WorkItemPropertiesApi.ts (4)
src/runtime.ts (2)
InitOverrideFunction(297-297)ApiResponse(392-395)src/models/IssuePropertyAPI.ts (2)
IssuePropertyAPI(35-180)IssuePropertyAPIFromJSON(193-195)src/models/IssuePropertyOptionAPI.ts (2)
IssuePropertyOptionAPI(20-129)IssuePropertyOptionAPIFromJSON(139-141)src/models/IssuePropertyValueAPIDetail.ts (2)
IssuePropertyValueAPIDetail(22-35)IssuePropertyValueAPIDetailFromJSON(46-48)
🪛 actionlint (1.7.7)
.github/workflows/publish-node-sdk.yml
37-37: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (21)
package.json (1)
3-3: LGTM! Version increment follows semantic versioning.The patch version bump from 0.1.3 to 0.1.4 appropriately reflects the bug fixes and type improvements mentioned in the PR objectives.
README.md (1)
1-1: LGTM! Documentation version references updated consistently.The version references in both the title and npm install command have been properly updated to match the package.json version change.
Also applies to: 39-39
.openapi-generator/FILES (1)
44-44: LGTM! Manifest correctly tracks new model files.The addition of
IssueDetail.tsandIssuePropertyValueAPIDetail.tsto the OpenAPI generator manifest aligns with the new detailed serializers mentioned in the PR objectives.Also applies to: 54-54
src/models/index.ts (1)
22-22: LGTM! New model exports properly added to public API.The exports for
IssueDetailandIssuePropertyValueAPIDetailcorrectly make these new detailed models available for consumption by SDK users.Also applies to: 32-32
src/models/IssueExpand.ts (2)
73-82: Verify breaking change and update JSDoc comments.The type change from object arrays to strings for
labelsandassigneesis a significant simplification that could be a breaking change for existing consumers.Additionally, the JSDoc comments don't reflect that these are now strings rather than arrays.
Please verify that:
- This change aligns with the actual API response format
- Existing consumers can handle this breaking change
- Consider updating the JSDoc comments to reflect the new string types
/** * - * @type {string} + * @type {string} - Comma-separated label names * @memberof IssueExpand */ readonly labels?: string; /** * - * @type {string} + * @type {string} - Comma-separated assignee identifiers * @memberof IssueExpand */ readonly assignees?: string;
270-271: LGTM! JSON parsing logic correctly updated for string types.The parsing logic has been appropriately simplified to handle the new string format for labels and assignees.
.github/workflows/publish-node-sdk.yml (1)
26-34: LGTM: Version extraction logic is robust.The version extraction step properly validates the extracted version and fails gracefully if the version cannot be determined from package.json.
src/models/IssueSearch.ts (1)
24-26: LGTM: Documentation improvement enhances clarity.The simplified documentation is more concise and clearly describes the purpose of the IssueSearch interface as providing search results for work items.
src/apis/WorkItemsApi.ts (2)
18-18: LGTM: Added necessary imports for IssueDetail type.The imports are correctly added to support the new IssueDetail type usage in the API methods.
Also applies to: 27-28
379-379: IssueDetail upgrade verified—no breaking changes foundA search for
retrieveWorkItemacross the TypeScript codebase only returned its definitions insrc/apis/WorkItemsApi.ts(no external calls to update), so switching the return type fromIssuetoIssueDetailis safe and requires no further action.src/models/IssuePropertyValueAPIDetail.ts (3)
15-35: LGTM: Well-structured interface with clear documentation.The interface properly represents aggregated issue property values with clear field descriptions and appropriate type definitions.
40-44: LGTM: Type guard implementation is correct.The
instanceOfIssuePropertyValueAPIDetailfunction properly validates both required fields (propertyIdandvalues) are present and defined.
50-59: LGTM: JSON conversion handles snake_case mapping correctly.The serialization and deserialization functions properly handle the conversion between
property_id(snake_case) andpropertyId(camelCase), following the established pattern in the codebase.Also applies to: 65-75
src/models/IssueDetail.ts (4)
37-44: LGTM: Comprehensive interface documentation is clear.The documentation accurately describes the interface as handling complete work item lifecycle with full relationship management.
233-238: LGTM: Type guard validates essential required fields.The instance check correctly validates the three required fields:
assignees,labels, andname, which are the non-optional fields in the interface.
244-281: LGTM: JSON deserialization handles complex type conversions correctly.The deserialization properly:
- Converts snake_case to camelCase field names
- Parses ISO date strings to Date objects
- Maps nested arrays using appropriate converters (UserLiteFromJSON, LabelFromJSON)
- Handles null/undefined values appropriately
287-318: LGTM: JSON serialization properly excludes readonly fields.The
Omittype in the function signature correctly excludes readonly fields that shouldn't be serialized. The date formatting is appropriate:
- Full ISO strings for timestamps (
toISOString())- Date-only strings for date fields (
toISOString().substring(0,10))src/apis/WorkItemPropertiesApi.ts (4)
22-22: LGTM! Proper import of new detailed type.The addition of
IssuePropertyValueAPIDetailimport and its corresponding JSON conversion functions aligns with the API's evolution to provide more detailed property value responses.Also applies to: 38-39
506-506: Correct array handling for list properties endpoint.The changes properly update the
listIssuePropertiesmethods to handle array responses:
- Return type changed from single
IssuePropertyAPItoArray<IssuePropertyAPI>- JSON parsing updated to map over the array using
jsonValue.map(IssuePropertyAPIFromJSON)This aligns with the backend API returning arrays for list operations.
Also applies to: 553-553, 560-560
569-569: Correct array handling for list property options endpoint.The changes properly update the
listIssuePropertyOptionsmethods to handle array responses:
- Return type changed from single
IssuePropertyOptionAPItoArray<IssuePropertyOptionAPI>- JSON parsing updated to map over the array using
jsonValue.map(IssuePropertyOptionAPIFromJSON)This maintains consistency with other list endpoints.
Also applies to: 616-616, 623-623
632-632: Proper migration to detailed property value type with array handling.The changes correctly:
- Update return type to
Array<IssuePropertyValueAPIDetail>(using the new detailed type)- Update JSON parsing to use
IssuePropertyValueAPIDetailFromJSONfor each array element- Maintain consistency between raw and non-raw method signatures
This addresses both the array response issue and adopts the more detailed property value model.
Also applies to: 686-686, 693-693
h-release action
Description
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores