From c22509eca214f4b1640d314659a2c6e303537497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 6 May 2024 11:59:49 +0200 Subject: [PATCH] Properly unset active channel in the server (#26846) * Properly unset active channel in the server * Address feedback --------- Co-authored-by: Mattermost Build --- server/channels/app/notification.go | 2 +- server/channels/app/notification_push.go | 10 +++++++--- server/channels/app/notification_push_test.go | 19 ++++++++++++++++++- .../sidebar_channel_menu/index.ts | 4 ++-- .../sidebar_channel_menu.test.tsx | 2 +- .../sidebar_channel_menu.tsx | 3 ++- .../src/components/team_controller/index.ts | 3 ++- .../team_controller/team_controller.tsx | 1 + .../mattermost-redux/src/actions/channels.ts | 15 +++++++++++++++ 9 files changed, 49 insertions(+), 10 deletions(-) diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 6e8a8551dc..8f11936b3f 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -618,7 +618,7 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea status = &model.Status{UserId: id, Status: model.StatusOffline, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if statusReason := DoesStatusAllowPushNotification(profileMap[id].NotifyProps, status, post.ChannelId); statusReason == "" { + if statusReason := DoesStatusAllowPushNotification(profileMap[id].NotifyProps, status, post.ChannelId, true); statusReason == "" { a.sendPushNotification( notification, profileMap[id], diff --git a/server/channels/app/notification_push.go b/server/channels/app/notification_push.go index 727c403d72..7acc161852 100644 --- a/server/channels/app/notification_push.go +++ b/server/channels/app/notification_push.go @@ -590,7 +590,7 @@ func (a *App) ShouldSendPushNotification(user *model.User, channelNotifyProps mo return false } - if statusAllowedReason := DoesStatusAllowPushNotification(user.NotifyProps, status, post.ChannelId); statusAllowedReason != "" { + if statusAllowedReason := DoesStatusAllowPushNotification(user.NotifyProps, status, post.ChannelId, false); statusAllowedReason != "" { a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypePush, statusAllowedReason) a.NotificationsLog().Debug("Notification not sent - status", mlog.String("type", model.NotificationTypePush), @@ -648,14 +648,18 @@ func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps m return "" } -func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *model.Status, channelID string) model.NotificationReason { +func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *model.Status, channelID string, isCRT bool) model.NotificationReason { // If User status is DND or OOO return false right away if status.Status == model.StatusDnd || status.Status == model.StatusOutOfOffice { return model.NotificationReasonUserStatus } pushStatus, ok := userNotifyProps[model.PushStatusNotifyProp] - if (pushStatus == model.StatusOnline || !ok) && (status.ActiveChannel != channelID || model.GetMillis()-status.LastActivityAt > model.StatusChannelTimeout) { + sendOnlineNotification := status.ActiveChannel != channelID || //We are in a different channel + model.GetMillis()-status.LastActivityAt > model.StatusChannelTimeout || //It has been a while since we were last active on this channel + isCRT //Is CRT, so being active in a channel doesn't mean you are seeing thread activity + + if (pushStatus == model.StatusOnline || !ok) && sendOnlineNotification { return "" } diff --git a/server/channels/app/notification_push_test.go b/server/channels/app/notification_push_test.go index 7213aa99a9..ff06f5c124 100644 --- a/server/channels/app/notification_push_test.go +++ b/server/channels/app/notification_push_test.go @@ -441,12 +441,14 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { away := &model.Status{UserId: userID, Status: model.StatusAway, Manual: false, LastActivityAt: 0, ActiveChannel: ""} online := &model.Status{UserId: userID, Status: model.StatusOnline, Manual: false, LastActivityAt: model.GetMillis(), ActiveChannel: ""} dnd := &model.Status{UserId: userID, Status: model.StatusDnd, Manual: true, LastActivityAt: model.GetMillis(), ActiveChannel: ""} + activeOnChannel := &model.Status{UserId: userID, Status: model.StatusOnline, Manual: false, LastActivityAt: model.GetMillis(), ActiveChannel: channelID} tt := []struct { name string userNotifySetting string status *model.Status channelID string + isCRT bool expected model.NotificationReason }{ { @@ -491,6 +493,21 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { channelID: "", expected: model.NotificationReasonUserIsActive, }, + { + name: "WHEN props is ONLINE and user is online and active within the channel", + userNotifySetting: model.StatusOnline, + status: activeOnChannel, + channelID: channelID, + expected: model.NotificationReasonUserIsActive, + }, + { + name: "WHEN props is ONLINE and user is online and active within a thread in the channel", + userNotifySetting: model.StatusOnline, + status: activeOnChannel, + channelID: channelID, + expected: "", + isCRT: true, + }, { name: "WHEN props is ONLINE and user is dnd with channel", userNotifySetting: model.StatusOnline, @@ -623,7 +640,7 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { t.Run(tc.name, func(t *testing.T) { userNotifyProps := make(map[string]string) userNotifyProps["push_status"] = tc.userNotifySetting - assert.Equal(t, tc.expected, DoesStatusAllowPushNotification(userNotifyProps, tc.status, tc.channelID)) + assert.Equal(t, tc.expected, DoesStatusAllowPushNotification(userNotifyProps, tc.status, tc.channelID, tc.isCRT)) }) } } diff --git a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/index.ts b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/index.ts index fa82bfbfa6..e94c5ab2d2 100644 --- a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/index.ts +++ b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/index.ts @@ -6,7 +6,7 @@ import type {ConnectedProps} from 'react-redux'; import type {Channel} from '@mattermost/types/channels'; -import {favoriteChannel, unfavoriteChannel, markChannelAsRead} from 'mattermost-redux/actions/channels'; +import {favoriteChannel, unfavoriteChannel, markMultipleChannelsAsRead} from 'mattermost-redux/actions/channels'; import Permissions from 'mattermost-redux/constants/permissions'; import {getCategoryInTeamWithChannel} from 'mattermost-redux/selectors/entities/channel_categories'; import {isFavoriteChannel} from 'mattermost-redux/selectors/entities/channels'; @@ -66,7 +66,7 @@ function mapStateToProps(state: GlobalState, ownProps: OwnProps) { } const mapDispatchToProps = { - markChannelAsRead, + markMultipleChannelsAsRead, markMostRecentPostInChannelAsUnread, favoriteChannel, unfavoriteChannel, diff --git a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.test.tsx b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.test.tsx index ad4410a84f..1f1f256532 100644 --- a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.test.tsx +++ b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.test.tsx @@ -43,7 +43,7 @@ describe('components/sidebar/sidebar_channel/sidebar_channel_menu', () => { onToggleMenu: jest.fn(), multiSelectedChannelIds: [], displayedChannels: [], - markChannelAsRead: jest.fn(), + markMultipleChannelsAsRead: jest.fn(), markMostRecentPostInChannelAsUnread: jest.fn(), favoriteChannel: jest.fn(), unfavoriteChannel: jest.fn(), diff --git a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.tsx b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.tsx index 64ea87bb43..91860f5e3f 100644 --- a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.tsx +++ b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.tsx @@ -37,7 +37,8 @@ const SidebarChannelMenu = (props: Props) => { let markAsReadUnreadMenuItem: JSX.Element | null = null; if (props.isUnread) { function handleMarkAsRead() { - props.markChannelAsRead(props.channel.id, true); + // We use mark multiple to not update the active channel in the server + props.markMultipleChannelsAsRead({[props.channel.id]: Date.now()}); trackEvent('ui', 'ui_sidebar_channel_menu_markAsRead'); } diff --git a/webapp/channels/src/components/team_controller/index.ts b/webapp/channels/src/components/team_controller/index.ts index 5fa897d9f6..886a8ac6a4 100644 --- a/webapp/channels/src/components/team_controller/index.ts +++ b/webapp/channels/src/components/team_controller/index.ts @@ -5,7 +5,7 @@ import {connect} from 'react-redux'; import type {ConnectedProps} from 'react-redux'; import type {RouteComponentProps} from 'react-router-dom'; -import {fetchAllMyTeamsChannelsAndChannelMembersREST, fetchChannelsAndMembers} from 'mattermost-redux/actions/channels'; +import {fetchAllMyTeamsChannelsAndChannelMembersREST, fetchChannelsAndMembers, unsetActiveChannelOnServer} from 'mattermost-redux/actions/channels'; import {getCurrentChannelId} from 'mattermost-redux/selectors/entities/channels'; import {getLicense, getConfig} from 'mattermost-redux/selectors/entities/general'; import {getCurrentTeamId, getMyTeams} from 'mattermost-redux/selectors/entities/teams'; @@ -55,6 +55,7 @@ const mapDispatchToProps = { markChannelAsReadOnFocus, initializeTeam, joinTeam, + unsetActiveChannelOnServer, }; const connector = connect(mapStateToProps, mapDispatchToProps); diff --git a/webapp/channels/src/components/team_controller/team_controller.tsx b/webapp/channels/src/components/team_controller/team_controller.tsx index 6f065fd094..8dc3113dea 100644 --- a/webapp/channels/src/components/team_controller/team_controller.tsx +++ b/webapp/channels/src/components/team_controller/team_controller.tsx @@ -104,6 +104,7 @@ function TeamController(props: Props) { function handleBlur() { window.isActive = false; blurTime.current = Date.now(); + props.unsetActiveChannelOnServer(); } function handleKeydown(event: KeyboardEvent) { diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts index 44255dc306..edd8c90f88 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts @@ -701,6 +701,21 @@ export function updateApproximateViewTime(channelId: string): ActionFuncAsync { }; } +export function unsetActiveChannelOnServer(): ActionFuncAsync { + return async (dispatch, getState) => { + try { + // The view channel api in the server handles the active channel + await Client4.viewMyChannel(''); + } catch (error) { + forceLogoutIfNecessary(error, dispatch, getState); + dispatch(logError(error)); + return {data: false}; + } + + return {data: true}; + }; +} + export function readMultipleChannels(channelIds: string[]): ActionFuncAsync { return async (dispatch, getState) => { let response;