[WEB-5237] feat: add workspace invitation and project member management endpoints #8059
Conversation
- Introduced `WorkspaceInviteSerializer` for handling workspace invites. - Added `WorkspaceInvitationsViewset` to manage invites (list, create, retrieve, update, delete). - Created `ProjectMemberSerializer` for managing project members. - Updated `ProjectMemberAPIEndpoint` to support creating and updating project members with appropriate permissions. - Enhanced URL routing to include new invite and member management paths.
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds workspace invitation serializers, views, and routes; splits project-member endpoints into list/create and detail; introduces a BaseViewSet with enhanced error handling and helpers; implements a permission system (ROLE enum, allow_permission decorator, workspace/project/page permissions); and re-exports new serializers, viewset, and permissions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as WorkspaceInvitationsViewset
participant Perm as WorkspaceOwnerPermission
participant Serializer as WorkspaceInviteSerializer
participant DB as Database
rect rgb(245,250,255)
note over Client,API: Create invitation (POST /workspaces/{slug}/invitations/)
Client->>API: POST { email, role }
API->>Perm: has_permission(request, view)
Perm-->>API: allowed / denied
alt allowed
API->>Serializer: validate_email(), validate_role(), validate()
Serializer->>DB: query WorkspaceMemberInvite by (workspace_slug, email)
alt exists
Serializer-->>API: ValidationError (EMAIL_ALREADY_INVITED)
API-->>Client: 400 Bad Request
else not exists
API->>DB: create WorkspaceMemberInvite(created_by=request.user, workspace=slug)
DB-->>API: created invite
API-->>Client: 201 Created
end
else denied
API-->>Client: 403 Forbidden
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Pull Request Overview
This PR adds functionality for managing project members and workspace invitations through new API endpoints. The main focus is enabling CRUD operations for project members with appropriate permission controls and creating a workspace invitation management system.
- Added
ProjectAdminPermissionclass to restrict certain operations to project admins - Created new POST, PATCH, and DELETE endpoints for managing project members
- Implemented workspace invitation viewset with full CRUD operations
- Extended guest role permissions to allow guests to perform certain issue operations
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/plane/app/permissions/project.py | Added ProjectAdminPermission class for admin-only operations |
| apps/api/plane/app/permissions/init.py | Exported new ProjectAdminPermission class |
| apps/api/plane/api/views/member.py | Added POST, PATCH, DELETE methods to ProjectMemberAPIEndpoint with dynamic permissions |
| apps/api/plane/api/views/issue.py | Extended allowed roles to include GUEST for issue operations |
| apps/api/plane/api/views/invite.py | New viewset for workspace invitation management |
| apps/api/plane/api/views/base.py | Added BaseViewSet class with common functionality and error handling |
| apps/api/plane/api/views/init.py | Exported new WorkspaceInvitationsViewset |
| apps/api/plane/api/urls/member.py | Added URL patterns for project member POST, PATCH, DELETE operations |
| apps/api/plane/api/urls/invite.py | New URL patterns for workspace invitations |
| apps/api/plane/api/urls/init.py | Included invitation URL patterns |
| apps/api/plane/api/serializers/member.py | New serializer for project member operations |
| apps/api/plane/api/serializers/invite.py | New serializer for workspace invitations |
| apps/api/plane/api/serializers/init.py | Exported new serializers |
Comments suppressed due to low confidence (1)
apps/api/plane/api/views/member.py:2
- Import of 'c' is not used.
from calendar import c
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/plane/api/views/issue.py (1)
1963-1969: Fix incorrect parameter type in permission checks.Lines 1964, 2037, and 2102 pass
request.user(User object) touser_has_issue_permission(), but the function signature at line 146 expectsuser_id(integer) as the first parameter. This causes incorrect behavior:
- Line 147:
user_id == issue.created_by_idwill always return False when comparing a User object to an integer- Line 152:
member_id=user_idfilter will fail or return incorrect resultsLine 1807 demonstrates the correct usage:
user_has_issue_permission(request.user.id, ...).Apply this diff to fix the issue:
if not user_has_issue_permission( - request.user, + request.user.id, project_id=project_id, issue=issue, allowed_roles=[ROLE.ADMIN.value, ROLE.MEMBER.value, ROLE.GUEST.value], allow_creator=True, ):This same fix should be applied at lines 1964, 2037, and 2102.
Also applies to: 2036-2042, 2101-2107
apps/api/plane/api/views/base.py (1)
114-124: Critical bug: returning exception object instead of response.Line 124 returns
exc(the exception object) instead ofresponse(the error response fromhandle_exception). This will cause the API to fail when exceptions occur, as Django expects a Response object, not an exception.Apply this diff:
def dispatch(self, request, *args, **kwargs): try: response = super().dispatch(request, *args, **kwargs) if settings.DEBUG: from django.db import connection print(f"{request.method} - {request.get_full_path()} of Queries: {len(connection.queries)}") return response except Exception as exc: response = self.handle_exception(exc) - return exc + return response
🧹 Nitpick comments (5)
apps/api/plane/api/serializers/invite.py (1)
49-56: Consider making uniqueness validation more defensive.The validation correctly prevents duplicate email invitations within a workspace. However, it doesn't exclude the current instance during updates, relying instead on the viewset's
partial_updatemethod to block email changes. While this works given the current implementation, the serializer could be more defensive by checking if it's an update operation and excludingself.instancefrom the uniqueness check.Example improvement:
def validate(self, data): slug = self.context["slug"] + queryset = WorkspaceMemberInvite.objects.filter(email=data["email"], workspace__slug=slug) + if self.instance: + queryset = queryset.exclude(pk=self.instance.pk) - if ( - data.get("email") - and WorkspaceMemberInvite.objects.filter(email=data["email"], workspace__slug=slug).exists() - ): + if data.get("email") and queryset.exists(): raise serializers.ValidationError("Email already invited", code="EMAIL_ALREADY_INVITED") return dataapps/api/plane/api/views/member.py (2)
166-171: Consider more explicit error handling for object retrieval.The
ProjectMember.objects.get()call will raiseDoesNotExistif the member isn't found, which is caught byhandle_exception. While this works, usingget_object_or_404or a try/except block would make the error handling more explicit and improve code clarity.Example:
+from django.shortcuts import get_object_or_404 + def patch(self, request, slug, project_id, pk): - project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + project_member = get_object_or_404(ProjectMember, project_id=project_id, workspace__slug=slug, pk=pk) serializer = ProjectMemberSerializer(project_member, data=request.data, partial=True, context={"slug": slug}) serializer.is_valid(raise_exception=True) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK)
181-185: Consider more explicit error handling (same as PATCH method).Like the
patchmethod, this usesProjectMember.objects.get()without explicit error handling. The soft-delete approach (settingis_active=False) is good for preserving history.Apply the same improvement as suggested for the patch method:
def delete(self, request, slug, project_id, pk): - project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + project_member = get_object_or_404(ProjectMember, project_id=project_id, workspace__slug=slug, pk=pk) project_member.is_active = False project_member.save() return Response(status=status.HTTP_204_NO_CONTENT)apps/api/plane/api/views/base.py (1)
165-277: Consider reducing code duplication between BaseAPIView and BaseViewSet.There's significant duplication of the
handle_exception,dispatch, and property methods (workspace_slug,project_id,fields,expand) betweenBaseAPIViewandBaseViewSet. While some duplication is expected due to different base classes (GenericAPIView vs ModelViewSet), the common logic could potentially be extracted into a shared mixin.Consider creating a shared mixin for common functionality:
class CommonAPIMethodsMixin: @property def workspace_slug(self): return self.kwargs.get("slug", None) @property def project_id(self): project_id = self.kwargs.get("project_id", None) if project_id: return project_id if resolve(self.request.path_info).url_name == "project": return self.kwargs.get("pk", None) @property def fields(self): fields = [field for field in self.request.GET.get("fields", "").split(",") if field] return fields if fields else None @property def expand(self): expand = [expand for expand in self.request.GET.get("expand", "").split(",") if expand] return expand if expand else NoneThen both base classes could inherit from this mixin.
apps/api/plane/api/views/invite.py (1)
85-90: Consider more explicit error handling for workspace retrieval.While
WorkspaceOwnerPermissionshould ensure the workspace exists, usingget_object_or_404or explicit error handling would make the code more defensive and clearer.+from django.shortcuts import get_object_or_404 + def create(self, request, slug): - workspace = Workspace.objects.get(slug=slug) + workspace = get_object_or_404(Workspace, slug=slug) serializer = WorkspaceInviteSerializer(data=request.data, context={"slug": slug}) serializer.is_valid(raise_exception=True) serializer.save(workspace=workspace, created_by=request.user) return Response(serializer.data, status=status.HTTP_201_CREATED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/api/plane/api/serializers/__init__.py(1 hunks)apps/api/plane/api/serializers/invite.py(1 hunks)apps/api/plane/api/serializers/member.py(1 hunks)apps/api/plane/api/urls/__init__.py(2 hunks)apps/api/plane/api/urls/invite.py(1 hunks)apps/api/plane/api/urls/member.py(1 hunks)apps/api/plane/api/views/__init__.py(1 hunks)apps/api/plane/api/views/base.py(2 hunks)apps/api/plane/api/views/invite.py(1 hunks)apps/api/plane/api/views/issue.py(3 hunks)apps/api/plane/api/views/member.py(3 hunks)apps/api/plane/app/permissions/__init__.py(1 hunks)apps/api/plane/app/permissions/project.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/__init__.pyapps/api/plane/api/serializers/member.pyapps/api/plane/api/serializers/invite.py
🧬 Code graph analysis (12)
apps/api/plane/api/serializers/__init__.py (2)
apps/api/plane/api/serializers/invite.py (1)
WorkspaceInviteSerializer(12-56)apps/api/plane/api/serializers/member.py (1)
ProjectMemberSerializer(10-35)
apps/api/plane/api/views/invite.py (3)
apps/api/plane/api/views/base.py (3)
BaseViewSet(165-277)get_queryset(174-179)filter_queryset(55-58)apps/api/plane/db/models/workspace.py (2)
WorkspaceMemberInvite(227-251)Workspace(115-178)apps/api/plane/api/serializers/invite.py (1)
WorkspaceInviteSerializer(12-56)
apps/api/plane/api/serializers/member.py (4)
apps/api/plane/db/models/project.py (1)
ProjectMember(192-236)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(194-224)apps/api/plane/api/serializers/base.py (1)
BaseSerializer(5-114)apps/api/plane/db/models/user.py (1)
User(38-164)
apps/api/plane/api/urls/invite.py (1)
apps/api/plane/api/views/invite.py (1)
WorkspaceInvitationsViewset(20-150)
apps/api/plane/api/serializers/invite.py (2)
apps/api/plane/db/models/workspace.py (1)
WorkspaceMemberInvite(227-251)apps/api/plane/api/serializers/base.py (1)
BaseSerializer(5-114)
apps/api/plane/api/views/__init__.py (1)
apps/api/plane/api/views/invite.py (1)
WorkspaceInvitationsViewset(20-150)
apps/api/plane/api/views/member.py (4)
apps/api/plane/api/serializers/member.py (1)
ProjectMemberSerializer(10-35)apps/api/plane/db/models/workspace.py (3)
Workspace(115-178)WorkspaceMember(194-224)save(188-191)apps/api/plane/db/models/project.py (4)
ProjectMember(192-236)save(157-159)save(169-171)save(208-218)apps/api/plane/app/permissions/project.py (2)
ProjectMemberPermission(52-78)ProjectAdminPermission(115-126)
apps/api/plane/api/views/base.py (4)
apps/api/plane/utils/exception_logger.py (1)
log_exception(9-20)apps/api/plane/utils/paginator.py (1)
BasePaginator(631-729)apps/api/plane/utils/core/mixins/view.py (1)
ReadReplicaControlMixin(6-20)apps/api/plane/app/views/base.py (6)
BaseViewSet(44-142)TimezoneMixin(30-41)handle_exception(66-105)handle_exception(163-200)fields(135-137)fields(225-227)
apps/api/plane/app/permissions/project.py (2)
apps/api/plane/db/models/project.py (2)
ProjectMember(192-236)ROLE(21-24)apps/api/plane/api/views/base.py (4)
workspace_slug(142-143)workspace_slug(257-258)project_id(146-152)project_id(261-267)
apps/api/plane/api/views/issue.py (1)
apps/api/plane/db/models/project.py (1)
ROLE(21-24)
apps/api/plane/app/permissions/__init__.py (1)
apps/api/plane/app/permissions/project.py (1)
ProjectAdminPermission(115-126)
apps/api/plane/api/urls/member.py (1)
apps/api/plane/api/views/member.py (1)
ProjectMemberAPIEndpoint(92-185)
⏰ 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: CodeQL analysis (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (17)
apps/api/plane/api/views/__init__.py (1)
56-57: LGTM!The import correctly exposes
WorkspaceInvitationsViewsetfor use in URL routing.apps/api/plane/api/urls/__init__.py (1)
11-24: LGTM!The invite patterns are correctly imported and included in the URL configuration, following the established pattern used for other URL modules.
apps/api/plane/api/serializers/__init__.py (1)
56-57: LGTM!The new serializers are correctly exported, making them accessible as part of the package's public API.
apps/api/plane/app/permissions/project.py (1)
115-127: LGTM!The
ProjectAdminPermissionclass correctly enforces admin-level authorization by verifying that the user is an active project admin. The implementation follows the established patterns in the codebase.apps/api/plane/app/permissions/__init__.py (1)
14-14: LGTM!The
ProjectAdminPermissionis correctly exported, making it available for use in views that require admin-level project access control.apps/api/plane/api/serializers/invite.py (2)
37-42: LGTM - Email validation is correctly implemented.The validation uses Django's built-in validator and properly converts exceptions to DRF's ValidationError format with an appropriate error code.
44-47: LGTM - Role validation correctly restricts to valid workspace roles.The validation ensures only permitted roles (ADMIN, MEMBER, GUEST) can be assigned, with a clear error code for invalid values.
apps/api/plane/api/views/member.py (2)
96-99: LGTM - Dynamic permission handling is well-designed.The method appropriately restricts mutating operations (POST, PATCH, DELETE) to project admins while allowing any project member to list members via GET requests.
151-155: LGTM - Project member creation follows standard patterns.The implementation correctly validates the member's workspace membership via the serializer and associates the member with the project.
apps/api/plane/api/views/base.py (3)
165-179: LGTM - BaseViewSet structure is well-designed.The class correctly extends the necessary mixins and base classes for a full-featured viewset with timezone handling, read replica control, pagination, and standard ModelViewSet capabilities.
181-240: LGTM - Exception handling with structured logging is well-implemented.The error handling provides appropriate HTTP status codes for different exception types and includes structured logging with error codes and messages. The use of different log levels (warning vs error) appropriately categorizes the severity of different error types.
242-254: LGTM - dispatch correctly returns response (unlike BaseAPIView).The exception handling in this dispatch method correctly returns the
responsefromhandle_exception, unlike the bug inBaseAPIView.dispatchat line 124.apps/api/plane/api/views/invite.py (5)
20-36: LGTM - Queryset filtering correctly scopes invitations to workspace.The
get_querysetmethod properly filters invitations by workspace slug and applies any configured filters. Theget_objectmethod retrieves individual invitations from this scoped queryset.
51-54: LGTM - Standard list implementation.The method correctly retrieves and serializes workspace invitations using the filtered queryset.
71-74: LGTM - Standard retrieve implementation.The method correctly retrieves and serializes a single workspace invitation.
108-120: LGTM - Excellent immutability enforcement for email field.The method correctly prevents email changes after invite creation, which is the appropriate business logic. Allowing partial updates of other fields (like role) while blocking email changes is well-designed.
137-150: LGTM - Excellent validation prevents data integrity issues.The method correctly prevents deletion of invitations that have been accepted or responded to, maintaining audit trail and preventing data loss. The business logic validation is well-designed.
- Replaced manual URL paths for workspace invitations with a DefaultRouter for cleaner routing. - Updated imports for better organization and clarity.
…utilities - Updated import paths for `WorkspaceOwnerPermission`, `ProjectMemberPermission`, and others to improve code organization. - Added new permission utility classes in `utils/permissions` for better role management and access control. - Implemented `allow_permission` decorator for streamlined permission checks across views.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/plane/api/views/member.py (1)
133-138: Soft-deleted members still leak into the GET response
delete()only flipsis_activetoFalse, but the GET endpoint keeps returning everyProjectMemberwithout filtering byis_active. As soon as someone is “removed”, their record continues to appear in the list, so clients still see members who should have been deleted. Please filter the queryset byis_active=Truebefore serializing.- project_members = ProjectMember.objects.filter(project_id=project_id, workspace__slug=slug).values_list( + project_members = ProjectMember.objects.filter( + project_id=project_id, + workspace__slug=slug, + is_active=True, + ).values_list( "member_id", flat=True )
🧹 Nitpick comments (1)
apps/api/plane/utils/permissions/page.py (1)
113-121: Rename misleading variable.The variable
project_member_existssuggests it's a boolean indicating membership, but it actually holds the result of the action access check (which is role-based permission validation).Apply this diff for clarity:
def _has_public_page_action_access(self, request, role): """ Check if the user has permission to access a public page and can perform operations on the page. """ - project_member_exists = self._check_project_action_access(request, role) - if not project_member_exists: + has_action_access = self._check_project_action_access(request, role) + if not has_action_access: return False return True
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/plane/api/views/invite.py(1 hunks)apps/api/plane/api/views/member.py(3 hunks)apps/api/plane/utils/permissions/__init__.py(1 hunks)apps/api/plane/utils/permissions/base.py(1 hunks)apps/api/plane/utils/permissions/page.py(1 hunks)apps/api/plane/utils/permissions/project.py(1 hunks)apps/api/plane/utils/permissions/workspace.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/utils/permissions/workspace.pyapps/api/plane/utils/permissions/page.py
🧬 Code graph analysis (7)
apps/api/plane/utils/permissions/base.py (2)
apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(194-224)apps/api/plane/db/models/project.py (1)
ProjectMember(192-236)
apps/api/plane/utils/permissions/workspace.py (1)
apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(194-224)
apps/api/plane/api/views/invite.py (5)
apps/api/plane/api/views/base.py (3)
BaseViewSet(165-277)get_queryset(174-179)filter_queryset(55-58)apps/api/plane/db/models/workspace.py (2)
WorkspaceMemberInvite(227-251)Workspace(115-178)apps/api/plane/api/serializers/invite.py (1)
WorkspaceInviteSerializer(12-56)apps/api/plane/utils/permissions/workspace.py (1)
WorkspaceOwnerPermission(47-54)apps/api/plane/app/views/workspace/invite.py (1)
WorkspaceInvitationsViewset(33-146)
apps/api/plane/utils/permissions/page.py (5)
apps/api/plane/db/models/project.py (1)
ProjectMember(192-236)apps/api/plane/db/models/page.py (1)
Page(19-73)apps/api/plane/utils/permissions/base.py (1)
ROLE(9-12)apps/api/plane/utils/permissions/project.py (5)
has_permission(10-49)has_permission(53-78)has_permission(82-112)has_permission(116-126)has_permission(130-139)apps/api/plane/app/views/page/base.py (1)
access(247-260)
apps/api/plane/utils/permissions/__init__.py (4)
apps/api/plane/utils/permissions/workspace.py (6)
WorkSpaceBasePermission(15-44)WorkspaceOwnerPermission(47-54)WorkSpaceAdminPermission(57-67)WorkspaceEntityPermission(70-86)WorkspaceViewerPermission(89-96)WorkspaceUserPermission(99-106)apps/api/plane/utils/permissions/project.py (5)
ProjectBasePermission(9-49)ProjectEntityPermission(81-112)ProjectMemberPermission(52-78)ProjectLitePermission(129-139)ProjectAdminPermission(115-126)apps/api/plane/utils/permissions/base.py (2)
allow_permission(15-73)ROLE(9-12)apps/api/plane/utils/permissions/page.py (1)
ProjectPagePermission(14-121)
apps/api/plane/utils/permissions/project.py (4)
apps/api/plane/db/models/project.py (1)
ProjectMember(192-236)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(194-224)apps/api/plane/utils/permissions/base.py (1)
ROLE(9-12)apps/api/plane/api/views/issue.py (1)
project_identifier(174-175)
apps/api/plane/api/views/member.py (4)
apps/api/plane/api/views/base.py (3)
BaseAPIView(48-162)project_id(146-152)project_id(261-267)apps/api/plane/api/serializers/member.py (1)
ProjectMemberSerializer(10-35)apps/api/plane/db/models/project.py (4)
ProjectMember(192-236)save(157-159)save(169-171)save(208-218)apps/api/plane/utils/permissions/project.py (2)
ProjectMemberPermission(52-78)ProjectAdminPermission(115-126)
⏰ 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 (3)
apps/api/plane/utils/permissions/project.py (2)
9-49: LGTM: Well-structured base permission logic.The
ProjectBasePermissionclass correctly implements a layered access control strategy:
- SAFE_METHODS require workspace membership
- POST requires workspace admin or member role
- Other methods require project admin OR (project member AND workspace admin)
This provides a solid foundation for project-level access control.
81-112: LGTM: Appropriate dual-path handling for project identification.The
ProjectEntityPermissionclass correctly handles two scenarios:
- Resources accessed via
project_identifier(lines 87-94)- Resources accessed via
project_id(lines 97-112)This flexibility supports different API endpoint patterns while maintaining consistent security checks.
apps/api/plane/utils/permissions/page.py (1)
1-2: Keep import as-is; review comment is based on incomplete analysis.The script reveals an import pattern across the codebase: all
project.pyfiles import ROLE fromplane.db.models.project, while allpage.pyfiles import fromplane.app.permissions. This naming-based split appears intentional, not an error. Although ROLE is defined identically in multiple locations (ADMIN=20, MEMBER=15, GUEST=5), the architectural separation by file type is consistent across ~40+ import statements in the codebase. The inconsistency flagged in the review exists but reflects an established design decision rather than a bug.
- Renamed `ProjectMemberAPIEndpoint` to `ProjectMemberListCreateAPIEndpoint` and introduced `ProjectMemberDetailAPIEndpoint` for better separation of concerns. - Updated URL routing to reflect the new endpoint structure, allowing for distinct handling of member listing, creation, and detail retrieval. - Enhanced the detail endpoint to support fetching and updating specific project members.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/api/plane/api/views/member.py (3)
2-2: Remove the unused import.This accidental import is not used anywhere in the file.
Apply this diff:
-from calendar import c
175-192: Fix response serializer and add error handling.Two issues here:
Response type mismatch: The OpenAPI schema (line 168) declares
ProjectMemberSerializeras the response, but the implementation returnsUserLiteSerializerdata. This loses the role information and doesn't match the API contract.Missing error handling: The
.get()calls on lines 189-190 will raise exceptions if the records don't exist, resulting in 500 errors instead of proper 404 responses.Apply this diff to fix both issues:
- # Get the workspace members that are present inside the workspace - project_members = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) - user = User.objects.get(id=project_members.member_id) - user = UserLiteSerializer(user).data - return Response(user, status=status.HTTP_200_OK) + try: + project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + except ProjectMember.DoesNotExist: + return Response( + {"error": "Project member not found"}, + status=status.HTTP_404_NOT_FOUND, + ) + + serializer = ProjectMemberSerializer(project_member) + return Response(serializer.data, status=status.HTTP_200_OK)
218-223: Add error handling for missing records.Both the PATCH and DELETE methods use
.get()without error handling (lines 219, 234). If the project member doesn't exist, this will raise an exception and return a 500 error instead of a proper 404 response.Apply this diff:
def patch(self, request, slug, project_id, pk): - project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + try: + project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + except ProjectMember.DoesNotExist: + return Response( + {"error": "Project member not found"}, + status=status.HTTP_404_NOT_FOUND, + ) serializer = ProjectMemberSerializer(project_member, data=request.data, partial=True, context={"slug": slug}) serializer.is_valid(raise_exception=True) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK) @extend_schema( operation_id="delete_project_member", summary="Delete project member", description="Delete a project member", tags=["Members"], parameters=[WORKSPACE_SLUG_PARAMETER, PROJECT_ID_PARAMETER], responses={204: OpenApiResponse(description="Project member deleted")}, ) def delete(self, request, slug, project_id, pk): - project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + try: + project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + except ProjectMember.DoesNotExist: + return Response( + {"error": "Project member not found"}, + status=status.HTTP_404_NOT_FOUND, + ) project_member.is_active = False project_member.save() return Response(status=status.HTTP_204_NO_CONTENT)Also applies to: 233-237
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/plane/api/urls/member.py(1 hunks)apps/api/plane/api/views/__init__.py(2 hunks)apps/api/plane/api/views/member.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/api/views/init.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (2)
apps/api/plane/api/urls/member.py (1)
apps/api/plane/api/views/member.py (3)
ProjectMemberListCreateAPIEndpoint(91-153)ProjectMemberDetailAPIEndpoint(157-237)WorkspaceMemberAPIEndpoint(28-88)
apps/api/plane/api/views/member.py (4)
apps/api/plane/api/views/base.py (3)
BaseAPIView(48-162)project_id(146-152)project_id(261-267)apps/api/plane/api/serializers/user.py (1)
UserLiteSerializer(9-34)apps/api/plane/api/serializers/member.py (1)
ProjectMemberSerializer(10-35)apps/api/plane/utils/permissions/project.py (2)
ProjectMemberPermission(52-78)ProjectAdminPermission(115-126)
🔇 Additional comments (3)
apps/api/plane/api/urls/member.py (1)
3-15: LGTM! Previous feedback addressed.The URL patterns now have unique names ("project-members" for list/create vs "project-member" for detail), and the GET method has been added to the detail endpoint as requested. The routing structure is clean and follows Django conventions.
apps/api/plane/api/views/member.py (2)
95-98: LGTM! Dynamic permission handling is correct.The
get_permissionsmethod appropriately restricts POST requests to project admins while allowing project members to list members. This is a standard DRF pattern for per-method permission control.
140-153: LGTM! Project member creation is properly validated.The POST endpoint correctly passes the workspace slug in the serializer context, which enables the
validate_membermethod to verify that the user is a workspace member before adding them to the project.
…ers and views - Updated `ProjectMemberSerializer` to include role validation, ensuring only valid roles are accepted. - Improved error handling for missing slug in member validation. - Refactored permission checks in `IssueAttachmentDetailAPIEndpoint` to use user ID instead of user object for consistency and clarity. - Cleaned up unused imports in `member.py` for better code organization.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
apps/api/plane/api/serializers/member.py (1)
25-26: Remove redundant None check.The
memberfield is defined withrequired=True(line 18), which already ensures the value cannot beNone. This explicit check is unnecessary.Apply this diff:
def validate_member(self, value): slug = self.context.get("slug") if not slug: raise serializers.ValidationError("Slug is required", code="INVALID_SLUG") - if not value: - raise serializers.ValidationError("Member is required", code="INVALID_MEMBER") if not WorkspaceMember.objects.filter(workspace__slug=slug, member=value).exists(): raise serializers.ValidationError("Member not found in workspace", code="INVALID_MEMBER") return valueapps/api/plane/api/views/member.py (5)
157-158: Remove unusedget_objectmethod.This method is never called by any view method (all use
ProjectMember.objects.get()directly). Additionally, it referencesself.get_queryset()which is not defined.Apply this diff:
class ProjectMemberDetailAPIEndpoint(ProjectMemberListCreateAPIEndpoint): - def get_object(self): - return self.get_queryset().get(pk=self.kwargs.get("pk")) -
188-190: Missing error handling and inefficient query pattern.Two issues:
ProjectMember.objects.get()andUser.objects.get()will raise exceptions if not found, resulting in 500 errors instead of proper 404 responses.- Two separate queries are inefficient when one query with
select_related("member")would suffice.Apply this diff:
- # Get the workspace members that are present inside the workspace - project_members = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) - user = User.objects.get(id=project_members.member_id) - user = UserLiteSerializer(user).data + # Get the project member with related user data + try: + project_member = ProjectMember.objects.select_related("member").get( + project_id=project_id, workspace__slug=slug, pk=pk + ) + except ProjectMember.DoesNotExist: + return Response( + {"error": "Project member not found"}, + status=status.HTTP_404_NOT_FOUND, + ) + user = UserLiteSerializer(project_member.member).data return Response(user, status=status.HTTP_200_OK)
193-206: Remove duplicate POST method.This POST method duplicates the parent class implementation (lines 139-152) and creates a duplicate OpenAPI
operation_id="create_project_member". POST operations should not be defined on detail endpoints.Apply this diff:
- @extend_schema( - operation_id="create_project_member", - summary="Create project member", - description="Create a new project member", - tags=["Members"], - parameters=[WORKSPACE_SLUG_PARAMETER, PROJECT_ID_PARAMETER], - responses={201: OpenApiResponse(description="Project member created", response=ProjectMemberSerializer)}, - request=OpenApiRequest(request=ProjectMemberSerializer), - ) - def post(self, request, slug, project_id): - serializer = ProjectMemberSerializer(data=request.data, context={"slug": slug}) - serializer.is_valid(raise_exception=True) - serializer.save(project_id=project_id) - return Response(serializer.data, status=status.HTTP_201_CREATED) -
218-218: Missing error handling forProjectMember.objects.get().If the project member doesn't exist, this will raise
ProjectMember.DoesNotExist, resulting in a 500 error instead of a proper 404 response.Apply this diff:
def patch(self, request, slug, project_id, pk): - project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + try: + project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + except ProjectMember.DoesNotExist: + return Response( + {"error": "Project member not found"}, + status=status.HTTP_404_NOT_FOUND, + ) serializer = ProjectMemberSerializer(project_member, data=request.data, partial=True, context={"slug": slug})
233-233: Missing error handling forProjectMember.objects.get().If the project member doesn't exist, this will raise
ProjectMember.DoesNotExist, resulting in a 500 error instead of a proper 204 response for an idempotent delete or 404 for clarity.Apply this diff:
def delete(self, request, slug, project_id, pk): - project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + try: + project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) + except ProjectMember.DoesNotExist: + return Response( + {"error": "Project member not found"}, + status=status.HTTP_404_NOT_FOUND, + ) project_member.is_active = False project_member.save()
🧹 Nitpick comments (1)
apps/api/plane/api/views/member.py (1)
91-91: Redundant permission_classes declaration.Line 91 sets
permission_classesbut is immediately overridden by theget_permissions()method (lines 94-97). The declaration on line 91 has no effect.Apply this diff:
class ProjectMemberListCreateAPIEndpoint(BaseAPIView): - permission_classes = [ProjectMemberPermission] use_read_replica = True
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/plane/api/serializers/member.py(1 hunks)apps/api/plane/api/views/issue.py(4 hunks)apps/api/plane/api/views/member.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/api/views/issue.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/member.py
🧬 Code graph analysis (2)
apps/api/plane/api/serializers/member.py (5)
apps/api/plane/db/models/project.py (1)
ProjectMember(192-236)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(194-224)apps/api/plane/db/models/user.py (1)
User(38-164)apps/api/plane/api/views/member.py (2)
get(65-87)get(117-137)apps/api/plane/api/serializers/invite.py (2)
validate_role(44-47)Meta(17-35)
apps/api/plane/api/views/member.py (6)
apps/api/plane/api/serializers/user.py (1)
UserLiteSerializer(9-34)apps/api/plane/api/serializers/member.py (1)
ProjectMemberSerializer(11-39)apps/api/plane/db/models/workspace.py (3)
Workspace(115-178)WorkspaceMember(194-224)save(188-191)apps/api/plane/db/models/project.py (4)
ProjectMember(192-236)save(157-159)save(169-171)save(208-218)apps/api/plane/utils/permissions/project.py (2)
ProjectMemberPermission(52-78)ProjectAdminPermission(115-126)apps/api/plane/utils/permissions/workspace.py (1)
WorkSpaceAdminPermission(57-67)
⏰ 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 (2)
apps/api/plane/api/serializers/member.py (1)
31-39: LGTM!The role validation correctly restricts values to valid ROLE constants, and the Meta configuration properly defines the serializer fields.
apps/api/plane/api/views/member.py (1)
139-152: LGTM!The POST implementation correctly uses
ProjectMemberSerializerwith workspace slug in context for validation, and passesproject_idtosave()for proper association.
…dpoint - Removed the `get_object` method from `ProjectMemberDetailAPIEndpoint` as it was redundant. - Deleted the `post` method for creating project members, streamlining the endpoint to focus on retrieval and updates only. - Updated the endpoint documentation to reflect the changes in functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/api/plane/api/views/member.py (2)
186-187: Verify error handling approach for .get() calls.While
BaseAPIView.handle_exception()automatically convertsDoesNotExistexceptions to 404 responses, explicit error handling would make the code clearer and allow custom error messages.Consider using
get_object_or_404()or try-except blocks:from django.shortcuts import get_object_or_404 # Option 1: Using get_object_or_404 project_member = get_object_or_404( ProjectMember, project_id=project_id, workspace__slug=slug, pk=pk ) user = get_object_or_404(User, id=project_member.member_id) # Option 2: Explicit try-except try: project_member = ProjectMember.objects.get( project_id=project_id, workspace__slug=slug, pk=pk ) user = User.objects.get(id=project_member.member_id) except ProjectMember.DoesNotExist: return Response( {"error": "Project member not found"}, status=status.HTTP_404_NOT_FOUND, ) except User.DoesNotExist: return Response( {"error": "User not found"}, status=status.HTTP_404_NOT_FOUND, )
165-165: Fix OpenAPI response schema mismatch.The schema declares
response=ProjectMemberSerializerbut the implementation returnsUserLiteSerializerdata (lines 186-189). This creates incorrect API documentation for consumers.Apply this diff:
- 200: OpenApiResponse(description="Project member", response=ProjectMemberSerializer), + 200: OpenApiResponse(description="Project member", response=UserLiteSerializer),
🧹 Nitpick comments (3)
apps/api/plane/api/views/member.py (3)
156-156: Reconsider inheritance structure.Inheriting from
ProjectMemberListCreateAPIEndpointcauses the detail endpoint to inherit thepost()method (lines 148-152), which is inappropriate for a detail endpoint that should only handle retrieve/update/delete operations.Consider either:
- Extracting shared logic to a base class and having both list-create and detail endpoints inherit from it
- Having
ProjectMemberDetailAPIEndpointinherit fromBaseAPIViewand duplicateget_permissions()
179-183: Consider extracting workspace validation.The workspace existence check is duplicated from the list GET method (lines 124-128) and missing from PATCH and DELETE methods. Consider extracting to a helper method for consistency.
Example:
def _validate_workspace(self, slug): """Validate workspace exists, return 400 if not.""" if not Workspace.objects.filter(slug=slug).exists(): return Response( {"error": "Provided workspace does not exist"}, status=status.HTTP_400_BAD_REQUEST, ) return None def get(self, request, slug, project_id, pk): error_response = self._validate_workspace(slug) if error_response: return error_response # ... rest of method
200-205: Add workspace validation for consistency.The PATCH method lacks the workspace existence check that's present in the GET method (lines 179-183). This creates inconsistent validation across endpoints.
Consider adding the same workspace check at the beginning of the method, or better yet, extract it to a helper method as suggested in the previous comment.
Also, the same error handling concerns from line 186-187 apply to line 201.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/api/views/member.py(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/views/member.py
🧬 Code graph analysis (1)
apps/api/plane/api/views/member.py (7)
apps/api/plane/api/views/base.py (3)
BaseAPIView(48-162)project_id(146-152)project_id(261-267)apps/api/plane/api/serializers/user.py (1)
UserLiteSerializer(9-34)apps/api/plane/api/serializers/member.py (1)
ProjectMemberSerializer(11-39)apps/api/plane/db/models/workspace.py (3)
Workspace(115-178)WorkspaceMember(194-224)save(188-191)apps/api/plane/db/models/project.py (4)
ProjectMember(192-236)save(157-159)save(169-171)save(208-218)apps/api/plane/utils/permissions/project.py (2)
ProjectMemberPermission(52-78)ProjectAdminPermission(115-126)apps/api/plane/utils/permissions/workspace.py (1)
WorkSpaceAdminPermission(57-67)
⏰ 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 (3)
apps/api/plane/api/views/member.py (3)
7-7: LGTM: Import additions are correct.The new imports (
OpenApiRequest,ProjectMemberSerializer,ProjectAdminPermission) are appropriately used throughout the file for request schema documentation, member serialization, and permission enforcement.Also applies to: 12-12, 14-14
94-97: LGTM: Proper method-level permission control.The dynamic permission selection correctly restricts write operations to project admins while allowing project members to view the member list.
139-152: LGTM: Project member creation is properly implemented.The endpoint correctly validates workspace membership via the serializer context and associates the new member with the specified project.
| project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk) | ||
| project_member.is_active = False | ||
| project_member.save() | ||
| return Response(status=status.HTTP_204_NO_CONTENT) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify soft delete implementation.
The code sets is_active = False but doesn't set the deleted_at field. Based on the ProjectMember model's unique constraint (condition=Q(deleted_at__isnull=True)), deleted_at is the primary soft-delete marker. Only setting is_active=False may prevent re-adding the same member later.
Consider setting deleted_at for proper soft delete:
from django.utils import timezone
def delete(self, request, slug, project_id, pk):
project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk)
project_member.is_active = False
project_member.deleted_at = timezone.now()
project_member.save()
return Response(status=status.HTTP_204_NO_CONTENT)Also, this method lacks the workspace validation present in the GET method (lines 179-183), and has the same error handling concerns as line 186.
I'll verify the concerns raised in this review comment by examining the ProjectMember model, checking soft delete patterns in the codebase, and validating the workspace validation issue.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find ProjectMember model definition
find . -type f -name "*.py" | xargs grep -l "class ProjectMember" | head -5
Length of output: 212
🏁 Script executed:
#!/bin/bash
# Search for deleted_at field in ProjectMember or related base classes
rg -nA 20 "class ProjectMember" --type=py | head -60Length of output: 4927
🏁 Script executed:
#!/bin/bash
# Check the member.py file to understand the GET method and error handling
rg -nB 5 -A 10 "def retrieve\(self, request" apps/api/plane/api/views/member.pyLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for other soft delete patterns in the codebase
rg -nP "\.delete_at\s*=" --type=py | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find BaseModel to understand deleted_at field
find . -type f -name "*.py" -path "*/models/*" | xargs grep -l "class BaseModel" | head -5Length of output: 34
🏁 Script executed:
#!/bin/bash
# Check the DELETE method in member.py with full context
rg -nB 15 -A 5 "def delete\(self, request, slug, project_id, pk\)" apps/api/plane/api/views/member.pyLength of output: 1118
🏁 Script executed:
#!/bin/bash
# Check BaseModel definition for deleted_at field
rg -nA 30 "class BaseModel" apps/api/plane/db/models/base.py | head -50Length of output: 1275
🏁 Script executed:
#!/bin/bash
# Get the full ProjectMember model definition including constraints
rg -nA 50 "class ProjectMember\(ProjectBaseModel\)" apps/api/plane/db/models/project.pyLength of output: 2261
🏁 Script executed:
#!/bin/bash
# Look for unique constraint in ProjectMember Meta class
rg -nA 10 "class ProjectMember" apps/api/plane/db/models/project.py | grep -A 10 "class Meta"Length of output: 377
🏁 Script executed:
#!/bin/bash
# Search for deleted_at references in project.py model
rg -n "deleted_at" apps/api/plane/db/models/project.pyLength of output: 1020
🏁 Script executed:
#!/bin/bash
# Check the full member.py view file to find GET/retrieve method
rg -nB 3 -A 15 "def get\(self, request, slug, project_id, pk\)" apps/api/plane/api/views/member.pyLength of output: 846
🏁 Script executed:
#!/bin/bash
# Look for patterns where is_active is used as primary soft delete marker
rg -nP "is_active\s*=\s*False" apps/api/plane/db/models/project.py | head -10Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Check ProjectBaseModel definition
rg -nA 20 "class ProjectBaseModel" apps/api/plane/db/models/project.py | head -40
Length of output: 892
🏁 Script executed:
#!/bin/bash
# Check AuditModel for deleted_at field
rg -nA 20 "class AuditModel" apps/api/plane/db/models/base.pyLength of output: 0
🏁 Script executed:
#!/bin/bash
# Verify soft delete patterns - check if other soft deletes set deleted_at
rg -nB 2 -A 2 "\.deleted_at\s*=" --type=py | head -40Length of output: 3209
🏁 Script executed:
#!/bin/bash
# Check if there are other delete methods in views that show the pattern
rg -nB 2 -A 6 "def delete\(self, request" apps/api/plane/api/views/ | head -50Length of output: 3532
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for deleted_at field definition in the entire codebase
rg -n "deleted_at\s*=" apps/api/plane/db/models/base.py
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get full AuditModel class from base.py
cat apps/api/plane/db/models/base.py | head -100Length of output: 1350
🏁 Script executed:
#!/bin/bash
# Confirm the issue: Check if ProjectMember inherits deleted_at from parent
rg -nA 5 "class.*AuditModel" apps/api/plane/db/models/base.pyLength of output: 196
🏁 Script executed:
#!/bin/bash
# Search for any imports or field definitions around deleted_at
rg -nB 5 -A 5 "deleted_at" apps/api/plane/db/models/base.py | head -30Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Find AuditModel in mixins
find . -type f -name "*.py" -path "*/mixins*" | xargs grep -l "class AuditModel" | head -5
Length of output: 29
🏁 Script executed:
#!/bin/bash
# Get AuditModel definition
rg -nA 15 "class AuditModel" --type=py | head -40Length of output: 326
🏁 Script executed:
#!/bin/bash
# Double-check by looking for where deleted_at field is actually defined
rg -n "deleted_at.*models\." --type=py | head -10Length of output: 678
Set deleted_at when soft-deleting project members.
The code sets only is_active = False but omits deleted_at, which is the primary soft-delete marker in this codebase. The ProjectMember model's unique constraint (condition=Q(deleted_at__isnull=True)) only prevents duplicates when deleted_at is null, so this incomplete soft delete prevents re-adding the same member later. This pattern is established throughout the codebase (see attachment.py:141, deletion_task.py:68, asset/v2.py:32 where deleted_at = timezone.now() is set).
Additionally, the DELETE method at line 216 lacks the workspace existence check present in the GET method (line 179), and both methods use .get() without error handling.
Suggested fix:
from django.utils import timezone
def delete(self, request, slug, project_id, pk):
# Check if the workspace exists (consistent with GET method)
if not Workspace.objects.filter(slug=slug).exists():
return Response(
{"error": "Provided workspace does not exist"},
status=status.HTTP_400_BAD_REQUEST,
)
project_member = ProjectMember.objects.get(project_id=project_id, workspace__slug=slug, pk=pk)
project_member.is_active = False
project_member.deleted_at = timezone.now()
project_member.save()
return Response(status=status.HTTP_204_NO_CONTENT)- Introduced new URL paths for managing project members, including a dedicated endpoint for listing and creating project members. - Added a detail endpoint for specific project members to enhance retrieval and update capabilities. - Updated existing URL patterns to maintain consistency and clarity in member management.
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 (1)
apps/api/plane/api/urls/member.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
apps/api/plane/api/urls/member.py (1)
apps/api/plane/api/views/member.py (3)
ProjectMemberListCreateAPIEndpoint(90-152)ProjectMemberDetailAPIEndpoint(156-219)WorkspaceMemberAPIEndpoint(27-87)
⏰ 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 (2)
apps/api/plane/api/urls/member.py (2)
3-3: LGTM: Import statement correctly updated.The imports now properly reference the split endpoints (
ProjectMemberListCreateAPIEndpointandProjectMemberDetailAPIEndpoint), which aligns with the endpoint separation in the URL patterns below.
7-16: LGTM: Past review feedback addressed.The duplicate URL pattern name issue from the previous review has been correctly fixed—the collection route uses
"project-members"(plural) and the detail route uses"project-member"(singular). The addition of the"get"method to the detail endpoint (line 14) also addresses the earlier request to retrieve a single record.
| path( | ||
| "workspaces/<str:slug>/projects/<uuid:project_id>/project-members/", | ||
| ProjectMemberListCreateAPIEndpoint.as_view(http_method_names=["get", "post"]), | ||
| name="project-members", | ||
| ), | ||
| path( | ||
| "workspaces/<str:slug>/projects/<uuid:project_id>/project-members/<uuid:pk>/", | ||
| ProjectMemberDetailAPIEndpoint.as_view(http_method_names=["patch", "delete", "get"]), | ||
| name="project-member", | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
Duplicate URL pattern names and redundant routes.
These routes introduce two critical issues:
-
Duplicate URL pattern names: The names
"project-members"(line 20) and"project-member"(line 25) conflict with the names at lines 10 and 15. Django'sreverse()function will not be able to resolve these names unambiguously. -
Redundant API endpoints: The
/project-members/routes (lines 17-26) appear functionally identical to the/members/routes (lines 7-16)—they use the same view classes, HTTP methods, and provide the same functionality. Having two different URL paths for the same resource violates RESTful API design principles and creates confusion for API consumers.
Questions:
- Is there a specific reason to maintain both
/members/and/project-members/endpoints? - If this is for backward compatibility during a transition period, it should be documented and one set should eventually be deprecated.
- If only one endpoint is needed, which one should be retained?
If both endpoints are truly required, apply this diff to fix the naming conflict:
path(
"workspaces/<str:slug>/projects/<uuid:project_id>/project-members/",
ProjectMemberListCreateAPIEndpoint.as_view(http_method_names=["get", "post"]),
- name="project-members",
+ name="project-members-alt",
),
path(
"workspaces/<str:slug>/projects/<uuid:project_id>/project-members/<uuid:pk>/",
ProjectMemberDetailAPIEndpoint.as_view(http_method_names=["patch", "delete", "get"]),
- name="project-member",
+ name="project-member-alt",
),However, the recommended solution is to remove the duplicate routes entirely unless there is a documented justification for maintaining both.
I'll verify the concerns in the review comment by examining the full URL configuration and checking for references throughout the codebase.
---
🏁 Script executed:
```shell
#!/bin/bash
# Display the complete member.py file to see full context
cat apps/api/plane/api/urls/member.py
Length of output: 1225
🏁 Script executed:
#!/bin/bash
# Search for reverse() calls using "project-members" or "project-member" names
rg -n "reverse\(" -A2 -B2 --type=py | grep -E "(project-members|project-member)" -A2 -B2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for any references to the URL names to understand usage
rg -nP '"project-members"|\'project-members\'|"project-member"|\'project-member\'' --type=pyLength of output: 68
Let me search more broadly for references and test usage:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for test files that might reference these project member endpoints
fd -e test.py -e tests.py --type f | head -20
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for any reverse calls in Python files to see patterns
rg -n 'reverse\s*\(' --type=py -A1 | head -40Length of output: 2834
🏁 Script executed:
#!/bin/bash
# Search for project-members string anywhere (broader search)
rg 'project-members' --type=py --type=jsonLength of output: 634
🏁 Script executed:
#!/bin/bash
# Check for TODO/FIXME comments in the member.py context
rg -n 'TODO|FIXME' apps/api/plane/api/urls/member.pyLength of output: 0
Remove duplicate URL pattern names and resolve redundant routes.
The codebase has confirmed duplicate URL pattern names in member.py:
- Lines 10 and 20: both define
name="project-members" - Lines 15 and 25: both define
name="project-member"
Django's reverse() function cannot resolve duplicate names unambiguously. A test file comment in test_project_app.py (line 19-20) explicitly acknowledges this problem: "unreliable due to duplicate 'name' values in URL patterns."
Additionally, both /members/ and /project-members/ route pairs use identical views and HTTP methods, creating redundant API endpoints for the same resource.
Fix required:
Either:
- Remove one set of routes entirely (recommended), or
- Rename the second pair to
name="project-members-alt"andname="project-member-alt"as a temporary measure
Determine which path (/members/ or /project-members/) should be the canonical endpoint and remove or deprecate the other.
🤖 Prompt for AI Agents
In apps/api/plane/api/urls/member.py around lines 17 to 26 there are duplicate
URL pattern names and redundant routes for the same views; remove the duplicate
route pair that uses the /members/ paths (keep /project-members/ as the
canonical endpoints), delete the duplicate path entries and their imports/usages
so only one set of routes remains, and ensure the remaining patterns use unique
names ("project-members" and "project-member"); update any references
(reverse(), tests) to point to the canonical names if needed.
Description
WorkspaceInviteSerializerfor handling workspace invites.WorkspaceInvitationsViewsetto manage invites (list, create, retrieve, update, delete).ProjectMemberSerializerfor managing project members.ProjectMemberAPIEndpointto support creating and updating project members with appropriate permissions.Type of Change
References
WEB-5237
Summary by CodeRabbit
New Features
Permissions
Backend