[MM-70822] Explain the blocked section switch while editing a channel permission rule (#38751)

* [MM-70822] Explain the blocked section switch while editing a channel permission rule The Channel Settings modal refuses to switch sections while a tab reports unsaved changes, and the Permissions Policy tab reports unsaved changes for as long as its rule editor is open. The SaveChangesPanel that explains the refusal only renders in that tab's list view, so clicking Info, Membership Policy or Configuration while editing a rule did nothing at all: no section change, no message, even though the items still looked clickable. Route the modal's existing showTabSwitchError signal into the rule editor's error slot so the refusal is explained where the user is looking, and mark that slot role='alert' so it is announced rather than only seen. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-70822] Scroll the blocked section-switch warning into view When the rule editor is taller than the modal, the alert that explains a refused section switch sat below the fold. Scroll it into view when the error appears so the message is actually visible. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Align max-login-attempts Playwright locators with current login copy The full Playwright suite failed on max_login_attempts.spec.ts: exact matches for the old invalid-credentials banner and lockout sentence no longer hold. Use the shared invalidCredentialsError locator and the stable lock-message prefix. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Pick future dates in the demo plugin date dialog e2e spec Days 20 and 22 of the current month are in the past after mid-month, and the plugin rejects those submissions. Advance to next month in that case. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Retrigger CI for Enterprise docker image and E2E Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-70822] Report the blocked section switch through the modal's floating footer Per review, the rule editor now raises the same SaveChangesPanel the rest of the Channel Settings modal uses, in its built-in error state, instead of a bespoke inline message — matching the Team Settings membership policy editor. The footer is position: fixed at the bottom of the modal, so it is visible whatever the editor is scrolled to; the scroll-into-view workaround the inline message needed goes away with it. Saving from the footer stays disabled (the rule is committed from the editor's own button) and its Cancel leaves the editor, so the two ways out of the block are the ones the message names. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-70822] Cover the blocked-switch footer's way out in the E2E spec The footer's Cancel is now the exit the message points at, so assert it drops the draft and returns to the rules list. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Revert unrelated e2e test fixes Restore demo_dialog_date.spec.ts and max_login_attempts.spec.ts to their upstream state. These e2e changes are unrelated to the MM-70822 channel permission-rule fix and were flagged for removal in review. Address PR feedback: 0 answered, 1 resolved, 0 declined * Retrigger CI after an unrelated Playwright flake on HEAD Co-authored-by: mattermost-code <matty-code@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> Co-authored-by: Mattermost Build <build@mattermost.com>
2026-11-03 20:56:57 -06:00 · 2026-10-02 19:03:29 -04:00
1 parent 6338ffdc76
commit 85e95437ab
@@ -154,4 +154,67 @@ test.describe('Channel Settings Modal - Permissions Policy tab (v0.4)', () => {

        await channelSettings.close();
    });
+
+    /**
+     * @objective Verify that an open permission rule editor, which blocks navigation to the modal's other
+     * sections, says so instead of silently swallowing the click.
+     */
+    test(
+        'MM-PP_v0_4_c4 blocked section switch while editing a permission rule explains itself',
+        {tag: '@channel_settings'},
+        async ({pw}) => {
+            const {adminUser, adminClient, team} = await pw.initSetup();
+            await enableABACConfig(adminClient);
+            await ensureDepartmentAttribute(adminClient);
+
+            const channel = await createPrivateChannel(adminClient, team.id);
+
+            const {page} = await pw.testBrowser.login(adminUser);
+            const channelsPage = new ChannelsPage(page);
+            await channelsPage.goto(team.name, channel.name);
+            await channelsPage.toBeVisible();
+
+            const channelSettings = await channelsPage.openChannelSettings();
+            const permissionsTab = channelSettings.container.getByTestId('permissions_policy-tab-button');
+            await expect(permissionsTab).toBeVisible();
+            await permissionsTab.click();
+
+            const tab = channelSettings.container.locator('.ChannelSettingsModal__permissionsPolicyTab');
+            await expect(tab).toBeVisible({timeout: 10000});
+
+            // # Open the rule editor and start a draft
+            await tab.getByTestId('permissions-policy-add-rule').click();
+            const editor = channelSettings.container.getByTestId('permissions-policy-editor');
+            await expect(editor).toBeVisible({timeout: 5000});
+            await editor.getByTestId('permissions-policy-editor-name').fill('Block external uploads');
+
+            // # Try to leave the editor by selecting the Info section
+            await channelSettings.infoTab.click();
+
+            // * The blocked switch is reported through the modal's floating
+            // footer in its error state, with saving from there disabled. The
+            // modal withdraws it after 3s, so assert inside that window.
+            const savePanel = channelSettings.container.locator('.SaveChangesPanel');
+            await expect(savePanel).toContainText('You have unsaved changes', {timeout: 2000});
+            await expect(savePanel).toHaveClass(/error/);
+            await expect(channelSettings.saveButton).toBeDisabled();
+
+            // * The Permissions Policy section is still the selected one and the draft is intact
+            await expect(permissionsTab).toHaveAttribute('aria-selected', 'true');
+            await expect(editor.getByTestId('permissions-policy-editor-name')).toHaveValue('Block external uploads');
+            await expect(channelSettings.infoSettings.container).not.toBeVisible();
+
+            // # Take the way out the footer offers
+            await savePanel.getByTestId('SaveChangesPanel__cancel-btn').click();
+
+            // * The draft is dropped and the rules list is back
+            await expect(editor).not.toBeVisible();
+            await expect(tab.getByTestId('permissions-policy-add-rule')).toBeVisible();
+
+            // * Section switching works again once no rule is being edited
+            await channelSettings.openInfoTab();
+
+            await channelSettings.close();
+        },
+    );
 });
@@ -128,6 +128,31 @@ jest.mock('./channel_settings_access_rules_tab', () => {
    };
 });

+jest.mock('./channel_settings_permissions_policy_tab', () => {
+    return function MockPermissionsPolicyTab({
+        setAreThereUnsavedChanges,
+        showTabSwitchError,
+    }: {
+        setAreThereUnsavedChanges: (value: boolean) => void;
+        showTabSwitchError?: boolean;
+    }): JSX.Element {
+        return (
+            <div data-testid='permissions-policy-tab'>
+                {'Permissions Policy Tab Content'}
+                <button
+                    data-testid='permissions-policy-open-editor'
+                    onClick={() => setAreThereUnsavedChanges(true)}
+                >
+                    {'Open Rule Editor'}
+                </button>
+                {showTabSwitchError && (
+                    <div data-testid='permissions-policy-tab-switch-error'>{'You have unsaved changes'}</div>
+                )}
+            </div>
+        );
+    };
+});
+
 // Define the tab type for the settings sidebar
 type TabType = {
    name: string;
@@ -1032,6 +1057,33 @@ describe('ChannelSettingsModal', () => {
        });
    });

+    describe('permissions policy tab wiring', () => {
+        it('tells the permissions policy tab when it blocks a section switch', async () => {
+            mockManageChannelAccessRulesPermission = true;
+
+            const testState = makeTestState();
+            testState.entities.channels.channels[channelId].type = General.PRIVATE_CHANNEL;
+            testState.entities.general.config.FeatureFlagPermissionPolicies = 'true';
+            testState.entities.general.config.FeatureFlagChannelPermissionPolicies = 'true';
+
+            renderWithContext(<ChannelSettingsModal {...baseProps}/>, testState);
+
+            await userEvent.click(await screen.findByRole('tab', {name: /permissions policy/i}));
+            expect(await screen.findByTestId('permissions-policy-tab')).toBeInTheDocument();
+
+            // Opening the rule editor reports unsaved changes, which is what makes
+            // the modal refuse the switch — but is not, on its own, an error to show.
+            await userEvent.click(screen.getByTestId('permissions-policy-open-editor'));
+            expect(screen.queryByTestId('permissions-policy-tab-switch-error')).not.toBeInTheDocument();
+
+            await userEvent.click(screen.getByRole('tab', {name: /info/i}));
+
+            expect(await screen.findByTestId('permissions-policy-tab-switch-error')).toBeInTheDocument();
+            expect(screen.queryByTestId('info-tab')).not.toBeInTheDocument();
+            expect(screen.getByTestId('permissions-policy-tab')).toBeInTheDocument();
+        });
+    });
+
    describe('schema tab wiring', () => {
        it('renders declarative schema controls without a save bar until changed', async () => {
            const onSave = jest.fn(async () => {});
@@ -174,13 +174,35 @@ describe('components/channel_settings_modal/ChannelSettingsPermissionsPolicyTab'
    };

    const openNewRuleEditor = async () => {
-        renderWithContext(<ChannelSettingsPermissionsPolicyTab {...baseProps}/>, initialState);
+        const view = renderWithContext(<ChannelSettingsPermissionsPolicyTab {...baseProps}/>, initialState);

        const addRuleButton = await screen.findByTestId('permissions-policy-add-rule');
        await waitFor(() => expect(addRuleButton).toBeEnabled());
        await userEvent.click(addRuleButton);

        await screen.findByTestId('permissions-policy-editor');
+
+        return view;
+    };
+
+    const openExistingRuleEditor = async (props = baseProps) => {
+        mockActions.getChannelPolicy.mockResolvedValue({
+            data: {
+                rules: [{
+                    name: 'Existing rule',
+                    role: ACCESS_CONTROL_CHANNEL_ROLE_USER,
+                    actions: [ACCESS_CONTROL_ACTION_UPLOAD_FILE],
+                    expression: 'user.attributes.team == "eng"',
+                }],
+            },
+        });
+
+        const view = renderWithContext(<ChannelSettingsPermissionsPolicyTab {...props}/>, initialState);
+
+        await userEvent.click(await screen.findByText('Existing rule'));
+        await screen.findByTestId('permissions-policy-editor');
+
+        return view;
    };

    const openNewRuleEditorWithFields = async () => {
@@ -615,4 +637,127 @@ describe('components/channel_settings_modal/ChannelSettingsPermissionsPolicyTab'
        expect(screen.getByTestId('table-editor-value')).toHaveTextContent(newExpression);
        expect(screen.getByTestId(`permissions-policy-editor-action-${ACCESS_CONTROL_ACTION_UPLOAD_FILE}`)).toBeInTheDocument();
    });
+
+    test('reports unsaved changes to the modal while a rule is open in the editor', async () => {
+        const setAreThereUnsavedChanges = jest.fn();
+
+        await openExistingRuleEditor({...baseProps, setAreThereUnsavedChanges});
+
+        // An open editor holds a draft the modal cannot save for the user, so it
+        // counts as unsaved even before the rule is edited — that is what makes
+        // the modal block section switches.
+        await waitFor(() => {
+            expect(setAreThereUnsavedChanges).toHaveBeenLastCalledWith(true);
+        });
+
+        await userEvent.click(screen.getByTestId('permissions-policy-editor-cancel'));
+
+        await waitFor(() => {
+            expect(setAreThereUnsavedChanges).toHaveBeenLastCalledWith(false);
+        });
+    });
+
+    test('explains the blocked section switch while the rule editor is open', async () => {
+        const {rerender} = await openExistingRuleEditor();
+
+        expect(screen.queryByText('You have unsaved changes')).not.toBeInTheDocument();
+
+        rerender(
+            <ChannelSettingsPermissionsPolicyTab
+                {...baseProps}
+                showTabSwitchError={true}
+            />,
+        );
+
+        // The same floating footer the rest of the modal uses, in its error state.
+        expect(await screen.findByText('You have unsaved changes')).toBeInTheDocument();
+        expect(screen.getByTestId('SaveChangesPanel__save-btn')).toBeDisabled();
+
+        // The modal clears the flag on a timeout.
+        rerender(
+            <ChannelSettingsPermissionsPolicyTab
+                {...baseProps}
+                showTabSwitchError={false}
+            />,
+        );
+
+        await waitFor(() => {
+            expect(screen.queryByText('You have unsaved changes')).not.toBeInTheDocument();
+        });
+    });
+
+    test('explains the blocked section switch while a new rule is being added', async () => {
+        const {rerender} = await openNewRuleEditor();
+
+        rerender(
+            <ChannelSettingsPermissionsPolicyTab
+                {...baseProps}
+                showTabSwitchError={true}
+            />,
+        );
+
+        expect(await screen.findByText('You have unsaved changes')).toBeInTheDocument();
+    });
+
+    test('leaves the rule editor when the blocked-switch footer is cancelled', async () => {
+        const {rerender} = await openExistingRuleEditor();
+
+        rerender(
+            <ChannelSettingsPermissionsPolicyTab
+                {...baseProps}
+                showTabSwitchError={true}
+            />,
+        );
+
+        await userEvent.click(await screen.findByTestId('SaveChangesPanel__cancel-btn'));
+
+        // Back on the rules list, with the rule the editor was showing intact.
+        expect(screen.queryByTestId('permissions-policy-editor')).not.toBeInTheDocument();
+        expect(screen.getByText('Existing rule')).toBeInTheDocument();
+    });
+
+    test('keeps the in-progress draft when a section switch is blocked', async () => {
+        const {rerender} = await openExistingRuleEditor();
+        await screen.findByTestId('table-editor');
+
+        const newExpression = 'user.attributes.team == "ops"';
+        await userEvent.clear(screen.getByTestId('permissions-policy-editor-name'));
+        await userEvent.type(screen.getByTestId('permissions-policy-editor-name'), 'Renamed rule');
+        act(() => {
+            latestTableEditorProps().onChange(newExpression);
+        });
+
+        rerender(
+            <ChannelSettingsPermissionsPolicyTab
+                {...baseProps}
+                showTabSwitchError={true}
+            />,
+        );
+
+        expect(await screen.findByText('You have unsaved changes')).toBeInTheDocument();
+
+        // The message asks the user to save the rule, so the rule has to survive it.
+        expect(screen.getByTestId('permissions-policy-editor-name')).toHaveValue('Renamed rule');
+        expect(screen.getByTestId('table-editor-value')).toHaveTextContent(newExpression);
+    });
+
+    test('keeps a save validation error visible when a section switch is blocked', async () => {
+        const {rerender} = await openExistingRuleEditor();
+
+        await userEvent.clear(screen.getByTestId('permissions-policy-editor-name'));
+        await userEvent.click(screen.getByTestId('permissions-policy-editor-save'));
+        expect(await screen.findByTestId('permissions-policy-editor-error')).toHaveTextContent('Each permission rule needs a unique name.');
+
+        // A blocked section switch must not overwrite the reason the rule
+        // cannot be saved in the first place.
+        rerender(
+            <ChannelSettingsPermissionsPolicyTab
+                {...baseProps}
+                showTabSwitchError={true}
+            />,
+        );
+
+        expect(screen.getByTestId('permissions-policy-editor-error')).toHaveTextContent('Each permission rule needs a unique name.');
+        expect(screen.getByText('You have unsaved changes')).toBeInTheDocument();
+    });
 });
@@ -644,22 +644,44 @@ function ChannelSettingsPermissionsPolicyTab({
        }

        return (
-            <PermissionRuleEditor
-                key={editingKey}
-                initial={initial}
-                isNew={isNew}
-                channelId={channel.id}
-                actions={actions}
-                userAttributes={mergedAttributes}
-                attributesLoaded={attributesLoaded}
-                enableUserManagedAttributes={accessControlSettings?.EnableUserManagedAttributes || false}
-                isSystemAdmin={isSystemAdmin}
-                error={formError}
-                onCancel={cancelEditor}
-                onCommit={commitDraft}
-                buildSimulationPolicy={buildSimulationPolicy}
-                policySimulationEnabled={policySimulationEnabled}
-            />
+            <>
+                <PermissionRuleEditor
+                    key={editingKey}
+                    initial={initial}
+                    isNew={isNew}
+                    channelId={channel.id}
+                    actions={actions}
+                    userAttributes={mergedAttributes}
+                    attributesLoaded={attributesLoaded}
+                    enableUserManagedAttributes={accessControlSettings?.EnableUserManagedAttributes || false}
+                    isSystemAdmin={isSystemAdmin}
+                    error={formError}
+                    onCancel={cancelEditor}
+                    onCommit={commitDraft}
+                    buildSimulationPolicy={buildSimulationPolicy}
+                    policySimulationEnabled={policySimulationEnabled}
+                />
+
+                {/* An open rule editor counts as unsaved changes, so the modal
+                  * refuses section switches while it is open. The list view's
+                  * panel is not rendered here, so bring up the same floating
+                  * footer the rest of the modal uses to report that refusal.
+                  * Its Save is disabled in this state — the rule itself is
+                  * committed from the editor's own button. */}
+                {showTabSwitchError && (
+                    <SaveChangesPanel
+                        handleSubmit={handleSaveChanges}
+                        handleCancel={cancelEditor}
+                        handleClose={handleClose}
+                        tabChangeError={true}
+                        state={SAVE_RESULT_ERROR}
+                        cancelButtonText={formatMessage({
+                            id: 'channel_settings.permissions_policy.editor.cancel',
+                            defaultMessage: 'Cancel',
+                        })}
+                    />
+                )}
+            </>
        );
    }