Skip to content

Support batched notifications#83

Draft
roznawsk wants to merge 2 commits into
mainfrom
fce-3315-notification-batching
Draft

Support batched notifications#83
roznawsk wants to merge 2 commits into
mainfrom
fce-3315-notification-batching

Conversation

@roznawsk

Copy link
Copy Markdown
Member

Description

Describe your changes.

Motivation and Context

Why is this change required? What problem does it solve? If it fixes an open
issue, please link to the issue here.

Documentation impact

  • Documentation update required
  • Documentation updated in another PR
  • No documentation update required

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)

@roznawsk roznawsk requested a review from Copilot June 11, 2026 20:11
@linear

linear Bot commented Jun 11, 2026

Copy link
Copy Markdown

FCE-3315

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds support for Fishjam “batched” webhook notifications by teaching the SDK’s webhook decoder to unpack ServerMessageNotificationBatch, wiring a new room creation option to enable batching, and making the notification delivery path resilient to batches.

Changes:

  • Add RoomOptions.batch_webhook_notifications and pass it through to the backend room config.
  • Extend receive_binary() to return an ordered list when the payload is a ServerMessageNotificationBatch.
  • Update notifier/test plumbing to handle batch-unpacked notifications and add tests around decoding batches.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
fishjam/_webhook_notifier.py Unpacks ServerMessageNotificationBatch and broadens receive_binary() return type to include lists.
fishjam/_ws_notifier.py Robustly handles notification batches on the WebSocket path by unpacking and dispatching each notification.
fishjam/api/_fishjam_client.py Adds batch_webhook_notifications to RoomOptions and forwards it into RoomConfig on room creation.
tests/support/webhook_notifier.py Updates the test webhook server to enqueue each notification when receive_binary() returns a list.
tests/test_webhook_notifier.py New unit tests validating receive_binary() behavior for single messages vs. batches (including filtering unsupported members).
tests/test_notifier.py Adds an integration-style test that enables batched webhook notifications for a room.
tests/test_allowed_notifications.py Clarifies that ServerMessageNotificationBatch is a transport wrapper that is unpacked by receive_binary().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread fishjam/api/_fishjam_client.py
Comment thread fishjam/api/_fishjam_client.py
Comment thread tests/test_notifier.py
Comment on lines +43 to 46
def receive_binary(
binary: bytes,
) -> Union[AllowedNotification, List[AllowedNotification], None]:
"""Transforms a received protobuf notification into a notification instance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants