Skip to content

Conversation

@juliajforesti
Copy link
Contributor

@juliajforesti juliajforesti commented Sep 23, 2025

Proposed changes (including videos or screenshots)

Adjust custom sound loop on Omnichannel continuous notification to play New room loop instead of New message loop

Issue(s)

Steps to test or reproduce

Further comments

CORE-1394

Summary by CodeRabbit

  • Bug Fixes

    • Omnichannel/Livechat continuous notifications now loop the “New room” sound when there are unread rooms, instead of looping the “New message” sound. This reduces repetitive message alerts and aligns the loop with room-level activity. One-off message sounds remain unchanged.
  • Chores

    • Updated related packages to include the new notification behavior.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 23, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 7.12.0, but it targets 7.11.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Sep 23, 2025

🦋 Changeset detected

Latest commit: 765f87c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 42 packages
Name Type
@rocket.chat/ui-contexts Patch
@rocket.chat/meteor Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/http-router Patch
@rocket.chat/model-typings Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/federation-service Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Walkthrough

Switches omnichannel continuous notification loop from “New message” to “New room.” Updates CustomSoundProvider API and UI context to expose playNewRoomLoop and remove playNewMessageLoop. Adjusts client hook to call the new room loop. Adds a changeset patch for @rocket.chat/ui-contexts and @rocket.chat/meteor.

Changes

Cohort / File(s) Summary
Client sound behavior & provider
apps/meteor/client/hooks/useOmnichannelContinuousSoundNotification.ts, apps/meteor/client/providers/CustomSoundProvider/CustomSoundProvider.tsx
Hook now invokes room loop instead of message loop for continuous notifications; provider adds playNewRoomLoop, removes playNewMessageLoop, and loops the room notification sound.
UI context contract update
packages/ui-contexts/src/CustomSoundContext.ts
CustomSoundContextValue updated: add playNewRoomLoop, remove playNewMessageLoop; default context updated accordingly.
Release metadata
.changeset/grumpy-points-deliver.md
Adds changeset patching @rocket.chat/ui-contexts and @rocket.chat/meteor to reflect sound loop change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Agent as Agent UI
  participant Hook as useOmnichannelContinuousSoundNotification
  participant Ctx as CustomSoundContext
  participant Provider as CustomSoundProvider
  participant Audio as AudioEngine

  Agent->>Hook: Unread Livechat room(s) detected
  Hook->>Ctx: notificationSounds.playNewRoomLoop()
  Ctx->>Provider: Delegate playNewRoomLoop
  Provider->>Audio: Start loop("newRoomNotification")
  Note over Audio: Continuous playback until cleared

  Agent->>Hook: No unread rooms / unmount
  Hook->>Ctx: notificationSounds.stop()
  Ctx->>Provider: Delegate stop
  Provider->>Audio: Stop playback
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A bunny drums a steady tune,
Not message pings, but rooms that croon—
A looping hop through queueing doors,
New burrows beep on rabbit floors.
With ears a-twitch, I nudge the loom:
“Play the room! Play the room!” 🐇🎶

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “fix: wrong sound on Omnichannel continuous notification” succinctly identifies that the pull request resolves an incorrect notification sound playback in the Omnichannel (Livechat) continuous notification feature, directly reflecting the core change of switching from the "New message" loop to the "New room" loop. It uses clear, descriptive language without extraneous details or file lists, and aligns with the developer’s primary objective as outlined in the PR description.
Linked Issues Check ✅ Passed The changes fully address CORE-1394 by modifying the omnichannel continuous notification hook to invoke playNewRoomLoop instead of playNewMessageLoop, updating the CustomSoundProvider to include the new room loop method and removing the message loop, and adjusting the CustomSoundContext interface and defaults accordingly. These code modifications ensure that when Livechat continuous sound notifications are enabled and there are unread rooms, the correct "New room" sound is played, meeting the linked issue’s requirement.
Out of Scope Changes Check ✅ Passed All modifications in this pull request are focused on updating the continuous notification sound logic for Omnichannel and adjusting related context and provider code; there are no unrelated features, refactors, or extraneous changes outside the scope of switching the loop sound as specified in the linked issue.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/customSound

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 411f130 and 1f5f93b.

📒 Files selected for processing (4)
  • .changeset/grumpy-points-deliver.md (1 hunks)
  • apps/meteor/client/hooks/useOmnichannelContinuousSoundNotification.ts (1 hunks)
  • apps/meteor/client/providers/CustomSoundProvider/CustomSoundProvider.tsx (1 hunks)
  • packages/ui-contexts/src/CustomSoundContext.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/client/providers/CustomSoundProvider/CustomSoundProvider.tsx (1)
apps/meteor/client/providers/CustomSoundProvider/lib/helpers.ts (1)
  • formatVolume (49-51)
🔇 Additional comments (5)
.changeset/grumpy-points-deliver.md (1)

1-6: LGTM!

The changeset correctly documents the patch-level changes to both affected packages with a clear description of the functionality change.

packages/ui-contexts/src/CustomSoundContext.ts (2)

33-33: LGTM!

The playNewRoomLoop method is correctly added to the type definition, following the same signature pattern as other notification sound methods.


63-63: LGTM!

The default implementation for playNewRoomLoop is consistent with the pattern used for other methods in the context.

apps/meteor/client/hooks/useOmnichannelContinuousSoundNotification.ts (1)

22-22: LGTM!

The hook correctly calls playNewRoomLoop() to play the continuous room notification sound, which aligns with the PR objective to switch from the message loop to the room loop.

apps/meteor/client/providers/CustomSoundProvider/CustomSoundProvider.tsx (1)

78-78: LGTM!

The playNewRoomLoop implementation correctly plays the room notification sound with looping enabled, using the appropriate volume setting. This properly supports the continuous notification functionality required by the omnichannel feature.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@juliajforesti juliajforesti added this to the 7.12.0 milestone Sep 23, 2025
@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.41%. Comparing base (d8270d8) to head (765f87c).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #37042      +/-   ##
===========================================
+ Coverage    67.40%   67.41%   +0.01%     
===========================================
  Files         3331     3329       -2     
  Lines       113519   113379     -140     
  Branches     20604    20713     +109     
===========================================
- Hits         76523    76440      -83     
+ Misses       34392    34338      -54     
+ Partials      2604     2601       -3     
Flag Coverage Δ
e2e 57.33% <0.00%> (+0.01%) ⬆️
unit 71.16% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

Makes sense to keep playNewMessageLoop ?

@juliajforesti juliajforesti marked this pull request as ready for review October 1, 2025 17:43
@juliajforesti juliajforesti requested a review from a team as a code owner October 1, 2025 17:43
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

LGTM!

@dougfabris dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label Oct 2, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Oct 2, 2025
@kodiakhq kodiakhq bot merged commit a25e88c into develop Oct 2, 2025
49 checks passed
@kodiakhq kodiakhq bot deleted the fix/customSound branch October 2, 2025 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants