⚙️ 47823 backend [ tasks ] Update "require completion by all" logic for groups#257
⚙️ 47823 backend [ tasks ] Update "require completion by all" logic for groups#257pneumojoseph wants to merge 13 commits into
Conversation
…ach task completion
…uire completion for all group users
…y "group" performer
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| super().delete_raw_performers() | ||
| self._delete_orphaned_performers() | ||
|
|
||
| def can_be_completed(self) -> bool: |
There was a problem hiding this comment.
Orphan cleanup drops GROUP_USER rows
High Severity
_delete_orphaned_performers builds a queryset of performers not linked from raw performers, which always includes GROUP_USER rows. When any orphan USER or GROUP performer is found, it deletes the entire queryset, wiping per-member GROUP_USER completion records needed for require-completion-by-all.
Reviewed by Cursor Bugbot for commit 08e729a. Configure here.
There was a problem hiding this comment.
🟡 Medium accounts/validators.py:12
user_is_last_performer() returns True (blocking deactivation) for a user whose own TaskPerformer row is soft-deleted, as long as the task has some other non-deleted performer. The chained filters .filter(taskperformer__is_deleted=False) and .filter(raw_performers__is_deleted=False) are applied as separate joins from on_performer(user.id), so each predicate can match a different related row — the user_id filter hits the user's row while the is_deleted=False filter is satisfied by a different performer. Use a single .filter(taskperformer__user_id=user.id, taskperformer__is_deleted=False) call (or Q object) so both conditions apply to the same TaskPerformer row. Apply the same fix for raw_performers.
if (
Task.objects.filter(
taskperformer__user_id=user.id,
taskperformer__is_deleted=False,
taskperformer__directly_status__ne=DirectlyStatus.DELETED,
raw_performers__is_deleted=False,
)
.raw_performers_count(1)
.filter(
status__in=(
TaskStatus.PENDING,
TaskStatus.DELAYED,
TaskStatus.ACTIVE,
),
).exists()
):
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/accounts/validators.py around lines 12-25:
`user_is_last_performer()` returns `True` (blocking deactivation) for a user whose own `TaskPerformer` row is soft-deleted, as long as the task has some other non-deleted performer. The chained filters `.filter(taskperformer__is_deleted=False)` and `.filter(raw_performers__is_deleted=False)` are applied as separate joins from `on_performer(user.id)`, so each predicate can match a different related row — the `user_id` filter hits the user's row while the `is_deleted=False` filter is satisfied by a different performer. Use a single `.filter(taskperformer__user_id=user.id, taskperformer__is_deleted=False)` call (or `Q` object) so both conditions apply to the same `TaskPerformer` row. Apply the same fix for `raw_performers`.
pneumojoseph
changed the title
- after remove user from a group - after remove group - after deactivate user
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 249a04b. Configure here.
| ) | ||
| if group_performer_tasks_ids: | ||
| check_and_complete_tasks.delay( | ||
| task_ids=group_performer_tasks_ids, |
There was a problem hiding this comment.
Task IDs list tuple shape
High Severity
_check_and_complete_tasks builds task ID lists with values_list('id') without flat=True, producing one-tuples instead of integers. check_and_complete_tasks then filters with id__in on those values, so removed or deactivated users may not trigger auto-completion of RCBA group tasks.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 249a04b. Configure here.
| completed_performers = task_performers.filter( | ||
| Q(is_completed=True) | ||
| | Q(is_completed=False, user_id=self.user.id) | ||
| | Q(is_completed=False, group__users=self.user.id), |
There was a problem hiding this comment.
Dual performer completion deadlock
High Severity
For RCBA tasks where the same user is both a direct user performer and a group member, by_user_or_group().first() can pick the group row first. After they finish via a group_user record, a later complete attempt raises UserAlreadyCompleteTask while the direct user performer row may still be incomplete, blocking task completion.
Reviewed by Cursor Bugbot for commit 249a04b. Configure here.
| for task in tasks: | ||
| if task.can_be_completed(): |
There was a problem hiding this comment.
🟠 High tasks/tasks.py:67
check_and_complete_tasks can force a task into COMPLETED status even after it has transitioned to PENDING or DELAYED, corrupting workflow state. The task is reloaded only by id and then completed if task.can_be_completed() returns true, but neither that method nor the loop checks task.is_active. Because this runs in a Celery task, the status can change between enqueue time and worker execution, so a task that is no longer active still gets force-completed. Consider guarding the loop with an task.is_active check before calling service.complete_task(task).
for task in tasks:
- if task.can_be_completed():
+ if task.is_active and task.can_be_completed():🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/tasks/tasks.py around lines 67-68:
`check_and_complete_tasks` can force a task into `COMPLETED` status even after it has transitioned to `PENDING` or `DELAYED`, corrupting workflow state. The task is reloaded only by `id` and then completed if `task.can_be_completed()` returns true, but neither that method nor the loop checks `task.is_active`. Because this runs in a Celery task, the status can change between enqueue time and worker execution, so a task that is no longer active still gets force-completed. Consider guarding the loop with an `task.is_active` check before calling `service.complete_task(task)`.
| service.update_users_counts() | ||
| self._check_and_complete_tasks() | ||
| self.identify(user) |
There was a problem hiding this comment.
🟠 High services/user.py:397
_deactivate() enqueues check_and_complete_tasks inside the transaction.atomic() block, so the Celery worker can run before the deactivation commits. task.can_be_completed() then still sees the user as active and returns False, so tasks are skipped with no retry — the new auto-completion path silently does nothing. Move the self._check_and_complete_tasks() call after the atomic block, or defer it with transaction.on_commit, so the worker only runs once the user's status = INACTIVE is persisted.
service.update_users_counts()
- self._check_and_complete_tasks()
self.identify(user)
self._check_and_complete_tasks()Also found in 1 other location(s)
backend/src/accounts/services/group.py:258
delete()now calls_check_and_complete_tasks()at line 258 beforeself.instance.delete()soft-deletes the group and cascades the relatedTaskPerformerrows._check_and_complete_tasks()immediately enqueuescheck_and_complete_tasks, so the worker can run against the still-existing group performer records, see the task as not completable, and exit. Because there is no second recheck after the delete finishes, tasks that should auto-complete after group deletion can remain stuck active.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/accounts/services/user.py around lines 397-399:
`_deactivate()` enqueues `check_and_complete_tasks` inside the `transaction.atomic()` block, so the Celery worker can run before the deactivation commits. `task.can_be_completed()` then still sees the user as active and returns `False`, so tasks are skipped with no retry — the new auto-completion path silently does nothing. Move the `self._check_and_complete_tasks()` call after the `atomic` block, or defer it with `transaction.on_commit`, so the worker only runs once the user's `status = INACTIVE` is persisted.
Also found in 1 other location(s):
- backend/src/accounts/services/group.py:258 -- `delete()` now calls `_check_and_complete_tasks()` at line 258 before `self.instance.delete()` soft-deletes the group and cascades the related `TaskPerformer` rows. `_check_and_complete_tasks()` immediately enqueues `check_and_complete_tasks`, so the worker can run against the still-existing group performer records, see the task as not completable, and exit. Because there is no second recheck after the delete finishes, tasks that should auto-complete after group deletion can remain stuck active.
| """ Check if it is possible to complete tasks | ||
| where the performer group is """ | ||
|
|
||
| group_performer_tasks_ids = list( |
There was a problem hiding this comment.
🟠 High services/group.py:164
_check_and_complete_tasks can enqueue and auto-complete tasks belonging to unrelated groups. The query Task.objects.active().active_for_group(self.instance.id) uses active_for_group, whose filter is Q(taskperformer__group_id=group_id) | Q(taskperformer__type=PerformerType.GROUP). The second branch matches any active task that has any group performer at all, regardless of which group. So removing users from one group can trigger check_and_complete_tasks on tasks whose only group performer is a different group, and those tasks may be auto-completed if they are otherwise completable. Consider using a query scoped to this group only (e.g. taskperformer__group_id=self.instance.id) instead of reusing the broad active_for_group queryset.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/accounts/services/group.py around line 164:
`_check_and_complete_tasks` can enqueue and auto-complete tasks belonging to unrelated groups. The query `Task.objects.active().active_for_group(self.instance.id)` uses `active_for_group`, whose filter is `Q(taskperformer__group_id=group_id) | Q(taskperformer__type=PerformerType.GROUP)`. The second branch matches any active task that has *any* group performer at all, regardless of which group. So removing users from one group can trigger `check_and_complete_tasks` on tasks whose only group performer is a different group, and those tasks may be auto-completed if they are otherwise completable. Consider using a query scoped to this group only (e.g. `taskperformer__group_id=self.instance.id`) instead of reusing the broad `active_for_group` queryset.
1 participant
Problem
When a task is set up so that every assignee must complete it individually, and the task is assigned to a user group, the system did not reliably track whether each group member had done their part. A task could stay open even after all active members had finished, or move forward before everyone had acted. Assignees who were later removed from the task could still be counted as outstanding. In addition, internal completion records for individual group members were incorrectly exposed in task details and activity history, making the performer list confusing for users.
Fix / Solution
Task.can_be_completed()to correctly evaluate completion whenrequire_completion_by_allis enabled, including group members, removed performers, inactive users, and deleted groups.Release notes
Changes
API
Tasks (
/v2/tasks/)GET /v2/tasks/:id—performerslist includes onlyuserandgrouptypes;group_userrecords are excluded.GET /v2/tasks/:id/events—task.performersin event payloads excludesgroup_userrecords.POST /v2/tasks/:id/complete— when a group member completes a task withrequire_completion_by_all=true, a hiddengroup_userperformer is created/updated, atask_completeevent is emitted for that user, and the task advances only after all group members (and other performers) have completed; removed/deleted performers are ignored in completion checks.POST /v2/tasks/:id/comment—task.performersin the response excludesgroup_userrecords.Workflows (
/workflows/)GET /workflows/:id/events—task.performersin event payloads excludesgroup_userrecords.POST /workflows/:id/comment—task.performersin the response excludesgroup_userrecords.Web-client
No frontend changes in this branch.
Test cases
API
POST /v2/tasks/:id/complete
fields_values) for a group-assigned task withrequire_completion_by_all=truewhere other group members have not yet completedgroup_userrecord created for the current user withis_completed=true;task_completeevent created for the user; task remains active (is_completed=false); workflow does not advancefields_valuesincluded) for a group-assigned task withrequire_completion_by_all=truewhere all group members have now completedrequire_completion_by_all=falsegroup_userrecord is alreadyis_completed=truedirectly_status=deleteddirectly_status=deletedChecklistIncompletedvalidation error in response bodyfields_values(wrong type or missing required field)GET /v2/tasks/:id
groupandgroup_userperformersperformerscontains only thegroupentry (one item); nogroup_userentriesperformerslists only non-deleted performersrequire_completion_by_all=truewhere current user has completed their part but task is still activeis_completed=trueGET /v2/tasks/:id/events
groupandgroup_userperformers after atask_completeeventtask.performersin each event contains onlygrouptype, nogroup_userlimitandoffsetquery parametersGET /workflows/:id/events
groupandgroup_userperformerstask.performersin events contains onlygrouptypelimitandoffsetquery parametersPOST /v2/tasks/:id/comment
text) on a task withgroup_userperformerstask.performerscontains onlygrouptypegroup_userPOST /workflows/:id/comment
group_userperformerstask.performerscontains onlygrouptypeWeb-client
No direct UI changes in this branch. Regression checks below cover API-driven behavior visible in the client.
group_userentries visibleNote
High Risk
Changes core task completion and workflow advancement for RCBA + groups, and triggers async auto-complete on membership changes—incorrect SQL or edge-case handling could complete tasks early or leave them stuck.
Overview
Fixes require completion by all for tasks assigned to groups by tracking each active member’s completion separately and only advancing the workflow when everyone who still counts has finished.
Completion logic:
Task.can_be_completed()now usesGetIncompletedTaskPerformersQueryfor RCBA cases (direct users, group members viaGROUP_USERcompletion rows, ignoring inactive users, soft-deleted groups, directly-deleted performers, and stale deletedGROUP_USERrows). Group members completing viacomplete_task_for_userget or create a hiddengroup_userTaskPerformer; analytics andtask_completeevents run in that path, and full task completion usescan_be_completed(by_user=...). API serializers expose onlyuserandgroupperformers.Data model:
TaskPerformeruniqueness becomes(task, user, type)for non-deleted rows (migration included).Side effects: Celery
check_and_complete_tasksruns after user deactivation and when users leave or a group is deleted, auto-completing eligible active group/user tasks as the account owner.Reviewed by Cursor Bugbot for commit 249a04b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Update 'require completion by all' logic to support group performers in task completion
PerformerType.GROUP_USERto track individual group member completions; when a group performer completes a task, aGROUP_USERTaskPerformerrecord is get-or-created for that user.WorkflowActionService._task_can_be_completedandTask.exclude_directly_deleted_taskperformer_setwithTask.can_be_completed(by_user=...), which uses a new raw SQL query (GetIncompletedTaskPerformersQuery) to resolve incomplete performers across both direct and group memberships.complete_taskintocomplete_task_for_user;complete_taskno longer acceptsby_useror emits events.check_and_complete_tasksCelery task, triggered when a user is deactivated or removed/deleted from a group, to asynchronously complete any affected tasks that become completable as a result.USERandGROUPtypes only, excludingGROUP_USERfrom API responses.TaskPerformeruniqueness constraint changes from(user, task)to a conditional(task, user, type)whereis_deleted=False, requiring a schema migration and allowing multiple rows per user/task across types.Macroscope summarized 249a04b.