MM-59952/MM-61438 Fix web app not responding to WS events for status (#29371)

* MM-59952/MM-61438 Fix web app not responding to WS events for status

* Add E2E tests

e2e-tests/cypress/tests/integration/channels/status/status_updates_spec.ts

@@ -0,0 +1,103 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+// ***************************************************************
+// - [#] indicates a test step (e.g. # Go to a page)
+// - [*] indicates an assertion (e.g. * Check the title)
+// - Use element ID when selecting an element. Create one if none.
+// ***************************************************************
+
+// Group: @channels @status
+
+describe('Status of current user', () => {
+    before(() => {
+        // # Login as test user and visit channel
+        cy.apiInitSetup({loginAfter: true}).then(({team, channel}) => {
+            cy.visit(`/${team.name}/channels/${channel.name}`);
+        });
+    });
+
+    it("Changes to the current user's status made from the profile/status dropdown should be shown in real time", () => {
+        // * The user's status should start as online
+        verifyStatus('online');
+
+        // # Open status menu
+        cy.uiGetSetStatusButton().click();
+
+        // # Change status to away
+        cy.findByText('Away').click();
+
+        // * The status should be updated to away
+        verifyStatus('away');
+
+        // # Open status menu
+        cy.uiGetSetStatusButton().click();
+
+        // # Change status to offline
+        cy.findByText('Offline').click();
+
+        // * The status should be updated to offline
+        verifyStatus('offline');
+
+        // # Open status menu
+        cy.uiGetSetStatusButton().click();
+
+        // # Change status back to online
+        cy.findByText('Online').click();
+
+        // * The status should be updated to online
+        verifyStatus('online');
+    });
+
+    it("Changes to the current user's status made using slash commands should be shown in real time", () => {
+        // * The user's status should start as online
+        verifyStatus('online');
+
+        // # Change status to away
+        cy.postMessage('/away ');
+
+        // * Wait for the response from the server
+        cy.findByText('You are now away').should('exist');
+
+        // * The status should be updated to away
+        verifyStatus('away');
+
+        // # Change status to offline
+        cy.postMessage('/offline ');
+
+        // * Wait for the response from the server
+        cy.findByText('You are now offline').should('exist');
+
+        // * The status should be updated to offline
+        verifyStatus('offline');
+
+        // # Change status to do not disturb
+        cy.postMessage('/dnd ');
+
+        // * Wait for the response from the server
+        cy.findByText('Do Not Disturb is enabled. You will not receive desktop or mobile push notifications until Do Not Disturb is turned off.').should('exist');
+
+        // * The status should be updated to offline
+        verifyStatus('dnd');
+
+        // # Change status back to online
+        cy.postMessage('/online ');
+
+        // * Wait for the response from the server
+        cy.findByText('You are now online').should('exist');
+
+        // * The status should be updated to online
+        verifyStatus('online');
+    });
+});
+
+function verifyStatus(status: 'online' | 'away' | 'offline' | 'dnd') {
+    cy.get('[aria-label="Current status: Online. Select to open profile and status menu."]').
+        should(status === 'online' ? 'exist' : 'not.exist');
+    cy.get('[aria-label="Current status: Away. Select to open profile and status menu."]').
+        should(status === 'away' ? 'exist' : 'not.exist');
+    cy.get('[aria-label="Current status: Offline. Select to open profile and status menu."]').
+        should(status === 'offline' ? 'exist' : 'not.exist');
+    cy.get('[aria-label="Current status: Do not disturb. Select to open profile and status menu."]').
+        should(status === 'dnd' ? 'exist' : 'not.exist');
+}

webapp/channels/src/actions/websocket_actions.jsx

@@ -475,7 +475,7 @@ export function handleEvent(msg) {
         break;
 
     case SocketEvents.STATUS_CHANGED:
-        handleStatusChangedEvent(msg);
+        dispatch(handleStatusChangedEvent(msg));
         break;
 
     case SocketEvents.HELLO:
@@ -752,7 +752,7 @@ export function handleNewPostEvent(msg) {
         ) {
             myDispatch({
                 type: UserTypes.RECEIVED_STATUSES,
-                data: [{[post.user_id]: UserStatuses.ONLINE}],
+                data: {[post.user_id]: UserStatuses.ONLINE},
             });
         }
     };
@@ -1301,11 +1301,11 @@ function addedNewGmUser(preference) {
     return preference.category === Constants.Preferences.CATEGORY_GROUP_CHANNEL_SHOW && preference.value === 'true';
 }
 
-function handleStatusChangedEvent(msg) {
+export function handleStatusChangedEvent(msg) {
-    dispatch({
+    return {
         type: UserTypes.RECEIVED_STATUSES,
-        data: [{[msg.data.user_id]: msg.data.status}],
+        data: {[msg.data.user_id]: msg.data.status},
-    });
+    };
 }
 
 function handleHelloEvent(msg) {

webapp/channels/src/actions/websocket_actions.test.jsx

@@ -1,7 +1,7 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {UserTypes, CloudTypes} from 'mattermost-redux/action_types';
+import {CloudTypes} from 'mattermost-redux/action_types';
 import {fetchMyCategories} from 'mattermost-redux/actions/channel_categories';
 import {getGroup} from 'mattermost-redux/actions/groups';
 import {
@@ -10,16 +10,18 @@ import {
 } from 'mattermost-redux/actions/posts';
 import {batchFetchStatusesProfilesGroupsFromPosts} from 'mattermost-redux/actions/status_profile_polling';
 import {getUser} from 'mattermost-redux/actions/users';
+import {getStatusForUserId} from 'mattermost-redux/selectors/entities/users';
 
 import {handleNewPost} from 'actions/post_actions';
 import {syncPostsInChannel} from 'actions/views/channel';
 import {closeRightHandSide} from 'actions/views/rhs';
+import realConfigureStore from 'store';
 import store from 'stores/redux_store';
 
 import mergeObjects from 'packages/mattermost-redux/test/merge_objects';
 import configureStore from 'tests/test_store';
 import {getHistory} from 'utils/browser_history';
-import Constants, {SocketEvents, UserStatuses, ActionTypes} from 'utils/constants';
+import Constants, {SocketEvents, ActionTypes, UserStatuses} from 'utils/constants';
 
 import {
     handleChannelUpdatedEvent,
@@ -37,6 +39,7 @@ import {
     handleAppsPluginDisabled,
     handleCloudSubscriptionChanged,
     handleGroupAddedMemberEvent,
+    handleStatusChangedEvent,
 } from './websocket_actions';
 
 jest.mock('mattermost-redux/actions/posts', () => ({
@@ -491,6 +494,8 @@ describe('handleNewPostEvent', () => {
         },
     };
 
+    const otherUserId = 'user2';
+
     test('should receive post correctly', () => {
         const testStore = configureStore(initialState);
 
@@ -508,9 +513,9 @@ describe('handleNewPostEvent', () => {
     });
 
     test('should set other user to online', () => {
-        const testStore = configureStore(initialState);
+        const testStore = realConfigureStore(initialState);
 
-        const post = {id: 'post1', channel_id: 'channel1', user_id: 'user2'};
+        const post = {id: 'post1', channel_id: 'channel1', user_id: otherUserId};
         const msg = {
             data: {
                 post: JSON.stringify(post),
@@ -518,18 +523,17 @@ describe('handleNewPostEvent', () => {
             },
         };
 
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(undefined);
+
         testStore.dispatch(handleNewPostEvent(msg));
 
-        expect(testStore.getActions()).toContainEqual({
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(UserStatuses.ONLINE);
-            type: UserTypes.RECEIVED_STATUSES,
-            data: [{[post.user_id]: UserStatuses.ONLINE}],
-        });
     });
 
     test('should not set other user to online if post was from autoresponder', () => {
-        const testStore = configureStore(initialState);
+        const testStore = realConfigureStore(initialState);
 
-        const post = {id: 'post1', channel_id: 'channel1', user_id: 'user2', type: Constants.AUTO_RESPONDER};
+        const post = {id: 'post1', channel_id: 'channel1', user_id: otherUserId, type: Constants.AUTO_RESPONDER};
         const msg = {
             data: {
                 post: JSON.stringify(post),
@@ -537,21 +541,23 @@ describe('handleNewPostEvent', () => {
             },
         };
 
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(undefined);
+
         testStore.dispatch(handleNewPostEvent(msg));
 
-        expect(testStore.getActions()).not.toContainEqual({
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(undefined);
-            type: UserTypes.RECEIVED_STATUSES,
-            data: [{[post.user_id]: UserStatuses.ONLINE}],
-        });
     });
 
     test('should not set other user to online if status was manually set', () => {
-        const testStore = configureStore({
+        const testStore = realConfigureStore({
             ...initialState,
             entities: {
                 ...initialState.entities,
                 users: {
                     ...initialState.entities.users,
+                    statuses: {
+                        [otherUserId]: UserStatuses.AWAY,
+                    },
                     isManualStatus: {
                         user2: true,
                     },
@@ -559,7 +565,7 @@ describe('handleNewPostEvent', () => {
             },
         });
 
-        const post = {id: 'post1', channel_id: 'channel1', user_id: 'user2'};
+        const post = {id: 'post1', channel_id: 'channel1', user_id: otherUserId};
         const msg = {
             data: {
                 post: JSON.stringify(post),
@@ -567,18 +573,19 @@ describe('handleNewPostEvent', () => {
             },
         };
 
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(UserStatuses.AWAY);
+
         testStore.dispatch(handleNewPostEvent(msg));
 
-        expect(testStore.getActions()).not.toContainEqual({
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(UserStatuses.AWAY);
-            type: UserTypes.RECEIVED_STATUSES,
-            data: [{[post.user_id]: UserStatuses.ONLINE}],
-        });
     });
 
     test('should not set other user to online based on data from the server', () => {
-        const testStore = configureStore(initialState);
+        const testStore = realConfigureStore(initialState);
 
-        const post = {id: 'post1', channel_id: 'channel1', user_id: 'user2'};
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(undefined);
+
+        const post = {id: 'post1', channel_id: 'channel1', user_id: otherUserId};
         const msg = {
             data: {
                 post: JSON.stringify(post),
@@ -588,10 +595,7 @@ describe('handleNewPostEvent', () => {
 
         testStore.dispatch(handleNewPostEvent(msg));
 
-        expect(testStore.getActions()).not.toContainEqual({
+        expect(testStore.getState().entities.users.statuses[otherUserId]).toBe(undefined);
-            type: UserTypes.RECEIVED_STATUSES,
-            data: [{[post.user_id]: UserStatuses.ONLINE}],
-        });
     });
 });
 
@@ -1213,3 +1217,56 @@ describe('handleLeaveTeam', () => {
         expect(store.dispatch).toHaveBeenCalledWith(expectedAction);
     });
 });
+
+describe('handleStatusChangedEvent', () => {
+    const currentUserId = 'user1';
+
+    function makeInitialState() {
+        return {
+            entities: {
+                users: {
+                    currentUserId,
+                    statuses: {
+                        [currentUserId]: 'online',
+                    },
+                },
+            },
+        };
+    }
+
+    test('should modify the status of the current user', () => {
+        const testStore = realConfigureStore(makeInitialState());
+
+        expect(getStatusForUserId(testStore.getState(), currentUserId)).toBe(UserStatuses.ONLINE);
+
+        testStore.dispatch(handleStatusChangedEvent({
+            event: SocketEvents.STATUS_CHANGED,
+            data: {
+                user_id: currentUserId,
+                status: UserStatuses.AWAY,
+            },
+        }));
+
+        expect(getStatusForUserId(testStore.getState(), currentUserId)).toBe(UserStatuses.AWAY);
+
+        testStore.dispatch(handleStatusChangedEvent({
+            event: SocketEvents.STATUS_CHANGED,
+            data: {
+                user_id: currentUserId,
+                status: UserStatuses.ONLINE,
+            },
+        }));
+
+        expect(getStatusForUserId(testStore.getState(), currentUserId)).toBe(UserStatuses.ONLINE);
+
+        testStore.dispatch(handleStatusChangedEvent({
+            event: SocketEvents.STATUS_CHANGED,
+            data: {
+                user_id: currentUserId,
+                status: UserStatuses.OFFLINE,
+            },
+        }));
+
+        expect(getStatusForUserId(testStore.getState(), currentUserId)).toBe(UserStatuses.OFFLINE);
+    });
+});