[WEB-3038]feat: home preferences #6308
Conversation
WalkthroughThis pull request introduces a feature for managing workspace home preferences in the Plane application. It includes the addition of a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
apiserver/plane/app/views/workspace/preference.py (3)
3-5: Remove duplicate import to avoid confusion.Line 5 re-imports
WorkspaceHomePreference, already imported on line 3. Removing the redundant import avoids redefinition warnings from linters and clarifies code usage.-from plane.db.models.workspace import WorkspaceHomePreference🧰 Tools
🪛 Ruff (0.8.2)
5-5: Redefinition of unused
WorkspaceHomePreferencefrom line 3Remove definition:
WorkspaceHomePreference(F811)
20-24: Optimize multiple database calls.Performing multiple
.exists()calls can be consolidated with a single query (e.g., filtering bykey__in=[...]) to reduce repetitive lookups and improve performance.🧰 Tools
🪛 Ruff (0.8.2)
20-20: Line too long (171 > 88)
(E501)
21-21: Line too long (163 > 88)
(E501)
22-22: Line too long (171 > 88)
(E501)
23-23: Line too long (173 > 88)
(E501)
24-24: Line too long (177 > 88)
(E501)
20-40: Address line length issues from static analysis.Refactor or wrap code across multiple lines to comply with the recommended limits. This also improves readability.
🧰 Tools
🪛 Ruff (0.8.2)
20-20: Line too long (171 > 88)
(E501)
21-21: Line too long (163 > 88)
(E501)
22-22: Line too long (171 > 88)
(E501)
23-23: Line too long (173 > 88)
(E501)
24-24: Line too long (177 > 88)
(E501)
27-27: Line too long (210 > 88)
(E501)
38-38: Line too long (136 > 88)
(E501)
apiserver/plane/db/models/page.py (1)
23-23: Switching to TextField.Removing a
max_lengthconstraint provides flexibility but check if large text inputs fornamealign with UI and search requirements.apiserver/plane/db/models/workspace.py (1)
347-385: Model addition looks good, but consider default widget ordering.The
WorkspaceHomePreferencemodel and its associated fields appear consistent with the rest of the code. However, the defaultsort_orderof65535might limit future expansions (e.g., a negative or ascending range). If needed, consider a more flexible approach or confirm that 65535 ordering is sufficient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apiserver/plane/app/serializers/workspace.py(1 hunks)apiserver/plane/app/urls/workspace.py(2 hunks)apiserver/plane/app/views/__init__.py(1 hunks)apiserver/plane/app/views/workspace/preference.py(1 hunks)apiserver/plane/db/migrations/0089_workspacehomepreference_and_more.py(1 hunks)apiserver/plane/db/migrations/0090_workspacehomepreference_sort_order.py(1 hunks)apiserver/plane/db/models/__init__.py(1 hunks)apiserver/plane/db/models/page.py(1 hunks)apiserver/plane/db/models/workspace.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/workspace/preference.py
5-5: Redefinition of unused WorkspaceHomePreference from line 3
Remove definition: WorkspaceHomePreference
(F811)
20-20: Line too long (171 > 88)
(E501)
21-21: Line too long (163 > 88)
(E501)
22-22: Line too long (171 > 88)
(E501)
23-23: Line too long (173 > 88)
(E501)
24-24: Line too long (177 > 88)
(E501)
27-27: Line too long (210 > 88)
(E501)
38-38: Line too long (136 > 88)
(E501)
apiserver/plane/app/serializers/workspace.py
15-15: plane.db.models.WorkspaceHomePreference imported but unused
Remove unused import: plane.db.models.WorkspaceHomePreference
(F401)
apiserver/plane/db/migrations/0090_workspacehomepreference_sort_order.py
3-3: django.db.models imported but unused
Remove unused import: django.db.models
(F401)
🔇 Additional comments (11)
apiserver/plane/db/migrations/0090_workspacehomepreference_sort_order.py (1)
1-2: Verify migration timestampThe migration is dated January 2, 2025. Since this is a future date, please ensure this won't cause issues with other migrations in the deployment pipeline.
✅ Verification successful
Migration timestamp is consistent with the repository's timeline
Looking at the migration history, this migration (0090) follows a clear chronological sequence with the previous migrations. The most recent migrations show:
- 0088: December 24, 2024
- 0089: January 2, 2025 07:47
- 0090: January 2, 2025 08:25 (current migration)
The timestamp is consistent with the repository's timeline and follows the correct sequential order. The migration timestamps are properly incremental, and there are no conflicting timestamps that would cause deployment issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any migrations with conflicting or future dates find . -type f -name "*.py" -path "*/migrations/*" -exec grep -l "Generated by Django" {} \; | while read -r file; do echo "=== $file ===" grep "Generated by Django" "$file" doneLength of output: 31639
apiserver/plane/db/migrations/0089_workspacehomepreference_and_more.py (4)
16-92: Good introduction of a separate model and constraints.The creation of
WorkspaceHomePreferencewith explicit constraints (both unique and unique-together) ensures data integrity, especially withdeleted_at__isnullin place for soft deletion. This approach is consistent with Django best practices.
93-103: Verify uniqueness constraints.When using both
UniqueConstraint(withdeleted_at__isnull) andunique_together={"workspace", "user", "key", "deleted_at"}, ensure you have tested scenarios where records are soft-deleted but need re-creation for the same(workspace, user, key). This could unintentionally trigger constraint violations if those records are not fully purged.
105-109: Confirm “text” field alignment with existing usage.Converting the
Pagemodel'snamefield to a text-based field may affect the structure of related queries or indexes. If frequent indexing or text-based lookups are used, confirm that performance is acceptable.
110-114: Sort order default usage.The default of 65535 is a common pattern for “lowest priority” or “end-of-list” positioning. Confirm whether this satisfies your sorting requirements long-term.
apiserver/plane/db/models/__init__.py (1)
72-72: Successful addition of the WorkspaceHomePreference import.This maintains consistency with new migrations. No issues detected.
apiserver/plane/app/views/workspace/preference.py (1)
16-18: Handle possible DoesNotExist exception.
Workspace.objects.get(slug=slug, owner=request.user)can raiseWorkspace.DoesNotExist. Consider graceful error handling (e.g., 404 response).apiserver/plane/app/views/__init__.py (1)
44-44: WorkspacePreferenceViewSet import looks correct.No concerns. Integrates well with the new feature set.
apiserver/plane/app/urls/workspace.py (2)
229-233: Path naming is consistent and clear.The new route for workspace home preferences aligns well with the existing naming convention (e.g.,
quick-links). This should help maintain clarity and consistency across the workspace endpoints.
30-31: Confirm the necessity of both imports.It appears both the
QuickLinkViewSetandWorkspacePreferenceViewSetare being imported. Verify that they're both used and that no extraneous imports remain.✅ Verification successful
Both imports are necessary and actively used in the workspace URLs
The verification confirms that both imports are required:
QuickLinkViewSetis used for list/create operations and retrieve/update/delete operations in the workspace URLsWorkspacePreferenceViewSetis used for workspace preference operationsBoth ViewSets are properly defined in their respective modules and are actively referenced in the URL patterns, making these imports necessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check usage of WorkspacePreferenceViewSet and QuickLinkViewSet to confirm they are referenced. rg "QuickLinkViewSet" rg "WorkspacePreferenceViewSet"Length of output: 888
apiserver/plane/db/models/workspace.py (1)
344-345: No issues with the updated__str__method.Confirm that the updated return format meets the logging or debugging requirements. It currently returns
<workspace_id> <url>.
apiserver/plane/db/migrations/0090_workspacehomepreference_sort_order.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apiserver/plane/app/views/workspace/preference.py (2)
31-43: Improve bulk creation robustness.The sort order calculation
(index + 1) * 1000could potentially overflow for large indices. Consider using a smaller multiplier or implementing a more robust ordering strategy.- sort_order=(index + 1) * 1000, + sort_order=(index + 1) * 100, # Smaller multiplierAlso, consider adding a maximum limit to prevent potential abuse:
+ if len(create_preference_keys) > 50: # Arbitrary limit + return Response( + {"error": "Too many preferences to create"}, + status=status.HTTP_400_BAD_REQUEST + )
13-49: Add docstring for better documentation.Consider adding a docstring to document the view's purpose, parameters, and response format.
class WorkspacePreferenceViewSet(BaseAPIView): + """ + API endpoint for managing workspace home preferences. + + Retrieves existing preferences for the authenticated user in the specified workspace. + Creates default preferences if they don't exist. + + Returns: + Response: List of preferences with their keys, enabled status, sort order, and config. + """ model = WorkspaceHomePreference
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apiserver/plane/app/serializers/workspace.py(4 hunks)apiserver/plane/app/urls/workspace.py(2 hunks)apiserver/plane/app/views/__init__.py(1 hunks)apiserver/plane/app/views/workspace/preference.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apiserver/plane/app/serializers/workspace.py
- apiserver/plane/app/views/init.py
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/workspace/preference.py
5-5: Redefinition of unused WorkspaceHomePreference from line 3
Remove definition: WorkspaceHomePreference
(F811)
🔇 Additional comments (2)
apiserver/plane/app/urls/workspace.py (2)
223-235: LGTM! URL patterns are well-structured.The new endpoints for quick links and home preferences follow the established URL pattern structure and naming conventions.
Line range hint
31-241: Verify URL pattern consistency across the codebase.Let's ensure the new endpoints maintain consistency with existing patterns.
✅ Verification successful
Let me gather more information to verify the URL patterns comprehensively.
Let me check one more specific pattern to complete the verification.
Based on the verification results, I can now provide the final response:
URL patterns are consistent with established conventions
The verification shows that:
- The workspace URLs consistently use
<str:slug>for workspace identification, with only one exception in the license module which is a different context (InstanceWorkSpaceEndpoint).- Resource identifiers consistently use
<uuid:pk>for specific resource identification.- The URL structure in the provided code follows the same pattern as other endpoints in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check URL pattern consistency for workspace endpoints # Check if all workspace URLs use slug parameter echo "Checking workspace URL patterns..." rg "path\(.*workspaces/.*" | grep -v "<str:slug>" # Check if all specific resource identifiers use UUID echo "Checking UUID usage in URLs..." rg "path\(.*/<\w+:pk>/" | grep -v "<uuid:pk>"Length of output: 370
Script:
#!/bin/bash # Check all URL patterns in the workspace-related files echo "=== Checking all workspace URL patterns ===" fd "urls.py" | xargs rg "path\(" -A 1 # Check specific identifiers in URL patterns echo "=== Checking identifier patterns ===" fd "urls.py" | xargs rg "<\w+:\w+>" # Check workspace viewsets to ensure consistent parameter naming echo "=== Checking workspace viewset parameter naming ===" ast-grep --pattern 'class $_WorkSpace$_ViewSet'Length of output: 7231
Script:
#!/bin/bash # Check workspace-related views and their parameter naming echo "=== Checking workspace view implementations ===" ast-grep --pattern 'class $_(Workspace|WorkSpace)$_' # Check URL patterns specifically in app/urls echo "=== Checking app URL patterns ===" fd "urls" -t f --full-path "app/urls" | xargs catLength of output: 380
* chore: cycles quick actions restructuring * chore: added additional actions to cycle list actions * chore: cycle quick action structure * chore: added additional actions to cycle list actions * chore: added end cycle hook * fix: updated end cycle export --------- Co-authored-by: gurusinath <[email protected]>
* fix: adding language support package * fix: language support implementation using mobx * fix: adding more languages for support * fix: profile settings translations * feat: added language support for sidebar and user settings * feat: added language support for deactivation modal * fix: added project sync after transfer issues (#6200) * code refactor and improvement (#6203) * chore: package code refactoring * chore: component restructuring and refactor * chore: comment create improvement * refactor: enhance workspace and project wrapper modularity (#6207) * [WEB-2678]feat: added functionality to add labels directly from dropdown (#6211) * enhancement:added functionality to add features directly from dropdown * fix: fixed import order * fix: fixed lint errors * chore: added common component for project activity (#6212) * chore: added common component for project activity * fix: added enum * fix: added enum for initiatives * - Do not clear temp files that are locked. (#6214) - Handle edge cases in sync workspace * fix: labels empty state for drop down (#6216) * refactor: remove cn helper function from the editor package (#6217) * * feat: added language support to issue create modal in sidebar * fix: project activity type * * fix: added missing translations * fix: modified translation for plurals * fix: fixed spanish translation * dev: language type error in space user profile types * fix: type fixes * chore: added alpha tag --------- Co-authored-by: sriram veeraghanta <[email protected]> Co-authored-by: Anmol Singh Bhatia <[email protected]> Co-authored-by: Prateek Shourya <[email protected]> Co-authored-by: Akshita Goyal <[email protected]> Co-authored-by: Satish Gandham <[email protected]> Co-authored-by: Aaryan Khandelwal <[email protected]> Co-authored-by: gurusinath <[email protected]>
* chore: re-order pages dropdown options * chore: re-order pages dropdown options
* Crud for wuick links * Validate quick link existence * Add custom method for destroy and retrieve * Add List method * Remove print statements * List all the workspace quick links * feat: endpoint to get recently active items * Resolve conflicts * Resolve conflicts * Add filter to only list required entities * Return required fields * Add filter * Add filter
sangeethailango
force-pushed
the
feat-home-widgets
branch
from
f511168 to
1b4575e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apiserver/plane/app/views/workspace/preference.py (2)
31-43: Handle potential race condition in bulk create operation.While
ignore_conflicts=Truehelps, there's still a potential race condition between checking existence and creation. Consider usingget_or_createin a transaction.- WorkspaceHomePreference.objects.bulk_create( - [ - WorkspaceHomePreference( - key=key, - user=request.user, - workspace=workspace, - sort_order=(index + 1) * 1000, - ) - for index, key in enumerate(create_preference_keys) - ], - batch_size=10, - ignore_conflicts=True, - ) + from django.db import transaction + with transaction.atomic(): + for index, key in enumerate(create_preference_keys): + WorkspaceHomePreference.objects.get_or_create( + key=key, + user=request.user, + workspace=workspace, + defaults={'sort_order': (index + 1) * 1000} + )
15-50: Add method documentation.Add a docstring to describe the method's purpose, parameters, and return value.
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE") def get(self, request, slug): + """ + Retrieve or initialize workspace home preferences for the current user. + + Args: + request: The HTTP request object + slug: The workspace slug + + Returns: + Response containing user's workspace preferences + """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apiserver/plane/app/serializers/workspace.py(4 hunks)apiserver/plane/app/urls/workspace.py(2 hunks)apiserver/plane/app/views/__init__.py(1 hunks)apiserver/plane/app/views/workspace/preference.py(1 hunks)apiserver/plane/db/models/__init__.py(1 hunks)apiserver/plane/db/models/workspace.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apiserver/plane/db/models/workspace.py
- apiserver/plane/db/models/init.py
- apiserver/plane/app/views/init.py
- apiserver/plane/app/urls/workspace.py
- apiserver/plane/app/serializers/workspace.py
🔇 Additional comments (2)
apiserver/plane/app/views/workspace/preference.py (2)
12-14: LGTM! Clean class definition following Django REST framework patterns.The class is properly structured and inherits from BaseAPIView with the correct model assignment.
16-17:⚠️ Potential issueAdd error handling for workspace retrieval.
Use
get_object_or_404to handle non-existent workspaces gracefully.- workspace = Workspace.objects.get(slug=slug) + from django.shortcuts import get_object_or_404 + workspace = get_object_or_404(Workspace, slug=slug)Likely invalid or redundant comment.
sangeethailango
force-pushed
the
feat-home-widgets
branch
from
7a2577a to
30434b4
Compare
sangeethailango
force-pushed
the
feat-home-widgets
branch
from
739e07a to
901164b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apiserver/plane/app/serializers/__init__.py(1 hunks)apiserver/plane/app/serializers/workspace.py(5 hunks)apiserver/plane/app/views/workspace/preference.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apiserver/plane/app/serializers/workspace.py
- apiserver/plane/app/views/workspace/preference.py
🔇 Additional comments (1)
apiserver/plane/app/serializers/__init__.py (1)
23-23: No concerns with this import statement.This line is consistent with the existing serializer import structure and introduces no further issues.
only insert missing keys use serializer to return data
Remove random generation of sort_order
Description
This PR will provide endpoints to create/patch widgets.
Type of Change
Summary by CodeRabbit
New Features
Improvements