feat: enhance Assign Role Wizard with multi-step functionality and scope management

Author: bra-i-am

src/authz-module/data/types.ts

@@ -38,14 +38,14 @@ export type PermissionsByRole = {
 };
 
 export interface PutAssignTeamMembersRoleResponse {
-  completed: { userIdentifier: string; status: string }[];
-  errors: { userIdentifier: string; error: string }[];
+  completed: { userIdentifier: string; scope: string; status: string }[];
+  errors: { userIdentifier: string; scope: string; error: string }[];
 }
 
 export interface AssignTeamMembersRoleRequest {
   users: string[];
   role: string;
-  scope: string;
+  scopes: string[];
 }
 
 export type ValidateUsersRequest = {

src/authz-module/index.scss

@@ -54,6 +54,11 @@
   }
 }
 
+.scope-list {
+  max-height: 500px;
+  overflow-y: auto;
+}
+
 .toast-container {
   // Ensure toast appears above modal
   z-index: 1000;

src/authz-module/wizard/AssignRoleWizard.test.tsx

@@ -2,37 +2,51 @@ import { screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { renderWrapper } from '@src/setupTest';
 import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
-import { useValidateUsers } from '../data/hooks';
+import { useValidateUsers, useAssignTeamMembersRole } from '../data/hooks';
 import { ToastManagerProvider } from '../libraries-manager/ToastManagerContext';
 import AssignRoleWizard from './AssignRoleWizard';
 
 jest.mock('@edx/frontend-platform/auth', () => ({
   getAuthenticatedUser: jest.fn(),
 }));
 
+jest.mock('../data/hooks', () => ({
+  useValidateUsers: jest.fn(),
+  useAssignTeamMembersRole: jest.fn(),
+}));
+
+jest.mock('./DefineApplicationScopeStep', () => ({
+  __esModule: true,
+  default: ({ onScopeToggle }: { onScopeToggle: (id: string) => void }) => (
+    <div>
+      <button type="button" data-testid="toggle-scope" onClick={() => onScopeToggle('lib:test:123')}>
+        Toggle scope
+      </button>
+    </div>
+  ),
+}));
+
 const mockUseValidateUsers = useValidateUsers as jest.Mock;
+const mockUseAssignTeamMembersRole = useAssignTeamMembersRole as jest.Mock;
 const mockValidateMutateAsync = jest.fn();
+const mockAssignMutateAsync = jest.fn();
 
 const renderWizard = (props = {}) => renderWrapper(
   <ToastManagerProvider>
     <AssignRoleWizard onClose={jest.fn()} {...props} />
   </ToastManagerProvider>,
 );
 
-// selectors
 const getUsersInput = () => screen.getByLabelText(/Add users by username or email/i);
 const getRoleRadio = (name: RegExp) => screen.getByRole('radio', { name });
 const getNextButton = () => screen.getByRole('button', { name: /^Next$/i });
 const getCancelButton = () => screen.getByRole('button', { name: /^Cancel$/i });
-const getStep2Heading = () => screen.queryByRole('heading', { name: 'Step 2' });
 
 describe('AssignRoleWizard — Step 1', () => {
   beforeEach(() => {
     jest.clearAllMocks();
-    mockUseValidateUsers.mockReturnValue({
-      mutateAsync: mockValidateMutateAsync,
-      isPending: false,
-    });
+    mockUseValidateUsers.mockReturnValue({ mutateAsync: mockValidateMutateAsync, isPending: false });
+    mockUseAssignTeamMembersRole.mockReturnValue({ mutateAsync: mockAssignMutateAsync, isPending: false });
     (getAuthenticatedUser as jest.Mock).mockReturnValue({ administrator: true });
   });
 
@@ -49,6 +63,11 @@ describe('AssignRoleWizard — Step 1', () => {
     expect(getUsersInput()).toHaveValue('alice');
   });
 
+  it('Next button is disabled without both users and a role', () => {
+    renderWizard();
+    expect(getNextButton()).toBeDisabled();
+  });
+
   it('selecting a different role replaces the previous selection', async () => {
     const user = userEvent.setup();
     renderWizard();
@@ -67,7 +86,7 @@ describe('AssignRoleWizard — Step 1', () => {
     await user.click(getNextButton());
     await waitFor(() => {
       expect(screen.getByText(/not associated with an account/i)).toBeInTheDocument();
-      expect(getStep2Heading()).not.toBeInTheDocument();
+      expect(screen.queryByTestId('toggle-scope')).not.toBeInTheDocument();
     });
   });
 
@@ -79,7 +98,7 @@ describe('AssignRoleWizard — Step 1', () => {
     await user.click(getRoleRadio(/Library Admin/i));
     await user.click(getNextButton());
     await waitFor(() => {
-      expect(screen.getByRole('heading', { name: 'Step 2' })).toBeInTheDocument();
+      expect(screen.getByTestId('toggle-scope')).toBeInTheDocument();
     });
   });
 
@@ -91,9 +110,8 @@ describe('AssignRoleWizard — Step 1', () => {
     await user.click(getRoleRadio(/Library Admin/i));
     await user.click(getNextButton());
     await waitFor(() => {
-      // Toast should appear with retry option
       expect(screen.getByRole('alert')).toBeInTheDocument();
-      expect(getStep2Heading()).not.toBeInTheDocument();
+      expect(screen.queryByTestId('toggle-scope')).not.toBeInTheDocument();
     });
   });
 
@@ -117,18 +135,11 @@ describe('AssignRoleWizard — Step 1', () => {
     renderWizard();
     await user.type(getUsersInput(), 'alice');
     await user.click(getRoleRadio(/Library Admin/i));
-
-    // First attempt fails - toast appears with retry
     await user.click(getNextButton());
     await waitFor(() => expect(screen.getByRole('alert')).toBeInTheDocument());
-
-    // Click retry button
-    const retryButton = screen.getByRole('button', { name: /retry/i });
-    await user.click(retryButton);
-
-    // Should advance to step 2 on retry
+    await user.click(screen.getByRole('button', { name: /retry/i }));
     await waitFor(() => {
-      expect(getStep2Heading()).toBeInTheDocument();
+      expect(screen.getByTestId('toggle-scope')).toBeInTheDocument();
     });
   });
 });
@@ -140,16 +151,15 @@ describe('AssignRoleWizard — Step 2', () => {
     await user.click(getRoleRadio(/Library Admin/i));
     await user.click(getNextButton());
     await waitFor(() => {
-      expect(screen.getByRole('heading', { name: 'Step 2' })).toBeInTheDocument();
+      expect(screen.getByTestId('toggle-scope')).toBeInTheDocument();
     });
   };
 
   beforeEach(() => {
     jest.clearAllMocks();
-    mockUseValidateUsers.mockReturnValue({
-      mutateAsync: mockValidateMutateAsync,
-      isPending: false,
-    });
+    mockUseValidateUsers.mockReturnValue({ mutateAsync: mockValidateMutateAsync, isPending: false });
+    mockUseAssignTeamMembersRole.mockReturnValue({ mutateAsync: mockAssignMutateAsync, isPending: false });
+    (getAuthenticatedUser as jest.Mock).mockReturnValue({ administrator: true });
   });
 
   it('Back button returns to Step 1', async () => {
@@ -161,119 +171,57 @@ describe('AssignRoleWizard — Step 2', () => {
       expect(getUsersInput()).toBeInTheDocument();
     });
   });
-});
-
-describe('AssignRoleWizard — Step 2', () => {
-  const mockOnScopeToggle = jest.fn();
 
-  beforeEach(() => {
-    jest.clearAllMocks();
-    mockOnScopeToggle.mockClear();
-    mockUseValidateUsers.mockReturnValue({
-      mutateAsync: mockValidateMutateAsync,
-      isPending: false,
-    });
-    (getAuthenticatedUser as jest.Mock).mockReturnValue({ administrator: true });
-  });
-
-  it('advances to Step 2 with valid users', async () => {
-    const user = userEvent.setup();
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
-    renderWizard();
-    await user.type(screen.getByTestId('users-input'), 'alice');
-    await user.click(screen.getByTestId('role-option-library_admin'));
-    await user.click(screen.getByRole('button', { name: /Next/i }));
-    await waitFor(() => {
-      expect(screen.getByTestId('stepper')).toHaveAttribute('data-active-step', 'define-application-scope');
-    });
-  });
-
-  it('calls onScopeToggle when scope is selected', async () => {
+  it('Save button is disabled when no scopes are selected', async () => {
     const user = userEvent.setup();
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
     renderWizard();
-    await user.type(screen.getByTestId('users-input'), 'alice');
-    await user.click(screen.getByTestId('role-option-library_admin'));
-    await user.click(screen.getByRole('button', { name: /Next/i }));
-    await waitFor(() => {
-      expect(screen.getByTestId('stepper')).toHaveAttribute('data-active-step', 'define-application-scope');
-    });
-    await user.click(screen.getByTestId('toggle-scope'));
-    // Scope toggle is called from the mocked component
+    await advanceToStep2(user);
+    expect(screen.getByRole('button', { name: /^Save$/i })).toBeDisabled();
   });
 
-  it('saves role assignment successfully', async () => {
+  it('saves role assignment successfully and closes the wizard', async () => {
     const user = userEvent.setup();
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
     const onClose = jest.fn();
+    mockAssignMutateAsync.mockResolvedValue({
+      completed: [{ userIdentifier: 'alice', scope: 'lib:test:123', status: 'role_added' }],
+      errors: [],
+    });
     renderWizard({ onClose });
-    await user.type(screen.getByTestId('users-input'), 'alice');
-    await user.click(screen.getByTestId('role-option-library_admin'));
-    await user.click(screen.getByRole('button', { name: /Next/i }));
+    await advanceToStep2(user);
+    await user.click(screen.getByTestId('toggle-scope'));
+    await user.click(screen.getByRole('button', { name: /^Save$/i }));
     await waitFor(() => {
-      expect(screen.getByTestId('stepper')).toHaveAttribute('data-active-step', 'define-application-scope');
+      expect(mockAssignMutateAsync).toHaveBeenCalledWith({
+        data: { users: ['alice'], role: 'library_admin', scopes: ['lib:test:123'] },
+      });
+      expect(onClose).toHaveBeenCalled();
     });
-    await user.click(screen.getByTestId('toggle-scope'));
-    await user.click(screen.getByRole('button', { name: /Save/i }));
-    // The save button is in Step 2
   });
 
-  // Skipped - complex integration test that requires proper scope selection propagation
-  // The error handling logic is tested in the code and works correctly in production
-  it.skip('shows error when save fails', async () => {
+  it('shows error toast when the API returns assignment errors', async () => {
     const user = userEvent.setup();
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
+    mockAssignMutateAsync.mockResolvedValue({
+      completed: [],
+      errors: [{ userIdentifier: 'alice', scope: 'lib:test:123', error: 'already has role' }],
+    });
     renderWizard();
-    await user.type(screen.getByTestId('users-input'), 'alice');
-    await user.click(screen.getByTestId('role-option-library_admin'));
-    await user.click(screen.getByRole('button', { name: /Next/i }));
+    await advanceToStep2(user);
+    await user.click(screen.getByTestId('toggle-scope'));
+    await user.click(screen.getByRole('button', { name: /^Save$/i }));
     await waitFor(() => {
-      expect(screen.getByTestId('stepper')).toHaveAttribute('data-active-step', 'define-application-scope');
+      expect(screen.getByRole('alert')).toBeInTheDocument();
     });
   });
 
-  it('goes back to Step 1', async () => {
+  it('shows error toast when save throws a network error', async () => {
     const user = userEvent.setup();
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
+    mockAssignMutateAsync.mockRejectedValue(new Error('Network error'));
     renderWizard();
-    await user.type(screen.getByTestId('users-input'), 'alice');
-    await user.click(screen.getByTestId('role-option-library_admin'));
-    await user.click(screen.getByRole('button', { name: /Next/i }));
-    await waitFor(() => {
-      expect(screen.getByTestId('stepper')).toHaveAttribute('data-active-step', 'define-application-scope');
-    });
-    await user.click(screen.getByRole('button', { name: /Back/i }));
+    await advanceToStep2(user);
+    await user.click(screen.getByTestId('toggle-scope'));
+    await user.click(screen.getByRole('button', { name: /^Save$/i }));
     await waitFor(() => {
-      expect(screen.getByTestId('stepper')).toHaveAttribute('data-active-step', 'select-users-and-role');
-    });
-  });
-});
-
-describe('AssignRoleWizard — error handling', () => {
-  beforeEach(() => {
-    jest.clearAllMocks();
-    mockUseValidateUsers.mockReturnValue({
-      mutateAsync: mockValidateMutateAsync,
-      isPending: false,
+      expect(screen.getByRole('alert')).toBeInTheDocument();
     });
-    (getAuthenticatedUser as jest.Mock).mockReturnValue({ administrator: true });
-  });
-
-  it('returns early when no role selected', async () => {
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
-    renderWizard();
-    await userEvent.type(screen.getByTestId('users-input'), 'alice');
-    await userEvent.click(screen.getByRole('button', { name: /Next/i }));
-    // Without a role, should not proceed
-  });
-
-  it('returns early when no scopes selected', async () => {
-    const user = userEvent.setup();
-    mockValidateMutateAsync.mockResolvedValue({ invalidUsers: [], validUsers: ['alice'] });
-    renderWizard();
-    await user.type(screen.getByTestId('users-input'), 'alice');
-    await user.click(screen.getByTestId('role-option-library_admin'));
-    await user.click(screen.getByRole('button', { name: /Next/i }));
-    // Without scopes in step 2, save returns early
   });
 });