From 01130a12be32ab4f012276230c9a3b076da01676 Mon Sep 17 00:00:00 2001 From: vinit717 Date: Mon, 25 Aug 2025 11:06:16 +0530 Subject: [PATCH 1/3] fix: update user role assignment during team creation --- todo/services/team_service.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/todo/services/team_service.py b/todo/services/team_service.py index 134ece79..fd6886a6 100644 --- a/todo/services/team_service.py +++ b/todo/services/team_service.py @@ -108,6 +108,8 @@ def create_team(cls, dto: CreateTeamDTO, created_by_user_id: str) -> CreateTeamR team_id_str = str(created_team.id) + cls._assign_user_role(created_by_user_id, team_id_str, "member") + cls._assign_user_role(created_by_user_id, team_id_str, "admin") cls._assign_user_role(created_by_user_id, team_id_str, "owner") for member_id in member_ids: @@ -115,7 +117,7 @@ def create_team(cls, dto: CreateTeamDTO, created_by_user_id: str) -> CreateTeamR cls._assign_user_role(member_id, team_id_str, "member") if dto.poc_id and dto.poc_id != created_by_user_id: - cls._assign_user_role(dto.poc_id, team_id_str, "owner") + cls._assign_user_role(dto.poc_id, team_id_str, "member") # Audit log for team creation AuditLogRepository.create( From 91cb3a63406d362b21a97d2497d1c4b092f7f49c Mon Sep 17 00:00:00 2001 From: vinit717 Date: Mon, 25 Aug 2025 21:53:22 +0530 Subject: [PATCH 2/3] refactor: update user role assignment to use RoleName constants in team service --- todo/services/team_service.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/todo/services/team_service.py b/todo/services/team_service.py index fd6886a6..81e5ee3c 100644 --- a/todo/services/team_service.py +++ b/todo/services/team_service.py @@ -7,6 +7,7 @@ from todo.repositories.team_creation_invite_code_repository import TeamCreationInviteCodeRepository from todo.repositories.team_repository import TeamRepository, UserTeamDetailsRepository from todo.constants.messages import AppMessages +from todo.constants.role import RoleName from todo.utils.invite_code_utils import generate_invite_code from typing import List from todo.models.audit_log import AuditLogModel @@ -108,16 +109,19 @@ def create_team(cls, dto: CreateTeamDTO, created_by_user_id: str) -> CreateTeamR team_id_str = str(created_team.id) - cls._assign_user_role(created_by_user_id, team_id_str, "member") - cls._assign_user_role(created_by_user_id, team_id_str, "admin") - cls._assign_user_role(created_by_user_id, team_id_str, "owner") + cls._assign_user_role(created_by_user_id, team_id_str, RoleName.MEMBER.value) + cls._assign_user_role(created_by_user_id, team_id_str, RoleName.ADMIN.value) + cls._assign_user_role(created_by_user_id, team_id_str, RoleName.OWNER.value) + + users_with_roles = {created_by_user_id} for member_id in member_ids: - if member_id != created_by_user_id: - cls._assign_user_role(member_id, team_id_str, "member") + if member_id not in users_with_roles: + cls._assign_user_role(member_id, team_id_str, RoleName.MEMBER.value) + users_with_roles.add(member_id) - if dto.poc_id and dto.poc_id != created_by_user_id: - cls._assign_user_role(dto.poc_id, team_id_str, "member") + if dto.poc_id and dto.poc_id not in users_with_roles: + cls._assign_user_role(dto.poc_id, team_id_str, RoleName.MEMBER.value) # Audit log for team creation AuditLogRepository.create( @@ -286,7 +290,7 @@ def join_team_by_invite_code(cls, invite_code: str, user_id: str) -> TeamDTO: UserTeamDetailsRepository.create(user_team) # NEW: Assign default member role using new role system - cls._assign_user_role(user_id, str(team.id), "member") + cls._assign_user_role(user_id, str(team.id), RoleName.MEMBER.value) # Audit log for team join AuditLogRepository.create( @@ -446,7 +450,7 @@ def add_team_members(cls, team_id: str, member_ids: List[str], added_by_user_id: # NEW: Assign default member roles using new role system for member_id in member_ids: - cls._assign_user_role(member_id, team_id, "member") + cls._assign_user_role(member_id, team_id, RoleName.MEMBER.value) # Audit log for team member addition for member_id in member_ids: From 50931bdf5212724dd2b22212cbd347c853a3a8a0 Mon Sep 17 00:00:00 2001 From: Shobhan Sundar Goutam <81035407+shobhan-sundar-goutam@users.noreply.github.com> Date: Fri, 5 Sep 2025 10:50:08 +0530 Subject: [PATCH 3/3] Merge pull request #270 from Real-Dev-Squad/refactor/remove-redundant-auth-checks refactor(auth): remove redundant authentication checks --- todo/views/task.py | 38 +++++++++-------------------------- todo/views/task_assignment.py | 26 ++++++------------------ 2 files changed, 16 insertions(+), 48 deletions(-) diff --git a/todo/views/task.py b/todo/views/task.py index a7e42731..f8e0c750 100644 --- a/todo/views/task.py +++ b/todo/views/task.py @@ -3,11 +3,10 @@ from rest_framework.response import Response from rest_framework import status from rest_framework.request import Request -from rest_framework.exceptions import AuthenticationFailed, ValidationError +from rest_framework.exceptions import ValidationError from django.conf import settings from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse from drf_spectacular.types import OpenApiTypes -from todo.middlewares.jwt_auth import get_current_user_info from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer from todo.serializers.create_task_serializer import CreateTaskSerializer from todo.serializers.update_task_serializer import UpdateTaskSerializer @@ -78,22 +77,18 @@ def get(self, request: Request): query = GetTaskQueryParamsSerializer(data=request.query_params) query.is_valid(raise_exception=True) if query.validated_data["profile"]: - user = get_current_user_info(request) - if not user: - raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED) status_filter = query.validated_data.get("status", "").upper() response = TaskService.get_tasks_for_user( - user_id=user["user_id"], + user_id=request.user_id, page=query.validated_data["page"], limit=query.validated_data["limit"], status_filter=status_filter, ) return Response(data=response.model_dump(mode="json"), status=status.HTTP_200_OK) - user = get_current_user_info(request) if query.validated_data["profile"]: response = TaskService.get_tasks_for_user( - user_id=user["user_id"], + user_id=request.user_id, page=query.validated_data["page"], limit=query.validated_data["limit"], ) @@ -109,7 +104,7 @@ def get(self, request: Request): limit=query.validated_data["limit"], sort_by=query.validated_data["sort_by"], order=query.validated_data.get("order"), - user_id=user["user_id"], + user_id=request.user_id, team_id=team_id, status_filter=status_filter, ) @@ -137,15 +132,13 @@ def post(self, request: Request): Returns: Response: HTTP response with created task data or error details """ - user = get_current_user_info(request) - serializer = CreateTaskSerializer(data=request.data) if not serializer.is_valid(): return self._handle_validation_errors(serializer.errors) try: - dto = CreateTaskDTO(**serializer.validated_data, createdBy=user["user_id"]) + dto = CreateTaskDTO(**serializer.validated_data, createdBy=request.user_id) response: CreateTaskResponse = TaskService.create_task(dto) return Response(data=response.model_dump(mode="json"), status=status.HTTP_201_CREATED) @@ -245,9 +238,8 @@ def get(self, request: Request, task_id: str): }, ) def delete(self, request: Request, task_id: str): - user = get_current_user_info(request) task_id = ObjectId(task_id) - TaskService.delete_task(task_id, user["user_id"]) + TaskService.delete_task(task_id, request.user_id) return Response(status=status.HTTP_204_NO_CONTENT) @extend_schema( @@ -283,8 +275,6 @@ def patch(self, request: Request, task_id: str): Can also be used to defer a task by using ?action=defer query parameter. """ action = request.query_params.get("action", "update") - user = get_current_user_info(request) - if action == "defer": serializer = DeferTaskSerializer(data=request.data) serializer.is_valid(raise_exception=True) @@ -292,7 +282,7 @@ def patch(self, request: Request, task_id: str): updated_task_dto = TaskService.defer_task( task_id=task_id, deferred_till=serializer.validated_data["deferredTill"], - user_id=user["user_id"], + user_id=request.user_id, ) elif action == "update": serializer = UpdateTaskSerializer(data=request.data, partial=True) @@ -302,7 +292,7 @@ def patch(self, request: Request, task_id: str): updated_task_dto = TaskService.update_task( task_id=task_id, validated_data=serializer.validated_data, - user_id=user["user_id"], + user_id=request.user_id, ) else: raise ValidationError({"action": ValidationErrors.UNSUPPORTED_ACTION.format(action)}) @@ -338,10 +328,6 @@ def patch(self, request: Request, task_id: str): Update both task details and assignee information in a single request. Similar to task creation but for updates. """ - user = get_current_user_info(request) - if not user: - raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED) - serializer = UpdateTaskSerializer(data=request.data, partial=True) if not serializer.is_valid(): @@ -350,7 +336,7 @@ def patch(self, request: Request, task_id: str): try: # Update the task using the service with validated data updated_task_dto = TaskService.update_task_with_assignee_from_dict( - task_id=task_id, validated_data=serializer.validated_data, user_id=user["user_id"] + task_id=task_id, validated_data=serializer.validated_data, user_id=request.user_id ) return Response(data=updated_task_dto.model_dump(mode="json"), status=status.HTTP_200_OK) @@ -429,10 +415,6 @@ class AssignTaskToUserView(APIView): }, ) def patch(self, request: Request, task_id: str): - user = get_current_user_info(request) - if not user: - raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED) - serializer = AssignTaskToUserSerializer(data=request.data) if not serializer.is_valid(): return Response(data={"errors": serializer.errors}, status=status.HTTP_400_BAD_REQUEST) @@ -441,7 +423,7 @@ def patch(self, request: Request, task_id: str): dto = CreateTaskAssignmentDTO( task_id=task_id, assignee_id=serializer.validated_data["assignee_id"], user_type="user" ) - response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, user["user_id"]) + response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, request.user_id) return Response(data=response.model_dump(mode="json"), status=status.HTTP_200_OK) except Exception as e: error_response = ApiErrorResponse( diff --git a/todo/views/task_assignment.py b/todo/views/task_assignment.py index 982f1699..66aa1918 100644 --- a/todo/views/task_assignment.py +++ b/todo/views/task_assignment.py @@ -2,13 +2,11 @@ from rest_framework.response import Response from rest_framework import status from rest_framework.request import Request -from rest_framework.exceptions import AuthenticationFailed from django.conf import settings from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse from drf_spectacular.types import OpenApiTypes from rest_framework import serializers -from todo.middlewares.jwt_auth import get_current_user_info from todo.serializers.create_task_assignment_serializer import CreateTaskAssignmentSerializer from todo.services.task_assignment_service import TaskAssignmentService from todo.dto.task_assignment_dto import CreateTaskAssignmentDTO @@ -53,17 +51,13 @@ def post(self, request: Request): Returns: Response: HTTP response with created assignment data or error details """ - user = get_current_user_info(request) - if not user: - raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED) - serializer = CreateTaskAssignmentSerializer(data=request.data) if not serializer.is_valid(): return Response(data={"errors": serializer.errors}, status=status.HTTP_400_BAD_REQUEST) try: dto = CreateTaskAssignmentDTO(**serializer.validated_data) - response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, user["user_id"]) + response: CreateTaskAssignmentResponse = TaskAssignmentService.create_task_assignment(dto, request.user_id) return Response(data=response.model_dump(mode="json"), status=status.HTTP_201_CREATED) @@ -177,10 +171,6 @@ def patch(self, request: Request, task_id: str): Set or update the executor for a team-assigned task. Only the SPOC can perform this action. For user assignments, this endpoint is not applicable. """ - user = get_current_user_info(request) - if not user: - raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED) - executor_id = request.data.get("executor_id") if not executor_id: return Response({"error": "executor_id is required"}, status=status.HTTP_400_BAD_REQUEST) @@ -204,7 +194,7 @@ def patch(self, request: Request, task_id: str): ) # Only SPOC can update executor - if not TeamRepository.is_user_spoc(str(assignment.assignee_id), user["user_id"]): + if not TeamRepository.is_user_spoc(str(assignment.assignee_id), request.user_id): return Response( {"error": "Only the SPOC can update executor for this team task."}, status=status.HTTP_403_FORBIDDEN ) @@ -224,14 +214,14 @@ def patch(self, request: Request, task_id: str): # Update executor_id try: updated_assignment = TaskAssignmentRepository.update_assignment( - task_id, executor_id, "user", user["user_id"] + task_id, executor_id, "user", request.user_id ) if not updated_assignment: # Get more details about why it failed import traceback print( - f"DEBUG: update_executor failed for task_id={task_id}, executor_id={executor_id}, user_id={user['user_id']}" + f"DEBUG: update_executor failed for task_id={task_id}, executor_id={executor_id}, user_id={request.user_id}" ) print(f"DEBUG: assignment details: {assignment}") return Response( @@ -257,7 +247,7 @@ def patch(self, request: Request, task_id: str): team_id=assignment.assignee_id, previous_executor_id=previous_executor_id, new_executor_id=executor_id, - spoc_id=user["user_id"], + spoc_id=request.user_id, action="reassign_executor", ) AuditLogRepository.create(audit_log) @@ -295,12 +285,8 @@ def delete(self, request: Request, task_id: str): Returns: Response: HTTP response with success or error details """ - user = get_current_user_info(request) - if not user: - raise AuthenticationFailed(ApiErrors.AUTHENTICATION_FAILED) - try: - success = TaskAssignmentService.delete_task_assignment(task_id, user["user_id"]) + success = TaskAssignmentService.delete_task_assignment(task_id, request.user_id) if not success: error_response = ApiErrorResponse( statusCode=404,