Skip to content

29499 backend [ websockets ] Websockets improvements#81

Open
EBirkenfeld wants to merge 9 commits into
masterfrom
backend/websockets/29499__websockets_improvements
Open

29499 backend [ websockets ] Websockets improvements#81
EBirkenfeld wants to merge 9 commits into
masterfrom
backend/websockets/29499__websockets_improvements

Conversation

@EBirkenfeld
Copy link
Copy Markdown
Collaborator

@EBirkenfeld EBirkenfeld commented Nov 25, 2025

1. Description (problem)

This PR completes the refactoring of the WebSocket infrastructure. Previously, the backend maintained several fragmented channels (/ws/notifications, /ws/workflows/new-task, /ws/workflows/removed-task, /ws/workflows/events), each with its own data format and consumer. The removed-task event was ambiguous and caused state conflicts on the client (it was unclear whether the task was removed due to completion or workflow cancellation/return).
Additionally, dataset and group events were being dispatched with a "double-wrapped" payload, breaking API contracts.

2. Context

Moving to a single, unified /ws/events channel simplifies the maintenance of both the web frontend and mobile app: clients no longer need to keep 5 different sockets open, just one. This also eliminates data races and lays the groundwork for future features (e.g., live list updates), as all events are now strictly typed and wrapped in a standard envelope.

3. Solution

  • All legacy WS routes and consumers have been completely removed.
  • All events (comments, mentions, system notifications) are now mapped to the notification_created type.
  • The ambiguous removed_task event has been split into two explicit events: task_completed (when a task is done) and task_deleted (when a task is recalled, returned, or the process is deleted).
  • Dispatch logic is unified in WebSocketService._send(), ensuring a standard envelope format for all messages: {id, date_created_tsp, type, data}.

4. Implementation Details

  • Removed legacy consumer classes and files from consumers.py.
  • Removed legacy socket routes from urls.py.
  • Replaced send_removed_task_notification with send_task_deleted_notification across tasks.py and websockets.py.
  • Eliminated double wrapping ({'data': {'data': ...}}) in send_dataset_*, send_user_*, and send_group_* methods within WebSocketService.
  • Updated mock assertions across all tests (~240 notification tests updated and passing).
  • Updated Public API Wiki documentation.

5. What to Test

5.1 Preconditions

  • Running dev environment with backend, Celery, and Redis active.
  • Two accounts prepared (User A and User B) in the same team.
  • DevTools -> Network -> WS filter opened to monitor incoming messages on the /ws/events channel.

5.2 Positive Scenarios

  1. New Notification (notification_created)
    • User A mentions User B in a task comment.
    • User B receives a type: "notification_created" event over the websocket.
  2. New Task (task_created)
    • User A starts a workflow where the first task is assigned to User B.
    • User B receives a type: "task_created" event with task data inside the payload.
  3. Task Completion (task_completed)
    • User A completes a task assigned to them.
    • The WS log shows a type: "task_completed" event.
  4. Task Deletion (task_deleted)
    • User A returns a task to the previous step (Return to).
    • The current performer of the cancelled task receives a type: "task_deleted" event.
  5. Dataset Update
    • User A renames or alters a dataset structure in settings.
    • A dataset_updated event is pushed to the WS channel with a flat payload (no data.data nesting).

5.3 Negative Scenarios and Edge Cases

  1. Missing connection to legacy channels
    • Attempt to connect to wss://.../ws/notifications or wss://.../ws/workflows/new-task via frontend or Postman.
    • Expected result: Server rejects the connection (404/Disconnect) since the routes are removed.
  2. Network errors and reconnection
    • Simulate an internet connection drop and restore.
    • Expected result: The client should automatically reconnect to /ws/events and correctly resume receiving notifications.

5.4 Verification Points

  • Verify payload structure in DevTools: any incoming event MUST contain id (UUID), date_created_tsp, type (string), and data (object or null).

5.5 API Checks

  • The contract has changed only for outgoing WebSocket messages.
  • The payload for datasets/users/groups is now flat. Example of a correct message:
    {"id": "...", "date_created_tsp": 160000000, "type": "dataset_updated", "data": {"id": 123, "name": "..."}}.

5.6 What Was NOT Tested

  • Locales were not tested (localization logic was untouched).
  • Mobile application was not tested in this PR (requires a separate check by the mobile team as they consume the same websocket feeds).

6. Testing Affected Areas (dependencies)

  • Task Lists (Dashboard / My Tasks): Since task_deleted and task_completed are now split, ensure that task lists (Kanban/Table views) correctly remove task cards upon receiving either of these event types.
  • Unread Indicator (Bell icon): Ensure the counter increments correctly upon receiving notification_created.

7. Refactoring

  • Significant simplification of WebSocketService: 4 different channels consolidated into 1.
  • Removed duplicated code from legacy consumers. Ensure there is no delivery performance degradation (though it should be improved due to multiplexing over a single connection).

8. Commits with Fixes

  • fix(websockets): resolve payload double wrapping and broken mock assertions
  • All commits in branch backend/websockets/29499__websockets_improvements

9. Release notes:

Backend WebSocket infrastructure completely refactored to use a single unified /ws/events channel. Deprecated multi-socket connections have been removed, fixing dataset payload bugs and splitting the ambiguous removed-task event into precise task_completed and task_deleted events.

Note

Consolidate all WebSocket traffic to a single /ws/events endpoint with a uniform message envelope

  • Removes NotificationsConsumer, NewTaskConsumer, RemovedTaskConsumer, and WorkflowEventConsumer; all WebSocket traffic now routes through EventsConsumer at /ws/events.
  • Adds a standard message envelope in WebSocketService._send containing id (uuid4), date_created_tsp, type, and data fields for every outgoing WebSocket message.
  • Renames notification methods and Celery tasks throughout: complete_tasktask_completed, removed_tasktask_deleted, workflow_eventevent_created/event_updated.
  • Adds new tasks and WebSocket methods: send_task_completed_websocket, send_event_created, send_event_updated.
  • Risk: any client connected to the old /ws/notifications, /ws/new_task, /ws/removed_task, or /ws/workflow_event endpoints will stop receiving events; callers that passed pre-wrapped envelopes to _send will now receive double-wrapped payloads (noted for send_dataset_deleted).

Changes since #81 opened

  • Consolidated multiple notification method types into a single notification_created method for WebSocket notifications [eb89a92]
  • Removed metadata envelope wrapper from dataset WebSocket messages [eb89a92]
  • Updated NotificationMethod enum to replace removed_task with notification_created [eb89a92]
📊 Macroscope summarized ee8cec8. 8 files reviewed, 4 issues evaluated, 1 issue filtered, 1 comment posted

🗂️ Filtered Issues

backend/src/notifications/services/websockets.py — 1 comment posted, 3 evaluated, 1 filtered
  • line 31: send_task_completed_websocket (line 173) passes method_name=NotificationMethod.task_completed but the method is named send_task_completed_websocket, suggesting it should use NotificationMethod.task_completed_websocket which is in ALLOWED_METHODS. Using task_completed means consumers cannot distinguish between websocket-specific task completion events and regular task completion events, defeating the purpose of having separate notification method types. [ Already posted ]

Note

High Risk
Breaking change for any client still on old WebSocket URLs or message type values; task-completion realtime behavior changed from removed-task to task-completed websocket in workflow completion paths.

Overview
This PR unifies real-time delivery on a single EventsConsumer channel (events_{user_id}) and standardizes outbound WebSocket payloads.

WebSocket shape and routing: WebSocketService._send now wraps every message with id, date_created_tsp, type, and data. Inbox-style updates (comments, mentions, overdue, etc.) publish as notification_created on the events channel instead of separate notifications_* groups. Task list updates use task_created / task_deleted; workflow timeline uses event_created / event_updated. Legacy consumers (NotificationsConsumer, NewTaskConsumer, RemovedTaskConsumer, WorkflowEventConsumer) are removed—clients must use /ws/events.

Renames and new tasks: Celery/API names shift from send_removed_task_notificationsend_task_deleted_notification, send_complete_task_notificationsend_task_completed_notification, and send_workflow_eventsend_event_created / send_event_updated. Push uses NotificationMethod.task_completed. A new send_task_completed_websocket path replaces some former “removed task” websocket calls when a task is completed (e.g. WorkflowActionService).

Call-site updates: Group membership, performers, workflow actions, and comment/event services are updated to the new task names; tests expect the envelope and /ws/events URLs.

Reviewed by Cursor Bugbot for commit eb89a92. Bugbot is set up for automated code reviews on this repo. Configure here.

…update notification methods

- Removed NewTaskConsumer, NotificationsConsumer, RemovedTaskConsumer, and WorkflowEventConsumer from the websocket and notifications services.
- Added new notification methods for task and event actions in enums.
- Updated WebSocketService to handle new message types and refactored notification sending logic.
- Adjusted WorkflowActionService and related tasks to include is_completed flag for task notifications.
@EBirkenfeld EBirkenfeld self-assigned this Nov 25, 2025
@EBirkenfeld EBirkenfeld added the Backend API changes request label Nov 25, 2025
Comment thread backend/src/notifications/services/websockets.py Outdated
Comment thread backend/src/notifications/tasks.py Outdated
Comment thread backend/src/notifications/tests/test_services/test_websockets.py Outdated
Comment thread backend/src/notifications/tests/test_services/test_websockets.py Outdated
Comment thread backend/src/notifications/tests/test_services/test_websockets.py
Comment thread backend/src/notifications/services/websockets.py Outdated
- Renamed `send_removed_task_deleted_notification` to `send_task_deleted_notification` across the codebase.
- Updated all relevant service and test files to reflect the new method name for consistency.
- Adjusted notification enums and websocket services to accommodate the changes.
Comment thread backend/src/notifications/services/websockets.py
Comment thread backend/src/processes/services/events.py
pneumojoseph
pneumojoseph previously approved these changes Dec 1, 2025
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ee8cec8. Configure here.

Comment thread backend/src/notifications/services/websockets.py
Comment thread backend/src/notifications/services/websockets.py
@EBirkenfeld EBirkenfeld removed the request for review from dahlia-lewis May 25, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend API changes request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants