Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
13e97ad
feat(team): restrict member removal to admins and reassign tasks
Hariom01010 Aug 23, 2025
ad26f3a
feat(remove-member): resolve comments by bot
Hariom01010 Aug 24, 2025
f25bc28
feat(remove-member): instantiate the exception raised
Hariom01010 Aug 24, 2025
8ef56b8
Merge branch 'develop' of https://github.com/Hariom01010/todo-backend…
Hariom01010 Aug 29, 2025
3f0bd93
Merge branch 'develop' of https://github.com/Hariom01010/todo-backend…
Hariom01010 Aug 31, 2025
7877f29
feat(remove-from-team): sync changes for task assignment to postgres
Hariom01010 Sep 2, 2025
f075980
feat(remove-member): update cannot remove POC exception message
Hariom01010 Sep 2, 2025
13485ff
Merge branch 'develop' of https://github.com/Hariom01010/todo-backend…
Hariom01010 Sep 3, 2025
6bb20aa
feat(remove-member-from-team): handle mixed ObjectId and string types…
Hariom01010 Sep 3, 2025
7713420
feat(remove-member-from-team): move UserTeamDetailsRepository to the
Hariom01010 Sep 3, 2025
b0e48d3
feat(remove-from-team): set updated_by to None in the ops for task
Hariom01010 Sep 3, 2025
f96e4c4
feat(remove-member): move task_reassignment function in transaction
Hariom01010 Sep 4, 2025
42c9485
feat(remove-member): fix formatting issues
Hariom01010 Sep 4, 2025
3b50a66
feat(remove-from-team): include is_active in unique constraint to pre…
Hariom01010 Sep 5, 2025
4b4ae55
feat(remove-from-team): remove unecessary comments
Hariom01010 Sep 9, 2025
bd88687
feat(remove-from-member): add serializer for path params validation
Hariom01010 Sep 9, 2025
3d20bfb
Merge branch 'develop' of https://github.com/Hariom01010/todo-backend…
Hariom01010 Sep 12, 2025
d567b3a
feat(remove-from-team): removed aggregation pipeline and use normal db
Hariom01010 Sep 16, 2025
01e59b8
feat(remove-from-todo): remove unecessary migration files
Hariom01010 Sep 16, 2025
bbc2132
feat(remove-from-team): remove task_count field from audit log model
Hariom01010 Sep 16, 2025
3bf3c2a
feat(remove-from-team): fix bug where reassigned todos not showing on
Hariom01010 Sep 17, 2025
3c6f245
feat(remove-from-team): move remove role and validation to individual
Hariom01010 Sep 17, 2025
8bcea47
feat(remove-from-team): fix formatting issues
Hariom01010 Sep 17, 2025
2059402
feat(remove-from-team): use bool return value instead of count for ta…
Hariom01010 Sep 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions todo/constants/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class ApiErrors:
USER_NOT_FOUND_GENERIC = "User not found."
SEARCH_QUERY_EMPTY = "Search query cannot be empty"
TASK_ALREADY_IN_WATCHLIST = "Task is already in the watchlist"
CANNOT_REMOVE_POC = "POC cannot be removed from a team"


# Validation error messages
Expand Down
18 changes: 18 additions & 0 deletions todo/exceptions/team_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from todo.constants.messages import ApiErrors

class BaseTeamException(Exception):
def __init__(self, message: str):
self.message = message
super().__init__(self.message)

class CannotRemoveOwnerException(BaseTeamException):
def __init__(self, message = ApiErrors.UNAUTHORIZED_TITLE):
super().__init__(message)

class NotTeamAdminException(BaseTeamException):
def __init__(self, message = ApiErrors.UNAUTHORIZED_TITLE):
super().__init__(message)

class CannotRemoveTeamPOC(BaseTeamException):
def __init__(self, message = ApiErrors.CANNOT_REMOVE_POC):
super().__init__(message)
2 changes: 2 additions & 0 deletions todo/models/audit_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ class AuditLogModel(Document):
assignee_to: PyObjectId | None = None
# For general user reference (who performed the action)
performed_by: PyObjectId | None = None
# For multiple task reassignments
task_count: str | None = None
61 changes: 59 additions & 2 deletions todo/repositories/task_assignment_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
from todo.models.task_assignment import TaskAssignmentModel
from todo.repositories.common.mongo_repository import MongoRepository
from todo.models.common.pyobjectid import PyObjectId


from todo.constants.task import TaskStatus
class TaskAssignmentRepository(MongoRepository):
collection_name = TaskAssignmentModel.collection_name

Expand Down Expand Up @@ -224,3 +223,61 @@ def deactivate_by_task_id(cls, task_id: str, user_id: str) -> bool:
return result.modified_count > 0
except Exception:
return False

@classmethod
def reassign_tasks_from_user_to_team(cls, user_id: str, team_id: str, removed_by_user_id: str):
"""
Reassign all tasks of user to team
"""
collection = cls.get_collection()
now = datetime.now(timezone.utc)
pipeline = [
{
"$match": {
"team_id": team_id,
"assignee_id": user_id,
"user_type": "user",
"is_active": True
}
},
{"$addFields": {"task_id_obj": { "$toObjectId": "$task_id" }}},
{"$lookup": {
"from": "tasks",
"localField": "task_id_obj",
"foreignField": "_id",
"as": "task_info"
}},
{ "$unwind": "$task_info" },
{ "$match": { "task_info.status": { "$ne": TaskStatus.DONE.value } } }
]
user_task_assignments = list(collection.aggregate(pipeline))
if not user_task_assignments:
return 0

# Deactivate current assignment (try using both ObjectId and string)
collection.update_many(
{"assignee_id": ObjectId(user_id), "team_id": ObjectId(team_id), "is_active": True },
{"$set": {"is_active": False, "updated_by": ObjectId(removed_by_user_id), "updated_at": now}})

collection.update_many(
{"assignee_id": user_id, "team_id": team_id, "is_active": True},
{"$set": {"is_active": False, "updated_by": ObjectId(removed_by_user_id), "updated_at": now}}
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Combine deactivation updates, include user_type guard, and match both string/ObjectId in a single pass.

Current approach runs two update_many calls without a user_type filter. Consolidate into one atomic query that also narrows to user assignments and handles both storage types.

-        # Deactivate current assignment (try using both ObjectId and string)
-        collection.update_many(
-            {"assignee_id": ObjectId(user_id), "team_id": ObjectId(team_id), "is_active": True },
-            {"$set": {"is_active": False, "updated_by": ObjectId(removed_by_user_id), "updated_at": now}})
-
-        collection.update_many(
-            {"assignee_id": user_id, "team_id": team_id, "is_active": True},
-            {"$set": {"is_active": False, "updated_by": ObjectId(removed_by_user_id), "updated_at": now}}
-        )
+        # Deactivate current user assignments for this team (support string/ObjectId)
+        collection.update_many(
+            {
+                "assignee_id": {"$in": assignee_match},
+                "team_id": {"$in": team_match},
+                "user_type": "user",
+                "is_active": True
+            },
+            {
+                "$set": {
+                    "is_active": False,
+                    "updated_by": ObjectId(removed_by_user_id),
+                    "updated_at": now
+                }
+            }
+        )

Committable suggestion skipped: line range outside the PR's diff.


new_assignments = []
for task in user_task_assignments:
new_assignments.append(TaskAssignmentModel(
_id=PyObjectId(),
task_id=task["task_id_obj"],
assignee_id=PyObjectId(team_id),
user_type="team",
created_by=PyObjectId(removed_by_user_id),
updated_by=None,
team_id=PyObjectId(team_id)
).model_dump(mode="json", by_alias=True, exclude_none=True))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Prefer repository create helpers or ensure consistent auditing fields when bypassing them.

Direct insert_many is fine for throughput, but it bypasses the repository’s create() hook that normalizes created_at/updated_at. Given TaskAssignmentModel sets created_at via default_factory, you’re safe. If you add any future hooks or audit side-effects to create(), this path will miss them. Consider documenting this or switching to create() in a follow-up if consistency becomes a concern.

🤖 Prompt for AI Agents
In todo/repositories/task_assignment_repository.py around lines 267 to 277, the
loop builds TaskAssignmentModel dicts and will be inserted directly (bypassing
the repository.create hooks) which can miss auditing/normalization logic; either
switch to calling the repository create() helper per item (or a batch helper) so
created_at/updated_at and any create-side effects run, or explicitly populate
and include all auditing fields that create() would set (created_at, updated_at,
any audit flags) and add an inline comment noting why direct insert_many is
intentionally used to avoid future drift.

if new_assignments:
collection.insert_many(new_assignments)
return len(new_assignments)



16 changes: 15 additions & 1 deletion todo/services/task_assignment_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from todo.models.audit_log import AuditLogModel
from todo.repositories.audit_log_repository import AuditLogRepository


class TaskAssignmentService:
@classmethod
def create_task_assignment(cls, dto: CreateTaskAssignmentDTO, user_id: str) -> CreateTaskAssignmentResponse:
Expand Down Expand Up @@ -149,3 +148,18 @@ def delete_task_assignment(cls, task_id: str, user_id: str) -> bool:
Delete task assignment by task ID.
"""
return TaskAssignmentRepository.delete_assignment(task_id, user_id)

@classmethod
def reassign_tasks_from_user_to_team(cls, user_id: str, team_id: str, user_type: str, removed_by_user_id: str):
"""
Reassign all tasks of user to team
"""
assignment = TaskAssignmentRepository.reassign_tasks_from_user_to_team(user_id, team_id, removed_by_user_id)
if assignment != 0:
AuditLogRepository.create(AuditLogModel(
team_id=PyObjectId(team_id),
performed_by = PyObjectId(removed_by_user_id),
task_count = str(assignment),
action="tasks_reassigned_to_team"
))

26 changes: 23 additions & 3 deletions todo/services/team_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
from todo.models.audit_log import AuditLogModel
from todo.repositories.audit_log_repository import AuditLogRepository
from todo.dto.responses.error_response import ApiErrorResponse, ApiErrorDetail

from todo.services.user_role_service import UserRoleService
from todo.constants.role import RoleScope, RoleName
from todo.services.task_assignment_service import TaskAssignmentService
from todo.exceptions.team_exceptions import (CannotRemoveOwnerException, NotTeamAdminException, CannotRemoveTeamPOC)
DEFAULT_ROLE_ID = "1"


Expand Down Expand Up @@ -479,7 +482,24 @@ class TeamOrUserNotFound(Exception):
@classmethod
def remove_member_from_team(cls, user_id: str, team_id: str, removed_by_user_id: str = None):
from todo.repositories.user_team_details_repository import UserTeamDetailsRepository

user_roles = UserRoleService.get_user_roles(user_id, RoleScope.TEAM.value, team_id)
user_role_ids = [roles["role_id"] for roles in user_roles]
team = TeamService.get_team_by_id(team_id)
# Authentication Checks
if user_id != removed_by_user_id:
if not UserRoleService.has_role(removed_by_user_id, RoleName.ADMIN.value, RoleScope.TEAM.value, team_id):
raise NotTeamAdminException
elif user_id == team.poc_id:
raise CannotRemoveTeamPOC
if user_id == team.created_by:
raise CannotRemoveOwnerException
if user_id == team.poc_id:
cls.update_team(team_id, UpdateTeamDTO(poc_id=team.created_by), removed_by_user_id)
# # Remove User Roles
for role_id in user_role_ids:
UserRoleService.remove_role_by_id(user_id, role_id, RoleScope.TEAM.value, team_id)
# Reassign Tasks:
TaskAssignmentService.reassign_tasks_from_user_to_team(user_id, team_id, 'team', removed_by_user_id)
success = UserTeamDetailsRepository.remove_member_from_team(user_id=user_id, team_id=team_id)
if not success:
raise cls.TeamOrUserNotFound()
Expand All @@ -488,7 +508,7 @@ def remove_member_from_team(cls, user_id: str, team_id: str, removed_by_user_id:
AuditLogRepository.create(
AuditLogModel(
team_id=PyObjectId(team_id),
action="member_removed_from_team",
action="member_removed_from_team" if user_id != removed_by_user_id else "member_left_team",
performed_by=PyObjectId(removed_by_user_id) if removed_by_user_id else PyObjectId(user_id),
)
)
Expand Down
2 changes: 2 additions & 0 deletions todo/views/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ def get(self, request: Request, team_id: str):
entry["status_from"] = log.status_from
if log.status_to:
entry["status_to"] = log.status_to
if log.task_count:
entry["task_count"] = log.task_count
timeline.append(entry)
return Response({"timeline": timeline}, status=status.HTTP_200_OK)

Expand Down
Loading