feat: restrict member removal to admins and reassign tasks #255
Conversation
- enforce auth so only admins can remove another member - automatically remove user roles on removal - reassign all non-DONE tasks of the removed user to the team - add audit log entry for task reassignment
Hariom01010
added
enhancement
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. Summary by CodeRabbit
WalkthroughIntroduces team member removal safeguards and task reassignment to team. Adds a new API error, a team exceptions module, an audit log field for task counts, repository/service methods to reassign tasks from a user to a team, enhanced team removal logic with role cleanup and audit logging, and exposes task_count in activity timeline. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant TS as TeamService
participant URS as UserRoleService
participant TR as TaskAssignmentRepository
participant TAS as TaskAssignmentService
participant AR as AuditLogRepository
participant V as Team View
rect rgb(245,248,255)
note over C,TS: Remove member from team
C->>TS: remove_member_from_team(team_id, user_id, removed_by_user_id)
TS->>TS: Validate remover is ADMIN if removing others
TS->>TS: Check not Owner/POC
TS->>URS: Remove all roles for user in team
TS->>TAS: reassign_tasks_from_user_to_team(user_id, team_id, removed_by_user_id)
TAS->>TR: reassign_tasks_from_user_to_team(...)
TR-->>TAS: count
TAS->>AR: create log "tasks_reassigned_to_team" (task_count)
TS->>AR: create log member_removed_from_team/member_left_team
TS-->>C: 200 OK
end
rect rgb(243,255,247)
note over V,AR: Timeline includes task_count if present
V->>AR: fetch audit logs
AR-->>V: logs (may include task_count)
V-->>C: timeline entries (+task_count when available)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone identified. Possibly related PRs
Suggested reviewers
Poem
✨ Finishing touches🧪 Generate unit tests
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 |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Incorrect Type for Task Count ▹ view | ✅ Fix detected |
 |
Non-descriptive Default Error Message ▹ view | ✅ Fix detected |
|
Inconsistent Exception Class Naming ▹ view | ✅ Fix detected |
|
Conflicting POC Removal Logic ▹ view | ✅ Fix detected |
|
Complex Authorization Flow ▹ view | ✅ Fix detected |
 |
Sequential Role Deletion Loop ▹ view | ✅ Fix detected |
 |
Method has too many responsibilities ▹ view | ✅ Fix detected |
|
Unclear abbreviation in error message ▹ view | ✅ Fix detected |
|
Sequential Service Calls Without Batching ▹ view | |
 |
Incomplete docstring for task reassignment method ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| todo/exceptions/team_exceptions.py | ✅ |
| todo/models/audit_log.py | ✅ |
| todo/constants/messages.py | ✅ |
| todo/services/task_assignment_service.py | ✅ |
| todo/repositories/task_assignment_repository.py | ✅ |
| todo/views/team.py | ✅ |
| todo/services/team_service.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
todo/services/team_service.py (1)
507-513: Include removed_member_id in audit log details for traceability.You already include details on member addition. Mirror that for removals to aid ops/debugging.
AuditLogRepository.create( AuditLogModel( team_id=PyObjectId(team_id), - 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), + action="member_removed_from_team" if user_id != removed_by_user_id else "member_left_team", + performed_by=PyObjectId(removed_by_user_id), + details={"removed_member_id": user_id}, ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
todo/constants/messages.py(1 hunks)todo/exceptions/team_exceptions.py(1 hunks)todo/models/audit_log.py(1 hunks)todo/repositories/task_assignment_repository.py(2 hunks)todo/services/task_assignment_service.py(1 hunks)todo/services/team_service.py(3 hunks)todo/views/team.py(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#231
File: todo/repositories/task_repository.py:93-109
Timestamp: 2025-07-25T20:12:36.483Z
Learning: Issue #215 in the Real-Dev-Squad/todo-backend repository addresses the problem where tasks assigned to team members disappear from the team's todo list. The expected behavior is that tasks assigned to individual team members should still be visible in the team's todo list, which is implemented by aggregating both direct team assignments and member assignments in the _get_assigned_task_ids_for_team method.
📚 Learning: 2025-07-25T20:12:36.483Z
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#231
File: todo/repositories/task_repository.py:93-109
Timestamp: 2025-07-25T20:12:36.483Z
Learning: In the todo-backend project, tasks can only be assigned to either a team (user_type = "team") or an individual user (user_type = "user"), never both simultaneously. When a POC reassigns a task from a team to an individual team member, the old team assignment is deactivated and a new user assignment is created, ensuring no overlapping assignments exist.
Applied to files:
todo/services/task_assignment_service.pytodo/repositories/task_assignment_repository.py
📚 Learning: 2025-07-25T20:12:36.483Z
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#231
File: todo/repositories/task_repository.py:93-109
Timestamp: 2025-07-25T20:12:36.483Z
Learning: In the todo-backend project's TaskAssignmentRepository, the update_assignment method ensures exclusive task assignments by deactivating all current active assignments for a task before creating a new assignment. This prevents any task from being simultaneously assigned to both a team and individual team members.
Applied to files:
todo/repositories/task_assignment_repository.py
📚 Learning: 2025-07-23T19:26:43.747Z
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#227
File: todo/repositories/task_repository.py:0-0
Timestamp: 2025-07-23T19:26:43.747Z
Learning: In the todo-backend project, the get_tasks_for_user method in TaskRepository is intentionally designed to return only tasks assigned to the user (not tasks created by them), while the count method includes both tasks created by and assigned to the user. This behavioral difference is by design to serve different use cases.
Applied to files:
todo/repositories/task_assignment_repository.py
📚 Learning: 2025-07-25T20:12:36.483Z
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#231
File: todo/repositories/task_repository.py:93-109
Timestamp: 2025-07-25T20:12:36.483Z
Learning: Issue #215 in the Real-Dev-Squad/todo-backend repository addresses the problem where tasks assigned to team members disappear from the team's todo list. The expected behavior is that tasks assigned to individual team members should still be visible in the team's todo list, which is implemented by aggregating both direct team assignments and member assignments in the _get_assigned_task_ids_for_team method.
Applied to files:
todo/repositories/task_assignment_repository.py
🧬 Code graph analysis (4)
todo/services/task_assignment_service.py (4)
todo/repositories/task_assignment_repository.py (2)
reassign_tasks_from_user_to_team(228-280)create(14-25)todo/repositories/audit_log_repository.py (2)
AuditLogRepository(6-22)create(10-16)todo/models/audit_log.py (1)
AuditLogModel(8-27)todo/models/common/pyobjectid.py (1)
PyObjectId(4-15)
todo/repositories/task_assignment_repository.py (4)
todo/constants/task.py (1)
TaskStatus(4-9)todo/services/task_assignment_service.py (1)
reassign_tasks_from_user_to_team(153-164)todo/models/task_assignment.py (1)
TaskAssignmentModel(10-41)todo/models/common/pyobjectid.py (1)
PyObjectId(4-15)
todo/exceptions/team_exceptions.py (1)
todo/constants/messages.py (1)
ApiErrors(25-57)
todo/services/team_service.py (7)
todo/services/user_role_service.py (4)
UserRoleService(10-131)get_user_roles(54-79)has_role(82-87)remove_role_by_id(46-51)todo/constants/role.py (2)
RoleScope(4-6)RoleName(9-13)todo/services/task_assignment_service.py (2)
TaskAssignmentService(17-164)reassign_tasks_from_user_to_team(153-164)todo/exceptions/team_exceptions.py (3)
CannotRemoveOwnerException(8-10)NotTeamAdminException(12-14)CannotRemoveTeamPOC(16-18)todo/repositories/user_role_repository.py (2)
get_user_roles(44-66)remove_role_by_id(77-95)todo/dto/update_team_dto.py (1)
UpdateTeamDTO(6-54)todo/repositories/task_assignment_repository.py (1)
reassign_tasks_from_user_to_team(228-280)
🪛 GitHub Actions: Tests
todo/services/task_assignment_service.py
[error] 1-1: Ruff format check would reformat this file. Command: 'ruff format --check'.
todo/repositories/task_assignment_repository.py
[error] 1-1: Ruff format check would reformat this file. Command: 'ruff format --check'.
todo/exceptions/team_exceptions.py
[error] 1-1: Ruff format check would reformat this file. Command: 'ruff format --check'.
todo/services/team_service.py
[error] 1-1: Ruff format check would reformat this file. Command: 'ruff format --check'.
🔇 Additional comments (6)
todo/exceptions/team_exceptions.py (1)
3-6: Consider adding str for cleaner exception messagesNot required, but implementing str ensures consistent message rendering across handlers/logs.
Example:
class BaseTeamException(Exception): def __init__(self, message: str): self.message = message super().__init__(self.message) + def __str__(self) -> str: + return self.messageLikely an incorrect or invalid review comment.
todo/services/task_assignment_service.py (1)
153-165: Ensure ID type consistency in aggregation matchIt looks like the aggregation pipeline’s initial
$matchis comparing the stored ObjectId values against supplied string IDs, so it will always return zero documents. The model and validators confirm that bothassignee_idandteam_idare saved as BSON ObjectId (viaPyObjectId), and inserts use Pydantic’s JSON dump—which converts ObjectIds to their string hex form—only for transmission, not in the DB. In Mongo, they remain ObjectIds.To fix this, cast the input strings to ObjectId in the pipeline rather than matching strings:
• In
reassign_tasks_from_user_to_team, change the first stage from:{ $match: { team_id: team_id, assignee_id: user_id, user_type: "user", is_active: true }}to:
{ $match: { team_id: { $eq: ObjectId(team_id) }, assignee_id: { $eq: ObjectId(user_id) }, user_type: "user", is_active: true }}or use the aggregation operator:
{ $addFields: { team_id_obj: { $toObjectId: "$team_id" }, assignee_id_obj: { $toObjectId: "$assignee_id" } }}, { $match: { team_id_obj: ObjectId(team_id), assignee_id_obj: ObjectId(user_id), user_type: "user", is_active: true }}• Remove the redundant string‐based matches in the
update_manycalls (you already handle both ObjectId and string there, but it’s better to normalize inputs at the top).With this change, your pipeline will correctly find active tasks for the user and then proceed to deactivate and reassign them.
⛔ Skipped due to learnings
Learnt from: Achintya-Chatterjee PR: Real-Dev-Squad/todo-backend#231 File: todo/repositories/task_repository.py:93-109 Timestamp: 2025-07-25T20:12:36.483Z Learning: In the todo-backend project, tasks can only be assigned to either a team (user_type = "team") or an individual user (user_type = "user"), never both simultaneously. When a POC reassigns a task from a team to an individual team member, the old team assignment is deactivated and a new user assignment is created, ensuring no overlapping assignments exist.Learnt from: Achintya-Chatterjee PR: Real-Dev-Squad/todo-backend#231 File: todo/repositories/task_repository.py:93-109 Timestamp: 2025-07-25T20:12:36.483Z Learning: Issue #215 in the Real-Dev-Squad/todo-backend repository addresses the problem where tasks assigned to team members disappear from the team's todo list. The expected behavior is that tasks assigned to individual team members should still be visible in the team's todo list, which is implemented by aggregating both direct team assignments and member assignments in the _get_assigned_task_ids_for_team method.todo/repositories/task_assignment_repository.py (2)
9-9: Import usage is correct.TaskStatus is used in the aggregation filter to exclude DONE tasks. No issues.
227-283: Formatting Updated by RuffRuff has been run on
todo/repositories/task_assignment_repository.py, the file was reformatted, and all checks now pass. No further changes are needed.todo/services/team_service.py (2)
15-18: New imports look correct and scoped.Imports line up with the new authorization and task reassignment flow. No issues.
483-517: Formatting Issue ResolvedThe file
todo/services/team_service.pyhas been reformatted with Ruff and now passes all checks. No further action required.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
- fix cannot remove error being shown for correct exception
Hariom01010
changed the title
…into remove-from-team-api
…into remove-from-team-api
- update postgres model for audit log to include task_count
- fix formatting
|
…into remove-from-team-api
@AnujChhikara can you specifically highlight where error message response is wrong |
… in task reassignment - Moves service imports to the top level in `team_service.py` for better code organization. - Modifies the `update_many` call in `task_assignment_repository.py` to use the `$in` operator. This ensures that `assignee_id` and `team_id` can be matched regardless of whether they are stored as ObjectIds or strings, preventing potential issues when deactivating task assignments during member removal.
assignment creation in reassign_tasks_from_user_to_team - fix formatting issues
Achintya-Chatterjee
changed the title
- add team exception in views - add assigned_from and assigned_to to audit log
…vent sync failures
- fix bug where done tasks were also being reassigned - fix bug where in postgres_user_role where data sync failed if a user was removed and added more than once - updated reassigned tasks status and add sync to postgres db
Sir i have removed the unecessary migration files. |
…sk reassignment function
5 participants
Date: 23 Aug 2025
Developer Name: @Hariom01010
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Added task_count to audit log collection to store how many tasks were reassigned.
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
proof.mp4
If non admin tries to remove user:
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Restrict removing a member from a team to admins only, prevent removing owner or POC, and reassign the removed user's tasks to the team with appropriate auditing and data consistency.
Why are these changes being made?
To enforce proper permissions when modifying team membership and to ensure that tasks owned or assigned to a removed user are safely reassigned and tracked across systems. This approach centralizes validation, preserves auditability, and maintains data integrity during reassignments.