MM-69466: re-fetch admin config on config_changed to prevent concurrent save clobber (#37338) (#37355)

(cherry picked from commit 68389fabbb)

Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>

e2e-tests/playwright/specs/functional/system_console/concurrent_config_save.spec.ts

@@ -0,0 +1,145 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import type {AdminConfig} from '@mattermost/types/config';
+
+import {expect, test} from '@mattermost/playwright-lib';
+
+// Wait for the GET /api/v4/config re-fetch that our config_changed websocket
+// handler triggers. This is the signal that entities.admin.config in the
+// browser's Redux store is now up-to-date with the server's current state.
+async function waitForAdminConfigRefresh(page: import('@playwright/test').Page) {
+    return page.waitForResponse(
+        (resp) => resp.url().endsWith('/api/v4/config') && resp.request().method() === 'GET' && resp.status() === 200,
+        {timeout: 10000},
+    );
+}
+
+// Each test case opens a System Console page, makes a parallel API change to a
+// completely separate config section (simulating another admin), then saves the
+// open page and asserts both changes survived.
+test.describe('System Console > Concurrent config saves', () => {
+    test('Emoji page: parallel change to TeamSettings is not clobbered', async ({pw}) => {
+        const {adminUser, adminClient} = await pw.initSetup();
+
+        if (!adminUser) {
+            throw new Error('Failed to create admin user');
+        }
+
+        // # Set a known baseline for both sections under test
+        await adminClient.patchConfig({
+            ServiceSettings: {EnableCustomEmoji: false},
+            TeamSettings: {MaxUsersPerTeam: 50},
+        } as Partial<AdminConfig>);
+
+        // # Open the Emoji settings page
+        const {page} = await pw.testBrowser.login(adminUser);
+        await page.goto('/admin_console/site_config/emoji');
+
+        const emojiSection = page.getByTestId('sysconsole_section_EmojiSettings');
+        await expect(emojiSection).toBeVisible();
+        const saveButton = emojiSection.getByRole('button', {name: 'Save'});
+
+        // # Simulate another admin saving an unrelated section via API.
+        // Set up the response listener before triggering the change so we don't
+        // miss the GET /api/v4/config that our config_changed handler dispatches.
+        const configRefresh = waitForAdminConfigRefresh(page);
+        await adminClient.patchConfig({TeamSettings: {MaxUsersPerTeam: 99}} as Partial<AdminConfig>);
+
+        // # Wait for the browser to re-fetch the admin config before we save,
+        // confirming that this.props.config is now fresh. Then assert the
+        // toggle's current state to confirm the Redux store update has
+        // propagated to the UI before we interact.
+        await configRefresh;
+        const emojiToggle = emojiSection.getByTestId('ServiceSettings.EnableCustomEmojitrue');
+        await expect(emojiToggle).not.toBeChecked();
+
+        // # Change a setting on the open Emoji page and save
+        await emojiToggle.click();
+        await saveButton.click();
+        await pw.waitUntil(async () => (await saveButton.textContent()) === 'Save');
+
+        // * Both the UI change and the parallel API change must be present
+        const finalConfig = await adminClient.getConfig();
+        expect(finalConfig.ServiceSettings.EnableCustomEmoji).toBe(true);
+        expect(finalConfig.TeamSettings.MaxUsersPerTeam).toBe(99);
+    });
+
+    test('Announcement Banner page: parallel change to ServiceSettings is not clobbered', async ({pw}) => {
+        const {adminUser, adminClient} = await pw.initSetup();
+
+        if (!adminUser) {
+            throw new Error('Failed to create admin user');
+        }
+
+        // # Set a known baseline for both sections under test
+        await adminClient.patchConfig({
+            AnnouncementSettings: {EnableBanner: false},
+            ServiceSettings: {EnableCustomEmoji: false},
+        } as Partial<AdminConfig>);
+
+        // # Open the Announcement Banner settings page
+        const {page} = await pw.testBrowser.login(adminUser);
+        await page.goto('/admin_console/site_config/announcement_banner');
+
+        const bannerSection = page.getByTestId('sysconsole_section_AnnouncementSettings');
+        await expect(bannerSection).toBeVisible();
+        const saveButton = bannerSection.getByRole('button', {name: 'Save'});
+
+        // # Simulate another admin saving an unrelated section via API
+        const configRefresh = waitForAdminConfigRefresh(page);
+        await adminClient.patchConfig({ServiceSettings: {EnableCustomEmoji: true}} as Partial<AdminConfig>);
+        await configRefresh;
+        const bannerToggle = bannerSection.getByTestId('AnnouncementSettings.EnableBannertrue');
+        await expect(bannerToggle).not.toBeChecked();
+
+        // # Change a setting on the open Announcement Banner page and save
+        await bannerToggle.click();
+        await saveButton.click();
+        await pw.waitUntil(async () => (await saveButton.textContent()) === 'Save');
+
+        // * Both changes must be present
+        const finalConfig = await adminClient.getConfig();
+        expect(finalConfig.AnnouncementSettings.EnableBanner).toBe(true);
+        expect(finalConfig.ServiceSettings.EnableCustomEmoji).toBe(true);
+    });
+
+    test('Users and Teams page: parallel change to EmailSettings is not clobbered', async ({pw}) => {
+        const {adminUser, adminClient} = await pw.initSetup();
+
+        if (!adminUser) {
+            throw new Error('Failed to create admin user');
+        }
+
+        // # Set a known baseline for both sections under test
+        await adminClient.patchConfig({
+            TeamSettings: {EnableJoinLeaveMessageByDefault: false},
+            EmailSettings: {EnableSignUpWithEmail: true},
+        } as Partial<AdminConfig>);
+
+        // # Open the Users and Teams settings page
+        const {page} = await pw.testBrowser.login(adminUser);
+        await page.goto('/admin_console/site_config/users_and_teams');
+
+        const teamSection = page.getByTestId('sysconsole_section_UserAndTeamsSettings');
+        await expect(teamSection).toBeVisible();
+        const saveButton = teamSection.getByRole('button', {name: 'Save'});
+
+        // # Simulate another admin saving EmailSettings via API
+        const configRefresh = waitForAdminConfigRefresh(page);
+        await adminClient.patchConfig({EmailSettings: {EnableSignUpWithEmail: false}} as Partial<AdminConfig>);
+        await configRefresh;
+        const joinLeaveToggle = teamSection.getByTestId('TeamSettings.EnableJoinLeaveMessageByDefaulttrue');
+        await expect(joinLeaveToggle).not.toBeChecked();
+
+        // # Toggle Enable Join/Leave messages on the Users and Teams page and save
+        await joinLeaveToggle.click();
+        await saveButton.click();
+        await pw.waitUntil(async () => (await saveButton.textContent()) === 'Save');
+
+        // * Both changes must be present
+        const finalConfig = await adminClient.getConfig();
+        expect(finalConfig.TeamSettings.EnableJoinLeaveMessageByDefault).toBe(true);
+        expect(finalConfig.EmailSettings.EnableSignUpWithEmail).toBe(false);
+    });
+});

webapp/channels/src/components/admin_console/admin_console.tsx

@@ -18,6 +18,7 @@ import SchemaAdminSettings from 'components/admin_console/schema_admin_settings'
 import SearchKeywordMarking from 'components/admin_console/search_keyword_marking';
 import AnnouncementBarController from 'components/announcement_bar';
 import BackstageNavbar from 'components/backstage/components/backstage_navbar';
+import useAdminConfigSync from 'components/common/hooks/useAdminConfigSync';
 import DiscardChangesModal from 'components/discard_changes_modal';
 import GlobalClassificationBanner from 'components/global_classification_banner';
 import ModalController from 'components/modal_controller';
@@ -87,6 +88,10 @@ const AdminConsole = (props: Props) => {
     const [search, setSearch] = useState('');
     const handleFocusScroller = useFocusScroller(props.location);
 
+    // Keep the config fresh while the System Console is open so that concurrent
+    // saves by different admins do not clobber each other.
+    useAdminConfigSync();
+
     useEffect(() => {
         props.actions.getConfig();
         props.actions.getEnvironmentConfig();

webapp/channels/src/components/common/hooks/useAdminConfigSync.test.ts

@@ -0,0 +1,96 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import {act} from '@testing-library/react';
+import * as ReactRedux from 'react-redux';
+
+import type {WebSocketMessage} from '@mattermost/client';
+import {WebSocketEvents} from '@mattermost/client';
+
+import {getConfig} from 'mattermost-redux/actions/admin';
+
+import {renderHookWithContext} from 'tests/react_testing_utils';
+import * as webSocketHooks from 'utils/use_websocket/hooks';
+
+import useAdminConfigSync from './useAdminConfigSync';
+
+jest.mock('mattermost-redux/actions/admin', () => ({
+    ...jest.requireActual('mattermost-redux/actions/admin'),
+    getConfig: jest.fn(() => ({type: 'MOCK_GET_ADMIN_CONFIG'})),
+}));
+
+jest.mock('utils/use_websocket/hooks', () => ({
+    useWebSocket: jest.fn(),
+    useWebSocketClient: jest.fn(),
+}));
+
+describe('useAdminConfigSync', () => {
+    const dispatchMock = jest.fn();
+    const addReconnectListener = jest.fn();
+    const removeReconnectListener = jest.fn();
+
+    let messageHandler: (msg: WebSocketMessage) => void;
+
+    beforeEach(() => {
+        jest.spyOn(ReactRedux, 'useDispatch').mockReturnValue(dispatchMock);
+
+        (webSocketHooks.useWebSocket as jest.Mock).mockImplementation(({handler}) => {
+            messageHandler = handler;
+        });
+        (webSocketHooks.useWebSocketClient as jest.Mock).mockReturnValue({
+            addReconnectListener,
+            removeReconnectListener,
+        });
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+        (getConfig as jest.Mock).mockClear();
+        dispatchMock.mockClear();
+        addReconnectListener.mockClear();
+        removeReconnectListener.mockClear();
+    });
+
+    test('dispatches getConfig on a config_changed event', () => {
+        renderHookWithContext(useAdminConfigSync);
+
+        act(() => {
+            messageHandler({event: WebSocketEvents.ConfigChanged} as WebSocketMessage);
+        });
+
+        expect(getConfig).toHaveBeenCalledTimes(1);
+        expect(dispatchMock).toHaveBeenCalledWith({type: 'MOCK_GET_ADMIN_CONFIG'});
+    });
+
+    test('ignores other websocket events', () => {
+        renderHookWithContext(useAdminConfigSync);
+
+        act(() => {
+            messageHandler({event: WebSocketEvents.Posted} as WebSocketMessage);
+        });
+
+        expect(getConfig).not.toHaveBeenCalled();
+    });
+
+    test('refetches on websocket reconnect', () => {
+        renderHookWithContext(useAdminConfigSync);
+
+        expect(addReconnectListener).toHaveBeenCalledTimes(1);
+        const reconnectListener = addReconnectListener.mock.calls[0][0];
+
+        act(() => {
+            reconnectListener();
+        });
+
+        expect(getConfig).toHaveBeenCalledTimes(1);
+    });
+
+    test('removes the reconnect listener on unmount', () => {
+        const {unmount} = renderHookWithContext(useAdminConfigSync);
+
+        act(unmount);
+
+        expect(removeReconnectListener).toHaveBeenCalledTimes(1);
+        expect(removeReconnectListener).toHaveBeenCalledWith(addReconnectListener.mock.calls[0][0]);
+    });
+});

webapp/channels/src/components/common/hooks/useAdminConfigSync.ts

@@ -0,0 +1,40 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import {useCallback, useEffect} from 'react';
+import {useDispatch} from 'react-redux';
+
+import type {WebSocketMessage} from '@mattermost/client';
+import {WebSocketEvents} from '@mattermost/client';
+
+import {getConfig} from 'mattermost-redux/actions/admin';
+
+import {useWebSocket, useWebSocketClient} from 'utils/use_websocket/hooks';
+
+/**
+ * Subscribes to config_changed WebSocket events and reconnects, re-fetching
+ * the full admin config each time so the in-memory state stays current.
+ */
+export default function useAdminConfigSync() {
+    const dispatch = useDispatch();
+    const wsClient = useWebSocketClient();
+
+    const refetch = useCallback(() => {
+        dispatch(getConfig());
+    }, [dispatch]);
+
+    const handleWebSocketMessage = useCallback((msg: WebSocketMessage) => {
+        if (msg.event === WebSocketEvents.ConfigChanged) {
+            refetch();
+        }
+    }, [refetch]);
+
+    useWebSocket({handler: handleWebSocketMessage});
+
+    useEffect(() => {
+        wsClient.addReconnectListener(refetch);
+        return () => {
+            wsClient.removeReconnectListener(refetch);
+        };
+    }, [wsClient, refetch]);
+}