Skip to content

Conversation

SamuelTR20
Copy link
Contributor

@SamuelTR20 SamuelTR20 commented Sep 29, 2025

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

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.

  • Backend/Serializers:
    • apps/api/plane/api/serializers/project.py:
      • In ProjectUpdateSerializer.update, validate estimate by its id 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

  • Bug Fixes
    • Resolved a validation issue when updating a project’s estimate. The API now correctly handles inputs provided as either an object or an ID, ensuring the correct estimate is referenced. This reduces unexpected 400 errors, improves compatibility with clients sending nested data, and stabilizes project update flows.

@Copilot Copilot AI review requested due to automatic review settings September 29, 2025 23:08
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

Updates 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

Cohort / File(s) Summary
Serializer update logic
apps/api/plane/api/serializers/project.py
Formatting tweak in class declaration. In update(), estimate validation now dereferences validated_data.get("estimate").id to perform existence checks, handling cases where an estimate object (not just an id) is supplied.

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my nose at tidy code,
A hop, a skip—validation mode.
If estimate’s an object, aha, the id!
I nibble bugs where they might hide.
One small space, one safer check—
Carrots saved, no broken spec. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description template’s “### Description” section contains only the placeholder comment and lacks a narrative of what was changed and why, and the “### References” section is empty instead of indicating related issue links or stating none apply, leaving required template fields incomplete. Please replace the placeholder under “### Description” with a clear summary of the code changes and their rationale, and update “### References” by linking any relevant issues or explicitly note that no related issues exist.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Fix PATCH project estimate validation bug” succinctly captures the primary change by specifying the HTTP method affected (PATCH), the resource (project estimate), and the nature of the update (validation bug fix), making it clear and appropriately scoped for someone skimming the merge history.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We 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:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@Copilot Copilot AI left a 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between ded3678 and d10a0bb.

📒 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. Extracting id from the Estimate instance aligns with DRF’s default handling of FK fields, and the None guard prevents attribute errors on missing estimates. No further changes needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants