feat: expand API resource coverage and harden test suite#53
feat: expand API resource coverage and harden test suite#53akhil-vamshi-konam wants to merge 8 commits into
Conversation
📝 WalkthroughWalkthroughThe SDK expands Plane API coverage with Collections, Estimates, Roles, Release sub-resources, Work Item relations, lite listings, additional property/page operations, updated models and exports, configurable test logging, dotenv-based test setup, and broader integration tests. ChangesAPI resource expansion
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (13)
src/api/Collections/Members.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse kebab-case for the new source filenames.
src/api/Collections/Members.ts#L1-L3: rename tomembers.tsand update imports.src/api/Collections/Pages.ts#L1-L11: rename topages.tsand update imports.src/api/Estimates.ts#L1-L11: rename toestimates.tsand update imports.src/api/Roles.ts#L1-L4: rename toroles.tsand update imports.As per coding guidelines,
src/**/*.tsfiles must “Use kebab-case for file names.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Collections/Members.ts` around lines 1 - 3, Rename src/api/Collections/Members.ts to members.ts, src/api/Collections/Pages.ts to pages.ts, src/api/Estimates.ts to estimates.ts, and src/api/Roles.ts to roles.ts; update every import or reference to use the new kebab-case filenames.Source: Coding guidelines
src/api/WorkItems/CustomRelations.ts (2)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse kebab-case resource filenames consistently.
src/api/WorkItems/CustomRelations.ts#L1-L3: rename tocustom-relations.ts.src/api/WorkItems/Dependencies.ts#L1-L7: rename todependencies.ts.src/api/WorkItems/Pages.ts#L1-L4: rename topages.ts.src/api/WorkItems/index.ts#L23-L25: update imports for the renamed modules.As per coding guidelines,
src/**/*.tsfilenames must use kebab-case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/WorkItems/CustomRelations.ts` around lines 1 - 3, Rename CustomRelations.ts to custom-relations.ts, Dependencies.ts to dependencies.ts, and Pages.ts to pages.ts to comply with kebab-case filename conventions. Update the imports in src/api/WorkItems/index.ts lines 23-25 to reference the renamed modules, preserving the existing exports and behavior.Source: Coding guidelines
51-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStandardize deletion methods as
del.
src/api/WorkItems/CustomRelations.ts#L51-L54: renameremovetodel.src/api/WorkItems/Dependencies.ts#L46-L49: renameremovetodel.src/api/WorkItems/Pages.ts#L62-L65: renamedeletetodel.Update the corresponding test call sites with the same API rename.
As per coding guidelines, standard resource methods must be named
list,create,retrieve,update, anddel.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/WorkItems/CustomRelations.ts` around lines 51 - 54, Standardize resource deletion methods to del: rename remove in src/api/WorkItems/CustomRelations.ts lines 51-54 and src/api/WorkItems/Dependencies.ts lines 46-49, and rename delete in src/api/WorkItems/Pages.ts lines 62-65. Update all corresponding test call sites to invoke del while preserving the existing deletion behavior.Source: Coding guidelines
src/api/WorkItems/Pages.ts (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the page-list query parameters.
params?: anybypasses SDK validation. Define an endpoint-specific query interface (or useRecord<string, unknown>until its fields are known).As per coding guidelines, avoid
anytypes; use proper typing orunknownwith type guards.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/WorkItems/Pages.ts` around lines 18 - 27, Replace the any-typed params argument in the list method with an endpoint-specific page-list query interface; if the supported fields are not yet known, use Record<string, unknown> instead. Preserve the existing request path and response typing while ensuring callers no longer bypass SDK type validation.Source: Coding guidelines
src/api/Pages.ts (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace public
anycontracts with SDK types.These methods weaken the exported API surface and violate the repository typing rule.
src/api/Pages.ts#L34-L35: introduce a typed page-list params contract.src/api/Pages.ts#L64-L65: use that same contract for project page listing.src/api/Projects.ts#L97-L98: return a typed work-log totals model, orunknownwith caller-side narrowing.As per coding guidelines,
src/**/*.ts: “Avoidanytypes; use proper typing orunknownwith type guards.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Pages.ts` around lines 34 - 35, Replace the public any contracts in src/api/Pages.ts lines 34-35 and 64-65 by defining and reusing a typed page-list parameters contract for listWorkspacePages and project page listing. In src/api/Projects.ts lines 97-98, replace the any work-log totals response with a typed totals model, or use unknown and require caller-side narrowing. Preserve the existing method behavior while complying with the repository’s no-any rule.Source: Coding guidelines
src/api/Releases/Links.ts (1)
17-23: 📐 Maintainability & Code Quality | 🔵 Trivial
params: anyviolates typing guideline.Will address via a consolidated comment covering the same pattern in
Comments.tsandWorkItems.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Releases/Links.ts` around lines 17 - 23, Replace the params: any type in the list method of Links.ts with the established typed query-parameter type used by the corresponding list methods in Comments.ts and WorkItems.ts, preserving optionality and existing request behavior.Source: Coding guidelines
src/api/Releases/WorkItems.ts (1)
17-23: 📐 Maintainability & Code Quality | 🔵 Trivial
params: anyviolates typing guideline.Will address via a consolidated comment covering the same pattern in
Comments.tsandLinks.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Releases/WorkItems.ts` around lines 17 - 23, Replace the any-typed params argument in WorkItems.list with the established typed query-parameter type used by the related Comments and Links list methods, preserving its optional behavior and existing request flow.Source: Coding guidelines
src/api/Releases/Comments.ts (1)
17-23: 📐 Maintainability & Code Quality | 🔵 Trivial
params: anyviolates typing guideline.As per coding guidelines,
src/**/*.tsshould avoidanytypes. This will be addressed via a consolidated comment covering the same pattern inLinks.tsandWorkItems.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Releases/Comments.ts` around lines 17 - 23, Replace the any type on the params argument of Comments.list with the appropriate specific parameter type already used by the API client, matching the established typing pattern in related methods such as Links and WorkItems while preserving the existing request and response handling.Source: Coding guidelines
tests/unit/releases/releases.test.ts (1)
13-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
afterAlldoesn't clean upcomment/linkon failure.Unlike
label/tag/release, the createdcommentandlinkare only deleted within their own dedicateditblocks (lines 203-205, 238-240). If an earlier test in the suite throws, these resources leak on the server.🧹 Proposed fix
afterAll(async () => { // Clean up created resources + if (link?.id) { + try { + await client.releases.links.delete(workspaceSlug, release.id!, link.id); + } catch (error) { + console.warn("Failed to delete release link:", error); + } + } + if (comment?.id) { + try { + await client.releases.comments.delete(workspaceSlug, release.id!, comment.id); + } catch (error) { + console.warn("Failed to delete release comment:", error); + } + } if (label?.id) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/releases/releases.test.ts` around lines 13 - 44, Add cleanup for the created comment and link in the afterAll teardown, guarded by their IDs and using the corresponding release comment/link deletion APIs. Follow the existing label, tag, and release cleanup pattern so resources are removed even when their dedicated tests fail.src/models/Page.ts (1)
20-25: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep server-managed fields out of
CreatePage.
Partial<Page>allows response-only fields such ascreated_byandupdated_byto be sent in create requests. DefineCreatePagefrom an explicitPick/Omitof createable fields, then addparent_idandcollection_idto that constrained DTO.As per coding guidelines, model DTOs should be separate and derived with
Pick,Omit, andPartial.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/Page.ts` around lines 20 - 25, Update the CreatePage type to derive from an explicit Pick, Omit, and/or Partial of fields allowed during creation rather than Partial<Page>, excluding server-managed response fields such as created_by and updated_by. Preserve the existing optional parent_id and collection_id additions.Source: Coding guidelines
src/models/Collection.ts (1)
42-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive request DTOs from the corresponding entity models.
The collection and estimate request interfaces manually duplicate response fields, allowing model and request contracts to drift. Use
Pick,Omit, andPartialconsistently at each site.
src/models/Collection.ts#L42-L57: deriveCreateCollectionandUpdateCollectionfromCollection.src/models/Collection.ts#L74-L82: derive member create/update DTOs fromCollectionMember.src/models/Estimate.ts#L24-L41: derive estimate create/update DTOs fromEstimate.src/models/Estimate.ts#L62-L78: derive estimate-point create/update DTOs fromEstimatePoint.As per coding guidelines, model Create/Update DTOs should use interfaces with
Pick,Omit, andPartial.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/Collection.ts` around lines 42 - 57, Derive all request DTOs from their corresponding entity models using interface composition with Pick, Omit, and Partial instead of duplicating fields. Update src/models/Collection.ts lines 42-57 for CreateCollection and UpdateCollection, src/models/Collection.ts lines 74-82 for the CollectionMember create/update DTOs, src/models/Estimate.ts lines 24-41 for Estimate create/update DTOs, and src/models/Estimate.ts lines 62-78 for EstimatePoint create/update DTOs; preserve the existing allowed and excluded fields, including Collection access remaining immutable after creation.Source: Coding guidelines
src/api/BaseResource.ts (1)
114-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the new DELETE query parameters.
The new
params?: anyexpands an untyped API surface. Introduce a shared query-parameter type, such asRecord<string, unknown>, while preserving any richer shapes required by Axios.As per coding guidelines,
src/**/*.tsmust avoidanyand use proper typing orunknownwith type guards.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/BaseResource.ts` around lines 114 - 119, Update the httpDelete method’s params argument to use a shared typed query-parameter definition instead of any, such as Record<string, unknown>, while retaining compatibility with Axios-supported richer parameter shapes. Ensure the related BaseResource API contains no any and uses unknown only with appropriate narrowing.Source: Coding guidelines
src/models/Role.ts (1)
1-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename new model files to kebab-case.
src/models/Role.ts#L1-L33: rename tosrc/models/role.tsand update imports/re-exports.src/models/WorkItemPage.ts#L1-L40: rename tosrc/models/work-item-page.tsand update imports/re-exports.As per coding guidelines, “Use kebab-case for file names.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/Role.ts` around lines 1 - 33, Rename src/models/Role.ts to src/models/role.ts and src/models/WorkItemPage.ts to src/models/work-item-page.ts, then update every import and re-export referencing these model modules to use the kebab-case paths. Preserve the existing Role and WorkItemPage symbols and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/BaseResource.ts`:
- Around line 158-162: Update the enableLogging branch in BaseResource’s
error-handling flow to replace raw error logging with a sanitized structured
representation. Include only safe diagnostic fields, excluding Authorization,
X-Api-Key, request data, response data, and other sensitive Axios details, while
preserving the existing conditional console.error behavior.
In `@src/api/Collections/index.ts`:
- Around line 59-62: Rename the public CRUD method `delete` to `del` in the
collections API while preserving its parameters and request behavior. Update all
callers in the collections unit tests to use `del`, ensuring no references to
the old method name remain.
In `@src/api/Customers/index.ts`:
- Around line 62-66: The customer API method should use the standard del naming
convention: rename Customers.deleteByExternalId to delByExternalId in
src/api/Customers/index.ts at lines 62-66, then update the corresponding test
call in tests/unit/customers/property-values.test.ts at lines 84-86 to use
delByExternalId.
In `@src/api/Estimates.ts`:
- Around line 48-50: Rename the public CRUD method delete to del in Estimates,
preserving its existing request and return behavior. Update all corresponding
calls and assertions in tests/unit/estimate.test.ts to use del, while leaving
the other standard resource methods unchanged.
In `@src/api/WorkItems/index.ts`:
- Around line 130-158: Update the parameter types used by listWorkspace and
listArchived to expose their documented query options, including filters,
order_by, cursor, per_page, fields, and expand where supported, while retaining
existing options. Add endpoint-appropriate interfaces near the existing
ListWorkItemsParams definitions and use them in both method signatures instead
of the overly restrictive shared type.
In `@src/api/WorkspaceWorkItemProperties/Options.ts`:
- Around line 54-56: Rename the public delete methods to del for consistency:
update Options.delete in src/api/WorkspaceWorkItemProperties/Options.ts:54-56
and WorkspaceWorkItemTypes.delete in
src/api/WorkspaceWorkItemTypes/index.ts:37-39; update the corresponding test
calls to options.del(...) at tests/unit/workspace-work-item-types.test.ts:83-85
and workspaceWorkItemTypes.del(...) at :95-97.
In `@tests/unit/customers/property-values.test.ts`:
- Around line 47-50: Strengthen the assertion in the “should read back the
bulk-set value” test by verifying the listed values include the expected
propertyId and the value “bulk value,” rather than only checking that the
response is defined. Keep the existing listValues call and use the returned
values collection for the content assertion.
In `@tests/unit/page.test.ts`:
- Around line 76-91: Do not let the workspace-page deletion test pass when
DELETE is unsupported: update the create/list/delete flow in
tests/unit/page.test.ts lines 76-91 to gate execution on a known server
capability or rethrow the 405, while removing the any and console usage. Apply
the same handling to project-page deletion in tests/unit/page.test.ts lines
93-107 so unsupported SDK operations remain visible and no test data is silently
left behind.
In `@tests/unit/work-items/archive.test.ts`:
- Around line 22-29: Update the archive test setup around completedState to
select an eligible completed or cancelled state; if neither exists, fail or skip
setup before invoking the archive operation. Ensure the work item state is
updated only when a valid archivable state has been found.
In `@tests/unit/workspace-work-item-types.test.ts`:
- Around line 86-91: Make live-test cleanup unconditional and failure-visible:
in tests/unit/workspace-work-item-types.test.ts at lines 86-91, let workspace
property deletion failures propagate instead of converting them to warnings; at
lines 95-98, register guarded work-item-type cleanup in afterAll; in
tests/unit/work-items/archive.test.ts at lines 32-40, restore archived state
when necessary and propagate deletion failures.
---
Nitpick comments:
In `@src/api/BaseResource.ts`:
- Around line 114-119: Update the httpDelete method’s params argument to use a
shared typed query-parameter definition instead of any, such as Record<string,
unknown>, while retaining compatibility with Axios-supported richer parameter
shapes. Ensure the related BaseResource API contains no any and uses unknown
only with appropriate narrowing.
In `@src/api/Collections/Members.ts`:
- Around line 1-3: Rename src/api/Collections/Members.ts to members.ts,
src/api/Collections/Pages.ts to pages.ts, src/api/Estimates.ts to estimates.ts,
and src/api/Roles.ts to roles.ts; update every import or reference to use the
new kebab-case filenames.
In `@src/api/Pages.ts`:
- Around line 34-35: Replace the public any contracts in src/api/Pages.ts lines
34-35 and 64-65 by defining and reusing a typed page-list parameters contract
for listWorkspacePages and project page listing. In src/api/Projects.ts lines
97-98, replace the any work-log totals response with a typed totals model, or
use unknown and require caller-side narrowing. Preserve the existing method
behavior while complying with the repository’s no-any rule.
In `@src/api/Releases/Comments.ts`:
- Around line 17-23: Replace the any type on the params argument of
Comments.list with the appropriate specific parameter type already used by the
API client, matching the established typing pattern in related methods such as
Links and WorkItems while preserving the existing request and response handling.
In `@src/api/Releases/Links.ts`:
- Around line 17-23: Replace the params: any type in the list method of Links.ts
with the established typed query-parameter type used by the corresponding list
methods in Comments.ts and WorkItems.ts, preserving optionality and existing
request behavior.
In `@src/api/Releases/WorkItems.ts`:
- Around line 17-23: Replace the any-typed params argument in WorkItems.list
with the established typed query-parameter type used by the related Comments and
Links list methods, preserving its optional behavior and existing request flow.
In `@src/api/WorkItems/CustomRelations.ts`:
- Around line 1-3: Rename CustomRelations.ts to custom-relations.ts,
Dependencies.ts to dependencies.ts, and Pages.ts to pages.ts to comply with
kebab-case filename conventions. Update the imports in
src/api/WorkItems/index.ts lines 23-25 to reference the renamed modules,
preserving the existing exports and behavior.
- Around line 51-54: Standardize resource deletion methods to del: rename remove
in src/api/WorkItems/CustomRelations.ts lines 51-54 and
src/api/WorkItems/Dependencies.ts lines 46-49, and rename delete in
src/api/WorkItems/Pages.ts lines 62-65. Update all corresponding test call sites
to invoke del while preserving the existing deletion behavior.
In `@src/api/WorkItems/Pages.ts`:
- Around line 18-27: Replace the any-typed params argument in the list method
with an endpoint-specific page-list query interface; if the supported fields are
not yet known, use Record<string, unknown> instead. Preserve the existing
request path and response typing while ensuring callers no longer bypass SDK
type validation.
In `@src/models/Collection.ts`:
- Around line 42-57: Derive all request DTOs from their corresponding entity
models using interface composition with Pick, Omit, and Partial instead of
duplicating fields. Update src/models/Collection.ts lines 42-57 for
CreateCollection and UpdateCollection, src/models/Collection.ts lines 74-82 for
the CollectionMember create/update DTOs, src/models/Estimate.ts lines 24-41 for
Estimate create/update DTOs, and src/models/Estimate.ts lines 62-78 for
EstimatePoint create/update DTOs; preserve the existing allowed and excluded
fields, including Collection access remaining immutable after creation.
In `@src/models/Page.ts`:
- Around line 20-25: Update the CreatePage type to derive from an explicit Pick,
Omit, and/or Partial of fields allowed during creation rather than
Partial<Page>, excluding server-managed response fields such as created_by and
updated_by. Preserve the existing optional parent_id and collection_id
additions.
In `@src/models/Role.ts`:
- Around line 1-33: Rename src/models/Role.ts to src/models/role.ts and
src/models/WorkItemPage.ts to src/models/work-item-page.ts, then update every
import and re-export referencing these model modules to use the kebab-case
paths. Preserve the existing Role and WorkItemPage symbols and behavior.
In `@tests/unit/releases/releases.test.ts`:
- Around line 13-44: Add cleanup for the created comment and link in the
afterAll teardown, guarded by their IDs and using the corresponding release
comment/link deletion APIs. Follow the existing label, tag, and release cleanup
pattern so resources are removed even when their dedicated tests fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 65b4bc25-f17d-47bc-acd6-cdaf4f4bb24f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (78)
README.mdjest.config.jspackage.jsonsrc/api/BaseResource.tssrc/api/Collections/Members.tssrc/api/Collections/Pages.tssrc/api/Collections/index.tssrc/api/Customers/Properties.tssrc/api/Customers/index.tssrc/api/Cycles.tssrc/api/Estimates.tssrc/api/Intake.tssrc/api/Members.tssrc/api/Modules.tssrc/api/Pages.tssrc/api/Projects.tssrc/api/Releases/Changelog.tssrc/api/Releases/Comments.tssrc/api/Releases/ItemLabels.tssrc/api/Releases/Labels.tssrc/api/Releases/Links.tssrc/api/Releases/Tags.tssrc/api/Releases/WorkItems.tssrc/api/Releases/index.tssrc/api/Roles.tssrc/api/Users.tssrc/api/WorkItemProperties/index.tssrc/api/WorkItemTypes.tssrc/api/WorkItems/CustomRelations.tssrc/api/WorkItems/Dependencies.tssrc/api/WorkItems/Pages.tssrc/api/WorkItems/index.tssrc/api/Workspace.tssrc/api/WorkspaceWorkItemProperties/Options.tssrc/api/WorkspaceWorkItemProperties/index.tssrc/api/WorkspaceWorkItemTypes/index.tssrc/client/plane-client.tssrc/index.tssrc/models/Collection.tssrc/models/Customer.tssrc/models/Cycle.tssrc/models/Estimate.tssrc/models/Intake.tssrc/models/Member.tssrc/models/Module.tssrc/models/Page.tssrc/models/Project.tssrc/models/Release.tssrc/models/Role.tssrc/models/User.tssrc/models/WorkItem.tssrc/models/WorkItemPage.tssrc/models/index.tstests/unit/collections.test.tstests/unit/customers/property-values.test.tstests/unit/epic.test.tstests/unit/estimate.test.tstests/unit/initiative.test.tstests/unit/intake.test.tstests/unit/lite-listings.test.tstests/unit/module.test.tstests/unit/page.test.tstests/unit/project-templates.test.tstests/unit/releases/releases.test.tstests/unit/role.test.tstests/unit/work-item-types/project-properties.test.tstests/unit/work-item-types/properties-options.test.tstests/unit/work-item-types/types.test.tstests/unit/work-items/archive.test.tstests/unit/work-items/custom-relations.test.tstests/unit/work-items/dependencies.test.tstests/unit/work-items/pages.test.tstests/unit/work-items/relations.test.tstests/unit/workflows/workflow.test.tstests/unit/workspace-project-states.test.tstests/unit/workspace-templates/workspace-templates.test.tstests/unit/workspace-work-item-types.test.tstests/unit/workspace.test.ts
💤 Files with no reviewable changes (1)
- src/api/Members.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/collections.test.ts (1)
7-11: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate each suite on its required feature flags.
The documentation says
WORKSPACE_PAGESandPRIVATE_COLLECTIONSare prerequisites, but every suite checks onlyconfig.workspaceSlug. A workspace without those flags will run the suites and fail instead of being skipped.Also applies to: 74-74, 154-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/collections.test.ts` around lines 7 - 11, Update the suite gates in “Collections API Tests” and the additional collection suites to check the feature flags required by each suite, not only config.workspaceSlug. Require WORKSPACE_PAGES for workspace-page cases and require both WORKSPACE_PAGES and PRIVATE_COLLECTIONS for private-collection cases, so unsupported configurations are skipped.
🧹 Nitpick comments (2)
tests/unit/work-items/pages.test.ts (1)
21-27: 🗄️ Data Integrity & Integration | 🔵 TrivialRun this live suite against a disposable or dedicated workspace.
Because the created page cannot be deleted, every execution leaves persistent test data. Isolate these tests from shared workspaces or provide server-side cleanup to prevent accumulation and cross-run pollution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/work-items/pages.test.ts` around lines 21 - 27, Update the setup around client.pages.createProjectPage in the test suite to require a disposable or dedicated workspace for these live tests, preventing persistent page data from polluting shared workspaces; if cleanup support is available, use it after each test instead.tests/unit/collections.test.ts (1)
91-99: 🗄️ Data Integrity & Integration | 🔵 TrivialRun page-creating suites against a disposable or dedicated workspace.
The standalone and inline pages cannot be deleted, so repeated executions permanently accumulate test data. Avoid shared workspaces or add server-side cleanup.
Also applies to: 140-150
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/collections.test.ts` around lines 91 - 99, Update the workspace setup and cleanup flow around createWorkspacePage and the affected standalone/inline page tests to use a disposable or dedicated workspace per test suite, rather than a shared workspace. Preserve the existing source/target collection cleanup and ensure the dedicated workspace is removed through the available server cleanup mechanism after the tests complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/epic.test.ts`:
- Around line 23-27: Update the “should retrieve an epic” test so it never
silently passes when epics.results is empty: require a known/seeded epic fixture
for client.epics.retrieve, or explicitly skip the test using the test
framework’s skip mechanism while preserving retrieval assertions when an epic
exists.
---
Outside diff comments:
In `@tests/unit/collections.test.ts`:
- Around line 7-11: Update the suite gates in “Collections API Tests” and the
additional collection suites to check the feature flags required by each suite,
not only config.workspaceSlug. Require WORKSPACE_PAGES for workspace-page cases
and require both WORKSPACE_PAGES and PRIVATE_COLLECTIONS for private-collection
cases, so unsupported configurations are skipped.
---
Nitpick comments:
In `@tests/unit/collections.test.ts`:
- Around line 91-99: Update the workspace setup and cleanup flow around
createWorkspacePage and the affected standalone/inline page tests to use a
disposable or dedicated workspace per test suite, rather than a shared
workspace. Preserve the existing source/target collection cleanup and ensure the
dedicated workspace is removed through the available server cleanup mechanism
after the tests complete.
In `@tests/unit/work-items/pages.test.ts`:
- Around line 21-27: Update the setup around client.pages.createProjectPage in
the test suite to require a disposable or dedicated workspace for these live
tests, preventing persistent page data from polluting shared workspaces; if
cleanup support is available, use it after each test instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7419c18a-f0be-4242-956b-7016c8ed7427
📒 Files selected for processing (8)
package.jsonsrc/api/BaseResource.tssrc/api/Pages.tssrc/models/WorkItem.tstests/unit/collections.test.tstests/unit/epic.test.tstests/unit/page.test.tstests/unit/work-items/pages.test.ts
💤 Files with no reviewable changes (2)
- tests/unit/page.test.ts
- src/api/Pages.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/api/BaseResource.ts
- src/models/WorkItem.ts
Saurabhkmr98
left a comment
There was a problem hiding this comment.
Reviewed via a 4-way parallel pass over new resources, WorkItems sub-resources, the Releases subtree, and fixes to existing resources. Most of the diff (Collections, Estimates, Roles, the Releases subtree, WorkItemProperties/WorkspaceWorkItemProperties/WorkspaceWorkItemTypes, and the fixes to pre-existing resources) checked out clean. Left two inline comments below.
(Note: I also spotted an unrelated pre-existing bug in src/api/WorkspaceTemplates/WorkItems.ts / src/api/ProjectTemplates/WorkItems.ts — unhyphenated workitems/templates/ vs the codebase's consistent work-items/ — but those files aren't touched by this PR, so I'm not commenting on them here; worth a separate issue.)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/workspace-work-item-types.test.ts`:
- Line 55: Update the test guard around config.projectId so missing projectId
explicitly skips the Jest test instead of returning from its body. Use the
repository’s conditional-test helper or define the case with it.skip, while
preserving normal assertion execution when projectId is available.
- Around line 57-59: Update the importToProject assertion in the workItemTypes
test to match the endpoint’s empty response: await the promise directly or
assert that it resolves to undefined, instead of using resolves.toBeDefined().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 12774670-b866-4af2-b4df-bbff206378d2
📒 Files selected for processing (1)
tests/unit/workspace-work-item-types.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/api/Epics.ts (2)
36-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the standard delete method
del.This newly added resource method violates the required standard API method name and makes the public surface inconsistent.
As per coding guidelines, standard resource methods must be named
list,create,retrieve,update, anddel.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Epics.ts` around lines 36 - 40, Rename the Epics resource method delete to del while preserving its parameters, return type, and existing httpDelete endpoint behavior; keep the standard methods list, create, retrieve, and update unchanged.Source: Coding guidelines
46-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove permissive
anyfrom public parameter contracts.The new Epic list signatures and the updated initiative pagination interface both bypass TypeScript validation with
any.
src/api/Epics.ts#L46-L62: define endpoint-specific list parameter interfaces instead ofparams?: any.src/models/Initiative.ts#L35-L38: replace theanyindex signature withunknown(or a bounded parameter type) so callers cannot silently pass unchecked values.As per coding guidelines,
src/**/*.tsmust avoidanytypes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/Epics.ts` around lines 46 - 62, Replace the permissive params?: any contracts in src/api/Epics.ts lines 46-62, covering EpicApi.list and EpicApi.listIssues, with endpoint-specific parameter interfaces that provide TypeScript validation while preserving pagination behavior. In src/models/Initiative.ts lines 35-38, replace the any index signature with unknown or an appropriately bounded parameter type. Ensure neither site uses any.Source: Coding guidelines
src/models/Epic.ts (2)
36-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the create DTO from
Epic.
UpdateEpiccorrectly usesPartial, butCreateEpicmanually repeats entity fields. UsePick<Epic, ...>plus the API-only*_idfields so entity and request contracts cannot drift.As per coding guidelines, model Create/Update DTOs must use
Pick,Omit, andPartial.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/Epic.ts` around lines 36 - 51, Update CreateEpic to derive entity-backed fields from Epic with Pick<Epic, ...>, while retaining the API-only state_id, parent_id, assignee_ids, and label_ids fields alongside any other request-only fields. Keep UpdateEpic as Partial<CreateEpic> and remove duplicated entity field declarations so the DTO stays synchronized with Epic.Source: Coding guidelines
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Work Item terminology throughout the Epic API.
The public model and resource names violate the required naming convention; retain the server’s
/issues/URL but rename the SDK symbols.
src/models/Epic.ts#L60-L60: renameEpicIssuetoEpicWorkItem.src/api/Epics.ts#L53-L74: renamelistIssues/addIssuestolistWorkItems/addWorkItemsand use the renamed model.tests/unit/epic.test.ts#L87-L102: update test calls for the renamed public methods.As per coding guidelines,
src/**/*.tsmust never use “Issue” in names and must use “Work Item” instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/Epic.ts` at line 60, Rename EpicIssue to EpicWorkItem in src/models/Epic.ts:60-60, and update src/api/Epics.ts:53-74 to rename listIssues/addIssues to listWorkItems/addWorkItems while using EpicWorkItem; retain the /issues/ server URL. Update the corresponding test calls in tests/unit/epic.test.ts:87-102, and ensure no src/**/*.ts public symbols use “Issue” terminology.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/epic.test.ts`:
- Around line 24-31: Update the catch block in the epic test to read the HTTP
status from error.response?.status and derive the message from
error.response?.data, including the relevant epics, error, or detail fields. Use
these Axios response properties for the 400/unsupported-epics check while
preserving the existing warning, skip behavior, and rethrow for other errors.
In `@tests/unit/work-items/work-items.test.ts`:
- Around line 166-170: Update the filtered results assertions in the work-items
test to verify that the created urgent work item is included by asserting
filtered.results contains urgent.id. Keep the existing non-empty and priority
assertions, using the created urgent fixture rather than relying only on
pre-existing urgent items.
---
Nitpick comments:
In `@src/api/Epics.ts`:
- Around line 36-40: Rename the Epics resource method delete to del while
preserving its parameters, return type, and existing httpDelete endpoint
behavior; keep the standard methods list, create, retrieve, and update
unchanged.
- Around line 46-62: Replace the permissive params?: any contracts in
src/api/Epics.ts lines 46-62, covering EpicApi.list and EpicApi.listIssues, with
endpoint-specific parameter interfaces that provide TypeScript validation while
preserving pagination behavior. In src/models/Initiative.ts lines 35-38, replace
the any index signature with unknown or an appropriately bounded parameter type.
Ensure neither site uses any.
In `@src/models/Epic.ts`:
- Around line 36-51: Update CreateEpic to derive entity-backed fields from Epic
with Pick<Epic, ...>, while retaining the API-only state_id, parent_id,
assignee_ids, and label_ids fields alongside any other request-only fields. Keep
UpdateEpic as Partial<CreateEpic> and remove duplicated entity field
declarations so the DTO stays synchronized with Epic.
- Line 60: Rename EpicIssue to EpicWorkItem in src/models/Epic.ts:60-60, and
update src/api/Epics.ts:53-74 to rename listIssues/addIssues to
listWorkItems/addWorkItems while using EpicWorkItem; retain the /issues/ server
URL. Update the corresponding test calls in tests/unit/epic.test.ts:87-102, and
ensure no src/**/*.ts public symbols use “Issue” terminology.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e89214a-eccd-4a19-a49c-29b0af139959
📒 Files selected for processing (10)
package.jsonsrc/api/Cycles.tssrc/api/Epics.tssrc/api/Modules.tssrc/api/WorkItems/index.tssrc/models/Epic.tssrc/models/Initiative.tssrc/models/WorkItem.tstests/unit/epic.test.tstests/unit/work-items/work-items.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/api/Modules.ts
- src/models/WorkItem.ts
Summary
Broadens the SDK's endpoint coverage with several new resources and sub-resources, corrects a set of wrong endpoint URLs and models, and makes the live test suite reliable and honest.
New resources
New sub-resources & methods
Fixes
Tests
Summary by CodeRabbit