[pull] master from mattermost:master

webapp/STYLE_GUIDE.md

@@ -87,7 +87,15 @@ The following guidelines should be applied to both new and existing code. Howeve
 - **Selectors**: Prefer using accessible RTL selectors to help ensure that components are accessible, roughly in this order: `getByRole` > `getByText`/`getByPlaceholderText` > `getByLabelText`/`getByAltText`/`getByTitle` > `getByTestId`. Usage of `getByTestId` should be rare.
 - **User Interactions**: Prefer `userEvent` over `fireEvent` for user interactions, and don't directly call methods on DOM elements to simulate events. RTL's `userEvent` simulates events the most realistically, and it ensures that component changes are properly wrapped in `act`.
     - **Async Interactions**: Always wait for all methods of `userEvent` as those methods are all asynchronous (e.g. `await userEvent.click(...)`).
-- **Usage of act**: `act` should only be used when performing any action that causes React to update and when that action does not alreadu go through a helper provided by RTL such as `userEvent`. Typically, most tests can be written without using `act` explicitly.
+    - **When fireEvent is acceptable**: Use `fireEvent` only in these specific cases where `userEvent` cannot be used:
+        - **Focus/Blur events**: `userEvent` doesn't have direct focus/blur methods. Use `fireEvent.focus()` and `fireEvent.blur()`.
+        - **Scroll events**: `userEvent` doesn't support scroll events. Use `fireEvent.scroll()`.
+        - **Image loading events**: `userEvent` doesn't support image loading events. Use `fireEvent.load()` and `fireEvent.error()`.
+        - **Document-level keyboard events**: `userEvent.keyboard()` requires element focus. Use `fireEvent.keyDown(document, ...)` for global keyboard shortcuts.
+        - **Fake timers**: `userEvent` doesn't work well with `jest.useFakeTimers()` and causes timeouts. Use `fireEvent.click()` when tests use fake timers.
+        - **Disabled elements**: `userEvent` respects CSS `pointer-events: none` on disabled elements. Use `fireEvent.click()` when testing that disabled element handlers are properly guarded.
+        - **MouseMove events**: `userEvent.hover()` only triggers mouseEnter/mouseOver, not mouseMove. Use `fireEvent.mouseMove()` when testing mouseMove handlers specifically.
+- **Usage of act**: `act` should only be used when performing any action that causes React to update and when that action does not already go through a helper provided by RTL such as `userEvent`. Typically, most tests can be written without using `act` explicitly.
 
 
 ### Dependencies & Packages

webapp/channels/src/actions/burn_on_read_deletion.test.ts

@@ -13,7 +13,6 @@ describe('burn_on_read_deletion actions', () => {
 
     beforeEach(() => {
         mockDispatch = jest.fn();
-        jest.clearAllMocks();
     });
 
     describe('burnPostNow', () => {

webapp/channels/src/actions/burn_on_read_posts.test.ts

@@ -28,10 +28,6 @@ describe('burn_on_read_posts actions', () => {
         ...mockPost,
     };
 
-    beforeEach(() => {
-        jest.clearAllMocks();
-    });
-
     describe('revealBurnOnReadPost', () => {
         const mockState = {
             entities: {

webapp/channels/src/actions/integration_actions.test.ts

@@ -253,10 +253,6 @@ describe('actions/integration_actions', () => {
     describe('lookupInteractiveDialog', () => {
         const {getDialogArguments} = require('mattermost-redux/selectors/entities/integrations');
 
-        beforeEach(() => {
-            jest.clearAllMocks();
-        });
-
         test('lookupInteractiveDialog with current channel', async () => {
             const testState = {
                 ...initialState,
@@ -341,10 +337,6 @@ describe('actions/integration_actions', () => {
     });
 
     describe('loadIncomingHooksAndProfilesForTeam', () => {
-        beforeEach(() => {
-            jest.clearAllMocks();
-        });
-
         test('should load hooks and profiles', async () => {
             const testStore = mockStore(initialState);
             await testStore.dispatch(Actions.loadIncomingHooksAndProfilesForTeam('team_id1', 0, 50, false));
@@ -368,10 +360,6 @@ describe('actions/integration_actions', () => {
     });
 
     describe('loadOutgoingHooksAndProfilesForTeam', () => {
-        beforeEach(() => {
-            jest.clearAllMocks();
-        });
-
         test('should load outgoing hooks and profiles', async () => {
             const testStore = mockStore(initialState);
             await testStore.dispatch(Actions.loadOutgoingHooksAndProfilesForTeam('team_id1', 1, 25));
@@ -388,10 +376,6 @@ describe('actions/integration_actions', () => {
     });
 
     describe('loadCommandsAndProfilesForTeam', () => {
-        beforeEach(() => {
-            jest.clearAllMocks();
-        });
-
         test('should load commands and profiles', async () => {
             const testStore = mockStore(initialState);
             await testStore.dispatch(Actions.loadCommandsAndProfilesForTeam('team_id1'));
@@ -408,10 +392,6 @@ describe('actions/integration_actions', () => {
     });
 
     describe('loadOAuthAppsAndProfiles', () => {
-        beforeEach(() => {
-            jest.clearAllMocks();
-        });
-
         test('should load OAuth apps with custom parameters', async () => {
             const testStore = mockStore(initialState);
             await testStore.dispatch(Actions.loadOAuthAppsAndProfiles(2, 30));
@@ -435,10 +415,6 @@ describe('actions/integration_actions', () => {
     });
 
     describe('loadOutgoingOAuthConnectionsAndProfiles', () => {
-        beforeEach(() => {
-            jest.clearAllMocks();
-        });
-
         test('should load outgoing OAuth connections', async () => {
             const testStore = mockStore(initialState);
             await testStore.dispatch(Actions.loadOutgoingOAuthConnectionsAndProfiles('team_id1', 1, 50));

webapp/channels/src/actions/websocket_actions.ts

@@ -958,7 +958,8 @@ export function handleLeaveTeamEvent(msg: WebSocketMessages.UserRemovedFromTeam)
     const currentUser = getCurrentUser(state);
 
     if (currentUser.id === msg.data.user_id) {
-        dispatch({type: TeamTypes.LEAVE_TEAM, data: {id: msg.data.team_id}});
+        // Include channel IDs so reducers can clean up posts/embeds for those channels
+        dispatch({type: TeamTypes.LEAVE_TEAM, data: {id: msg.data.team_id, channelIds: channels}});
 
         // if they are on the team being removed redirect them to default team
         if (getCurrentTeamId(state) === msg.data.team_id) {

webapp/channels/src/components/access_history_modal/access_history_modal.test.tsx

@@ -1,12 +1,11 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {fireEvent, screen, waitFor} from '@testing-library/react';
 import React from 'react';
 
 import AccessHistoryModal from 'components/access_history_modal/access_history_modal';
 
-import {renderWithContext} from 'tests/react_testing_utils';
+import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
 
 jest.mock('components/audit_table', () => {
     return jest.fn().mockImplementation(() => {
@@ -85,7 +84,7 @@ describe('components/AccessHistoryModal', () => {
         );
 
         await waitFor(() => screen.getByText('Access History'));
-        fireEvent.click(screen.getByLabelText('Close'));
+        await userEvent.click(screen.getByLabelText('Close'));
 
         expect(onHide).toHaveBeenCalledTimes(1);
     });

webapp/channels/src/components/activity_log_modal/activity_log_modal.test.tsx

@@ -1,15 +1,14 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {fireEvent, screen, waitFor} from '@testing-library/react';
 import React from 'react';
 import type {MouseEvent} from 'react';
 
 import {General} from 'mattermost-redux/constants';
 
 import ActivityLogModal from 'components/activity_log_modal/activity_log_modal';
 
-import {renderWithContext} from 'tests/react_testing_utils';
+import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
 
 jest.mock('components/activity_log_modal/components/activity_log', () => {
     return jest.fn().mockImplementation(({submitRevoke, currentSession}) => {
@@ -115,7 +114,7 @@ describe('components/ActivityLogModal', () => {
             />,
         );
 
-        fireEvent.click(screen.getByTestId('activity-log'));
+        await userEvent.click(screen.getByTestId('activity-log'));
 
         expect(revokeSession).toHaveBeenCalledTimes(1);
         expect(revokeSession).toHaveBeenCalledWith('user1', 'session1');
@@ -137,7 +136,7 @@ describe('components/ActivityLogModal', () => {
         );
 
         await waitFor(() => screen.getByText('Active Sessions'));
-        fireEvent.click(screen.getByLabelText('Close'));
+        await userEvent.click(screen.getByLabelText('Close'));
 
         expect(onHide).toHaveBeenCalledTimes(1);
     });

webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.test.tsx

@@ -1,12 +1,11 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {screen, waitFor, fireEvent} from '@testing-library/react';
 import React from 'react';
 
 import type {UserProfile} from '@mattermost/types/users';
 
-import {renderWithContext} from 'tests/react_testing_utils';
+import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
 import {TestHelper} from 'utils/test_helper';
 
 import TestResultsModal from './test_modal';
@@ -86,8 +85,6 @@ describe('TestResultsModal', () => {
     };
 
     beforeEach(() => {
-        jest.clearAllMocks();
-
         // Mock Redux thunk that returns successful user search response
         mockSearchUsers.mockReturnValue(() => Promise.resolve({
             data: {
@@ -97,10 +94,6 @@ describe('TestResultsModal', () => {
         }));
     });
 
-    afterEach(() => {
-        jest.clearAllMocks();
-    });
-
     it('should render modal with proper title and structure', async () => {
         renderWithContext(<TestResultsModal {...defaultProps}/>);
 
@@ -147,7 +140,8 @@ describe('TestResultsModal', () => {
 
         // Perform search
         const searchInput = screen.getByTestId('search-input');
-        fireEvent.change(searchInput, {target: {value: 'test search'}});
+        await userEvent.clear(searchInput);
+        await userEvent.type(searchInput, 'test search');
 
         await waitFor(() => {
             expect(mockSearchUsers).toHaveBeenCalledWith('test search', '', 50);
@@ -172,7 +166,7 @@ describe('TestResultsModal', () => {
 
         // Click next page - this should trigger pagination with page=2 which translates to 20 users per page
         const nextPageButton = screen.getByTestId('next-page-button');
-        fireEvent.click(nextPageButton);
+        await userEvent.click(nextPageButton);
 
         // The nextPage function gets called with page 1 (second page), but since it's above USERS_PER_PAGE (10)
         // but less than USERS_TO_FETCH (50), it should use the cursor logic and call with the last user's ID
@@ -199,7 +193,7 @@ describe('TestResultsModal', () => {
 
         // Click next page while loading
         const nextPageButton = screen.getByTestId('next-page-button');
-        fireEvent.click(nextPageButton);
+        await userEvent.click(nextPageButton);
 
         // Should not make additional call while loading
         expect(mockSearchUsers).toHaveBeenCalledTimes(1);
@@ -241,7 +235,7 @@ describe('TestResultsModal', () => {
 
         // Find and click the close button
         const closeButton = screen.getByLabelText('Close');
-        fireEvent.click(closeButton);
+        await userEvent.click(closeButton);
 
         await waitFor(() => {
             expect(mockOnExited).toHaveBeenCalled();
@@ -302,15 +296,16 @@ describe('TestResultsModal', () => {
 
         // Perform first search
         const searchInput = screen.getByTestId('search-input');
-        fireEvent.change(searchInput, {target: {value: 'search1'}});
+        await userEvent.clear(searchInput);
+        await userEvent.type(searchInput, 'search1');
 
         await waitFor(() => {
             expect(mockSearchUsers).toHaveBeenCalledWith('search1', '', 50);
         });
 
         // Perform second search
-        fireEvent.change(searchInput, {target: {value: ''}});
-        fireEvent.change(searchInput, {target: {value: 'search2'}});
+        await userEvent.clear(searchInput);
+        await userEvent.type(searchInput, 'search2');
 
         await waitFor(() => {
             expect(mockSearchUsers).toHaveBeenCalledWith('search2', '', 50);

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

@@ -1,10 +1,11 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {fireEvent, render, screen} from '@testing-library/react';
 import React from 'react';
 import {MemoryRouter} from 'react-router-dom';
 
+import {render, screen, userEvent} from 'tests/react_testing_utils';
+
 import BlockableLink from './blockable_link';
 
 jest.mock('utils/browser_history', () => ({
@@ -34,18 +35,18 @@ describe('components/admin_console/blockable_link/BlockableLink', () => {
         expect(screen.getByRole('link')).toHaveAttribute('href', '/admin_console/test');
     });
 
-    test('should navigate directly when not blocked', () => {
+    test('should navigate directly when not blocked', async () => {
         render(
             <MemoryRouter>
                 <BlockableLink {...defaultProps}/>
             </MemoryRouter>,
         );
 
-        fireEvent.click(screen.getByText('Link Text'));
+        await userEvent.click(screen.getByText('Link Text'));
         expect(defaultProps.actions.deferNavigation).not.toHaveBeenCalled();
     });
 
-    test('should defer navigation when blocked', () => {
+    test('should defer navigation when blocked', async () => {
         const blockedProps = {
             ...defaultProps,
             blocked: true,
@@ -57,11 +58,11 @@ describe('components/admin_console/blockable_link/BlockableLink', () => {
             </MemoryRouter>,
         );
 
-        fireEvent.click(screen.getByText('Link Text'));
+        await userEvent.click(screen.getByText('Link Text'));
         expect(blockedProps.actions.deferNavigation).toHaveBeenCalled();
     });
 
-    test('should call custom onClick handler if provided', () => {
+    test('should call custom onClick handler if provided', async () => {
         const onClickProps = {
             ...defaultProps,
             onClick: jest.fn(),
@@ -73,7 +74,7 @@ describe('components/admin_console/blockable_link/BlockableLink', () => {
             </MemoryRouter>,
         );
 
-        fireEvent.click(screen.getByText('Link Text'));
+        await userEvent.click(screen.getByText('Link Text'));
         expect(onClickProps.onClick).toHaveBeenCalled();
     });
 

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

@@ -3,7 +3,7 @@
 
 import React from 'react';
 
-import {renderWithContext, screen, fireEvent} from 'tests/react_testing_utils';
+import {renderWithContext, screen, userEvent} from 'tests/react_testing_utils';
 
 import CheckboxSetting from './checkbox_setting';
 
@@ -26,7 +26,7 @@ describe('components/admin_console/CheckboxSetting', () => {
         expect(container).toMatchSnapshot();
     });
 
-    test('onChange', () => {
+    test('onChange', async () => {
         const onChange = jest.fn();
         renderWithContext(
             <CheckboxSetting
@@ -41,7 +41,7 @@ describe('components/admin_console/CheckboxSetting', () => {
         const checkbox: HTMLInputElement = screen.getByRole('checkbox');
         expect(checkbox).not.toBeChecked();
 
-        fireEvent.click(checkbox);
+        await userEvent.click(checkbox);
 
         expect(onChange).toHaveBeenCalledTimes(1);
         expect(onChange).toHaveBeenCalledWith('string.id', true);