diff --git a/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.test.tsx b/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.test.tsx index 8f7d8f1b243..7803c253694 100644 --- a/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.test.tsx +++ b/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.test.tsx @@ -480,7 +480,9 @@ describe('ChannelSettingsConfigurationTab', () => { describe('Share channel with connected workspaces', () => { beforeEach(() => { const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + const {fetchChannelRemotes} = require('mattermost-redux/actions/shared_channels'); getRemotesForChannel.mockReturnValue([]); + fetchChannelRemotes.mockImplementation(() => ({type: 'MOCK_ACTION', data: []})); }); it('should render ShareChannelWithWorkspaces section when canManageSharedChannels is true', () => { @@ -519,6 +521,172 @@ describe('ChannelSettingsConfigurationTab', () => { expect(screen.getByText('Add workspace')).toBeInTheDocument(); }); + it('should disable sharing when saved with toggle on but no workspaces added', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + + getRemotesForChannel.mockReturnValue([]); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toBeDisabled(); + }); + + const toggle = screen.getByTestId('shareChannelWithWorkspacesToggle-button'); + await userEvent.click(toggle); + expect(toggle).toHaveClass('active'); + expect(screen.getByText('Add workspace')).toBeInTheDocument(); + + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(screen.getByText('Settings saved')).toBeInTheDocument(); + }); + + await waitFor(() => { + expect(screen.queryByRole('button', {name: 'Save'})).not.toBeInTheDocument(); + }, {timeout: 2000}); + + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toHaveClass('active'); + expect(screen.queryByText('Add workspace')).not.toBeInTheDocument(); + }); + + it('should show save again after adding a workspace when sharing was enabled and saved without workspaces', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + const {fetchChannelRemotes} = require('mattermost-redux/actions/shared_channels'); + + getRemotesForChannel.mockReturnValue([]); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toBeDisabled(); + }); + + const fetchCallsAfterMount = fetchChannelRemotes.mock.calls.length; + + await userEvent.click(screen.getByTestId('shareChannelWithWorkspacesToggle-button')); + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(fetchChannelRemotes.mock.calls.length).toBeGreaterThan(fetchCallsAfterMount); + }); + + await waitFor(() => { + expect(screen.queryByRole('button', {name: 'Save'})).not.toBeInTheDocument(); + }, {timeout: 2000}); + + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toHaveClass('active'); + + await userEvent.click(screen.getByTestId('shareChannelWithWorkspacesToggle-button')); + await userEvent.click(screen.getByRole('button', {name: /Add workspace/i})); + await waitFor(() => { + expect(screen.getByRole('menuitem', {name: 'Nebula Networks'})).toBeInTheDocument(); + }); + await userEvent.click(screen.getByRole('menuitem', {name: 'Nebula Networks'})); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Remove Nebula Networks/i})).toBeInTheDocument(); + }); + + expect(await screen.findByRole('button', {name: 'Save'})).toBeInTheDocument(); + }); + + it('should preserve local workspace removals when server remotes refresh before save', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + + const remote = { + remote_id: 'remote1', + name: 'nebula', + display_name: 'Nebula Networks', + create_at: 0, + delete_at: 0, + last_ping_at: Date.now(), + site_url: 'https://nebula.example.com', + }; + let remotes = [remote]; + getRemotesForChannel.mockImplementation(() => remotes); + + const {rerender} = renderWithContext( + , + ); + + const removeButton = await screen.findByRole('button', {name: /Remove Nebula Networks/i}); + await userEvent.click(removeButton); + + expect(screen.queryByRole('button', {name: /Remove Nebula Networks/i})).not.toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Save'})).toBeInTheDocument(); + + remotes = [{...remote}]; + getRemotesForChannel.mockImplementation(() => remotes); + rerender( + , + ); + + expect(screen.queryByRole('button', {name: /Remove Nebula Networks/i})).not.toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Save'})).toBeInTheDocument(); + }); + + it('should preserve sharing toggle off when server remotes refresh before save', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + + const remote = { + remote_id: 'remote1', + name: 'nebula', + display_name: 'Nebula Networks', + create_at: 0, + delete_at: 0, + last_ping_at: Date.now(), + site_url: 'https://nebula.example.com', + }; + let remotes = [remote]; + getRemotesForChannel.mockImplementation(() => remotes); + + const {rerender} = renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).toHaveClass('active'); + }); + + await userEvent.click(screen.getByTestId('shareChannelWithWorkspacesToggle-button')); + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toHaveClass('active'); + + remotes = [{...remote}]; + getRemotesForChannel.mockImplementation(() => remotes); + rerender( + , + ); + + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toHaveClass('active'); + expect(screen.getByRole('button', {name: 'Save'})).toBeInTheDocument(); + }); + it('when shared channel changes include only adding workspaces, save calls invite and fetchChannelRemotes', async () => { const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); const {fetchChannelRemotes} = require('mattermost-redux/actions/shared_channels'); @@ -555,6 +723,55 @@ describe('ChannelSettingsConfigurationTab', () => { expect(fetchChannelRemotes).toHaveBeenLastCalledWith('channel1', true); }); + it('should clear pending save status after workspace is saved', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + const {fetchChannelRemotes} = require('mattermost-redux/actions/shared_channels'); + + const savedRemote = { + remote_id: 'remote1', + name: 'nebula', + display_name: 'Nebula Networks', + create_at: 0, + delete_at: 0, + last_ping_at: Date.now(), + site_url: 'https://nebula.example.com', + }; + + getRemotesForChannel.mockReturnValue([]); + fetchChannelRemotes.mockImplementation(() => ({ + type: 'MOCK_ACTION', + data: [savedRemote], + })); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toBeDisabled(); + }); + + await userEvent.click(screen.getByTestId('shareChannelWithWorkspacesToggle-button')); + await userEvent.click(screen.getByRole('button', {name: /Add workspace/i})); + await waitFor(() => { + expect(screen.getByRole('menuitem', {name: 'Nebula Networks'})).toBeInTheDocument(); + }); + await userEvent.click(screen.getByRole('menuitem', {name: 'Nebula Networks'})); + + await waitFor(() => { + expect(screen.getByText('Pending save')).toBeInTheDocument(); + }); + + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(screen.queryByText('Pending save')).not.toBeInTheDocument(); + }); + }); + it('when shared channel changes include removing a connection, confirm modal is shown before save', async () => { const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); const {Client4} = require('mattermost-redux/client'); @@ -636,6 +853,106 @@ describe('ChannelSettingsConfigurationTab', () => { expect(fetchChannelRemotes).toHaveBeenLastCalledWith('channel1', true); }); + it('should disable sharing after toggling off and confirming unshare', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + const {Client4} = require('mattermost-redux/client'); + + const initialRemotes = [ + { + remote_id: 'remote1', + name: 'nebula', + display_name: 'Nebula Networks', + create_at: 0, + delete_at: 0, + last_ping_at: Date.now(), + site_url: 'https://nebula.example.com', + }, + ]; + getRemotesForChannel.mockReturnValue(initialRemotes); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).toHaveClass('active'); + }); + + await userEvent.click(screen.getByTestId('shareChannelWithWorkspacesToggle-button')); + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(screen.getByText(/Are you sure you want to unshare\?/)).toBeInTheDocument(); + }); + await userEvent.click(screen.getByRole('button', {name: /Yes, unshare/})); + + await waitFor(() => { + expect(Client4.sharedChannelRemoteUninvite).toHaveBeenCalledWith('remote1', 'channel1'); + }); + + getRemotesForChannel.mockReturnValue([]); + + await waitFor(() => { + expect(screen.queryByRole('button', {name: 'Save'})).not.toBeInTheDocument(); + }, {timeout: 2000}); + + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).not.toHaveClass('active'); + expect(screen.queryByText('Add workspace')).not.toBeInTheDocument(); + }); + + it('should clear unsaved changes after toggling off and confirming unshare', async () => { + const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); + const {Client4} = require('mattermost-redux/client'); + + const initialRemotes = [ + { + remote_id: 'remote1', + name: 'nebula', + display_name: 'Nebula Networks', + create_at: 0, + delete_at: 0, + last_ping_at: Date.now(), + site_url: 'https://nebula.example.com', + }, + ]; + getRemotesForChannel.mockReturnValue(initialRemotes); + + const setAreThereUnsavedChanges = jest.fn(); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByTestId('shareChannelWithWorkspacesToggle-button')).toHaveClass('active'); + }); + + await userEvent.click(screen.getByTestId('shareChannelWithWorkspacesToggle-button')); + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(screen.getByText(/Are you sure you want to unshare\?/)).toBeInTheDocument(); + }); + await userEvent.click(screen.getByRole('button', {name: /Yes, unshare/})); + + await waitFor(() => { + expect(Client4.sharedChannelRemoteUninvite).toHaveBeenCalledWith('remote1', 'channel1'); + }); + + await waitFor(() => { + expect(setAreThereUnsavedChanges).toHaveBeenLastCalledWith(false); + }); + }); + it('when user cancels remove modal, uninvite is not called', async () => { const {getRemotesForChannel} = require('mattermost-redux/selectors/entities/shared_channels'); const {Client4} = require('mattermost-redux/client'); diff --git a/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.tsx b/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.tsx index 15286907bae..6c514b8d7a5 100644 --- a/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.tsx +++ b/webapp/channels/src/components/channel_settings_modal/channel_settings_configuration_tab.tsx @@ -68,6 +68,27 @@ function bannerHasChanges(originalBannerInfo: Channel['banner_info'], updatedBan originalBannerInfo?.enabled !== updatedBannerInfo?.enabled; } +type SharingSnapshot = { + enabled: boolean; + remotes: WorkspaceWithStatus[]; +}; + +function remoteIdsKey(remotes: Array<{remote_id?: string; name?: string}>) { + return remotes.map((r) => r.remote_id || r.name).sort().join(','); +} + +function copyRemotes(remotes: WorkspaceWithStatus[]) { + return remotes.map((r) => ({...r})); +} + +function remotesWithoutPendingSave(remotes: WorkspaceWithStatus[]) { + return remotes.map((remote) => { + const saved = {...remote}; + delete saved.pendingSave; + return saved; + }); +} + function ChannelSettingsConfigurationTab({ channel, setAreThereUnsavedChanges, @@ -232,7 +253,6 @@ function ChannelSettingsConfigurationTab({ const autoTranslationSubHeading = formatMessage({id: 'channel_translation.label.subtext', defaultMessage: 'When enabled, messages in this channel will be translated to members\' own languages. Members can opt-out of this from the channel menu to view the original message instead.'}); const initialIsChannelAutotranslated = useSelector((state: GlobalState) => isChannelAutotranslatedSelector(state, channel.id)); - const initialRemotes = useSelector((state: GlobalState) => getRemotesForChannel(state, channel.id)); const [isChannelAutotranslated, setIsChannelAutotranslated] = useState(initialIsChannelAutotranslated); const hasAutoTranslationChanges = isChannelAutotranslated !== initialIsChannelAutotranslated; @@ -241,36 +261,46 @@ function ChannelSettingsConfigurationTab({ }, []); // Shared channels section - const [workspaceRemotes, setWorkspaceRemotes] = useState(() => - (initialRemotes || []).map((r) => ({...r})), - ); + const initialRemotes = useSelector((state: GlobalState) => getRemotesForChannel(state, channel.id)); + const userEditedSharingRef = useRef(false); + + const [savedSharing, setSavedSharing] = useState(() => ({ + enabled: (initialRemotes || []).length > 0, + remotes: copyRemotes(initialRemotes || []), + })); + const [sharingEnabled, setSharingEnabled] = useState(savedSharing.enabled); + const [workspaceRemotes, setWorkspaceRemotes] = useState(() => copyRemotes(savedSharing.remotes)); const [showRemoveSharingConfirmModal, setShowRemoveSharingConfirmModal] = useState(false); - // Key to force re-render the ShareChannelWithWorkspaces component - // on reset and on save - const [shareChannelKey, setShareChannelKey] = useState(Date.now()); + const hasWorkspaceChanges = sharingEnabled !== savedSharing.enabled || + remoteIdsKey(workspaceRemotes) !== remoteIdsKey(savedSharing.remotes); - // Track the toggle state to detect when sharing is explicitly disabled on a channel - // that has channel.shared=true but no remotes loaded (e.g. after page reload). - // Both are frozen at mount time so they don't drift apart due to async channel hydration. - const initialSharingEnabled = useRef(channel.shared || (initialRemotes || []).length > 0); - const [sharingEnabled, setSharingEnabled] = useState(channel.shared || (initialRemotes || []).length > 0); + const applySharingSnapshot = useCallback((remotes: WorkspaceWithStatus[]) => { + const copies = remotesWithoutPendingSave(copyRemotes(remotes)); + const enabled = copies.length > 0; + setSavedSharing({enabled, remotes: copies}); + setSharingEnabled(enabled); + setWorkspaceRemotes(copies); + }, []); - // Freeze initialRemoteIds in state and update it atomically with workspaceRemotes (in - // useDidUpdate below) so that the two never diverge in the same render. If we computed - // initialRemoteIds live from the Redux selector, a fetchChannelRemotes response could - // update initialRemotes one render before useDidUpdate syncs workspaceRemotes, causing a - // spurious hasWorkspaceChanges=true that triggers the "discard changes" dialog on close. - const [frozenInitialRemoteIds, setFrozenInitialRemoteIds] = useState( - () => (initialRemotes || []).map((r) => r.remote_id || r.name).sort().join(','), - ); - const currentRemoteIds = workspaceRemotes.map((r) => r.remote_id || r.name).sort().join(','); - const hasWorkspaceChanges = frozenInitialRemoteIds !== currentRemoteIds || - sharingEnabled !== initialSharingEnabled.current; + const commitSharingAfterSave = useCallback((remotes: WorkspaceWithStatus[]) => { + applySharingSnapshot(remotes); + userEditedSharingRef.current = false; + }, [applySharingSnapshot]); + + const handleSharingToggle = useCallback((enabled: boolean) => { + userEditedSharingRef.current = true; + setSharingEnabled(enabled); + }, []); + + const handleWorkspaceRemotesChange = useCallback((update: React.SetStateAction) => { + userEditedSharingRef.current = true; + setWorkspaceRemotes(update); + }, []); const confirmModalMessages = useMemo(() => { const workspaceRemoteIdSet = new Set(workspaceRemotes.map((r) => r.remote_id || r.name)); - const remotesToRemove = (initialRemotes || []).filter((r) => !workspaceRemoteIdSet.has(r.remote_id || r.name)); + const remotesToRemove = savedSharing.remotes.filter((r) => !workspaceRemoteIdSet.has(r.remote_id || r.name)); const channelDisplayName = channel.display_name || channel.name; const removeCount = remotesToRemove.length; const workspaceNames = remotesToRemove.map((r) => r.display_name || r.name || r.remote_id || ''); @@ -306,7 +336,7 @@ function ChannelSettingsConfigurationTab({ channel.name, formatList, formatMessage, - initialRemotes, + savedSharing.remotes, workspaceRemotes, ]); @@ -317,18 +347,13 @@ function ChannelSettingsConfigurationTab({ }, [canManageSharedChannels, channel.id, dispatch]); useDidUpdate(() => { - if (initialRemotes && canManageSharedChannels) { - // Update both frozen baseline and working copy atomically so they never diverge. - setFrozenInitialRemoteIds( - initialRemotes.map((r) => r.remote_id || r.name).sort().join(','), - ); - setWorkspaceRemotes(initialRemotes.map((r) => ({...r}))); - initialSharingEnabled.current = initialRemotes.length > 0; - if (initialRemotes.length > 0) { - setSharingEnabled(true); - } + if (!canManageSharedChannels || userEditedSharingRef.current) { + return; } - }, [canManageSharedChannels, initialRemotes]); + + const fromServer = (initialRemotes || []).map((r) => ({...r})); + applySharingSnapshot(fromServer); + }, [applySharingSnapshot, canManageSharedChannels, initialRemotes]); const handleCancelRemoveSharing = useCallback(() => { setShowRemoveSharingConfirmModal(false); @@ -463,11 +488,11 @@ function ChannelSettingsConfigurationTab({ } if (canManageSharedChannels && hasWorkspaceChanges) { - const initialIds = new Set((initialRemotes || []).map((r) => r.remote_id || r.name)); + const savedIds = new Set(savedSharing.remotes.map((r) => r.remote_id || r.name)); const currentIds = new Set(workspaceRemotes.map((r) => r.remote_id || r.name)); const toAdd = workspaceRemotes.filter((w) => w.pendingSave).map((w) => w.remote_id || w.name); - const toRemove = Array.from(initialIds).filter((id) => !currentIds.has(id)); + const toRemove = Array.from(savedIds).filter((id) => !currentIds.has(id)); let errorCount = 0; let lastError: ServerError | undefined; @@ -490,8 +515,7 @@ function ChannelSettingsConfigurationTab({ errorCount++; } } - await dispatch(fetchChannelRemotes(channel.id, true)); - setShareChannelKey(Date.now()); + const fetchResult = await dispatch(fetchChannelRemotes(channel.id, true)); if (errorCount === 1) { handleServerError(lastError as ServerError); @@ -502,6 +526,12 @@ function ChannelSettingsConfigurationTab({ defaultMessage: 'There has been errors while sharing the channel with some workspaces. Please try again.', })); } + + if (errorCount === 0) { + const refreshedRemotes = (fetchResult as {data?: WorkspaceWithStatus[]})?.data ?? workspaceRemotes; + commitSharingAfterSave(refreshedRemotes); + } + return errorCount === 0; } @@ -521,11 +551,12 @@ function ChannelSettingsConfigurationTab({ initialBannerInfo, initialClassificationState.enabled, initialIsChannelAutotranslated, - initialRemotes, isChannelAutotranslated, selectedClassificationId, + savedSharing, updatedChannelBanner, workspaceRemotes, + commitSharingAfterSave, ]); const performSave = useCallback(async () => { @@ -549,7 +580,7 @@ function ChannelSettingsConfigurationTab({ const handleSaveChanges = useCallback(async () => { if (canManageSharedChannels && hasWorkspaceChanges) { const currentIds = new Set(workspaceRemotes.map((r) => r.remote_id || r.name)); - const remotesToRemove = (initialRemotes || []).filter( + const remotesToRemove = savedSharing.remotes.filter( (r) => !currentIds.has(r.remote_id || r.name), ); @@ -560,7 +591,7 @@ function ChannelSettingsConfigurationTab({ } await performSave(); - }, [canManageSharedChannels, hasWorkspaceChanges, initialRemotes, performSave, workspaceRemotes]); + }, [canManageSharedChannels, hasWorkspaceChanges, performSave, savedSharing.remotes, workspaceRemotes]); const handleConfirmRemoveSharing = useCallback(async () => { setShowRemoveSharingConfirmModal(false); @@ -581,13 +612,11 @@ function ChannelSettingsConfigurationTab({ setSelectedClassificationId(initialClassificationState.classificationId); if (canManageSharedChannels) { - setSharingEnabled(initialSharingEnabled.current); - if (initialRemotes) { - setWorkspaceRemotes(initialRemotes.map((r) => ({...r}))); - setShareChannelKey(Date.now()); - } + setSharingEnabled(savedSharing.enabled); + setWorkspaceRemotes(copyRemotes(savedSharing.remotes)); + userEditedSharingRef.current = false; } - }, [canManageSharedChannels, initialBannerInfo, initialClassificationState, initialRemotes]); + }, [canManageSharedChannels, initialBannerInfo, initialClassificationState, savedSharing]); const handleClose = useCallback(() => { setSaveChangesPanelState(undefined); @@ -625,12 +654,11 @@ function ChannelSettingsConfigurationTab({ isStacked={true} /> )} diff --git a/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/share_channel_with_workspaces.tsx b/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/share_channel_with_workspaces.tsx index 2b348324420..82e7cc25383 100644 --- a/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/share_channel_with_workspaces.tsx +++ b/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/share_channel_with_workspaces.tsx @@ -10,7 +10,6 @@ import type {RemoteClusterInfo} from '@mattermost/types/shared_channels'; import {Client4} from 'mattermost-redux/client'; -import useDidUpdate from 'components/common/hooks/useDidUpdate'; import Toggle from 'components/toggle'; import AddWorkspaceDropdown, {type RemoteToAdd} from './add_workspace_dropdown'; @@ -65,10 +64,6 @@ export default function ShareChannelWithWorkspaces({ const currentRemoteIds = useMemo(() => new Set(remotes.map((w) => w.remote_id || w.name)), [remotes]); - useDidUpdate(() => { - onRemotesChange(remotes); - }, [remotes, onRemotesChange]); - const handleToggle = useCallback(() => { const next = !enabled; onToggle?.(next);