[SILO-1026] feat: add estimates external API endpoints#8664
Open
Saurabhkmr98 wants to merge 5 commits intopreviewfrom
Open
[SILO-1026] feat: add estimates external API endpoints#8664Saurabhkmr98 wants to merge 5 commits intopreviewfrom
Saurabhkmr98 wants to merge 5 commits intopreviewfrom
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an estimates feature: serializers (Estimate, EstimatePoint) with validation, three REST endpoints for project estimates and points, URL routing, a migration and model enum for estimate type choices, and OpenAPI parameters/examples/decorators. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Endpoint
participant Serializer
participant DB as Database
Client->>API: POST /workspaces/{slug}/projects/{id}/estimates
API->>API: check existing estimate
alt exists
API-->>Client: 409 Conflict
else
API->>Serializer: validate payload (context: workspace, project)
Serializer->>Serializer: inject workspace/project into data
Serializer-->>API: validated data
API->>DB: create Estimate
DB-->>API: created Estimate
API-->>Client: 201 Created + Estimate
end
Client->>API: POST /workspaces/{slug}/projects/{id}/estimates/{estimate_id}/points
API->>API: ensure estimate exists
alt missing
API-->>Client: 404 Not Found
else
API->>Serializer: validate list of points (many=True)
Serializer->>Serializer: validate each point (non-empty, max 20 chars)
Serializer-->>API: validated points
API->>DB: bulk_create EstimatePoint records
DB-->>API: created points
API-->>Client: 201 Created + points array
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 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 |
Saurabhkmr98
requested review from
Prashant-Surya,
dheeru0198 and
sriramveeraghanta
and removed request for
sriramveeraghanta
…t-estimates_external_api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/plane/utils/openapi/decorators.py (1)
299-310: Consider includingESTIMATE_ID_PARAMETERin defaults.Since estimate point endpoints are always nested under an estimate (
/estimates/<estimate_id>/estimate-points/), includingESTIMATE_ID_PARAMETERin the defaults would reduce boilerplate at call sites. However, this follows the existing pattern where nested decorators don't include parent IDs by default, so the current implementation is acceptable.📝 Optional enhancement
+from .parameters import WORKSPACE_SLUG_PARAMETER, PROJECT_ID_PARAMETER, ESTIMATE_ID_PARAMETER + def estimate_point_docs(**kwargs): """Decorator for estimate point-related endpoints""" defaults = { "tags": ["Estimate Points"], - "parameters": [WORKSPACE_SLUG_PARAMETER, PROJECT_ID_PARAMETER], + "parameters": [WORKSPACE_SLUG_PARAMETER, PROJECT_ID_PARAMETER, ESTIMATE_ID_PARAMETER], "responses": { }, } return extend_schema(**_merge_schema_options(defaults, kwargs))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/plane/utils/openapi/decorators.py` around lines 299 - 310, The decorator estimate_point_docs currently sets defaults for tags and parameters but omits ESTIMATE_ID_PARAMETER, causing callers to repeatedly add the parent estimate ID; update the defaults dict in estimate_point_docs to include ESTIMATE_ID_PARAMETER alongside WORKSPACE_SLUG_PARAMETER and PROJECT_ID_PARAMETER so nested estimate-point endpoints automatically document the parent estimate ID, keeping the rest of the defaults (tags and responses) unchanged and using the existing _merge_schema_options/extend_schema flow.apps/api/plane/utils/openapi/parameters.py (1)
499-505: Consider adding examples for consistency with other UUID parameters.Other UUID path parameters in this file (e.g.,
PROJECT_ID_PARAMETER,CYCLE_ID_PARAMETER,MODULE_ID_PARAMETER) includeexampleswith sample UUIDs and descriptions. Adding an example here would maintain consistency in OpenAPI documentation.📝 Proposed fix
ESTIMATE_ID_PARAMETER = OpenApiParameter( name="estimate_id", description="Estimate ID", required=True, type=OpenApiTypes.UUID, location=OpenApiParameter.PATH, + examples=[ + OpenApiExample( + name="Example estimate ID", + value="550e8400-e29b-41d4-a716-446655440000", + description="A typical estimate UUID", + ) + ], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/plane/utils/openapi/parameters.py` around lines 499 - 505, The ESTIMATE_ID_PARAMETER OpenApiParameter is missing an examples entry; add an examples argument similar to PROJECT_ID_PARAMETER/CYCLE_ID_PARAMETER/MODULE_ID_PARAMETER using an OpenApiExample with a sample UUID and a short description (e.g., "Sample estimate id") so the path parameter documentation is consistent with other UUID parameters; update the ESTIMATE_ID_PARAMETER definition to include examples=[OpenApiExample(value="00000000-0000-0000-0000-000000000000", description="Sample estimate id")] (or equivalent) ensuring imports are present if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/plane/api/views/estimate.py`:
- Around line 47-69: The post method currently uses Project.objects.get(...) and
Workspace.objects.get(...), which raise exceptions and make the subsequent
if-not checks unreachable; replace both calls with
Project.objects.filter(id=project_id, workspace__slug=slug).first() and
Workspace.objects.filter(slug=slug).first() (matching the file's pattern), then
check for None and return the 404 Response as written; ensure you still obtain
workspace and project to pass into serializer_class when creating the estimate
and keep the existing project_estimate check/serializer flow unchanged.
---
Nitpick comments:
In `@apps/api/plane/utils/openapi/decorators.py`:
- Around line 299-310: The decorator estimate_point_docs currently sets defaults
for tags and parameters but omits ESTIMATE_ID_PARAMETER, causing callers to
repeatedly add the parent estimate ID; update the defaults dict in
estimate_point_docs to include ESTIMATE_ID_PARAMETER alongside
WORKSPACE_SLUG_PARAMETER and PROJECT_ID_PARAMETER so nested estimate-point
endpoints automatically document the parent estimate ID, keeping the rest of the
defaults (tags and responses) unchanged and using the existing
_merge_schema_options/extend_schema flow.
In `@apps/api/plane/utils/openapi/parameters.py`:
- Around line 499-505: The ESTIMATE_ID_PARAMETER OpenApiParameter is missing an
examples entry; add an examples argument similar to
PROJECT_ID_PARAMETER/CYCLE_ID_PARAMETER/MODULE_ID_PARAMETER using an
OpenApiExample with a sample UUID and a short description (e.g., "Sample
estimate id") so the path parameter documentation is consistent with other UUID
parameters; update the ESTIMATE_ID_PARAMETER definition to include
examples=[OpenApiExample(value="00000000-0000-0000-0000-000000000000",
description="Sample estimate id")] (or equivalent) ensuring imports are present
if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91f23a0f-f0da-4be5-9a35-990c74be990a
📒 Files selected for processing (10)
apps/api/plane/api/serializers/__init__.pyapps/api/plane/api/serializers/estimate.pyapps/api/plane/api/urls/estimate.pyapps/api/plane/api/views/estimate.pyapps/api/plane/db/migrations/0121_alter_estimate_type.pyapps/api/plane/db/models/estimate.pyapps/api/plane/utils/openapi/__init__.pyapps/api/plane/utils/openapi/decorators.pyapps/api/plane/utils/openapi/examples.pyapps/api/plane/utils/openapi/parameters.py
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
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Documentation