feat: add project_id and workspace_search to AdvancedSearchWorkItem#24
feat: add project_id and workspace_search to AdvancedSearchWorkItem#24lifeiscontent wants to merge 1 commit intomakeplane:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded eight optional filter fields and a serializer to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plane/models/query_params.py (1)
98-105: Implementation is correct; optional improvement for empty list handling.The wrap serializer correctly post-processes the default serialization to convert lists to comma-separated strings for django-filters compatibility. The
handler(self)signature is valid in Pydantic v2.Minor consideration: an empty list
[]will serialize to"", producing a query parameter like?assignee_id__in=. If this is undesired, filter out empty strings:Optional: exclude empty list results
`@model_serializer`(mode="wrap") def _serialize(self, handler): # type: ignore[no-untyped-def] """Serialize list fields as comma-separated strings for django-filters.""" data = handler(self) for key, value in data.items(): if isinstance(value, list): data[key] = ",".join(str(v) for v in value) + return {k: v for k, v in data.items() if v != ""} - return data🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plane/models/query_params.py` around lines 98 - 105, The current _serialize method converts list fields to comma-separated strings but leaves empty lists as an empty string (e.g., ?assignee_id__in=); update _serialize (the model_serializer wrapper that calls handler(self)) to skip or remove keys whose value is an empty list so they are not serialized as empty strings—detect list values in the data dict after handler(self) and if the list is empty, delete data[key] (or set it to None/omit it) instead of converting to ",".join, while still converting non-empty lists as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plane/models/query_params.py`:
- Around line 98-105: The current _serialize method converts list fields to
comma-separated strings but leaves empty lists as an empty string (e.g.,
?assignee_id__in=); update _serialize (the model_serializer wrapper that calls
handler(self)) to skip or remove keys whose value is an empty list so they are
not serialized as empty strings—detect list values in the data dict after
handler(self) and if the list is empty, delete data[key] (or set it to None/omit
it) instead of converting to ",".join, while still converting non-empty lists as
before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53d91f60-ecbd-42b1-a54d-e0c3e241f99c
📒 Files selected for processing (1)
plane/models/query_params.py
There was a problem hiding this comment.
Pull request overview
Adds first-class server-side filtering to the SDK’s WorkItemQueryParams, aligning the Python client with Plane’s REST API filtering capabilities and avoiding client-side filtering workarounds.
Changes:
- Added filter fields to
WorkItemQueryParams(assignee/state/priority/labels/creator + draft/archived flags). - Added a Pydantic
model_serializerto serialize list filters as comma-separated strings for django-filtersBaseInFiltercompatibility.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @model_serializer(mode="wrap") | ||
| def _serialize(self, handler): # type: ignore[no-untyped-def] | ||
| """Serialize list fields as comma-separated strings for django-filters.""" |
There was a problem hiding this comment.
The @model_serializer is currently untyped and relies on # type: ignore[no-untyped-def], which conflicts with the repo’s strict mypy config. Please type the serializer signature (e.g., using Pydantic’s SerializerFunctionWrapHandler and optional SerializationInfo) so we can remove the type ignore and keep type safety.
| state_group__in: list[str] | None = Field( | ||
| None, | ||
| description="Filter by state groups (backlog, unstarted, started, completed, cancelled)", | ||
| ) | ||
| priority__in: list[str] | None = Field( | ||
| None, | ||
| description="Filter by priority levels (urgent, high, medium, low, none)", | ||
| ) |
There was a problem hiding this comment.
state_group__in and priority__in are typed as list[str], but the codebase already defines constrained Literal types (GroupEnum, PriorityEnum) used across models. Consider typing these as list[GroupEnum] | None and list[PriorityEnum] | None to prevent invalid values at compile/validation time and keep consistency with the rest of the SDK.
| """Serialize list fields as comma-separated strings for django-filters.""" | ||
| data = handler(self) | ||
| for key, value in data.items(): | ||
| if isinstance(value, list): |
There was a problem hiding this comment.
The model-level serializer converts any list-valued field into a comma-separated string. That’s fine for the current __in filters, but it will also affect any future list fields added to WorkItemQueryParams (even if the API expects repeated query params instead of CSV). Consider limiting this conversion to the known *__in keys (or an explicit allowlist) to avoid surprising serialization changes later.
| if isinstance(value, list): | |
| if key.endswith("__in") and isinstance(value, list): |
| @model_serializer(mode="wrap") | ||
| def _serialize(self, handler): # type: ignore[no-untyped-def] | ||
| """Serialize list fields as comma-separated strings for django-filters.""" | ||
| data = handler(self) | ||
| for key, value in data.items(): | ||
| if isinstance(value, list): | ||
| data[key] = ",".join(str(v) for v in value) | ||
| return data |
There was a problem hiding this comment.
New serialization behavior (converting list filters to comma-separated strings) isn’t covered by tests. Please add a unit test that asserts WorkItemQueryParams(assignee_id__in=[...]).model_dump(exclude_none=True) produces a comma-separated string, since this behavior is critical for server-side filtering to work with requests query encoding.
lifeiscontent
force-pushed
the
feat/work-item-query-filters
branch
from
94cf652 to
1b79a9a
Compare
lifeiscontent
changed the title
…chWorkItem Add project_id and workspace_search fields to AdvancedSearchWorkItem model. These fields are already supported by the API endpoint (WorkItemAdvancedSearchRequestSerializer) but were missing from the SDK model.
lifeiscontent
force-pushed
the
feat/work-item-query-filters
branch
from
1b79a9a to
544ea4f
Compare
Summary
Adds
project_idandworkspace_searchfields toAdvancedSearchWorkItemmodel, and adds filter fields toWorkItemQueryParams.Changes
AdvancedSearchWorkItemproject_id— scope search to a specific projectworkspace_search— search across all projects in the workspaceThese fields are already supported by the API (
WorkItemAdvancedSearchRequestSerializer) but were missing from the SDK model.WorkItemQueryParamsassignee_id__in,state_group__in,priority__in, etc.) for future use when the public API list endpoint addsIssueFilterSetsupport.Verified
project_idandworkspace_searchtested end-to-endCompanion PR
advanced_search_work_itemsMCP toolSummary by CodeRabbit