-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[WEB-5890] migration: added getting_started_checklist, tips, explored_feature fields on the workspace member table #8489
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?
Conversation
📝 WalkthroughWalkthroughThree database models receive new JSON and Boolean field additions: WorkspaceMember gains getting_started_checklist, tips, and explored_features fields; User gains is_password_reset_required; Profile gains notification_view_mode with FULL/Compact choices. A corresponding Django migration applies these schema changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
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: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/db/migrations/0114_workspacemember_explored_features_and_more.pyapps/api/plane/db/models/workspace.py
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/db/models/workspace.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/migrations/0113_webhook_version.py:7-14
Timestamp: 2025-12-29T08:58:46.563Z
Learning: In the Plane codebase, when adding product tour or onboarding fields via migrations, it's intentional to set existing records to `True` (completed) while having the model default to `False` for new records. This ensures existing users don't see tours they don't need.
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.
Applied to files:
apps/api/plane/db/migrations/0114_workspacemember_explored_features_and_more.py
🧬 Code graph analysis (1)
apps/api/plane/db/migrations/0114_workspacemember_explored_features_and_more.py (1)
apps/api/plane/utils/exporters/schemas/base.py (1)
JSONField(127-140)
⏰ 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 packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/api/plane/db/migrations/0114_workspacemember_explored_features_and_more.py (1)
6-28: LGTM! Migration structure is correct.The migration is well-formed with appropriate field types and defaults. The use of
default=dictfor JSONField is correct in Django migrations.
apps/api/plane/db/migrations/0116_workspacemember_explored_features_and_more.py
Show resolved
Hide resolved
|
Linked to Plane Work Item(s) This comment was auto-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: 0
🧹 Nitpick comments (1)
apps/api/plane/db/models/user.py (1)
228-230: Consider reducingmax_lengthfor tighter storage.The field only stores
"full"or"compact"(max 7 characters), butmax_length=255is specified. A smaller value likemax_length=20would be more precise while still allowing headroom for future values.This is a minor optimization and not blocking.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/db/migrations/0116_workspacemember_explored_features_and_more.pyapps/api/plane/db/models/user.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/migrations/0113_webhook_version.py:7-14
Timestamp: 2025-12-29T08:58:46.563Z
Learning: In the Plane codebase, when adding product tour or onboarding fields via migrations, it's intentional to set existing records to `True` (completed) while having the model default to `False` for new records. This ensures existing users don't see tours they don't need.
📚 Learning: 2025-12-29T08:58:46.563Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/migrations/0113_webhook_version.py:7-14
Timestamp: 2025-12-29T08:58:46.563Z
Learning: In the Plane codebase, when adding product tour or onboarding fields via migrations, it's intentional to set existing records to `True` (completed) while having the model default to `False` for new records. This ensures existing users don't see tours they don't need.
Applied to files:
apps/api/plane/db/migrations/0116_workspacemember_explored_features_and_more.py
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.
Applied to files:
apps/api/plane/db/migrations/0116_workspacemember_explored_features_and_more.pyapps/api/plane/db/models/user.py
🧬 Code graph analysis (1)
apps/api/plane/db/migrations/0116_workspacemember_explored_features_and_more.py (1)
apps/api/plane/utils/exporters/schemas/base.py (2)
BooleanField(97-105)JSONField(127-140)
⏰ 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 (5)
apps/api/plane/db/models/user.py (2)
87-87: LGTM!The field follows the existing naming convention and is logically placed with other
is_*boolean fields.
195-198: LGTM!Clean implementation using Django's
TextChoicespattern. Inner class placement withinProfileis appropriate.apps/api/plane/db/migrations/0116_workspacemember_explored_features_and_more.py (3)
6-10: LGTM!Migration structure and dependencies are correctly defined.
23-37: Verify if existing workspace members should have pre-populated tour data.Based on learnings from the Plane codebase, when adding product tour or onboarding fields, existing records are typically set to a "completed" state so existing users don't see tours they don't need.
The current migration adds these fields with
default=dict, meaning both new and existing workspace members will have empty dictionaries. If existing members should skip the tour, consider adding aRunPythondata migration to pre-populate these fields for existing records.Please confirm whether existing workspace members should:
- See the new product tour (current behavior with empty dict), or
- Skip the tour (would require a data migration to populate completed state)
13-22: LGTM!The
notification_view_modeandis_password_reset_requiredfield additions are correctly defined with appropriate defaults.
Description
getting_started_checklist,tips,explored_featuresin theWorkspaceMembertable for the new product tour.is_password_reset_requiredin the User table.notification_view_modein the Profile table.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.