[MM-68652] Fix saved state on channel settings shared channels (#37173)

* [MM-68652] Fix saved state on channel settings shared channels

* Address feedback and simplify flow

* Address comments
This commit is contained in:
Daniel Espino García
2026-07-09 11:12:45 +02:00
committed by GitHub
parent c4620b9f26
commit 384b404f6c
3 changed files with 398 additions and 58 deletions
@@ -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(
<ChannelSettingsConfigurationTab
{...baseProps}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
channel={{...mockChannel, shared: true}}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
channel={{...mockChannel, shared: true}}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
channel={{...mockChannel, shared: true}}
canManageSharedChannels={true}
/>,
);
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(
<ChannelSettingsConfigurationTab
{...baseProps}
channel={{...mockChannel, shared: true}}
canManageSharedChannels={true}
setAreThereUnsavedChanges={setAreThereUnsavedChanges}
/>,
);
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');
@@ -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<WorkspaceWithStatus[]>(() =>
(initialRemotes || []).map((r) => ({...r})),
);
const initialRemotes = useSelector((state: GlobalState) => getRemotesForChannel(state, channel.id));
const userEditedSharingRef = useRef(false);
const [savedSharing, setSavedSharing] = useState<SharingSnapshot>(() => ({
enabled: (initialRemotes || []).length > 0,
remotes: copyRemotes(initialRemotes || []),
}));
const [sharingEnabled, setSharingEnabled] = useState(savedSharing.enabled);
const [workspaceRemotes, setWorkspaceRemotes] = useState<WorkspaceWithStatus[]>(() => 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<WorkspaceWithStatus[]>) => {
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}
/>
<ShareChannelWithWorkspaces
key={shareChannelKey}
remotes={workspaceRemotes}
initialRemotes={initialRemotes}
onRemotesChange={setWorkspaceRemotes}
initialRemotes={savedSharing.remotes}
onRemotesChange={handleWorkspaceRemotesChange}
enabled={sharingEnabled}
onToggle={setSharingEnabled}
onToggle={handleSharingToggle}
/>
</>
)}
@@ -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);