-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: New preference for Voice call notifications #37505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: a8697ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
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 |
WalkthroughAdds a new user preference "desktopNotificationVoiceCalls": UI toggle in account preferences, preference read in client hooks, server default setting, REST typing and end-to-end test inclusion, and an English translation key. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Preferences UI
participant Hook as useAccountPreferencesValues
participant VOIP as useDesktopNotifications
participant OS as Desktop Notification API
UI->>Hook: toggle `desktopNotificationVoiceCalls` (set preference)
Hook-->>VOIP: preference value (desktopNotificationVoiceCalls)
alt preference enabled
VOIP->>OS: dispatch notification for incoming voice call
OS-->>VOIP: delivered / shown
else preference disabled
VOIP--xOS: abort dispatch (no notification)
note right of VOIP: short-circuited by preference
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37505 +/- ##
===========================================
+ Coverage 68.98% 68.99% +0.01%
===========================================
Files 3358 3358
Lines 114240 114241 +1
Branches 20537 20537
===========================================
+ Hits 78803 78821 +18
+ Misses 33345 33331 -14
+ Partials 2092 2089 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 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.
📒 Files selected for processing (6)
apps/meteor/client/views/account/preferences/PreferencesNotificationsSection.tsx(2 hunks)apps/meteor/client/views/account/preferences/useAccountPreferencesValues.ts(3 hunks)apps/meteor/server/settings/accounts.ts(1 hunks)apps/meteor/tests/end-to-end/api/miscellaneous.ts(1 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/rest-typings/src/v1/users/UsersSetPreferenceParamsPOST.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/server/settings/accounts.ts
📚 Learning: 2025-11-07T14:50:33.544Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37423
File: packages/i18n/src/locales/en.i18n.json:18-18
Timestamp: 2025-11-07T14:50:33.544Z
Learning: Rocket.Chat settings: in apps/meteor/ee/server/settings/abac.ts, the Abac_Cache_Decision_Time_Seconds setting uses invalidValue: 0 as the fallback when ABAC is unlicensed. With a valid license, admins can still set the value to 0 to intentionally disable the ABAC decision cache.
Applied to files:
apps/meteor/server/settings/accounts.ts
🧬 Code graph analysis (1)
apps/meteor/client/views/account/preferences/useAccountPreferencesValues.ts (1)
packages/ui-contexts/src/index.ts (1)
useUserPreference(83-83)
🔇 Additional comments (9)
apps/meteor/tests/end-to-end/api/miscellaneous.ts (1)
196-196: LGTM!The test correctly includes the new
desktopNotificationVoiceCallspreference key in the validation array for the/meendpoint response.packages/rest-typings/src/v1/users/UsersSetPreferenceParamsPOST.ts (2)
57-57: LGTM!The type definition correctly adds the optional boolean field for the new voice call notification preference.
269-272: LGTM!The JSON schema correctly validates the new preference with the appropriate type and nullability settings, consistent with other boolean preferences.
apps/meteor/server/settings/accounts.ts (1)
460-464: LGTM!The server setting is correctly configured with appropriate type, visibility, and default value. The setting enables desktop notifications for voice calls by default (opt-out model), and the translation key
Notification_Desktop_show_voice_callsis properly defined in the i18n files.apps/meteor/client/views/account/preferences/PreferencesNotificationsSection.tsx (2)
97-97: LGTM!The ID declaration follows the established pattern used for other form fields in this component.
155-166: All verifications passed—implementation is complete and correct.
- Translation key
Notification_Desktop_show_voice_callsexists at packages/i18n/src/locales/en.i18n.json:3765- Server preference
Accounts_Default_User_Preferences_desktopNotificationVoiceCallsconfigured at apps/meteor/server/settings/accounts.ts:460- API type definition and schema validation properly updated at packages/rest-typings/src/v1/users/UsersSetPreferenceParamsPOST.ts:57 and 269-271
The toggle is fully integrated with all backend dependencies properly aligned.
apps/meteor/client/views/account/preferences/useAccountPreferencesValues.ts (3)
41-41: LGTM!The type definition is correct and consistent with other boolean preferences in the interface.
81-81: LGTM!The preference retrieval correctly uses
useUserPreferencewith proper typing and matches the field name used in the form.
113-113: LGTM!The new preference is correctly included in the returned object, completing the data flow from user preference to form.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37505 +/- ##
===========================================
- Coverage 68.98% 68.97% -0.01%
===========================================
Files 3359 3359
Lines 114213 114214 +1
Branches 20535 20535
===========================================
- Hits 78788 78780 -8
- Misses 33334 33344 +10
+ Partials 2091 2090 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui-voip/src/v2/useDesktopNotifications.ts (1)
26-26: LGTM! Optional: Consider explicit type parameter.The hook call correctly reads the new preference. For enhanced type safety and consistency with usage patterns elsewhere in the codebase, you could optionally add an explicit type parameter:
- const desktopNotificationsEnabled = useUserPreference('desktopNotificationVoiceCalls'); + const desktopNotificationsEnabled = useUserPreference<boolean>('desktopNotificationVoiceCalls');
📜 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.
📒 Files selected for processing (3)
.changeset/spicy-radios-sleep.md(1 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/ui-voip/src/v2/useDesktopNotifications.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/i18n/src/locales/en.i18n.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
📚 Learning: 2025-11-17T22:38:48.631Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
Applied to files:
.changeset/spicy-radios-sleep.mdpackages/ui-voip/src/v2/useDesktopNotifications.ts
📚 Learning: 2025-11-19T12:32:29.673Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.673Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
.changeset/spicy-radios-sleep.md
📚 Learning: 2025-11-17T14:30:36.271Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 37491
File: packages/desktop-api/src/index.ts:17-27
Timestamp: 2025-11-17T14:30:36.271Z
Learning: In the Rocket.Chat desktop API (`packages/desktop-api/src/index.ts`), the `CustomNotificationOptions` type has an optional `id` field by design. Custom notifications dispatched without an ID cannot be closed programmatically using `closeCustomNotification`, and this is intentional.
Applied to files:
.changeset/spicy-radios-sleep.mdpackages/ui-voip/src/v2/useDesktopNotifications.ts
🧬 Code graph analysis (1)
packages/ui-voip/src/v2/useDesktopNotifications.ts (1)
packages/ui-contexts/src/index.ts (1)
useUserPreference(83-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
packages/ui-voip/src/v2/useDesktopNotifications.ts (3)
1-1: LGTM!The import is correct and aligns with the pattern used elsewhere in the codebase for reading user preferences.
30-32: LGTM!The early return correctly prevents desktop notifications when the preference is disabled. The placement before the
window.RocketChatDesktopchecks is optimal and avoids unnecessary work.
80-80: LGTM!The dependency array correctly includes
desktopNotificationsEnabled, ensuring the effect re-runs when the preference changes. All dependencies are properly tracked..changeset/spicy-radios-sleep.md (1)
1-8: LGTM!The changeset correctly documents the feature addition with appropriate minor version bumps for all affected packages. The description clearly summarizes the new user preference functionality.
Proposed changes (including videos or screenshots)
Issue(s)
VGA-67
Steps to test or reproduce
Further comments
Summary by CodeRabbit