From 0809ce7a62c3484e6eace956fc945b8aeac794f1 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Tue, 1 Jul 2025 20:48:57 +0200 Subject: [PATCH] [MM-64630] Fix an issue where multiple channels can't be removed from policies (#32164) * Fix an issue where multiple channels can't be removed from policies * actually fix the issue * use hardcoded limit * simplify removal --- server/channels/app/access_control.go | 1 + server/channels/app/access_control_test.go | 2 +- .../policy_details.test.tsx.snap | 2 - .../channel_list/channel_list.tsx | 10 +---- .../policy_details/policy_details.tsx | 38 ++++++++----------- 5 files changed, 18 insertions(+), 35 deletions(-) diff --git a/server/channels/app/access_control.go b/server/channels/app/access_control.go index affdef1c35..3aba611854 100644 --- a/server/channels/app/access_control.go +++ b/server/channels/app/access_control.go @@ -183,6 +183,7 @@ func (a *App) UnassignPoliciesFromChannels(rctx request.CTX, policyID string, ch cps, _, err := a.Srv().Store().AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{ Type: model.AccessControlPolicyTypeChannel, ParentID: policyID, + Limit: 1000, }) if err != nil { return model.NewAppError("UnassignPoliciesFromChannels", "app.pap.unassign_access_control_policy_from_channels.app_error", nil, err.Error(), http.StatusInternalServerError) diff --git a/server/channels/app/access_control_test.go b/server/channels/app/access_control_test.go index 6c966c8b3d..a9534e0aea 100644 --- a/server/channels/app/access_control_test.go +++ b/server/channels/app/access_control_test.go @@ -341,7 +341,7 @@ func TestAssignAccessControlPolicyToChannels(t *testing.T) { }) } -func TestUnAssignPoliciesFromChannels(t *testing.T) { +func TestUnassignPoliciesFromChannels(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap b/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap index 4548e42efa..ac5864f187 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap @@ -142,7 +142,6 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh channelsToAdd={Object {}} channelsToRemove={Object {}} onRemoveCallback={[Function]} - onUndoRemoveCallback={[Function]} policyId="policy1" /> @@ -346,7 +345,6 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh channelsToAdd={Object {}} channelsToRemove={Object {}} onRemoveCallback={[Function]} - onUndoRemoveCallback={[Function]} policyId="" /> diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx b/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx index 91dbbddc4d..2d052eb7a7 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx @@ -30,7 +30,6 @@ type Props = { filters: ChannelSearchOpts; policyId?: string; onRemoveCallback: (channel: ChannelWithTeamData) => void; - onUndoRemoveCallback: (channel: ChannelWithTeamData) => void; channelsToRemove: Record; channelsToAdd: Record; actions: { @@ -196,16 +195,9 @@ export default class ChannelList extends React.PureComponent { }; private removeChannel = (channel: ChannelWithTeamData) => { - const {channelsToRemove, onRemoveCallback, onUndoRemoveCallback} = this.props; + const {onRemoveCallback} = this.props; const {page} = this.state; - // Toggle between adding and removing the channel - if (channelsToRemove[channel.id] === channel) { - // If the channel is already marked for removal, undo it - onUndoRemoveCallback(channel); - return; - } - // If the channel is not marked for removal, mark it onRemoveCallback(channel); diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx b/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx index cfc40432af..c9a52e4f9d 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx @@ -279,38 +279,31 @@ function PolicyDetails({ } }; - const handleChannelChanges = (channels: ChannelWithTeamData[], isAdding: boolean) => { + const addToNewChannels = (channels: ChannelWithTeamData[]) => { setChannelChanges((prev) => { const newChanges = cloneDeep(prev); - - channels.forEach((channel) => { - if (isAdding) { - if (newChanges.removed[channel.id]) { - delete newChanges.removed[channel.id]; - newChanges.removedCount--; - } else { - newChanges.added[channel.id] = channel; - } - } else if (newChanges.added[channel.id]) { - delete newChanges.added[channel.id]; - } else if (!newChanges.removed[channel.id]) { - newChanges.removedCount++; - newChanges.removed[channel.id] = channel; + channels.forEach((channel: ChannelWithTeamData) => { + if (newChanges.removed[channel.id]?.id === channel.id) { + delete newChanges.removed[channel.id]; + newChanges.removedCount--; + } else { + newChanges.added[channel.id] = channel; } }); - return newChanges; }); setSaveNeeded(true); actions.setNavigationBlocked(true); }; - const handleUndoRemove = (channel: ChannelWithTeamData) => { + const addToRemovedChannels = (channel: ChannelWithTeamData) => { setChannelChanges((prev) => { const newChanges = cloneDeep(prev); - if (newChanges.removed[channel.id]) { - delete newChanges.removed[channel.id]; - newChanges.removedCount--; + if (newChanges.added[channel.id]?.id === channel.id) { + delete newChanges.added[channel.id]; + } else if (newChanges.removed[channel.id]?.id !== channel.id) { + newChanges.removedCount++; + newChanges.removed[channel.id] = channel; } return newChanges; }); @@ -518,8 +511,7 @@ function PolicyDetails({ handleChannelChanges([channel], false)} - onUndoRemoveCallback={handleUndoRemove} + onRemoveCallback={(channel) => addToRemovedChannels(channel)} channelsToRemove={channelChanges.removed} channelsToAdd={channelChanges.added} policyId={policyId} @@ -575,7 +567,7 @@ function PolicyDetails({ {addChannelOpen && ( setAddChannelOpen(false)} - onChannelsSelected={(channels) => handleChannelChanges(channels, true)} + onChannelsSelected={(channels) => addToNewChannels(channels)} groupID={''} alreadySelected={Object.values(channelChanges.added).map((channel) => channel.id)} excludeAccessControlPolicyEnforced={true}