[INFRA-394] feat: add workspace-level API resources to SDK#39
Conversation
- Add WorkspaceTemplates resource with workitem, project, and page template sub-resources (list/create/update/delete each) - Add WorkspaceWorkItemTypes resource (list/create/update) with WorkspaceWorkItemTypeProperties sub-resource (list/link/unlink) - Add WorkspaceWorkItemProperties resource (list/create/update/delete) with WorkspaceWorkItemPropertyOptions sub-resource (list/create/update) - Add WorkspaceProjectLabels resource (list/create/update/delete) - Add WorkspaceProjectStates resource (list/create/update/delete) - Add WorkItemRelationDefinitions resource with is_default/is_active query param filters (list/create/update/delete) - Add Releases resource (list/create/update) with ReleaseTags, ReleaseLabels, and ReleaseItemLabels sub-resources - Add Pydantic models for all new resources - Register all 7 resources on PlaneClient - Export all new classes from plane/__init__.py - Add unit tests for all new resources Co-authored-by: Plane AI <noreply@plane.so>
|
Warning Review limit reached
More reviews will be available in 47 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds workspace-scoped API clients and Pydantic models for releases, work-item-relation-definitions, workspace templates, work item types/properties, and project labels/states; integrates them into PlaneClient and adds smoke/integration tests exercising CRUD lifecycles. ChangesWorkspace API extensions
🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
tests/unit/test_workspace_work_item_types.py (1)
45-59: 🏗️ Heavy liftAdd smoke coverage for property link/unlink mutations on
workspace_work_item_types.properties.This file only exercises
list, so regressions in the new mutation endpoints can ship unnoticed. A small create-link + delete-unlink lifecycle test would materially improve confidence.🤖 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/test_workspace_work_item_types.py` around lines 45 - 59, Add a new test in the TestWorkspaceWorkItemTypeProperties suite that, after obtaining a workspace WIT (as in test_list_properties_for_workspace_wit), performs a minimal create/link then delete/unlink lifecycle using the properties mutation methods: call client.workspace_work_item_types.properties.link(workspace_slug, wit.id, <minimal payload>) to link a property, assert the returned link object exists, then call client.workspace_work_item_types.properties.unlink(workspace_slug, wit.id, linked_id) to remove it and assert the unlink succeeded (or that the linked id is no longer present in properties.list). Ensure you handle the case when no WITs exist by skipping like the existing test.
🤖 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 `@plane/api/releases/item_labels.py`:
- Around line 25-49: The public methods on the release labels resource use
non-CRUD verbs: rename method add to create and remove to delete while
preserving their signatures and return types (change def add(self, ...) ->
list[ReleaseLabel] to def create(...) and def remove(...) to def delete(...));
update docstrings to reflect the CRUD names, adjust any internal calls or
external usages/tests that reference add/remove to call create/delete, and
ensure any exported symbols (e.g., __all__ or client wrappers) are updated so
the old names are not left dangling.
In `@plane/api/workspace_work_item_types/properties.py`:
- Around line 26-53: The link/unlink methods violate the CRUD + DTO contract:
rename link -> create and unlink -> delete, change both signatures to accept a
WorkspaceWorkItemTypePropertyLink DTO (not raw property_id), serialize the DTO
with .model_dump(exclude_none=True) when sending the request, and validate POST
responses with WorkItemProperty.model_validate(); for create call
self._post(f"{workspace_slug}/work-item-types/{type_id}/properties/",
dto.model_dump(exclude_none=True)) and for delete accept the same DTO but
extract dto.property_id to call
self._delete(f"{workspace_slug}/work-item-types/{type_id}/properties/{dto.property_id}/")
(delete returns None), updating references to the old link/unlink methods
accordingly.
In `@tests/unit/test_releases.py`:
- Around line 37-47: The test currently swallows all exceptions by wrapping the
client.releases.update call in a broad try/except and issuing warnings.warn,
which masks API failures; change this to let exceptions propagate (remove the
broad try/except) so the test fails on API errors, or if you must handle a known
SDK error, catch the specific SDK exception type (e.g., YourSdkError) and call
pytest.fail(...) with the error message; update the block around
client.releases.update/UpdateRelease accordingly and apply the same fix to the
other occurrence around lines 125-130 that also warns instead of failing.
- Around line 96-98: The test assumes model IDs are always set but release.id
and label.id are optional; before calling
client.releases.item_labels.list(workspace_slug, release.id) or passing
label_ids to endpoints, assert or validate that release.id and label.id are not
None (e.g., assert release.id is not None and assert label.id is not None) and
use the validated values; update the tests around the release/label handling
(including the similar block around lines 109-121) to extract and check ids
first, then call client.releases.item_labels.list and any endpoints that accept
label_ids with the guaranteed non-None id values.
In `@tests/unit/test_work_item_relation_definitions.py`:
- Around line 57-60: The teardown currently uses a blind catch "except Exception
as exc" around client.work_item_relation_definitions.delete(workspace_slug,
created.id); either replace this with a narrow exception type(s) that the delete
call can raise (e.g., HTTPError, ApiError, or the client-specific exception) and
handle/log exc via warnings.warn, or if the broad catch is intentional for
best-effort cleanup, mark it explicitly with a noqa to satisfy Ruff (add "#
noqa: BLE001" to the except line) and include a short comment explaining why the
broad catch is necessary; locate this in the test teardown where variables
workspace_slug, created.id, client.work_item_relation_definitions.delete and
warnings.warn are used.
In `@tests/unit/test_workspace_templates.py`:
- Line 49: Narrow the teardown exception handling by replacing the broad "except
Exception as exc" around the client.workspace_templates.*.delete(...) calls with
a specific "except HttpError as exc" so only HTTP response errors are caught;
import HttpError from plane.errors.errors at the top of
tests/unit/test_workspace_templates.py and update the three except blocks (the
ones wrapping client.workspace_templates.create(...).delete(...) teardown calls)
to use HttpError instead of Exception so other exceptions still fail the test.
In `@tests/unit/test_workspace_work_item_types.py`:
- Around line 56-58: The test currently uses wit.id without checking for None
which can produce an invalid URL; add an explicit non-null assertion for the
work item type ID before calling
client.workspace_work_item_types.properties.list — e.g., assert wit.id is not
None (with a short message referencing workspace_slug and wit) — then use wit.id
safely when calling
client.workspace_work_item_types.properties.list(workspace_slug, wit.id) and
assert the returned props is a list.
---
Nitpick comments:
In `@tests/unit/test_workspace_work_item_types.py`:
- Around line 45-59: Add a new test in the TestWorkspaceWorkItemTypeProperties
suite that, after obtaining a workspace WIT (as in
test_list_properties_for_workspace_wit), performs a minimal create/link then
delete/unlink lifecycle using the properties mutation methods: call
client.workspace_work_item_types.properties.link(workspace_slug, wit.id,
<minimal payload>) to link a property, assert the returned link object exists,
then call client.workspace_work_item_types.properties.unlink(workspace_slug,
wit.id, linked_id) to remove it and assert the unlink succeeded (or that the
linked id is no longer present in properties.list). Ensure you handle the case
when no WITs exist by skipping like the existing test.
🪄 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
Run ID: ada12412-353c-461c-b488-db92bdc00225
📒 Files selected for processing (34)
plane/__init__.pyplane/api/releases/__init__.pyplane/api/releases/base.pyplane/api/releases/item_labels.pyplane/api/releases/labels.pyplane/api/releases/tags.pyplane/api/work_item_relation_definitions.pyplane/api/workspace_project_labels.pyplane/api/workspace_project_states.pyplane/api/workspace_templates/__init__.pyplane/api/workspace_templates/base.pyplane/api/workspace_templates/page_templates.pyplane/api/workspace_templates/project_templates.pyplane/api/workspace_templates/workitem_templates.pyplane/api/workspace_work_item_properties/__init__.pyplane/api/workspace_work_item_properties/base.pyplane/api/workspace_work_item_properties/options.pyplane/api/workspace_work_item_types/__init__.pyplane/api/workspace_work_item_types/base.pyplane/api/workspace_work_item_types/properties.pyplane/client/plane_client.pyplane/models/releases.pyplane/models/work_item_relation_definitions.pyplane/models/work_item_types.pyplane/models/workspace_project_labels.pyplane/models/workspace_project_states.pyplane/models/workspace_templates.pytests/unit/test_releases.pytests/unit/test_work_item_relation_definitions.pytests/unit/test_workspace_project_labels.pytests/unit/test_workspace_project_states.pytests/unit/test_workspace_templates.pytests/unit/test_workspace_work_item_properties.pytests/unit/test_workspace_work_item_types.py
- rename ReleaseItemLabels.add/remove → create/delete (CRUD naming) - rename WorkspaceWorkItemTypeProperties.link/unlink → create/delete; create now accepts WorkspaceWorkItemTypePropertyLink DTO directly - test_releases: remove broad try/except around update (let it propagate); assert release.id and label.id are not None before use; update add/remove call sites to create/delete - test_work_item_relation_definitions: narrow except Exception → HttpError in teardown - test_workspace_templates: narrow all 3 teardown except Exception → HttpError - test_workspace_work_item_types: assert wit.id is not None before properties.list; add lifecycle test for property create/delete Co-authored-by: Plane AI <noreply@plane.so>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_workspace_work_item_types.py (1)
38-48: ⚡ Quick winRemove unnecessary try/finally block.
The
try/finallyblock with onlypassin thefinallyclause serves no purpose and should be removed for clarity. If no cleanup is needed (per the comment), thetry/finallywrapper is unnecessary.♻️ Proposed simplification
- try: - updated = client.workspace_work_item_types.update( - workspace_slug, - created.id, - UpdateWorkItemType(description="Updated description"), - ) - assert updated.id == created.id - assert updated.description == "Updated description" - finally: - # No delete endpoint on workspace WITs per spec — log a warning if needed - pass + updated = client.workspace_work_item_types.update( + workspace_slug, + created.id, + UpdateWorkItemType(description="Updated description"), + ) + assert updated.id == created.id + assert updated.description == "Updated description"🤖 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/test_workspace_work_item_types.py` around lines 38 - 48, Remove the redundant try/finally around the update assertion: locate the block invoking client.workspace_work_item_types.update(...) with UpdateWorkItemType(description="Updated description") and the subsequent asserts against created.id and updated.description, and simply run the update and asserts without the surrounding try/finally (keep the comment about no-delete behavior if desired).
🤖 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/test_workspace_work_item_types.py`:
- Around line 86-105: Remove the try/except that catches HttpError and calls
warnings.warn so API failures in the property link/unlink lifecycle are not
swallowed; specifically delete the except HttpError block and the warnings.warn
call and allow exceptions from
client.workspace_work_item_types.properties.create, .list and .delete to
propagate (so test failures surface) instead of being converted to warnings.
---
Nitpick comments:
In `@tests/unit/test_workspace_work_item_types.py`:
- Around line 38-48: Remove the redundant try/finally around the update
assertion: locate the block invoking
client.workspace_work_item_types.update(...) with
UpdateWorkItemType(description="Updated description") and the subsequent asserts
against created.id and updated.description, and simply run the update and
asserts without the surrounding try/finally (keep the comment about no-delete
behavior if desired).
🪄 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
Run ID: 6253fc04-e0be-4301-b2ad-dc8e74c92385
📒 Files selected for processing (6)
plane/api/releases/item_labels.pyplane/api/workspace_work_item_types/properties.pytests/unit/test_releases.pytests/unit/test_work_item_relation_definitions.pytests/unit/test_workspace_templates.pytests/unit/test_workspace_work_item_types.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/test_work_item_relation_definitions.py
- tests/unit/test_workspace_templates.py
- remove try/except HttpError in test_create_delete_property_link so API failures surface as test failures instead of warnings - remove redundant try/finally around update assertions in test_create_update_workspace_work_item_type - drop now-unused warnings and HttpError imports Co-authored-by: Plane AI <noreply@plane.so>
Summary
WorkspaceTemplatesresource withwork_items,projects, andpagessub-resources — list/create/update/delete at/workspaces/{slug}/workitems|projects|pages/templates/WorkspaceWorkItemTypesresource (list/create/update) withWorkspaceWorkItemTypePropertiessub-resource for linking/unlinking propertiesWorkspaceWorkItemPropertiesresource (list/create/update/delete) withWorkspaceWorkItemPropertyOptionssub-resource (list/create/update)WorkspaceProjectLabelsresource (list/create/update/delete)WorkspaceProjectStatesresource (list/create/update/delete)WorkItemRelationDefinitionsresource with optionalis_default/is_activequery filters (list/create/update/delete)Releasesresource (list/create/update) withReleaseTags,ReleaseLabels, andReleaseItemLabelssub-resourcesBaseResource, registered onPlaneClient, exported fromplane/__init__.pyTest plan
tests/unit/test_workspace_templates.py— list/create/update/delete for workitem, project, page templatestests/unit/test_workspace_work_item_types.py— list/create/update + property link/unlinktests/unit/test_workspace_work_item_properties.py— CRUD + options sub-resourcetests/unit/test_workspace_project_labels.py— CRUDtests/unit/test_workspace_project_states.py— CRUDtests/unit/test_work_item_relation_definitions.py— list with filters + CRUDtests/unit/test_releases.py— CRUD + tags/labels/item-labels sub-resourcesCo-authored-by: Plane AI noreply@plane.so
Summary by CodeRabbit
New Features
Tests