-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[SILO-699] chore: add check for feature enabled for module and cycle create #8146
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
[SILO-699] chore: add check for feature enabled for module and cycle create #8146
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughAdded project feature-flag checks to two serializers: CycleCreateSerializer now validates project.cycle_view (and converts start_date with is_start_date=True), and ModuleCreateSerializer now validates project.module_view before proceeding with existing validation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Serializer
participant ProjectDB as Project DB
Client->>Serializer: POST / create cycle or module
Serializer->>Serializer: validate() begins
Serializer->>ProjectDB: GET /Project by project_id
ProjectDB-->>Serializer: Project (or not found)
alt project missing
rect rgb(255,200,200)
Note over Serializer: raise ValidationError (missing project)
Serializer-->>Client: 400 ValidationError
end
else project present
alt feature flag enabled
rect rgb(200,220,255)
Note over Serializer: continue validation (dates, members, etc.)
Serializer-->>Client: 201 Created (or validation success)
end
else feature flag disabled
rect rgb(255,200,200)
Note over Serializer: raise ValidationError (feature disabled)
Serializer-->>Client: 400 ValidationError
end
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-07-23T18:18:06.875ZApplied to files:
🧬 Code graph analysis (1)apps/api/plane/api/serializers/module.py (2)
⏰ 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). (2)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/api/serializers/cycle.py(2 hunks)apps/api/plane/api/serializers/module.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dheeru0198
Repo: makeplane/plane PR: 7625
File: apps/api/plane/bgtasks/workspace_seed_task.py:95-98
Timestamp: 2025-09-12T07:29:36.083Z
Learning: In the Plane codebase, workspace seed data should have project features (cycle_view, module_view, issue_views_view) enabled by default, even when regular project creation has these features disabled. This provides users with a complete demo experience in the seeded workspace.
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 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.
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7969
File: apps/api/plane/api/views/cycle.py:1218-1228
Timestamp: 2025-10-17T10:46:13.825Z
Learning: In the cycle transfer functionality in apps/api/plane/api/views/cycle.py, transfers should only be allowed from completed cycles (end_date < timezone.now()), not from active or draft cycles. The guard should block when end_date is None or end_date >= timezone.now().
📚 Learning: 2025-10-17T10:46:13.825Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7969
File: apps/api/plane/api/views/cycle.py:1218-1228
Timestamp: 2025-10-17T10:46:13.825Z
Learning: In the cycle transfer functionality in apps/api/plane/api/views/cycle.py, transfers should only be allowed from completed cycles (end_date < timezone.now()), not from active or draft cycles. The guard should block when end_date is None or end_date >= timezone.now().
Applied to files:
apps/api/plane/api/serializers/cycle.py
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 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/cycle.pyapps/api/plane/api/serializers/module.py
🧬 Code graph analysis (2)
apps/api/plane/api/serializers/cycle.py (2)
apps/api/plane/db/models/project.py (1)
Project(65-159)apps/api/plane/tests/contract/api/test_cycles.py (1)
project(12-26)
apps/api/plane/api/serializers/module.py (1)
apps/api/plane/db/models/project.py (1)
Project(65-159)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/api/serializers/cycle.py (1)
80-80: Clarify the purpose of theis_start_date=Trueparameter.This change adds an
is_start_date=Trueparameter to theconvert_to_utccall, which is not mentioned in the PR description. The PR objective states adding feature checks for module and cycle creation, not modifying timezone conversion behavior.Please clarify:
- Is this change intentional and related to the feature flag validation?
- What is the purpose of the
is_start_dateparameter and how does it affect UTC conversion?- Should this be part of a separate PR focused on timezone handling?
| project = Project.objects.get(id=self.context.get("project_id")) | ||
| if not project.module_view: | ||
| raise serializers.ValidationError("Modules are not enabled for this project") |
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.
Handle missing project gracefully to prevent unhandled exceptions.
The code uses Project.objects.get() which will raise Project.DoesNotExist if the project is not found or if project_id is None. Additionally, if project_id is not present in the context, this will fail unexpectedly.
Apply this diff to add defensive checks:
- def validate(self, data):
- project = Project.objects.get(id=self.context.get("project_id"))
- if not project.module_view:
- raise serializers.ValidationError("Modules are not enabled for this project")
+ def validate(self, data):
+ project_id = self.context.get("project_id")
+ if not project_id:
+ raise serializers.ValidationError("Project ID is required")
+
+ project = Project.objects.filter(id=project_id).first()
+ if not project:
+ raise serializers.ValidationError("Project not found")
+
+ if not project.module_view:
+ raise serializers.ValidationError("Modules are not enabled for this project")🤖 Prompt for AI Agents
In apps/api/plane/api/serializers/module.py around lines 57 to 59, the code
calls Project.objects.get(id=self.context.get("project_id")) which will raise an
unhandled Project.DoesNotExist (or fail when project_id is None); update it to
first validate that project_id exists in self.context and then retrieve the
project safely (either use Project.objects.filter(id=project_id).first() and
check for None, or wrap get() in a try/except Project.DoesNotExist), and raise
serializers.ValidationError with a clear message (e.g., "Project not found" or
"project_id missing") when the project_id is missing or the project cannot be
found, instead of letting an exception bubble up.
|
@Saurabhkmr98 The coderabbit reviews are valid. Please fix those. |
…create (makeplane#8146) * add check for feature enabled for module and cycle create * add more checks
…create (makeplane#8146) * add check for feature enabled for module and cycle create * add more checks
…create (makeplane#8146) * add check for feature enabled for module and cycle create * add more checks
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.