chore: Add fields to issue_version and profile tables, and create new sticky table #6198
Conversation
… new sticky table
|
Warning Rate limit exceeded@gurusainath has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces significant changes to the database schema involving the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
sainath seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
apiserver/plane/db/models/sticky.py (1)
9-32: Add a__str__method to theStickymodelImplementing a
__str__method will enhance the readability when instances ofStickyare printed or logged.Consider adding the following method:
def __str__(self): return f"{self.name} ({self.workspace})"apiserver/plane/db/models/user.py (2)
29-35: Consider reducing code duplication with the default onboarding function.While the implementation is correct, consider refactoring to avoid duplication with
get_default_onboarding(). You could parameterize the function or use inheritance.Here's a suggested approach:
-def get_mobile_default_onboarding(): - return { - "profile_complete": False, - "workspace_create": False, - "workspace_join": False, - } +def get_default_onboarding(include_workspace_invite=True): + steps = { + "profile_complete": False, + "workspace_create": False, + "workspace_join": False, + } + if include_workspace_invite: + steps["workspace_invite"] = False + return steps + +def get_mobile_default_onboarding(): + return get_default_onboarding(include_workspace_invite=False)
191-191: Consider adding JSON schema validation for mobile_onboarding_step.The
mobile_onboarding_stepJSONField would benefit from schema validation to ensure data consistency.Example implementation:
from django.core.exceptions import ValidationError import jsonschema MOBILE_ONBOARDING_SCHEMA = { "type": "object", "properties": { "profile_complete": {"type": "boolean"}, "workspace_create": {"type": "boolean"}, "workspace_join": {"type": "boolean"} }, "required": ["profile_complete", "workspace_create", "workspace_join"] } def validate_mobile_onboarding(value): try: jsonschema.validate(instance=value, schema=MOBILE_ONBOARDING_SCHEMA) except jsonschema.exceptions.ValidationError as e: raise ValidationError(f"Invalid mobile onboarding data: {str(e)}") class Profile(TimeAuditModel): # ... other fields ... mobile_onboarding_step = models.JSONField( default=get_mobile_default_onboarding, validators=[validate_mobile_onboarding] )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py(1 hunks)apiserver/plane/db/models/__init__.py(1 hunks)apiserver/plane/db/models/issue.py(4 hunks)apiserver/plane/db/models/sticky.py(1 hunks)apiserver/plane/db/models/user.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py
38-38: Line too long (141 > 88)
(E501)
43-43: Line too long (165 > 88)
(E501)
63-63: Line too long (141 > 88)
(E501)
68-68: Line too long (99 > 88)
(E501)
69-69: Line too long (101 > 88)
(E501)
70-70: Line too long (103 > 88)
(E501)
71-71: Line too long (140 > 88)
(E501)
79-79: Line too long (94 > 88)
(E501)
80-80: Line too long (200 > 88)
(E501)
81-81: Line too long (144 > 88)
(E501)
82-82: Line too long (206 > 88)
(E501)
83-83: Line too long (138 > 88)
(E501)
95-95: Line too long (99 > 88)
(E501)
96-96: Line too long (101 > 88)
(E501)
97-97: Line too long (103 > 88)
(E501)
98-98: Line too long (140 > 88)
(E501)
103-103: Line too long (91 > 88)
(E501)
104-104: Line too long (200 > 88)
(E501)
105-105: Line too long (142 > 88)
(E501)
106-106: Line too long (165 > 88)
(E501)
107-107: Line too long (143 > 88)
(E501)
108-108: Line too long (206 > 88)
(E501)
109-109: Line too long (149 > 88)
(E501)
apiserver/plane/db/models/issue.py
18-18: .base.BaseModel imported but unused
Remove unused import: .base.BaseModel
(F401)
🔇 Additional comments (3)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py (1)
19-117: Migration script appears correct
The migration file successfully implements the intended schema changes, including adding and removing fields and creating new models. There are no issues detected in the migration operations.
🧰 Tools
🪛 Ruff (0.8.2)
38-38: Line too long (141 > 88)
(E501)
43-43: Line too long (165 > 88)
(E501)
63-63: Line too long (141 > 88)
(E501)
68-68: Line too long (99 > 88)
(E501)
69-69: Line too long (101 > 88)
(E501)
70-70: Line too long (103 > 88)
(E501)
71-71: Line too long (140 > 88)
(E501)
79-79: Line too long (94 > 88)
(E501)
80-80: Line too long (200 > 88)
(E501)
81-81: Line too long (144 > 88)
(E501)
82-82: Line too long (206 > 88)
(E501)
83-83: Line too long (138 > 88)
(E501)
95-95: Line too long (99 > 88)
(E501)
96-96: Line too long (101 > 88)
(E501)
97-97: Line too long (103 > 88)
(E501)
98-98: Line too long (140 > 88)
(E501)
103-103: Line too long (91 > 88)
(E501)
104-104: Line too long (200 > 88)
(E501)
105-105: Line too long (142 > 88)
(E501)
106-106: Line too long (165 > 88)
(E501)
107-107: Line too long (143 > 88)
(E501)
108-108: Line too long (206 > 88)
(E501)
109-109: Line too long (149 > 88)
(E501)
apiserver/plane/db/models/__init__.py (1)
81-81: Import of Sticky model is correctly added
The Sticky model is properly imported, making it available throughout the application.
apiserver/plane/db/models/user.py (1)
189-192: Verify relationship with existing onboarding fields.
The new is_mobile_onboarded field seems to overlap with the existing is_onboarded field. Consider:
- Documenting the distinction between these fields
- Whether these could be combined into a single field with an enum
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py (2)
60-86: Consider adding constraints to Sticky model fields.The model structure is good, but consider these improvements:
- Add max_length to TextField
nameto prevent excessive long names- Add validation for color fields to ensure valid color formats
- ('name', models.TextField()), + ('name', models.TextField(max_length=1000)), # adjust limit as needed - ('color', models.CharField(blank=True, max_length=255, null=True)), + ('color', models.CharField(blank=True, max_length=255, null=True, validators=[RegexValidator(r'^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$')])),🧰 Tools
🪛 Ruff (0.8.2)
63-63: Line too long (99 > 88)
(E501)
64-64: Line too long (101 > 88)
(E501)
65-65: Line too long (103 > 88)
(E501)
66-66: Line too long (140 > 88)
(E501)
74-74: Line too long (94 > 88)
(E501)
75-75: Line too long (200 > 88)
(E501)
76-76: Line too long (144 > 88)
(E501)
77-77: Line too long (206 > 88)
(E501)
78-78: Line too long (138 > 88)
(E501)
87-111: Consider adding an index for efficient version querying.The IssueDescriptionVersion model looks good, but consider adding an index on
last_saved_atfor better query performance when retrieving version history.('last_saved_at', models.DateTimeField(default=django.utils.timezone.now)), + models.Index(fields=['last_saved_at'], name='idx_issue_desc_version_last_saved'),🧰 Tools
🪛 Ruff (0.8.2)
90-90: Line too long (99 > 88)
(E501)
91-91: Line too long (101 > 88)
(E501)
92-92: Line too long (103 > 88)
(E501)
93-93: Line too long (140 > 88)
(E501)
98-98: Line too long (91 > 88)
(E501)
99-99: Line too long (200 > 88)
(E501)
100-100: Line too long (142 > 88)
(E501)
101-101: Line too long (165 > 88)
(E501)
102-102: Line too long (143 > 88)
(E501)
103-103: Line too long (206 > 88)
(E501)
104-104: Line too long (149 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py(1 hunks)apiserver/plane/db/models/issue.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/db/models/issue.py
18-18: .base.BaseModel imported but unused
Remove unused import: .base.BaseModel
(F401)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py
38-38: Line too long (141 > 88)
(E501)
58-58: Line too long (141 > 88)
(E501)
63-63: Line too long (99 > 88)
(E501)
64-64: Line too long (101 > 88)
(E501)
65-65: Line too long (103 > 88)
(E501)
66-66: Line too long (140 > 88)
(E501)
74-74: Line too long (94 > 88)
(E501)
75-75: Line too long (200 > 88)
(E501)
76-76: Line too long (144 > 88)
(E501)
77-77: Line too long (206 > 88)
(E501)
78-78: Line too long (138 > 88)
(E501)
90-90: Line too long (99 > 88)
(E501)
91-91: Line too long (101 > 88)
(E501)
92-92: Line too long (103 > 88)
(E501)
93-93: Line too long (140 > 88)
(E501)
98-98: Line too long (91 > 88)
(E501)
99-99: Line too long (200 > 88)
(E501)
100-100: Line too long (142 > 88)
(E501)
101-101: Line too long (165 > 88)
(E501)
102-102: Line too long (143 > 88)
(E501)
103-103: Line too long (206 > 88)
(E501)
104-104: Line too long (149 > 88)
(E501)
🔇 Additional comments (3)
apiserver/plane/db/migrations/0087_remove_issueversion_description_and_more.py (2)
19-39: LGTM! Migration operations for IssueVersion model are well-structured.
The removal of description fields and addition of the activity field follows Django's migration best practices.
🧰 Tools
🪛 Ruff (0.8.2)
38-38: Line too long (141 > 88)
(E501)
40-54: Verify the behavior of get_mobile_default_onboarding function.
While the mobile onboarding fields are well-structured, ensure that the get_mobile_default_onboarding function:
- Returns a consistent default value
- Handles any potential exceptions
- Is lightweight as it will be called for each new profile
✅ Verification successful
Function implementation verified - safe and consistent
The get_mobile_default_onboarding function is well-implemented:
- Returns a consistent, static dictionary with boolean flags
- No exception paths or complex logic that could fail
- Lightweight implementation with simple dictionary construction
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find the implementation of get_mobile_default_onboarding
ast-grep --pattern 'def get_mobile_default_onboarding():
$$$'
Length of output: 446
apiserver/plane/db/models/issue.py (1)
18-18: Remove unused import.
The BaseModel import is not used in this file.
🧰 Tools
🪛 Ruff (0.8.2)
18-18: .base.BaseModel imported but unused
Remove unused import: .base.BaseModel
(F401)
Description
issue_versionandprofiletables to enhance data handling and support additional features.stickytable for managing specific data requirements.Type of Change
Summary by CodeRabbit
New Features
StickyandIssueDescriptionVersion.profilemodel to enhance the mobile onboarding experience.Bug Fixes
issueversionmodel to improve data integrity.Documentation