[SILO-1276] feat: add work item page links, page listing, intake status, and archive methods#36
Open
akhil-vamshi-konam wants to merge 6 commits into
Open
[SILO-1276] feat: add work item page links, page listing, intake status, and archive methods#36akhil-vamshi-konam wants to merge 6 commits into
akhil-vamshi-konam wants to merge 6 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds work-item page linking to the Plane Python SDK: new Pydantic models, a WorkItemPages CRUD resource, Pages list methods, WorkItems integration and archived listing, Intake status update, related model/enum adjustments, tests across the surface, and a version bump. ChangesWork Item Page Linking
Sequence Diagram(s)sequenceDiagram
participant Client
participant WorkItemPages
participant API
Client->>WorkItemPages: list(workspace_slug, project_id, work_item_id, params?)
WorkItemPages->>API: GET /workspaces/{workspace_slug}/projects/{project_id}/work-items/{work_item_id}/pages
API-->>WorkItemPages: PaginatedWorkItemPageResponse
Client->>WorkItemPages: create(workspace_slug, project_id, work_item_id, CreateWorkItemPage)
WorkItemPages->>API: POST /workspaces/{workspace_slug}/projects/{project_id}/work-items/{work_item_id}/pages (body CreateWorkItemPage)
API-->>WorkItemPages: WorkItemPage
Client->>WorkItemPages: retrieve(..., work_item_page_id)
WorkItemPages->>API: GET /.../pages/{work_item_page_id}
API-->>WorkItemPages: WorkItemPage
Client->>WorkItemPages: delete(..., work_item_page_id)
WorkItemPages->>API: DELETE /.../pages/{work_item_page_id}
API-->>WorkItemPages: (no content)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/pages.py`:
- Around line 24-25: The endpoint URL strings for the pages list calls are
missing a trailing slash, causing redirects; update the calls that invoke
self._get to use f"{workspace_slug}/pages/" (and the other similar call around
the block that returns PaginatedPageResponse.model_validate) so the URL ends
with '/' per convention; search for uses of self._get with the "pages" path in
this file and add the trailing slash to each occurrence.
In `@plane/api/work_items/pages.py`:
- Around line 20-21: The list method currently accepts a raw dict parameter
`params: dict | None` (in the `list` function returning
`PaginatedWorkItemPageResponse`); change it to accept a Pydantic query DTO
(e.g., a new or existing QueryModel type) and before calling `_get` serialize it
with `query_model.model_dump(exclude_none=True)` instead of passing a raw dict,
then validate the response with
`PaginatedWorkItemPageResponse.model_validate()`; apply the same change (replace
raw dict params with the DTO + model_dump serialization and response
model_validate) to the other affected resource methods referenced around lines
30-33.
- Around line 31-33: The WorkItemPages endpoints are missing required trailing
slashes; update each endpoint string in the WorkItemPages resource to end with a
"/" (e.g. change
f"{workspace_slug}/projects/{project_id}/work-items/{work_item_id}/pages" to
f"{workspace_slug}/projects/{project_id}/work-items/{work_item_id}/pages/"), and
make the same change for the three other similar endpoint strings in the
WorkItemPages class so all requests follow the
`{base_path}/api/v1{resource_base_path}/{endpoint}/` convention.
In `@tests/unit/test_work_item_pages.py`:
- Around line 31-40: The page fixture creates a workspace page via
client.pages.create_workspace_page but never deletes it; convert the fixture to
a teardown-style (use yield) so after tests run you call the pages deletion API
(e.g., client.pages.delete_workspace_page or the appropriate client.pages.delete
method) with the created page's identifier to remove the page; update the
fixture named page to return (yield) the created CreatePage result and then call
the deletion in the finally/after-yield block to avoid leaking test pages.
- Around line 26-29: Replace the bare "except Exception: pass" around the
cleanup call to client.work_items.delete(workspace_slug, project.id, wi.id) with
targeted exception handling: import PlaneError from plane.errors and catch only
PlaneError (e.g., "except PlaneError: pass" or better "except PlaneError as e:
log.warning(...)") for expected SDK/cleanup failures, and allow all other
exceptions to propagate so unexpected errors fail the test; apply the same
change for the other cleanup calls at the same pattern (the other
client.work_items.delete and similar blocks).
🪄 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: d8a5716a-deaa-4f76-a6a7-6bf003cd21e7
📒 Files selected for processing (5)
plane/api/pages.pyplane/api/work_items/base.pyplane/api/work_items/pages.pyplane/models/work_item_pages.pytests/unit/test_work_item_pages.py
…ed work items method for work items
akhil-vamshi-konam
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
plane/models/work_item_properties.py (3)
134-135: ⚡ Quick winAlign parameter type annotations with field optionality.
Both
property_typeandrelation_typeare optional inUpdateWorkItemProperty, so their serializer parameters should reflect that with| None.♻️ Proposed fix for type annotation consistency
`@field_serializer`("property_type") -def serialize_property_type(self, value: PropertyType) -> str | None: +def serialize_property_type(self, value: PropertyType | None) -> str | None: return value.value if value else None `@field_serializer`("relation_type") -def serialize_relation_type(self, value: RelationType) -> str | None: +def serialize_relation_type(self, value: RelationType | None) -> str | None: return value.value if value else NoneAlso applies to: 138-139
🤖 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/models/work_item_properties.py` around lines 134 - 135, The serializer parameter annotations must match the optional fields: update the signatures of serialize_property_type and serialize_relation_type so they accept PropertyType | None and RelationType | None respectively (instead of non-optional types), and ensure their return logic remains the same (return value.value if value else None) to handle None inputs correctly.
47-48: ⚡ Quick winAlign parameter type annotations with field optionality.
The serializer parameter types should match the field types. Since
relation_typeis optional (RelationType | None), the parameter should beRelationType | None. Theproperty_typefield is required in this model, so its serializer parameter typePropertyTypeis correct, but the return type should remainstr(notstr | None) unlessproperty_typecan actually beNoneat serialization time.♻️ Proposed fix for type annotation consistency
`@field_serializer`("property_type") -def serialize_property_type(self, value: PropertyType) -> str | None: +def serialize_property_type(self, value: PropertyType) -> str: - return value.value if value else None + return value.value `@field_serializer`("relation_type") -def serialize_relation_type(self, value: RelationType) -> str | None: +def serialize_relation_type(self, value: RelationType | None) -> str | None: return value.value if value else NoneAlso applies to: 51-52
🤖 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/models/work_item_properties.py` around lines 47 - 48, The serialize_property_type signature and body must match the model field optionality: update serialize_property_type(self, value: PropertyType) -> str to return value.value (no None branch) since property_type is required, and update serialize_relation_type to accept RelationType | None (or Optional[RelationType]) and return str | None, preserving the conditional return (value.value if value else None); ensure you reference the property_type and relation_type fields and the methods serialize_property_type and serialize_relation_type when making the edits.
75-76: ⚡ Quick winAlign parameter type annotations with field optionality.
The serializer parameter types should match the field types. Since
relation_typeis optional (RelationType | None), the parameter should beRelationType | None. Theproperty_typefield is required in this model, so its serializer parameter typePropertyTypeis correct, but the return type should remainstr(notstr | None) unlessproperty_typecan actually beNoneat serialization time.♻️ Proposed fix for type annotation consistency
`@field_serializer`("property_type") -def serialize_property_type(self, value: PropertyType) -> str | None: +def serialize_property_type(self, value: PropertyType) -> str: - return value.value if value else None + return value.value `@field_serializer`("relation_type") -def serialize_relation_type(self, value: RelationType) -> str | None: +def serialize_relation_type(self, value: RelationType | None) -> str | None: return value.value if value else NoneAlso applies to: 79-80
🤖 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/models/work_item_properties.py` around lines 75 - 76, The serializer type hints are inconsistent with field optionality: update serialize_property_type so its signature reflects that property_type is required (keep parameter type as PropertyType and change the return type to str, not Optional) and update serialize_relation_type to accept RelationType | None and return str | None (since relation_type is optional); adjust the type annotations on the serialize_property_type and serialize_relation_type functions to match the property_type and relation_type field optionality.
🤖 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/intake.py`:
- Around line 113-115: The endpoint path passed to self._patch in the
update_status flow is missing a trailing slash; update the f-string used there
(the call that builds
f"{workspace_slug}/projects/{project_id}/intake-issues/{work_item_id}/status")
to include a trailing slash ("/status/") so it conforms to the SDK URL
convention used across plane/api/*.py and ensure any related callers still work
with the updated path.
In `@plane/api/work_items/base.py`:
- Around line 257-259: The archived-work-items endpoint URL is missing the
required trailing slash; update the f-string passed to self._get (the call
producing response) from
f"{workspace_slug}/projects/{project_id}/archived-work-items" to include a
trailing "/" so it becomes
f"{workspace_slug}/projects/{project_id}/archived-work-items/"; ensure this
change is applied in the method containing the response = self._get(...) call so
the SDK follows the "{base_path}/api/v1{resource_base_path}/{endpoint}/"
convention.
- Around line 21-23: Remove the duplicated import of WorkItemPages: in the
import block where WorkItemPages is imported twice alongside WorkItemRelations
(from .pages import WorkItemPages; from .relations import WorkItemRelations),
keep only a single import statement for WorkItemPages and remove the redundant
duplicate to satisfy Ruff/isort rules and clean up imports.
In `@tests/unit/test_intake.py`:
- Around line 120-121: The current tests silently return when the fixture
`intake_work_item` lacks an `issue`, causing false passes; replace the early
`return` that checks `if not (hasattr(intake_work_item, "issue") and
intake_work_item.issue): return` with an explicit `pytest.skip("fixture setup
incomplete: missing issue")` (or a hard `assert` that `intake_work_item.issue`
is present) so test outcomes are explicit; apply the same change in both
affected test functions that perform this `hasattr`/`issue` check.
In `@tests/unit/test_pages.py`:
- Around line 48-60: Wrap the page-creation assertions in a try/finally in the
tests that create pages (e.g., test_list_workspace_pages_contains_created_page
and the similar test at lines 71-83) so created pages are always deleted: after
calling client.pages.create_workspace_page (and the analogous
create_project_page), capture the created.id, run assertions in try, and in
finally call the appropriate deletion method (client.pages.delete_page or
client.pages.delete_workspace_page / delete_project_page as applicable) using
the captured id to ensure cleanup even if assertions fail; ensure you reference
the created variable and response/results to locate the creation and listing
calls.
In `@tests/unit/test_work_items.py`:
- Line 8: The import line with AdvancedSearchWorkItem, CreateWorkItem,
CreateWorkItemLink, UpdateWorkItem, and UpdateWorkItemLink exceeds the
100-character limit; split or wrap the import to multiple lines (e.g., use
parentheses and one symbol per line or separate imports) so the line length
complies with Black/Ruff formatting while keeping the same symbols imported from
plane.models.work_items.
- Around line 358-361: The teardown currently swallows all exceptions from
client.work_items.delete(workspace_slug, project.id, wi.id) which hides cleanup
failures; update the teardown to either let the exception propagate (remove the
try/except) so failures surface, or catch only the specific expected exception
(e.g., a NotFound or HTTPError) and re-raise or log unexpected exceptions;
locate the delete call in the test teardown and change the try/except block
accordingly so you do not silently pass on all Exception types.
---
Nitpick comments:
In `@plane/models/work_item_properties.py`:
- Around line 134-135: The serializer parameter annotations must match the
optional fields: update the signatures of serialize_property_type and
serialize_relation_type so they accept PropertyType | None and RelationType |
None respectively (instead of non-optional types), and ensure their return logic
remains the same (return value.value if value else None) to handle None inputs
correctly.
- Around line 47-48: The serialize_property_type signature and body must match
the model field optionality: update serialize_property_type(self, value:
PropertyType) -> str to return value.value (no None branch) since property_type
is required, and update serialize_relation_type to accept RelationType | None
(or Optional[RelationType]) and return str | None, preserving the conditional
return (value.value if value else None); ensure you reference the property_type
and relation_type fields and the methods serialize_property_type and
serialize_relation_type when making the edits.
- Around line 75-76: The serializer type hints are inconsistent with field
optionality: update serialize_property_type so its signature reflects that
property_type is required (keep parameter type as PropertyType and change the
return type to str, not Optional) and update serialize_relation_type to accept
RelationType | None and return str | None (since relation_type is optional);
adjust the type annotations on the serialize_property_type and
serialize_relation_type functions to match the property_type and relation_type
field optionality.
🪄 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: b509d5f5-52f1-4967-b0fd-80f912f4c8d1
📒 Files selected for processing (11)
plane/api/intake.pyplane/api/work_items/base.pyplane/api/work_items/pages.pyplane/models/enums.pyplane/models/projects.pyplane/models/work_item_properties.pyplane/models/work_items.pypyproject.tomltests/unit/test_intake.pytests/unit/test_pages.pytests/unit/test_work_items.py
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- plane/api/work_items/pages.py
akhil-vamshi-konam
changed the title
akhil-vamshi-konam
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Description
New sub-resource:
client.work_items.pagescreate(workspace_slug, project_id, work_item_id, data)— link a page to a work itemlist(workspace_slug, project_id, work_item_id)— list all linked pagesretrieve(workspace_slug, project_id, work_item_id, work_item_page_id)— retrieve a specific linkdelete(workspace_slug, project_id, work_item_id, work_item_page_id)— remove a page linkPages resource (
client.pages)list_workspace_pages(workspace_slug)— list all workspace pageslist_project_pages(workspace_slug, project_id)— list all pages in a projectIntake resource (
client.intake)update_status(workspace_slug, project_id, work_item_id, data)— triage an intake work item (accept, decline, snooze, duplicate)Work items resource (
client.work_items)list_archived(workspace_slug, project_id)— list archived work itemsarchive(workspace_slug, project_id, work_item_id)— archive a work itemunarchive(workspace_slug, project_id, work_item_id)— restore an archived work itemType of Change
Test Scenarios
tests/unit/test_work_item_pages.py— create, list, retrieve, delete page linkstests/unit/test_pages.py— list workspace/project pagestests/unit/test_intake.py— accept and decline intake work items viaupdate_statustests/unit/test_work_items.py— archive, unarchive, list archived, create/update links with title