feat: rich filters + pql on work-item list endpoints, add list_workspace [WEB-7393][PAI-1461]#37
Conversation
…orkspace` Expose the new external API's structured filtering capability on every work-item list method in the SDK. - WorkItemQueryParams now accepts `filters: dict[str, Any] | None` (the existing `pql: str | None` field is unchanged). A small `_prepare_work_item_params` helper JSON-encodes `filters` into the `filters=` query string the API expects. - New `WorkItems.list_workspace(workspace_slug, params)` calling `GET /workspaces/<slug>/work-items/` — paginated, total_results, spans every project the caller can view. - `WorkItems.list` and `WorkItems.list_archived` now route through `_prepare_work_item_params`, so `filters` and `pql` work uniformly. - `Cycles.list_work_items` and `Modules.list_work_items` accept `WorkItemQueryParams` (typed path) with the same serialization, and still accept a plain `Mapping[str, Any]` for backwards compatibility. - Real-API unit tests for `filters` on `list`, plus `list_workspace` with and without `filters`. - Bump version to 0.2.13. Backend: makeplane/plane-ee#7376 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR extends the Plane Python SDK with structured query filtering for work items. It introduces a ChangesWork Item Filtering Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
|
Linked to Plane Work Item(s)
This comment was auto-generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/cycles.py`:
- Around line 162-163: Replace the non-compliant endpoint segments and add
trailing slashes: change any occurrences of "cycle-issues" in the URL
construction to "work-items" and ensure the URLs follow the pattern
"{base_path}/api/v1{resource_base_path}/{endpoint}/" with a trailing "/" (e.g.
update the string
f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/cycle-issues" to use
"work-items" and append "/" ), and apply the same fix to the other occurrences
referenced (the similar URL constructions at the other reported locations) so
all endpoint strings end with a trailing slash and use "work-items" instead of
"issue"/"cycle-issues".
- Around line 142-160: Update Cycles.list_work_items to accept only the DTO type
by changing the params handling to require WorkItemQueryParams | None (remove
the Mapping branch) and call
_prepare_work_item_params(params.model_dump(exclude_none=True)) when params is
provided; ensure the outgoing request uses that serialized mapping and validate
the response with PaginatedCycleWorkItemResponse.model_validate(...) before
returning. Target the Cycles.list_work_items function and use
WorkItemQueryParams, _prepare_work_item_params, and
PaginatedCycleWorkItemResponse.model_validate to locate and implement the
change.
In `@plane/api/modules.py`:
- Around line 161-162: The endpoint string using "module-issues" should be
renamed to use "module-work-items" and must include a trailing slash to follow
API conventions; update the f-string at the call that builds the modules
endpoint (the line containing
f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-issues") to
f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-work-items/"
ensuring the URL matches the required
{base_path}/api/v1{resource_base_path}/{endpoint}/ pattern and preserves
existing query_params handling.
- Around line 141-159: The list_work_items method currently accepts a raw
Mapping which bypasses DTO validation; change the signature to accept only
WorkItemQueryParams (remove Mapping[str, Any] | None), and when building
query_params use params.model_dump(exclude_none=True) (or call
_prepare_work_item_params(params) if that helper returns a dict from the DTO),
removing the raw-mapping branch; also ensure the method validates responses
using PaginatedModuleWorkItemResponse.model_validate(...) before returning.
In `@plane/api/work_items/base.py`:
- Around line 199-201: Several endpoint f-strings used in calls to
self._get/_post are missing the terminal slash (e.g.
f"{workspace_slug}/projects/{project_id}/work-items" in the response =
self._get(...) call); update these formatted paths to include a trailing '/' so
they follow the API convention. Locate calls referencing workspace_slug,
project_id, work_item_id (and uses of _prepare_work_item_params) and append '/'
to each endpoint string passed into self._get and similar HTTP helpers (also
adjust the other occurrences in the same module referenced near the other
f-strings) to ensure every "{...}/..." path ends with '/'.
In `@tests/unit/test_work_items.py`:
- Around line 140-147: The current test only checks counts and presence which
can pass if filtering is ignored; update the assertions in the test (referencing
filtered, unfiltered, created.id, filtered.results, filtered.total_results) to
explicitly verify filter semantics by asserting that every item in
filtered.results satisfies the urgent filter predicate (e.g., item.priority ==
"urgent" or item.is_urgent — use the actual field on the WorkItem model) and
keep the pagination tolerance separate: still allow created.id to be absent from
this page if filtered.total_results > 0, but do not use that condition to bypass
the requirement that all returned items match the filter.
🪄 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: dc8a21e8-6990-49e6-b9ad-737b19edcc49
📒 Files selected for processing (7)
README.mdplane/api/cycles.pyplane/api/modules.pyplane/api/work_items/base.pyplane/models/query_params.pypyproject.tomltests/unit/test_work_items.py
| params: WorkItemQueryParams | Mapping[str, Any] | None = None, | ||
| ) -> PaginatedCycleWorkItemResponse: | ||
| """List work items in a cycle. | ||
|
|
||
| Supports the same ``filters`` and ``pql`` query parameters as | ||
| :meth:`WorkItems.list`. | ||
|
|
||
| Args: | ||
| workspace_slug: The workspace slug identifier | ||
| project_id: UUID of the project | ||
| cycle_id: UUID of the cycle | ||
| params: Optional query parameters | ||
| params: Optional query parameters. Prefer ``WorkItemQueryParams``; | ||
| a plain mapping is also accepted for backwards compatibility | ||
| and is passed through unchanged. | ||
| """ | ||
| if isinstance(params, WorkItemQueryParams): | ||
| query_params: Mapping[str, Any] | None = _prepare_work_item_params(params) | ||
| else: | ||
| query_params = params |
There was a problem hiding this comment.
Enforce DTO-only params for resource method inputs
Cycles.list_work_items still accepts plain mappings (Line 142, Line 159-Line 160). This conflicts with the resource contract and weakens typed validation for filters/pql input.
Suggested direction
- def list_work_items(
+ def list_work_items(
self,
workspace_slug: str,
project_id: str,
cycle_id: str,
- params: WorkItemQueryParams | Mapping[str, Any] | None = None,
+ params: WorkItemQueryParams | None = None,
) -> PaginatedCycleWorkItemResponse:
@@
- if isinstance(params, WorkItemQueryParams):
- query_params: Mapping[str, Any] | None = _prepare_work_item_params(params)
- else:
- query_params = params
+ query_params: Mapping[str, Any] | None = (
+ _prepare_work_item_params(params) if params is not None else None
+ )As per coding guidelines, "Resource methods must accept Pydantic DTOs, serialize with model_dump(exclude_none=True), and validate responses with Model.model_validate()."
🤖 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 `@plane/api/cycles.py` around lines 142 - 160, Update Cycles.list_work_items to
accept only the DTO type by changing the params handling to require
WorkItemQueryParams | None (remove the Mapping branch) and call
_prepare_work_item_params(params.model_dump(exclude_none=True)) when params is
provided; ensure the outgoing request uses that serialized mapping and validate
the response with PaginatedCycleWorkItemResponse.model_validate(...) before
returning. Target the Cycles.list_work_items function and use
WorkItemQueryParams, _prepare_work_item_params, and
PaginatedCycleWorkItemResponse.model_validate to locate and implement the
change.
| f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/cycle-issues", | ||
| params=query_params, |
There was a problem hiding this comment.
Endpoint path naming/convention is out of policy
Line 162 uses cycle-issues, and Lines 162, 195, and 206 omit trailing /. This violates endpoint naming and URL-format rules for API resources.
Suggested patch
- f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/cycle-issues",
+ f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/cycle-work-items/",
@@
- self._post(f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/archive", {})
+ self._post(f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/archive/", {})
@@
- self._delete(f"{workspace_slug}/projects/{project_id}/archived-cycles/{cycle_id}/unarchive")
+ self._delete(
+ f"{workspace_slug}/projects/{project_id}/archived-cycles/{cycle_id}/unarchive/"
+ )As per coding guidelines, "Never use 'Issue' in endpoint or parameter names — always use 'Work Item' instead" and "All API endpoints should end with a trailing / and follow URL convention: {base_path}/api/v1{resource_base_path}/{endpoint}/."
Also applies to: 195-195, 206-206
🤖 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 `@plane/api/cycles.py` around lines 162 - 163, Replace the non-compliant
endpoint segments and add trailing slashes: change any occurrences of
"cycle-issues" in the URL construction to "work-items" and ensure the URLs
follow the pattern "{base_path}/api/v1{resource_base_path}/{endpoint}/" with a
trailing "/" (e.g. update the string
f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/cycle-issues" to use
"work-items" and append "/" ), and apply the same fix to the other occurrences
referenced (the similar URL constructions at the other reported locations) so
all endpoint strings end with a trailing slash and use "work-items" instead of
"issue"/"cycle-issues".
| params: WorkItemQueryParams | Mapping[str, Any] | None = None, | ||
| ) -> PaginatedModuleWorkItemResponse: | ||
| """List work items in a module. | ||
|
|
||
| Supports the same ``filters`` and ``pql`` query parameters as | ||
| :meth:`WorkItems.list`. | ||
|
|
||
| Args: | ||
| workspace_slug: The workspace slug identifier | ||
| project_id: UUID of the project | ||
| module_id: UUID of the module | ||
| params: Optional query parameters | ||
| params: Optional query parameters. Prefer ``WorkItemQueryParams``; | ||
| a plain mapping is also accepted for backwards compatibility | ||
| and is passed through unchanged. | ||
| """ | ||
| if isinstance(params, WorkItemQueryParams): | ||
| query_params: Mapping[str, Any] | None = _prepare_work_item_params(params) | ||
| else: | ||
| query_params = params |
There was a problem hiding this comment.
Keep list_work_items strictly typed to query DTO
Line 141 and Line 159 permit raw mappings, bypassing DTO constraints and creating a second unvalidated input path.
Suggested direction
- def list_work_items(
+ def list_work_items(
self,
workspace_slug: str,
project_id: str,
module_id: str,
- params: WorkItemQueryParams | Mapping[str, Any] | None = None,
+ params: WorkItemQueryParams | None = None,
) -> PaginatedModuleWorkItemResponse:
@@
- if isinstance(params, WorkItemQueryParams):
- query_params: Mapping[str, Any] | None = _prepare_work_item_params(params)
- else:
- query_params = params
+ query_params: Mapping[str, Any] | None = (
+ _prepare_work_item_params(params) if params is not None else None
+ )As per coding guidelines, "Resource methods must accept Pydantic DTOs, serialize with model_dump(exclude_none=True), and validate responses with Model.model_validate()."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| params: WorkItemQueryParams | Mapping[str, Any] | None = None, | |
| ) -> PaginatedModuleWorkItemResponse: | |
| """List work items in a module. | |
| Supports the same ``filters`` and ``pql`` query parameters as | |
| :meth:`WorkItems.list`. | |
| Args: | |
| workspace_slug: The workspace slug identifier | |
| project_id: UUID of the project | |
| module_id: UUID of the module | |
| params: Optional query parameters | |
| params: Optional query parameters. Prefer ``WorkItemQueryParams``; | |
| a plain mapping is also accepted for backwards compatibility | |
| and is passed through unchanged. | |
| """ | |
| if isinstance(params, WorkItemQueryParams): | |
| query_params: Mapping[str, Any] | None = _prepare_work_item_params(params) | |
| else: | |
| query_params = params | |
| params: WorkItemQueryParams | None = None, | |
| ) -> PaginatedModuleWorkItemResponse: | |
| """List work items in a module. | |
| Supports the same ``filters`` and ``pql`` query parameters as | |
| :meth:`WorkItems.list`. | |
| Args: | |
| workspace_slug: The workspace slug identifier | |
| project_id: UUID of the project | |
| module_id: UUID of the module | |
| params: Optional query parameters. Prefer ``WorkItemQueryParams``; | |
| a plain mapping is also accepted for backwards compatibility | |
| and is passed through unchanged. | |
| """ | |
| query_params: Mapping[str, Any] | None = ( | |
| _prepare_work_item_params(params) if params is not None else None | |
| ) |
🤖 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 `@plane/api/modules.py` around lines 141 - 159, The list_work_items method
currently accepts a raw Mapping which bypasses DTO validation; change the
signature to accept only WorkItemQueryParams (remove Mapping[str, Any] | None),
and when building query_params use params.model_dump(exclude_none=True) (or call
_prepare_work_item_params(params) if that helper returns a dict from the DTO),
removing the raw-mapping branch; also ensure the method validates responses
using PaginatedModuleWorkItemResponse.model_validate(...) before returning.
| f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-issues", | ||
| params=params, | ||
| params=query_params, |
There was a problem hiding this comment.
module-issues path and missing trailing slash violate API conventions
Line 161 uses module-issues and does not end the endpoint with /.
Suggested patch
- f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-issues",
+ f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-work-items/",As per coding guidelines, "Never use 'Issue' in endpoint or parameter names — always use 'Work Item' instead" and "All API endpoints should end with a trailing / and follow URL convention: {base_path}/api/v1{resource_base_path}/{endpoint}/."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-issues", | |
| params=params, | |
| params=query_params, | |
| f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-work-items/", | |
| params=query_params, |
🤖 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 `@plane/api/modules.py` around lines 161 - 162, The endpoint string using
"module-issues" should be renamed to use "module-work-items" and must include a
trailing slash to follow API conventions; update the f-string at the call that
builds the modules endpoint (the line containing
f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-issues") to
f"{workspace_slug}/projects/{project_id}/modules/{module_id}/module-work-items/"
ensuring the URL matches the required
{base_path}/api/v1{resource_base_path}/{endpoint}/ pattern and preserves
existing query_params handling.
| response = self._get( | ||
| f"{workspace_slug}/projects/{project_id}/work-items", | ||
| params=_prepare_work_item_params(params), |
There was a problem hiding this comment.
Add trailing slashes to touched endpoint paths.
These changed URLs still omit the terminal /, which breaks the API path convention and can cause routing/redirect inconsistencies.
Suggested patch
- f"{workspace_slug}/projects/{project_id}/work-items",
+ f"{workspace_slug}/projects/{project_id}/work-items/",
params=_prepare_work_item_params(params),
@@
- f"{workspace_slug}/work-items",
+ f"{workspace_slug}/work-items/",
params=_prepare_work_item_params(params),
@@
- f"{workspace_slug}/projects/{project_id}/archived-work-items",
+ f"{workspace_slug}/projects/{project_id}/archived-work-items/",
params=_prepare_work_item_params(params),
@@
- self._delete(f"{workspace_slug}/projects/{project_id}/work-items/{work_item_id}/unarchive")
+ self._delete(f"{workspace_slug}/projects/{project_id}/work-items/{work_item_id}/unarchive/")As per coding guidelines, "All API endpoints should end with a trailing / and follow URL convention: {base_path}/api/v1{resource_base_path}/{endpoint}/".
Also applies to: 234-236, 319-320, 352-352
🤖 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 `@plane/api/work_items/base.py` around lines 199 - 201, Several endpoint
f-strings used in calls to self._get/_post are missing the terminal slash (e.g.
f"{workspace_slug}/projects/{project_id}/work-items" in the response =
self._get(...) call); update these formatted paths to include a trailing '/' so
they follow the API convention. Locate calls referencing workspace_slug,
project_id, work_item_id (and uses of _prepare_work_item_params) and append '/'
to each endpoint string passed into self._get and similar HTTP helpers (also
adjust the other occurrences in the same module referenced near the other
f-strings) to ensure every "{...}/..." path ends with '/'.
There was a problem hiding this comment.
Pull request overview
Adds support for the Plane external API’s “rich” work-item filtering by introducing structured filters alongside existing pql across work-item list endpoints, and exposes a new workspace-scoped work-item listing method in the SDK.
Changes:
- Add
filters: dict[str, Any] | NonetoWorkItemQueryParamsand serialize it as a JSON string in thefilters=query parameter. - Add
WorkItems.list_workspace(...)for workspace-wide work-item listing (paginated, includestotal_results). - Extend cycles/modules
list_work_itemsto acceptWorkItemQueryParamswhile keepingMappingsupport, plus update tests/docs and bump version to 0.2.13.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
plane/models/query_params.py |
Adds filters and expands pql documentation on WorkItemQueryParams. |
plane/api/work_items/base.py |
Implements _prepare_work_item_params (JSON-encodes filters) and adds list_workspace; routes list methods through the helper. |
plane/api/cycles.py |
Accepts WorkItemQueryParams for cycle work-item listing and applies the shared param-prep path. |
plane/api/modules.py |
Accepts WorkItemQueryParams for module work-item listing and applies the shared param-prep path. |
tests/unit/test_work_items.py |
Adds real-API tests for structured filters and for list_workspace (with/without filters). |
README.md |
Documents structured filters vs PQL and adds usage examples including workspace-scoped listing. |
pyproject.toml |
Bumps package version to 0.2.13. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if isinstance(params, WorkItemQueryParams): | ||
| query_params: Mapping[str, Any] | None = _prepare_work_item_params(params) | ||
| else: | ||
| query_params = params | ||
| response = self._get( |
| from .work_items.base import _prepare_work_item_params | ||
|
|
||
|
|
| if isinstance(params, WorkItemQueryParams): | ||
| query_params: Mapping[str, Any] | None = _prepare_work_item_params(params) | ||
| else: | ||
| query_params = params | ||
| response = self._get( |
| from .work_items.base import _prepare_work_item_params | ||
|
|
||
|
|
| assert created.id in [item.id for item in filtered.results] or ( | ||
| # If the workspace has many urgent items, the created one may be | ||
| # on a later page — accept either presence on this page or that | ||
| # at least one urgent item is returned overall. | ||
| filtered.total_results |
| @@ -748,9 +748,55 @@ The SDK provides comprehensive Pydantic v2 models for all API operations. | |||
|
|
|||
| - `BaseQueryParams` - Base query parameters | |||
| - `PaginatedQueryParams` - Pagination support (per_page, page) | |||
Drop the `or filtered.total_results > 0` fallback (which could pass even when filtering was a no-op) and assert directly that every returned item has `priority == "urgent"` — that's the property the test is meant to prove. Addresses CodeRabbit review.
Copilot caught a real bug: `Cycles.list_work_items` / `Modules.list_work_items` accept a plain `Mapping[str, Any]` for BC, but when callers passed `filters` as a dict through that path the SDK forwarded it to `requests` unchanged. The API expects `filters` as a JSON-encoded string, so the request would produce an invalid query parameter. - Extend `prepare_work_item_params` (renamed from `_prepare_work_item_params` since it's now used across resources, no longer private to the work_items module) to accept either a `WorkItemQueryParams` DTO or a plain mapping, and normalise `filters` to a JSON string in both paths. - Simplify `Cycles.list_work_items` and `Modules.list_work_items` to use the unified helper — drops the `isinstance` branch. - Fix README: `PaginatedQueryParams` is cursor-based (cursor, per_page), not (per_page, page) — that combination has never existed in the model. Addresses Copilot review on PR #37.
Summary
Exposes the new external API's structured filtering capability across every work-item list method in the SDK, plus a new
list_workspacemethod for the workspace-scoped paginated list endpoint added in makeplane/plane-ee#7376.Changes
WorkItemQueryParamsnow acceptsfilters: dict[str, Any] | None. The existingpql: str | Nonefield is unchanged. A small_prepare_work_item_paramshelper JSON-encodesfiltersinto thefilters=query string the API expects; everything else passes through as-is.WorkItems.list_workspace(workspace_slug, params)callingGET /workspaces/<slug>/work-items/— paginated envelope with fulltotal_results. Spans every project in the workspace the caller can view (per-project authorization and conditional grants are honored server-side).WorkItems.listandWorkItems.list_archivednow route through_prepare_work_item_params, sofiltersandpqlwork uniformly.Cycles.list_work_itemsandModules.list_work_itemsacceptWorkItemQueryParams(typed path) with the same serialization, and still accept a plainMapping[str, Any]for backwards compatibility.filtersonlist, pluslist_workspacewith and withoutfilters.Backend
feat/web-7393-external-workitem-rich-filter-pql) — this PR depends on those endpoints being deployed.Test plan
ruff check— clean on touched filesblack --check— clean (auto-formatted)_prepare_work_item_params(WorkItemQueryParams(filters={...}, pql='...', per_page=5))returns the expected serialized dictPLANE_BASE_URL,PLANE_API_KEY,WORKSPACE_SLUGagainst an env with the plane-ee branch deployed;pytest tests/unit/test_work_items.py -k "filter or workspace"should passReferences
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests