-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-4723] fix: disable project features on project create #7625
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
Conversation
WalkthroughDefaults for three Project boolean fields are changed to False via a migration; workspace seeding explicitly enables those flags when creating projects. A Session.user_id index was added. A new license Instance boolean field was added with a migration. An unused import was removed from a serializer. No public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
actor Seeder as Workspace Seeder
participant SeedTask as workspace_seed_task.py
participant ORM as Django ORM
participant DB as Database
Seeder->>SeedTask: create_project_and_member(payload)
Note over SeedTask: Inject flags: cycle_view=true<br/>module_view=true<br/>issue_views_view=true<br/>created_by_id set
SeedTask->>ORM: Project.objects.create(..., flags, created_by_id)
ORM->>DB: INSERT Project row (model defaults are False for these fields)
DB-->>ORM: Created row
ORM-->>SeedTask: Project instance
SeedTask-->>Seeder: return created Project
Note over DB: Migration sets defaults to False for new non-seeded creations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
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: 1
🧹 Nitpick comments (2)
apps/api/plane/db/models/project.py (2)
92-94: Optional: add help_text to make admin/API intent explicit.Helps future readers understand these are feature toggles intended to start disabled.
Apply this diff if you want inline documentation (will generate a small migration):
- module_view = models.BooleanField(default=False) - cycle_view = models.BooleanField(default=False) - issue_views_view = models.BooleanField(default=False) + module_view = models.BooleanField(default=False, help_text="Enable Modules feature for this project") + cycle_view = models.BooleanField(default=False, help_text="Enable Cycles feature for this project") + issue_views_view = models.BooleanField(default=False, help_text="Enable Issue Views for this project")
92-94: Naming nit: consider less redundant field names.“issue_views_view” is a bit awkward. If you ever plan a cleanup, names like “modules_enabled”, “cycles_enabled”, and “issue_views_enabled” read cleaner and convey boolean-ness. Out of scope to change now due to migration/churn, just flagging for backlog.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/api/plane/app/serializers/project.py(0 hunks)apps/api/plane/db/migrations/0102_alter_project_cycle_view_and_more.py(1 hunks)apps/api/plane/db/models/project.py(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/plane/app/serializers/project.py
🔇 Additional comments (1)
apps/api/plane/db/migrations/0102_alter_project_cycle_view_and_more.py (1)
1-28: Migration aligns with model updates—merge after local plan check.
- Confirmed that
0102_alter_project_cycle_view_and_more.pyis the last file inapps/api/plane/db/migrationsand flips the defaults forcycle_view,issue_views_view, andmodule_viewtoFalse.- The
Projectmodel inapps/api/plane/db/models/project.pydeclares those three fields withdefault=False.- No other migrations or code locations introduce unexpected changes to these fields.
After merging, in your development environment run:
python manage.py makemigrations --check python manage.py migrate --plan
| module_view = models.BooleanField(default=False) | ||
| cycle_view = models.BooleanField(default=False) | ||
| issue_views_view = models.BooleanField(default=False) |
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.
💡 Verification agent
🧩 Analysis chain
Defaults flipped to False align with PR objective; verify call sites and tests.
This change will disable Modules, Cycles, and Issue Views for newly created projects while leaving existing rows untouched. Page view remains enabled (Line 95), which matches the stated requirement.
Run this script to find places that may assume these flags default to True, update factories/fixtures, and catch brittle tests:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==> Find all references to the flipped flags (backend + frontend)"
rg -n -C2 -S -g '!**/migrations/**' -g '!**/node_modules/**' --type-add 'web:*.{ts,tsx,js,jsx}' --type py --type web \
-e '\bmodule_view\b' -e '\bcycle_view\b' -e '\bissue_views_view\b'
echo
echo "==> Project creations that may rely on old defaults (ensure flags are set explicitly when needed)"
rg -n -C2 -S -g '!**/migrations/**' --type py -e '\bProject\.(objects\.)?create\(' -e '\bProject\('
echo
echo "==> Factories/seeds likely to need updates"
rg -n -C2 -S --type py -e 'Factory\b|factory_boy|ProjectFactory|ProjectFaker|seed_.*project'Length of output: 59374
Update Tests and Factories for Flipped Defaults
The Boolean defaults for module_view, cycle_view, and issue_views_view have been flipped to False in apps/api/plane/db/models/project.py; all existing code paths were scanned and numerous call sites in both frontend and backend correctly guard on these flags (e.g. shouldRender: project.cycle_view, cycleViewDisabled={!currentProjectDetails?.cycle_view}, OpenAPI query parameter definitions, etc.). However, several tests and factories still rely on the old defaults:
• Backend unit/contract tests invoking Project.objects.create(...) without explicit flags will now get False instead of the previous True
– apps/api/plane/tests/contract/api/test_labels.py (line 12): add module_view=False, cycle_view=False, issue_views_view=False to the Project.objects.create call
– apps/api/plane/tests/unit/serializers/test_issue_recent_visit.py (line 41): likewise include explicit defaults
– All other tests under apps/api/plane/tests/contract/app/test_project_app.py that call Project.objects.create(...) without flags should be updated similarly
• ProjectFactory in apps/api/plane/tests/factories.py needs to set the new defaults so factories produce the intended state by default:
class ProjectFactory(factory.django.DjangoModelFactory):
class Meta:
model = Project
+ module_view = False
+ cycle_view = False
+ issue_views_view = False• Background‐task seeders creating projects without flags must also pass them explicitly:
– apps/api/plane/bgtasks/workspace_seed_task.py: include module_view=False, cycle_view=False, issue_views_view=False in the Project.objects.create(...) call
– apps/api/plane/bgtasks/dummy_data_task.py: same update for its project creation
Please update these locations to explicitly set the three new flags when creating projects in tests, factories, and seed tasks, and then re‐run the test suite to confirm all expectations align with the new defaults.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| module_view = models.BooleanField(default=False) | |
| cycle_view = models.BooleanField(default=False) | |
| issue_views_view = models.BooleanField(default=False) | |
| class ProjectFactory(factory.django.DjangoModelFactory): | |
| class Meta: | |
| model = Project | |
| module_view = False | |
| cycle_view = False | |
| issue_views_view = False |
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 updates the default project feature visibility settings so that new projects are created with most features disabled by default, keeping only pages enabled. This change aims to simplify the initial project experience while allowing users to enable features as needed.
Key changes:
- Modified default values for project feature flags from
TruetoFalse - Created database migration to implement the schema changes
- Updated seed data to explicitly enable all features for workspace initialization
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
apps/api/plane/db/models/project.py |
Changed default values for module_view, cycle_view, and issue_views_view from True to False |
apps/api/plane/db/migrations/0105_alter_project_cycle_view_and_more.py |
Added Django migration to update the default field values in the database |
apps/api/plane/bgtasks/workspace_seed_task.py |
Explicitly enabled all project views in seed data creation to maintain existing behavior |
apps/api/plane/app/serializers/project.py |
Removed unused import validate_binary_data |
Comments suppressed due to low confidence (1)
apps/api/plane/app/serializers/project.py:1
- The removal of the unused import
validate_binary_datais good cleanup, but ensure this change is intentional and not accidentally removing a dependency that might be needed elsewhere in this file.
# Third party imports
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/plane/app/serializers/project.py(0 hunks)apps/api/plane/bgtasks/workspace_seed_task.py(1 hunks)apps/api/plane/db/migrations/0105_alter_project_cycle_view_and_more.py(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/plane/app/serializers/project.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🔇 Additional comments (1)
apps/api/plane/db/migrations/0105_alter_project_cycle_view_and_more.py (1)
12-28: Defaults flipped to False — aligns with requirementAltering
cycle_view,issue_views_view, andmodule_viewto default False matches “disable features on create.” No data migration is included, which is fine if the change targets only newly created projects.If policy also applies to existing projects, follow up with a separate data migration to backfill these fields to False for legacy rows where appropriate.
Index user_id field in Session model
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 (3)
apps/api/plane/license/models/instance.py (1)
41-42: Clarify intent and make field operationally friendly (help_text, editable, optional index).If this flag is computed/managed by the system, mark it non-editable and document it. Add an index only if you expect frequent filtering on this column.
- # field for validating if the current version is deprecated - is_current_version_deprecated = models.BooleanField(default=False) + # indicates whether `current_version` is deprecated per support policy + is_current_version_deprecated = models.BooleanField( + default=False, + help_text="True when the instance's current_version is outside the supported window.", + editable=False, # flip to True only if this is user-editable + db_index=True, # drop if not a common filter + )apps/api/plane/license/migrations/0006_instance_is_current_version_deprecated.py (1)
12-18: Consider a follow-up data/update path.If any existing instances are already on deprecated versions, you may need either a data migration or a background task to initialize this flag accurately post-deploy.
apps/api/plane/db/models/session.py (1)
16-16: Remove field-level db_index on Session.user_id; create a concurrent partial indexSessions is high‑write; ALTERing to add an index will block writes. Remove db_index from the field and add a partial index (WHERE user_id IS NOT NULL) created concurrently.
- user_id = models.CharField(null=True, max_length=50, db_index=True) + user_id = models.CharField(null=True, max_length=50)Add to the model Meta:
from django.db.models import Q class Session(AbstractBaseSession): ... class Meta(AbstractBaseSession.Meta): db_table = "sessions" indexes = [ models.Index( fields=["user_id"], name="sessions_user_id_not_null_idx", condition=Q(user_id__isnull=False), ), ]Migration guidance (concise): use migrations.SeparateDatabaseAndState — in state_operations do AlterField (remove db_index) + AddIndex; in database_operations create the same index concurrently using django.contrib.postgres.operations.AddIndexConcurrently (project already uses AddIndexConcurrently in apps/api/plane/db/migrations/0103_*).
Relevant usage: Session.objects.filter(user_id=request.user.id) in apps/api/plane/app/views/user/base.py:152 — index will be useful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/plane/db/migrations/0105_alter_project_cycle_view_and_more.py(1 hunks)apps/api/plane/db/models/session.py(1 hunks)apps/api/plane/license/migrations/0006_instance_is_current_version_deprecated.py(1 hunks)apps/api/plane/license/models/instance.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/db/migrations/0105_alter_project_cycle_view_and_more.py
⏰ 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)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/api/plane/license/models/instance.py (1)
41-42: Who updates this denormalized flag?Search shows only the field declaration (apps/api/plane/license/models/instance.py:42) and its migration — no writers/readers found. Add a clear updater (signal, scheduled job, or service) to flip this when support windows change; if an updater already exists, point to its location and add tests.
apps/api/plane/license/migrations/0006_instance_is_current_version_deprecated.py (1)
12-18: Schema addition looks good.Field is added with a sane default; migration is minimal and safe.
…#7625) * fix: disbale project features on project create * Implement migration 0105 to alter project cycle view fields to Boolean with default values * Add project view settings in workspace seed task * Add is_current_version_deprecated field to Instance model Index user_id field in Session model --------- Co-authored-by: pablohashescobar <nikhilschacko@gmail.com>
Description
Type of Change
Summary by CodeRabbit
Chores
New Features