[WEB-6815] chore: added support for pql #27
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded an optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
🧹 Nitpick comments (2)
plane/models/query_params.py (2)
51-62: Consider updating the docstring to includepql.The docstring documents inherited parameters from
PaginatedQueryParamsbut doesn't mention the newly addedpqlparameter that's now inherited fromBaseQueryParams.📝 Suggested docstring update
class WorkItemQueryParams(PaginatedQueryParams): """Query parameters for work item list endpoints. Inherits all documented query parameters from PaginatedQueryParams: - cursor: Pagination cursor - expand: Comma-separated fields to expand - external_id: External system identifier - external_source: External system source name - fields: Comma-separated fields to include - order_by: Field to order by (prefix with '-' for descending) - per_page: Number of results per page (1-100) + - pql: PQL filter expression """🤖 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 51 - 62, Update the WorkItemQueryParams docstring to list the inherited pql parameter (from BaseQueryParams) along with the other inherited fields; locate the WorkItemQueryParams class docstring and add a bullet like "- pql: Piped query language string to filter results" (or similar brief description) so the documentation reflects that pql is available via inheritance from BaseQueryParams/PaginatedQueryParams.
31-31: Improve the field description wording.The description "Field to apply PQL filters" is grammatically awkward—"Field" doesn't work as the subject here. Consider rewording for clarity.
📝 Suggested description improvement
- pql: str | None = Field(None, description="Field to apply PQL filters") + pql: str | None = Field(None, description="PQL filter expression")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plane/models/query_params.py` at line 31, Update the Field description for the pql attribute on the QueryParams model: replace the awkward "Field to apply PQL filters" with a clearer phrase such as "PQL query string used to filter results" (or similar), so the pql: str | None = Field(..., description="...") reads as a descriptive noun phrase explaining that this value is a PQL expression used to filter query results.
🤖 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 51-62: Update the WorkItemQueryParams docstring to list the
inherited pql parameter (from BaseQueryParams) along with the other inherited
fields; locate the WorkItemQueryParams class docstring and add a bullet like "-
pql: Piped query language string to filter results" (or similar brief
description) so the documentation reflects that pql is available via inheritance
from BaseQueryParams/PaginatedQueryParams.
- Line 31: Update the Field description for the pql attribute on the QueryParams
model: replace the awkward "Field to apply PQL filters" with a clearer phrase
such as "PQL query string used to filter results" (or similar), so the pql: str
| None = Field(..., description="...") reads as a descriptive noun phrase
explaining that this value is a PQL expression used to filter query results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8f8b61c-7077-4fa9-a3e1-7422aa92fe2f
📒 Files selected for processing (1)
plane/models/query_params.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/test_work_items.py (1)
34-45: Consider asserting non-empty results to prevent vacuous test passes.If the API returns an empty list (due to no matching items or a PQL bug), the
forloop executes zero iterations and the priority assertion is vacuously satisfied. The test would pass even if the PQL filter is broken.Consider either:
- Adding
assert len(response.results) > 0if the test environment is expected to have high-priority items- Or creating a high-priority work item as a fixture before testing the filter
💡 Optional fix to guard against empty results
def test_list_work_items_with_pql_filter( self, client: PlaneClient, workspace_slug: str, project: Project ) -> None: """Test listing work items with a PQL filter.""" params = WorkItemQueryParams(pql='priority IN ("high")') response = client.work_items.list(workspace_slug, project.id, params=params) assert response is not None assert hasattr(response, "results") assert isinstance(response.results, list) + # Ensure we have results to validate the filter actually works + assert len(response.results) > 0, "No high-priority work items found to validate filter" for item in response.results: assert item.priority == "high"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_work_items.py` around lines 34 - 45, The test test_list_work_items_with_pql_filter currently can pass vacuously when response.results is empty; update the test to assert non-empty results or ensure a high-priority item exists before querying: either add an assertion like assert len(response.results) > 0 after obtaining response.results, or create a high-priority work item via the client (using client.work_items.create or your fixture helper) prior to calling WorkItemQueryParams and client.work_items.list so the PQL filter actually has matching data to validate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/test_work_items.py`:
- Around line 34-45: The test test_list_work_items_with_pql_filter currently can
pass vacuously when response.results is empty; update the test to assert
non-empty results or ensure a high-priority item exists before querying: either
add an assertion like assert len(response.results) > 0 after obtaining
response.results, or create a high-priority work item via the client (using
client.work_items.create or your fixture helper) prior to calling
WorkItemQueryParams and client.work_items.list so the PQL filter actually has
matching data to validate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa59d9df-b63e-4e8d-b9b3-8665e73814ec
📒 Files selected for processing (1)
tests/unit/test_work_items.py
There was a problem hiding this comment.
Pull request overview
Adds client-side support for sending PQL (Plane Query Language) filters when listing work items, along with a smoke test to validate filtered list behavior.
Changes:
- Extended shared query param models to include an optional
pqlquery parameter. - Added a work items list test that uses
pqlto filter by priority.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
plane/models/query_params.py |
Introduces pql on the base query params model so it can be serialized into request query strings. |
tests/unit/test_work_items.py |
Adds a smoke test covering list filtering via pql and updates imports accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/test_work_items.py`:
- Around line 34-54: The test_list_work_items_with_pql_filter test is weak
because it can pass if the pql is ignored; update it to create a second, clearly
non-matching work item (e.g., priority="low" or name that doesn't match the PQL)
using client.work_items.create, then call client.work_items.list with
WorkItemQueryParams(pql='priority IN ("high")') and assert the created
high-priority item's id is present and the non-matching item's id is not present
in response.results; ensure you still validate response is not None, has
.results, and results is a list before checking IDs.
- Around line 38-54: The test creates a work item (created_item via
client.work_items.create) but never deletes it; wrap the creation and assertions
in a try/finally (or add teardown) and call the work-items deletion API (e.g.,
client.work_items.delete(workspace_slug, project.id, created_item.id) or the
appropriate remove method) in the finally block to ensure the created_item is
removed regardless of test outcome; ensure created_item is defined before
calling delete (guard against create failures) and handle/ignore NotFound errors
if delete races with other cleanup.
🪄 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: 1bbf77db-8dd3-4a09-82b4-2c7a1c54c029
📒 Files selected for processing (2)
plane/models/query_params.pytests/unit/test_work_items.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plane/models/query_params.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
PQL support for work items.
Type of Change
Summary by CodeRabbit
New Features
Tests