feat: project-level property APIs, workspace CRUD completions, and resilient work item models#42
Conversation
…work item property management API endpoints
…property and type operations
|
Warning Review limit reached
More reviews will be available in 34 minutes and 58 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)
📝 WalkthroughWalkthroughThis PR expands the work item properties and types API surface by introducing workspace-scoped and project-scoped CRUD operations, adjusts core models to support flexible relationship payloads, and adds comprehensive test coverage for all new functionality. ChangesWork Item Properties & Types API Expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
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: 9
🧹 Nitpick comments (1)
tests/unit/test_work_items.py (1)
95-95: ⚡ Quick winRemove redundant import.
WorkItemQueryParamsis already imported at the module level (line 7), so this local import is unnecessary.♻️ Proposed fix
def test_list_workspace_work_items_with_pagination( self, client: PlaneClient, workspace_slug: str ) -> None: """Test workspace work item listing respects per_page param.""" - from plane.models.query_params import WorkItemQueryParams - params = WorkItemQueryParams(per_page=5)🤖 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_work_items.py` at line 95, Remove the redundant local import of WorkItemQueryParams in tests/unit/test_work_items.py (the extra "from plane.models.query_params import WorkItemQueryParams" shown in the diff) since WorkItemQueryParams is already imported at module level; simply delete this duplicate import line and run the tests to confirm there are no other missing imports or name collisions.
🤖 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/work_item_properties/base.py`:
- Around line 108-110: The project-level API paths in
plane/api/work_item_properties/base.py are missing trailing slashes; update the
formatted endpoint strings (e.g.,
f"{workspace_slug}/projects/{project_id}/work-item-properties", and the other
similar occurrences referenced in the comment) to include a trailing "/" so they
follow the convention ({base_path}/api/v1{resource_base_path}/{endpoint}/).
Locate each _get/_post/_put call that uses those f-strings (the occurrences
around the provided diffs) and append "/" to the end of the path string,
ensuring all new project-level endpoints end with a trailing slash.
- Line 191: The current return uses response.get("properties", []) which can
yield non-list values; replace that single return with a guarded/coercing block:
fetch props = response.get("properties", None), if isinstance(props, str) return
[props], if isinstance(props, list) return [p for p in props if isinstance(p,
str)] (or map to str if you prefer), otherwise return [] (or raise TypeError if
stricter behavior is desired) so callers always receive list[str]; update the
code around the return response.get("properties", []) to implement this
validation/coercion.
In `@plane/api/work_items/base.py`:
- Line 193: The endpoint string f"{workspace_slug}/work-items" must include a
trailing slash per API conventions; update the call that builds the URL (the
field using f"{workspace_slug}/work-items", params=query_params) to use
f"{workspace_slug}/work-items/" so the final request path ends with a "/" and
matches the {base_path}/api/v1{resource_base_path}/{endpoint}/ pattern.
In `@plane/api/workspace_work_item_types/properties.py`:
- Around line 25-29: The code currently does return list(response) immediately
after calling
self._get(f"{workspace_slug}/work-item-types/{type_id}/properties/"), which will
silently iterate dict keys if the API changes; update this method to validate
the response shape before converting: after the self._get(...) call
assert/inspect that response is a list (e.g., isinstance(response, list)), and
if not, raise a clear error (or handle the expected envelope like
response.get("items") if your API uses that) including the actual response
contents for debugging; ensure the error mentions the workspace_slug and type_id
so callers can trace the failing request.
In `@tests/unit/test_work_item_properties.py`:
- Around line 711-716: Replace the broad "except Exception: pass" in the
teardown calls to client.work_item_properties.delete_project with a targeted
handler: catch the specific client/API exception type thrown by the SDK (e.g.,
ApiError/HTTPError or the library's specific exception) and either log the error
or re-raise/assert so cleanup failures are visible; apply the same change to
both occurrences that call client.work_item_properties.delete_project (the
blocks using workspace_slug, project.id, created.id). Ensure you import the
specific exception class and include the caught exception in the log message
instead of silencing it.
- Line 751: The create_project call can raise an HTTP 400 in environments that
block project-level property creation; wrap the line that calls
client.work_item_properties.create_project(workspace_slug, project.id,
prop_data) in a try/except that catches the HTTPError (or the client-specific
exception) and if the exception's response/status_code is 400 invoke pytest.skip
with the same message used in the prior attach/detach test so the test is
skipped consistently; keep the original variable name prop and only skip on that
specific 400 condition.
In `@tests/unit/test_workspace_work_item_properties.py`:
- Around line 173-179: The teardown is catching all exceptions with "except
Exception" which hides real failures; change the try/except around
client.workspace_work_item_properties.delete(workspace_slug, prop.id) to either
remove the broad except so exceptions propagate and fail the test, or catch a
concrete SDK error type (e.g., ApiError/SDKError) imported from the SDK and
log/warn only that specific exception; update the except clause to reference the
concrete exception class and remove the generic "Exception" handler.
In `@tests/unit/test_workspace_work_item_types.py`:
- Around line 57-58: The test uses wit.id (from types[0]) without ensuring it's
not None; add a guard to assert or raise if wit.id is None before calling
client.workspace_work_item_types.retrieve(workspace_slug, wit.id). Locate the
variables wit and types in this test, verify wit is not None, then do an
explicit assert wit.id is not None (or a conditional that fails the test with a
clear message) so the subsequent retrieve call always receives a non-optional
id.
- Around line 44-47: The teardown currently swallows all errors in the cleanup:
replace the broad "except Exception: pass" around
client.workspace_work_item_types.delete(workspace_slug, created.id) with a
targeted handling strategy—either let errors propagate (remove the try/except)
so test failures surface, or catch the client-specific exception (e.g.,
ApiException/HTTPError) and log it or re-raise after logging; alternatively move
deletion into a pytest finalizer/fixture to ensure deterministic cleanup and
visible failures. Ensure you reference client.workspace_work_item_types.delete,
workspace_slug, and created.id when updating the code.
---
Nitpick comments:
In `@tests/unit/test_work_items.py`:
- Line 95: Remove the redundant local import of WorkItemQueryParams in
tests/unit/test_work_items.py (the extra "from plane.models.query_params import
WorkItemQueryParams" shown in the diff) since WorkItemQueryParams is already
imported at module level; simply delete this duplicate import line and run the
tests to confirm there are no other missing imports or name collisions.
🪄 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: ce9e705b-7a9c-4871-8472-7121a62024d6
📒 Files selected for processing (11)
plane/api/work_item_properties/base.pyplane/api/work_items/base.pyplane/api/workspace_work_item_properties/base.pyplane/api/workspace_work_item_properties/options.pyplane/api/workspace_work_item_types/base.pyplane/api/workspace_work_item_types/properties.pyplane/models/work_items.pytests/unit/test_work_item_properties.pytests/unit/test_work_items.pytests/unit/test_workspace_work_item_properties.pytests/unit/test_workspace_work_item_types.py
…rties are disabled by workspace settings
Summary
list_project,create_project,retrieve_project,update_project,delete_project) and type-attachment helpers (attach_to_type,detach_from_type) onwork_item_propertieswork_items.list_workspace()for cross-project work item queriesretrieveanddeleteonworkspace_work_item_typesretrieveanddeleteonworkspace_work_item_properties.optionsretrieveonworkspace_work_item_propertiesWorkspaceWorkItemTypeProperties.list()return type: API returns flat["uuid", ...]— changed fromlist[WorkItemProperty]tolist[str]WorkItem/WorkItemDetail/WorkItemExpandmodels:name:str→str | None(API omits it in some list responses)project,type:str | None→str | dict | None(expanded responses return nested objects)assignees,labels: barelist[T]→Field(default_factory=list)(missing keys no longer raiseValidationError)Test plan
work_item_properties.list_project()returns all properties for a projectwork_item_properties.attach_to_type()/detach_from_type()link and unlink propertieswork_items.list_workspace()paginates across all projectsworkspace_work_item_types.retrieve()/delete()return correct types, no 404workspace_work_item_properties.retrieve()resolves a known UUIDworkspace_work_item_properties.options.retrieve()/delete()work correctlyworkspace_work_item_types.properties.list()returnslist[str], notlist[WorkItemProperty]WorkItem.model_validate(...)succeeds whennameis absent orprojectis an expanded dictSummary by CodeRabbit
Release Notes