29499 backend [ websockets ] Websockets improvements

[EBirkenfeld]

Unify websocket notifications under EventsConsumer, standardize message envelope, and rename task events to task_created/task_deleted/task_completed across services and routes

• Replace legacy consumers and routes with a single /ws/events endpoint using EventsConsumer
• Standardize websocket payloads to {id, date_created_tsp, type, data} via WebSocketService._send
• Rename event types and tasks: complete_task→task_completed, removed_task→task_deleted, add task_created, event_created, event_updated, and task_completed_websocket
• Update push/websocket services and workflow/task services to emit new types and target events_{user_id} groups
• Update tests and imports to the new task and event names

📍Where to Start

Start with the message envelope and allowed methods in WebSocketService in websockets.py, then review task entrypoints in tasks.py.

Changes since #81 opened

• Refactored WebSocketService._send to remove the message_type parameter and use method_name directly as the message type field [6345ee9]
• Renamed notification tasks send_removed_task_deleted_notification to send_task_deleted_notification and send_removed_task_completed_notification to send_task_completed_websocket throughout the codebase [6345ee9]
• Updated NotificationMethod enum and WebSocketService.ALLOWED_METHODS to add task_completed_websocket and remove notification_created [6345ee9]
• Added WebSocketService.send_task_completed_websocket method and updated send_new_task_websocket to use task_created as method_name [6345ee9]
• Updated test_consumer_send_notification__received test to expect websocket payload type NotificationMethod.comment instead of NotificationMethod.notification_created [6345ee9]

---

📊 Macroscope summarized 0f65d7b. 12 files reviewed, 28 issues evaluated, 26 issues filtered, 0 comments posted

🗂️ Filtered Issues

backend/src/accounts/services/group.py — 0 comments posted, 2 evaluated, 2 filtered

• line 139: send_notification_task.delay(...) will raise AttributeError at runtime because src.notifications.tasks.send_task_deleted_notification has been redefined later in the module without the @shared_task decorator, shadowing the original Celery task object. As a result, the imported send_task_deleted_notification is a plain function lacking .delay. This affects the call at this site in _send_users_notification when send_notification_task is send_task_deleted_notification. [ Already posted ]
• line 154: send_notification_task.delay(...) will raise AttributeError at runtime here because send_task_deleted_notification imported from src.notifications.tasks is a plain function (shadowing the Celery task due to a later redefinition without @shared_task). _send_removed_users_notifications passes this object into _send_users_notification, which then calls .delay on it. [ Already posted ]

backend/src/notifications/services/push.py — 0 comments posted, 2 evaluated, 2 filtered

• line 121: UserNotifications.objects.get(user_id=user_id) in _send_to_apps can raise DoesNotExist if a counter row is missing for the user, aborting push delivery for all app tokens and causing the task to fail. Add a safe fetch (get_or_create or graceful fallback) before incrementing count_unread_push_in_ios_app. [ Low confidence ]
• line 122: Badge counter counter.count_unread_push_in_ios_app is incremented once per app device token inside the loop, causing multiple increments for a single notification when a user has multiple devices. The badge should reflect unread notifications, not number of devices. Move the counter increment outside the token loop and reuse the same value for all device messages. [ Low confidence ]

backend/src/notifications/services/websockets.py — 0 comments posted, 9 evaluated, 8 filtered

• line 71: _sync_send and _async_send assume get_channel_layer() returns a valid layer. If it returns None (misconfiguration or disabled channels), accessing group_send will raise at runtime. Add a guard to fail gracefully or skip sending. [ Low confidence ]
• line 87: _async_send creates a local background_tasks = set() and registers task.add_done_callback(background_tasks.discard), but never adds the task to the set. The callback is a no-op and the local set is discarded when the function returns. This is dead code and provides no cleanup or tracking, potentially hiding intent to retain tasks to avoid premature GC or to manage lifecycle. [ Low confidence ]
• line 89: _async_send uses get_event_loop() and loop.create_task(...) without ensuring a running loop in the current thread. In typical Django sync contexts, get_event_loop() can raise RuntimeError (no loop set) or create tasks that never run. Additionally, exceptions in the task are not handled, causing "Task exception was never retrieved" warnings; and background_tasks is a local set that is never populated (only used in discard callback), providing no retention and no cleanup/visibility. [ Low confidence ]
• line 145: _get_serialized_notification is called with notification that can be None (e.g., in send_overdue_task, send_task_completed, send_resume_workflow, send_delay_workflow, send_due_date_changed). NotificationsSerializer(instance=None).data will attempt to serialize a None instance and raise at runtime in DRF. Guard against None or make the parameter non-optional. [ Low confidence ]
• line 145: Multiple send methods pass notification=None to _get_serialized_notification, which constructs NotificationsSerializer(instance=None) and then accesses .data. In DRF, calling .data with instance=None can raise at runtime (e.g., AssertionError or attribute errors during representation). There are no guards ensuring notification is non-null. [ Low confidence ]
• line 171: send_task_completed_websocket sends the wrong notification type: it uses NotificationMethod.task_completed instead of the more specific NotificationMethod.task_completed_websocket. This breaks the externally visible contract by emitting an event type that does not match the method name and likely downstream expectations for websocket-specific completion events. [ Already posted ]
• line 171: send_task_completed_websocket sends with method_name=NotificationMethod.task_completed instead of the more specific NotificationMethod.task_completed_websocket. This breaks contract parity: clients expecting the websocket-specific event type will not receive it, and ALLOWED_METHODS includes both, making the mismatch detectable. [ Already posted ]
• line 186: Same notification=None risk exists in other methods that default notification to None and immediately serialize it: send_resume_workflow, send_delay_workflow, and send_due_date_changed. Each can raise at runtime when notification is omitted. [ Low confidence ]

backend/src/notifications/tasks.py — 0 comments posted, 3 evaluated, 3 filtered

• line 254: send_task_deleted_notification is defined twice: first as a Celery task (@shared_task(base=NotificationTask)) and then redefined later as a plain function. The second definition overwrites the first module symbol, so callers importing send_task_deleted_notification will see a regular function without the .delay/.apply_async attributes, causing AttributeError at runtime (e.g., when calling send_task_deleted_notification.delay(...)). Celery will have registered the original task object, but the exported symbol no longer references it, breaking the module-level API expected by callers and tests. [ Already posted ]
• line 897: send_event_updated is defined twice: first as a Celery task (@shared_task(base=NotificationTask)) and then redefined later as a plain function. The latter overwrites the task object in the module namespace, so send_event_updated.delay(...) will fail with AttributeError, and code expecting a Celery task will break. [ Already posted ]
• line 904: The condition if new_actions_ids: attempts to evaluate a Django QuerySet (ValuesListQuerySet from WorkflowEventAction.objects.watched().only_ids()) in a boolean context, which raises ValueError: The truth value of a QuerySet is ambiguous. Use an explicit emptiness check, e.g., if new_actions_ids.exists(): or materialize to a list first: ids = list(new_actions_ids); if ids: and then use ids for subsequent operations. [ Out of scope ]

backend/src/processes/services/events.py — 0 comments posted, 3 evaluated, 3 filtered

• line 655: _send_event_updated calls send_event_updated.delay(...). In this same commit, notifications.tasks redefines send_event_updated without the @shared_task decorator, overwriting the decorated task and removing the .delay attribute. Any call here will raise AttributeError: 'function' object has no attribute 'delay' at runtime. [ Already posted ]
• line 777: In update, if force_save is left as default False, the call to self.partial_update(force_save=force_save, **kwargs) only mutates the in-memory instance and defers saving. This method does not call self.save() later, so changes may never persist to the database while notifications and analytics are sent based on unsaved state. [ Low confidence ]
• line 780: When attachments is an empty list in update, with_attachments is set to False, but _update_attachments is not called because the code only runs self._update_attachments(attachments) under if attachments:. Existing attachments remain linked, leaving the database state inconsistent with with_attachments=False and the intended semantics of clearing attachments by passing an empty list. [ Out of scope ]

backend/src/processes/services/tasks/groups.py — 0 comments posted, 1 evaluated, 1 filtered

• line 104: In _delete_group_actions, when no individual (non-group) completed performer exists, the code falls back to a completed group and then does group.users.first().user. group.users.first() returns a UserModel (not an object with a .user attribute). Accessing .user will raise AttributeError. The same pattern appears earlier with .first().user on a TaskPerformer that could be None if no matching performer exists, but here the branch is only taken when completed performers exist; still, the .user after group.users.first() is incorrect. [ Out of scope ]

backend/src/processes/services/tasks/performers.py — 0 comments posted, 1 evaluated, 1 filtered

• line 100: In _delete_actions (TaskPerformersService), the same fallback branch uses group.users.first().user. group.users.first() already returns a UserModel, so accessing .user raises AttributeError at runtime. [ Out of scope ]

backend/src/processes/services/tasks/task_version.py — 0 comments posted, 2 evaluated, 2 filtered

• line 301: When there are no performers after update, default_performer is chosen as account.get_owner() if the workflow is external, otherwise workflow.workflow_starter. If workflow.workflow_starter is None (which is not guarded against), the code will call self.instance.add_raw_performer(default_performer) and self.instance.performers.add(default_performer) with None, and also attempt to access default_performer.id/email/is_new_tasks_subscriber, causing a runtime error (ValueError or AttributeError). Add a guard to ensure a non-null fallback performer or handle the absence explicitly before using it. [ Low confidence ]
• line 376: send_task_deleted_notification is invoked with .delay(...), but in src.notifications.tasks it is defined twice: first as a Celery @shared_task, then later (per the provided diff) redefined without the decorator. The later plain function overrides the task object in the module namespace, so send_task_deleted_notification.delay(...) will raise AttributeError at runtime because the overwritten function doesn't have a delay attribute. Ensure there is a single decorated task definition or import the decorated task under a different name to avoid shadowing. [ Already posted ]

backend/src/processes/services/workflow_action.py — 0 comments posted, 4 evaluated, 3 filtered

• line 96: force_delay_workflow does not validate the date argument. If date <= timezone.now(), computed duration = date - now becomes zero or negative. The code then sets start_date=now and uses this duration for Delay.duration and estimated_end_date = start_date + duration. This yields an immediate or already-expired delay, undermining the intended "force delay until future date" behavior and can cause confusing notifications and state transitions. [ Low confidence ]
• line 131: force_delay_workflow emits external side effects (Celery tasks via .delay and websocket/event dispatch through WorkflowEventService._after_create_actions) inside a transaction.atomic() block. If the transaction later rolls back, notifications and events may already be dispatched, causing inconsistent state for recipients. Use transaction.on_commit(...) to schedule these dispatches after a successful commit. [ Low confidence ]
• line 145: WorkflowEventService.force_delay_workflow_event is called with delay that may be None if self.workflow.tasks.active() yields no tasks. In that case, inside WorkflowEventService.force_delay_workflow_event, DelayEventJsonSerializer(delay).data will be constructed with a None instance, causing a runtime error. The force_delay_workflow method has no guard ensuring at least one active task or validating delay before passing it. [ Low confidence ]

backend/src/processes/services/workflows/workflow_version.py — 0 comments posted, 1 evaluated, 1 filtered

• line 60: send_task_deleted_notification.delay(...) will raise AttributeError at runtime because the imported send_task_deleted_notification symbol is a plain function (it shadows the earlier @shared_task definition due to a later redefinition in src.notifications.tasks). This site invokes .delay when notifying about tasks deleted by version update. [ Already posted ]