-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix PATCH project estimate validation bug #7872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: preview
Are you sure you want to change the base?
Fix PATCH project estimate validation bug #7872
Conversation
WalkthroughUpdates a serializer in apps/api/plane/api/serializers/project.py: minor formatting tweak and a logic change in ProjectUpdateSerializer.update to validate the estimate field by using its id when an estimate object is provided. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Endpoint
participant Serializer as ProjectUpdateSerializer
participant DB as Estimate Store
Client->>API: PATCH /projects/{id} { estimate: {...} or id }
API->>Serializer: validate & update(data)
alt estimate provided
Serializer->>Serializer: extract estimate_id<br/>(obj.id if object, else value)
Serializer->>DB: check Estimate.exists(estimate_id)
alt exists
DB-->>Serializer: ok
Serializer-->>API: proceed with update
API-->>Client: 200 Updated
else not found
DB-->>Serializer: not found
Serializer-->>API: raise ValidationError
API-->>Client: 400 Invalid estimate
end
else no estimate
Serializer-->>API: proceed without estimate change
API-->>Client: 200 Updated
end
note over Serializer: Changed: uses estimate.id when object is provided
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug in the PATCH endpoint validation for project estimates by correctly accessing the estimate ID when validating that an estimate belongs to a project.
- Fixed estimate validation logic to properly access the estimate object's ID attribute
- Ensures PATCH requests with estimate updates work correctly when the estimate exists
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/plane/api/serializers/project.py (1)
19-19
: Remove extra space in class declaration.There's an extra space between
class
and the class name. This should be a single space for consistency with Python conventions.Apply this diff to fix the formatting:
-class ProjectCreateSerializer(BaseSerializer): +class ProjectCreateSerializer(BaseSerializer):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/api/serializers/project.py
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
PR: makeplane/plane#7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/api/serializers/project.py
🧬 Code graph analysis (1)
apps/api/plane/api/serializers/project.py (1)
apps/api/plane/api/serializers/base.py (1)
BaseSerializer
(5-114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint API
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/api/serializers/project.py (1)
119-125
: Approve estimate validation fix. Extractingid
from theEstimate
instance aligns with DRF’s default handling of FK fields, and theNone
guard prevents attribute errors on missing estimates. No further changes needed.
Description
Type of Change
Screenshots and Media (if applicable)
Not applicable.
Test Scenarios
Verified that PATCH requests updating the estimate field now succeed when the estimate exists.
Verified that other fields in the Project PATCH still work as expected.
References
Note
Fixes project update validation to check the estimate by its id, allowing PATCH updates when a valid estimate is provided.
apps/api/plane/api/serializers/project.py
:ProjectUpdateSerializer.update
, validateestimate
by itsid
when checking project association to allow PATCH updates with an existing estimate.Written by Cursor Bugbot for commit d10a0bb. This will update automatically on new commits. Configure here.
Summary by CodeRabbit