[WEB-6815] chore: added support for pql #27
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 46 seconds. ⌛ 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 (1)
📝 WalkthroughWalkthroughAdded an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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