From 4c6ac0a432ecf8eab08568f6f394bb9f0f36fff5 Mon Sep 17 00:00:00 2001 From: M-ZubairAhmed Date: Fri, 27 Sep 2024 18:17:23 +0000 Subject: [PATCH] [MM-60218] Resettting to default doesnt reset to user's global notification setting in a channel's setting (#28046) --- .../channel_notifications_spec.js | 117 +- .../src/actions/notification_actions.jsx | 67 +- .../src/actions/notification_actions.test.js | 166 +- .../channel_notifications_modal.test.tsx.snap | 2254 +++++++++-------- .../channel_notifications_modal.scss | 7 +- .../channel_notifications_modal.test.tsx | 813 +++++- .../channel_notifications_modal.tsx | 408 +-- .../__snapshots__/index.test.tsx.snap | 15 + .../reset_to_default_button/index.test.tsx | 173 ++ .../reset_to_default_button/index.tsx | 117 + .../channel_notifications_modal/utils.tsx | 123 +- .../user_settings_notifications.tsx | 9 +- .../modals/components/base_setting_item.tsx | 8 +- .../components/checkbox_setting_item.tsx | 2 + .../modals/components/radio_setting_item.tsx | 4 +- webapp/channels/src/i18n/en.json | 12 +- webapp/channels/src/utils/constants.tsx | 1 + .../src/utils/notification_sounds.tsx | 52 +- webapp/platform/types/src/channels.ts | 4 +- webapp/platform/types/src/users.ts | 4 +- 20 files changed, 2839 insertions(+), 1517 deletions(-) create mode 100644 webapp/channels/src/components/channel_notifications_modal/reset_to_default_button/__snapshots__/index.test.tsx.snap create mode 100644 webapp/channels/src/components/channel_notifications_modal/reset_to_default_button/index.test.tsx create mode 100644 webapp/channels/src/components/channel_notifications_modal/reset_to_default_button/index.tsx diff --git a/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/channel_notifications_spec.js b/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/channel_notifications_spec.js index 4263a5d522..cf267b3035 100644 --- a/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/channel_notifications_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/channel_notifications_spec.js @@ -142,82 +142,97 @@ describe('CRT Desktop notifications', () => { it('MM-T4417_2 Click on sameMobileSettingsDesktop and check if additional settings still appears', () => { cy.visit(testChannelUrl); + + // # Open channel's notification preferences cy.uiOpenChannelMenu('Notification Preferences'); - cy.get('#desktopNotification-mention').scrollIntoView().should('be.visible').click().then(() => { - cy.get('[data-testid="desktopReplyThreads"]').scrollIntoView().should('be.visible').click(); - }); - cy.get('.channel-notifications-settings-modal__body').scrollTo('center').get('[data-testid="desktopReplyThreads"]').should('be.visible').click(); - cy.get('.channel-notifications-settings-modal__body').get('[data-testid="sameMobileSettingsDesktop"]').scrollIntoView().click().should('be.checked').then(() => { - cy.findByText('Notify me about…').should('not.be.visible'); + // * As per previous conditions the mobile and desktop settings should be the same + cy.get('[data-testid="sameMobileSettingsDesktop"]').scrollIntoView().should('be.checked'); + + // * Verify that Notify me about section of mobile settings is not visible + cy.get('[data-testid="mobile-notify-me-radio-section"]').should('not.exist'); + + // # Now uncheck the sameMobileSettingsDesktop so that mobile and desktop settings are different + cy.get('[data-testid="sameMobileSettingsDesktop"]').scrollIntoView().should('be.visible').click(); + + // * Verify that Notify me about section of mobile settings is visible + cy.get('[data-testid="mobile-notify-me-radio-section"]').should('be.visible').scrollIntoView().within(() => { + cy.findByText('Notify me about…').should('be.visible'); + + // # Click on mentions option + cy.get('[data-testid="MobileNotification-mention"]').should('be.visible').click(); }); - // check the box to see if the additional settings appears - cy.get('.channel-notifications-settings-modal__body').get('[data-testid="sameMobileSettingsDesktop"]').scrollIntoView().click(); - cy.get('.mm-modal-generic-section-item__title').should('be.visible').and('contain', 'Notify me about'); - - cy.get('#MobileNotification-all').should('be.visible').click(); - cy.get('#MobileNotification-mention').should('be.visible').click().then(() => { - cy.get('[data-testid="mobileReplyThreads"]').should('be.visible').click(); + // * Verify that Thread reply notifications section of mobile settings is visible + cy.get('[data-testid="mobile-reply-threads-checkbox-section"]').should('be.visible').scrollIntoView().within(() => { + cy.findByText('Notify me about replies to threads I’m following').should('be.visible'); }); - cy.get('#MobileNotification-none').should('be.visible').click(); - cy.get('[data-testid="autoFollowThreads"]').should('be.visible').click(); - - // # Save the changes - cy.findByText('Save').should('be.visible').click(); + // # Close the modal + cy.get('body').type('{esc}'); }); it('MM-T4417_3 Trigger notifications only on mention replies when channel setting is unchecked', () => { + // # Visit the test channel cy.visit(testChannelUrl); - // Setup notification spy - spyNotificationAs('notifySpy', 'granted'); + // # Setup notification spy + spyNotificationAs('notifySpy1', 'granted'); + + // # Open channel's notification preferences for test channel cy.uiOpenChannelMenu('Notification Preferences'); + + // # Select "Mentions, direct messages, and keywords only" as notify me about option cy.get('#desktopNotification-mention').scrollIntoView().should('be.visible').click(); + // # Unselect "Notify me about replies to threads I’m following" + cy.get('[data-testid="desktopReplyThreads"]').scrollIntoView().should('be.visible').then(($el) => { + if ($el.is(':checked')) { + cy.wrap($el).click(); + } + }); + + // # Select notification checkbox active + cy.get('[data-testid="desktopNotificationSoundsCheckbox"]').scrollIntoView().should('be.visible').then(($el) => { + if (!$el.is(':checked')) { + cy.wrap($el).click(); + } + }); + + // # Select a notification sound from dropdown + cy.get('#desktopNotificationSoundsSelect').scrollIntoView().should('be.visible').click(); + cy.findByText('Crackle').should('be.visible').click(); + // # Save the changes cy.findByText('Save').should('be.visible').click(); - // # Post a root message as other user - cy.postMessageAs({sender, message: 'This is a not followed root message', channelId: testChannelId, rootId: ''}).then(({id: postId}) => { - // # Switch to town-square so that unread notifications in test channel may be triggered - cy.uiClickSidebarItem('town-square'); + // # Go to the town-square channel + cy.visit(`/${testTeam.name}/channels/town-square`); - // # Post a message in unfollowed thread as another user - cy.postMessageAs({sender, message: 'This is a reply to the unfollowed thread', channelId: testChannelId, rootId: postId}); + // # Post a root message as other user in the test channel + cy.postMessageAs({sender, message: 'This is the root message which will not have a at-mention in thread', channelId: testChannelId, rootId: ''}).then(({id: postId}) => { + // # Post a message in the thread without at-mention + cy.postMessageAs({sender, message: 'Reply without at-mention', channelId: testChannelId, rootId: postId}).then(() => { + // * Verify Notification stub was not called for threads which does not have at-mention as per channel settings + cy.get('@notifySpy1').should('not.be.called'); + }); - // * Verify stub was not called for unfollowed thread - cy.get('@notifySpy').should('not.be.called'); + // # Cleanup + cy.apiDeletePost(postId); }); - // # Visit channel - cy.visit(testChannelUrl); + // Setup another notification spy + spyNotificationAs('notifySpy2', 'granted'); - // Setup notification spy - spyNotificationAs('notifySpy', 'granted'); - - // # Post a message - cy.postMessage('Hi there, this is a root message'); - - // # Get post id of message - cy.getLastPostId().then((postId) => { - // # Switch to town-square so that unread notifications in test channel may be triggered - cy.uiClickSidebarItem('town-square'); - - // # Post a message in original thread as another user - cy.postMessageAs({sender, message: 'This is a reply to the root post', channelId: testChannelId, rootId: postId}); - - // * Verify stub was not called - cy.get('@notifySpy').should('not.be.called'); - - // # Post a mention message in original thread as another user - const message = `@${receiver.username} this is a mention to receiver`; + // # Post another message in the test channel + cy.postMessageAs({sender, message: 'This is another root message which will have a at-mention in thread', channelId: testChannelId, rootId: ''}).then(({id: postId}) => { + const message = `Reply with at-mention @${receiver.username}`; + // # Post a message in the thread with at-mention cy.postMessageAs({sender, message, channelId: testChannelId, rootId: postId}); - // * Verify stub was called with correct title and body - cy.get('@notifySpy').should('have.been.calledWithMatch', `Reply in ${testChannelName}`, (args) => { + // * Verify Notification stub was called with correct title and body with at-mention as per channel settings + cy.get('@notifySpy2').should('have.been.calledWithMatch', `Reply in ${testChannelName}`, (args) => { expect(args.body, `Notification body: "${args.body}" should match: "${message}"`).to.equal(`@${sender.username}: ${message}`); return true; }); diff --git a/webapp/channels/src/actions/notification_actions.jsx b/webapp/channels/src/actions/notification_actions.jsx index 03f1c0d941..88a5ca60c4 100644 --- a/webapp/channels/src/actions/notification_actions.jsx +++ b/webapp/channels/src/actions/notification_actions.jsx @@ -18,11 +18,11 @@ import {getChannelURL, getPermalinkURL} from 'selectors/urls'; import {isThreadOpen} from 'selectors/views/threads'; import {getHistory} from 'utils/browser_history'; -import Constants, {NotificationLevels, UserStatuses, IgnoreChannelMentions} from 'utils/constants'; +import Constants, {NotificationLevels, UserStatuses, IgnoreChannelMentions, DesktopSound} from 'utils/constants'; import DesktopApp from 'utils/desktop_api'; import {stripMarkdown, formatWithRenderer} from 'utils/markdown'; import MentionableRenderer from 'utils/markdown/mentionable_renderer'; -import * as NotificationSounds from 'utils/notification_sounds'; +import {DesktopNotificationSounds, ding} from 'utils/notification_sounds'; import {showNotification} from 'utils/notifications'; import {cjkrPattern, escapeRegex} from 'utils/text_formatting'; import {isDesktopApp, isMobileApp} from 'utils/user_agent'; @@ -30,21 +30,52 @@ import * as Utils from 'utils/utils'; import {runDesktopNotificationHooks} from './hooks'; -const getSoundFromChannelMemberAndUser = (member, user) => { - if (member?.notify_props?.desktop_sound) { - return member.notify_props.desktop_sound === 'on'; +/** + * This function is used to determine if the desktop sound is enabled. + * It checks if desktop sound is defined in the channel member and if not, it checks if it's defined in the user preferences. + */ +export function isDesktopSoundEnabled(channelMember, user) { + const soundInChannelMemberNotifyProps = channelMember?.notify_props?.desktop_sound; + const soundInUserNotifyProps = user?.notify_props?.desktop_sound; + + if (soundInChannelMemberNotifyProps === DesktopSound.ON) { + return true; } - return !user.notify_props || user.notify_props.desktop_sound === 'true'; -}; - -const getNotificationSoundFromChannelMemberAndUser = (member, user) => { - if (member?.notify_props?.desktop_notification_sound) { - return member.notify_props.desktop_notification_sound; + if (soundInChannelMemberNotifyProps === DesktopSound.OFF) { + return false; } - return user.notify_props?.desktop_notification_sound ? user.notify_props.desktop_notification_sound : 'Bing'; -}; + if (soundInChannelMemberNotifyProps === DesktopSound.DEFAULT) { + return soundInUserNotifyProps ? soundInUserNotifyProps === 'true' : true; + } + + if (soundInUserNotifyProps) { + return soundInUserNotifyProps === 'true'; + } + + return true; +} + +/** + * This function returns the desktop notification sound from the channel member and user. + * It checks if desktop notification sound is defined in the channel member and if not, it checks if it's defined in the user preferences. + * If neither is defined, it returns the default sound 'BING'. + */ +export function getDesktopNotificationSound(channelMember, user) { + const notificationSoundInChannelMember = channelMember?.notify_props?.desktop_notification_sound; + const notificationSoundInUser = user?.notify_props?.desktop_notification_sound; + + if (notificationSoundInChannelMember && notificationSoundInChannelMember !== DesktopNotificationSounds.DEFAULT) { + return notificationSoundInChannelMember; + } + + if (notificationSoundInUser && notificationSoundInUser !== DesktopNotificationSounds.DEFAULT) { + return notificationSoundInUser; + } + + return DesktopNotificationSounds.BING; +} /** * @returns {import('mattermost-redux/types/actions').ThunkActionFunc, GlobalState>} @@ -259,7 +290,7 @@ export function sendDesktopNotification(post, msgProps) { } //Play a sound if explicitly set in settings - const sound = getSoundFromChannelMemberAndUser(member, user); + const desktopSoundEnabled = isDesktopSoundEnabled(member, user); // Notify if you're not looking in the right channel or when // the window itself is not active @@ -285,7 +316,7 @@ export function sendDesktopNotification(post, msgProps) { notify = true; } - let soundName = getNotificationSoundFromChannelMemberAndUser(member, user); + let soundName = getDesktopNotificationSound(member, user); const updatedState = getState(); let url = getChannelURL(updatedState, channel, teamId); @@ -295,7 +326,7 @@ export function sendDesktopNotification(post, msgProps) { } // Allow plugins to change the notification, or re-enable a notification - const args = {title, body, silent: !sound, soundName, url, notify}; + const args = {title, body, silent: !desktopSoundEnabled, soundName, url, notify}; const hookResult = await dispatch(runDesktopNotificationHooks(post, msgProps, channel, teamId, args)); if (hookResult.error) { dispatch(logError(hookResult.error)); @@ -309,8 +340,8 @@ export function sendDesktopNotification(post, msgProps) { const result = dispatch(notifyMe(title, body, channel, teamId, silent, soundName, url)); //Don't add extra sounds on native desktop clients - if (sound && !isDesktopApp() && !isMobileApp()) { - NotificationSounds.ding(soundName); + if (desktopSoundEnabled && !isDesktopApp() && !isMobileApp()) { + ding(soundName); } return result; diff --git a/webapp/channels/src/actions/notification_actions.test.js b/webapp/channels/src/actions/notification_actions.test.js index 0ee8e91984..e0d70971a3 100644 --- a/webapp/channels/src/actions/notification_actions.test.js +++ b/webapp/channels/src/actions/notification_actions.test.js @@ -7,7 +7,7 @@ import Constants, {NotificationLevels, UserStatuses} from 'utils/constants'; import * as NotificationSounds from 'utils/notification_sounds'; import * as utils from 'utils/notifications'; -import {sendDesktopNotification} from './notification_actions'; +import {sendDesktopNotification, isDesktopSoundEnabled, getDesktopNotificationSound} from './notification_actions'; describe('notification_actions', () => { describe('sendDesktopNotification', () => { @@ -195,7 +195,7 @@ describe('notification_actions', () => { expect(spy).toHaveBeenCalledWith({ body: '@username: Where is Jessica Hyde?', requireInteraction: false, - silent: true, + silent: false, title: 'Utopia', onClick: expect.any(Function), }); @@ -415,7 +415,7 @@ describe('notification_actions', () => { expect(spy).toHaveBeenCalledWith({ body: '@username: Where is Jessica Hyde?', requireInteraction: false, - silent: true, + silent: false, title: 'Reply in Utopia', onClick: expect.any(Function), }); @@ -509,3 +509,163 @@ describe('notification_actions', () => { }); }); }); + +describe('isDesktopSoundEnabled', () => { + test('should return channel member sound if it exists', () => { + const channelMember1 = { + notify_props: { + desktop_sound: 'on', + }, + }; + const user1 = { + notify_props: { + desktop_sound: 'false', + }, + }; + expect(isDesktopSoundEnabled(channelMember1, user1)).toBe(true); + + const channelMember2 = { + notify_props: { + desktop_sound: 'off', + }, + }; + const user2 = { + notify_props: { + desktop_sound: 'false', + }, + }; + expect(isDesktopSoundEnabled(channelMember2, user2)).toBe(false); + + const channelMember3 = { + notify_props: { + desktop_sound: 'default', + }, + }; + const user3 = { + notify_props: { + desktop_sound: 'false', + }, + }; + expect(isDesktopSoundEnabled(channelMember3, user3)).toBe(false); + + const channelMember4 = { + notify_props: { + desktop_sound: 'default', + }, + }; + const user4 = { + notify_props: { + desktop_sound: 'true', + }, + }; + expect(isDesktopSoundEnabled(channelMember4, user4)).toBe(true); + + const channelMember5 = { + notify_props: { + desktop_sound: 'on', + }, + }; + const user5 = { + notify_props: { + desktop_sound: '', + }, + }; + expect(isDesktopSoundEnabled(channelMember5, user5)).toBe(true); + }); + + test('should return user sound if channel member sound is not defined', () => { + const channelMember1 = { + notify_props: { + desktop_sound: '', + }, + }; + const user1 = { + notify_props: { + desktop_sound: 'true', + }, + }; + expect(isDesktopSoundEnabled(channelMember1, user1)).toBe(true); + + const channelMember2 = { + notify_props: { + desktop_sound: '', + }, + }; + const user2 = { + notify_props: { + desktop_sound: 'false', + }, + }; + expect(isDesktopSoundEnabled(channelMember2, user2)).toBe(false); + + const channelMember3 = { + notify_props: {}, + }; + const user3 = { + notify_props: { + desktop_sound: 'false', + }, + }; + expect(isDesktopSoundEnabled(channelMember3, user3)).toBe(false); + }); + + test('should return default if both channel member and user are not defined', () => { + const channelMember = {}; + const user = {}; + expect(isDesktopSoundEnabled(channelMember, user)).toBe(true); + }); +}); + +describe('getDesktopNotificationSound', () => { + test('should return channel member notification sound if it exists', () => { + const channelMember1 = { + notify_props: { + desktop_notification_sound: 'default', + }, + }; + const user1 = { + notify_props: { + desktop_notification_sound: 'Crackle', + }, + }; + expect(getDesktopNotificationSound(channelMember1, user1)).toBe('Crackle'); + + const channelMember2 = { + notify_props: { + desktop_notification_sound: 'default', + }, + }; + const user2 = { + notify_props: { + desktop_notification_sound: '', + }, + }; + expect(getDesktopNotificationSound(channelMember2, user2)).toBe('Bing'); + + const channelMember3 = { + notify_props: { + desktop_notification_sound: 'Crackle', + }, + }; + const user3 = { + notify_props: { + desktop_notification_sound: 'Bing', + }, + }; + expect(getDesktopNotificationSound(channelMember3, user3)).toBe('Crackle'); + }); + + test('should return user notification sound if channel member sound is not defined', () => { + const channelMember1 = {}; + const user1 = { + notify_props: { + desktop_notification_sound: 'Crackle', + }, + }; + expect(getDesktopNotificationSound(channelMember1, user1)).toBe('Crackle'); + + const channelMember2 = {}; + const user2 = {}; + expect(getDesktopNotificationSound(channelMember2, user2)).toBe('Bing'); + }); +}); diff --git a/webapp/channels/src/components/channel_notifications_modal/__snapshots__/channel_notifications_modal.test.tsx.snap b/webapp/channels/src/components/channel_notifications_modal/__snapshots__/channel_notifications_modal.test.tsx.snap index c4148ed0e4..6a7dfc6a0b 100644 --- a/webapp/channels/src/components/channel_notifications_modal/__snapshots__/channel_notifications_modal.test.tsx.snap +++ b/webapp/channels/src/components/channel_notifications_modal/__snapshots__/channel_notifications_modal.test.tsx.snap @@ -1,512 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`components/channel_notifications_modal/ChannelNotificationsModal should Ignore mentions for @channel, @here and @all 1`] = ` -Object { - "asFragment": [Function], - "baseElement": - + + +
+
+
+

+ Follow all threads in this channel +

+

+ When enabled, all new replies in this channel will be automatically followed and will appear in your Threads view. +

+
+
-

- Notify me about… -

- -
@@ -949,7 +419,7 @@ Object { } `; -exports[`components/channel_notifications_modal/ChannelNotificationsModal should check the options in the mobile notifications 1`] = ` +exports[`ChannelNotificationsModal should check the options in the mobile notifications 1`] = ` Object { "asFragment": [Function], "baseElement": Desktop Notifications +

- All new messages + All new messages (default)

@@ -1267,20 +746,11 @@ Object { Mobile Notifications @@ -1319,6 +789,7 @@ Object {

- All new messages + All new messages + +

+
+ + +
+
+
+

+ Follow all threads in this channel +

+

+ When enabled, all new replies in this channel will be automatically followed and will appear in your Threads view. +

+
+
+
+
+
+
@@ -1455,7 +976,7 @@ Object { } `; -exports[`components/channel_notifications_modal/ChannelNotificationsModal should not show other settings if channel is mute 1`] = ` +exports[`ChannelNotificationsModal should not show other settings if channel is mute 1`] = ` Object { "asFragment": [Function], "baseElement":
+
+
+
+

+ Follow all threads in this channel +

+

+ When enabled, all new replies in this channel will be automatically followed and will appear in your Threads view. +

+
+
+
+
+
+ +
+
+
+
+
Desktop Notifications +

- All new messages + All new messages (default)

@@ -2028,20 +1607,11 @@ Object { Mobile Notifications @@ -2068,6 +1638,7 @@ Object { class="mm-modal-generic-section-item__fieldset-checkbox" >
- -
- - - -
- - - - - , - "container":