Fixes bug where user/team role add modal state is not cleared on close

2026-01-17 04:31:21 -03:30 · 2021-05-26 16:32:22 -04:00 · 2021-05-26 16:32:22 -04:00 · fa7a459e50
commit fa7a459e50
parent 4e129d3d04
6 changed files with 101 additions and 96 deletions
--- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx
+++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx
@ -101,7 +101,9 @@ function SelectResourceStep({
        headerRow={
          <HeaderRow qsConfig={QS_Config(sortColumns)}>
            {sortColumns.map(({ name, key }) => (
-              <HeaderCell sortKey={key}>{name}</HeaderCell>
+              <HeaderCell sortKey={key} key={key}>
+                {name}
+              </HeaderCell>
            ))}
          </HeaderRow>
        }
--- a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx
+++ b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx
@ -33,7 +33,7 @@ const CheckboxListItem = ({

      {columns?.length > 0 ? (
        columns.map(col => (
-          <Td aria-label={col.name} dataLabel={col.key}>
+          <Td aria-label={col.name} dataLabel={col.key} key={col.key}>
            {item[col.key]}
          </Td>
        ))
--- a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.jsx
+++ b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.jsx
@ -1,14 +1,9 @@
-import React, { useState, useCallback, useEffect, useContext } from 'react';
-
+import React, { useState, useCallback } from 'react';
 import { t } from '@lingui/macro';
 import { useParams } from 'react-router-dom';
 import styled from 'styled-components';
-import { Button, DropdownItem } from '@patternfly/react-core';
-import { KebabifiedContext } from '../../contexts/Kebabified';
-import useRequest, { useDismissableError } from '../../util/useRequest';
+import useRequest from '../../util/useRequest';
 import SelectableCard from '../SelectableCard';
-import AlertModal from '../AlertModal';
-import ErrorDetail from '../ErrorDetail';
 import Wizard from '../Wizard/Wizard';
 import useSelected from '../../util/useSelected';
 import SelectResourceStep from '../AddRole/SelectResourceStep';
@ -22,21 +17,20 @@ const Grid = styled.div`
  grid-template-columns: repeat(auto-fill, minmax(250px, 1fr));
 `;

-function UserAndTeamAccessAdd({ title, onFetchData, apiModel }) {
+function UserAndTeamAccessAdd({
+  title,
+  onFetchData,
+  apiModel,
+  onClose,
+  onError,
+}) {
  const [selectedResourceType, setSelectedResourceType] = useState(null);
-  const [isWizardOpen, setIsWizardOpen] = useState(false);
  const [stepIdReached, setStepIdReached] = useState(1);
  const { id: userId } = useParams();
-  const { isKebabified, onKebabModalChange } = useContext(KebabifiedContext);
  const {
    selected: resourcesSelected,
    handleSelect: handleResourceSelect,
  } = useSelected([]);
-  useEffect(() => {
-    if (isKebabified) {
-      onKebabModalChange(isWizardOpen);
-    }
-  }, [isKebabified, isWizardOpen, onKebabModalChange]);

  const {
    selected: rolesSelected,
@ -60,13 +54,10 @@ function UserAndTeamAccessAdd({ title, onFetchData, apiModel }) {

      await Promise.all(roleRequests);
      onFetchData();
-      setIsWizardOpen(false);
    }, [onFetchData, rolesSelected, apiModel, userId, resourcesSelected]),
    {}
  );

-  const { error, dismissError } = useDismissableError(saveError);
-
  const steps = [
    {
      id: 1,
@ -128,56 +119,22 @@ function UserAndTeamAccessAdd({ title, onFetchData, apiModel }) {
    },
  ];

-  if (error) {
-    return (
-      <AlertModal
-        aria-label={t`Associate role error`}
-        isOpen={error}
-        variant="error"
-        title={t`Error!`}
-        onClose={dismissError}
-      >
-        {t`Failed to associate role`}
-        <ErrorDetail error={error} />
-      </AlertModal>
-    );
+  if (saveError) {
+    onError(saveError);
+    onClose();
  }

  return (
-    <>
-      {isKebabified ? (
-        <DropdownItem
-          key="add"
-          component="button"
-          aria-label={t`Add`}
-          onClick={() => setIsWizardOpen(true)}
-        >
-          {t`Add`}
-        </DropdownItem>
-      ) : (
-        <Button
-          ouiaId="access-add-button"
-          variant="primary"
-          aria-label={t`Add`}
-          onClick={() => setIsWizardOpen(true)}
-          key="add"
-        >
-          {t`Add`}
-        </Button>
-      )}
-      {isWizardOpen && (
-        <Wizard
-          isOpen={isWizardOpen}
-          title={title}
-          steps={steps}
-          onClose={() => setIsWizardOpen(false)}
-          onNext={({ id }) =>
-            setStepIdReached(stepIdReached < id ? id : stepIdReached)
-          }
-          onSave={handleWizardSave}
-        />
-      )}
-    </>
+    <Wizard
+      isOpen
+      title={title}
+      steps={steps}
+      onClose={onClose}
+      onNext={({ id }) =>
+        setStepIdReached(stepIdReached < id ? id : stepIdReached)
+      }
+      onSave={handleWizardSave}
+    />
  );
 }

--- a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx
+++ b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx
@ -9,6 +9,9 @@ import UserAndTeamAccessAdd from './UserAndTeamAccessAdd';

 jest.mock('../../api');

+const onError = jest.fn();
+const onClose = jest.fn();
+
 describe('<UserAndTeamAccessAdd/>', () => {
  const resources = {
    data: {
@ -57,24 +60,21 @@ describe('<UserAndTeamAccessAdd/>', () => {
        <UserAndTeamAccessAdd
          apiModel={UsersAPI}
          onFetchData={() => {}}
+          onClose={onClose}
          title="Add user permissions"
+          onError={onError}
        />
      );
    });
-    await waitForElement(wrapper, 'Button[aria-label="Add"]');
+    wrapper.update();
  });
  afterEach(() => {
    jest.resetAllMocks();
  });
  test('should mount properly', async () => {
-    expect(wrapper.find('Button[aria-label="Add"]').length).toBe(1);
-    act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')());
-    wrapper.update();
    expect(wrapper.find('PFWizard').length).toBe(1);
  });
  test('should disable steps', async () => {
-    act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')());
-    wrapper.update();
    expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true);
    expect(
      wrapper
@ -122,8 +122,6 @@ describe('<UserAndTeamAccessAdd/>', () => {
    JobTemplatesAPI.read.mockResolvedValue(resources);
    JobTemplatesAPI.readOptions.mockResolvedValue(options);
    UsersAPI.associateRole.mockResolvedValue({});
-    act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')());
-    wrapper.update();
    await act(async () =>
      wrapper.find('SelectableCard[label="Job templates"]').prop('onClick')({
        fetchItems: JobTemplatesAPI.read,
@ -179,15 +177,16 @@ describe('<UserAndTeamAccessAdd/>', () => {
    await expect(UsersAPI.associateRole).toHaveBeenCalled();
  });

-  test('should close wizard', async () => {
-    act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')());
+  test('should close wizard on cancel', async () => {
+    await act(async () =>
+      wrapper.find('Button[children="Cancel"]').prop('onClick')()
+    );
    wrapper.update();
-    act(() => wrapper.find('PFWizard').prop('onClose')());
-    wrapper.update();
-    expect(wrapper.find('PFWizard').length).toBe(0);
+    expect(onClose).toHaveBeenCalledTimes(1);
  });

  test('should throw error', async () => {
+    expect(onError).toHaveBeenCalledTimes(0);
    JobTemplatesAPI.read.mockResolvedValue(resources);
    JobTemplatesAPI.readOptions.mockResolvedValue(options);
    UsersAPI.associateRole.mockRejectedValue(
@ -210,9 +209,6 @@ describe('<UserAndTeamAccessAdd/>', () => {
      }),
    }));

-    act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')());
-    wrapper.update();
-
    await act(async () =>
      wrapper.find('SelectableCard[label="Job templates"]').prop('onClick')({
        fetchItems: JobTemplatesAPI.read,
@ -261,6 +257,6 @@ describe('<UserAndTeamAccessAdd/>', () => {

    await expect(UsersAPI.associateRole).toHaveBeenCalled();
    wrapper.update();
-    expect(wrapper.find('AlertModal').length).toBe(1);
+    expect(onError).toHaveBeenCalled();
  });
 });
--- a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx
+++ b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx
@ -1,8 +1,6 @@
 import React, { useCallback, useEffect, useState } from 'react';
 import { useLocation } from 'react-router-dom';
-
 import { t } from '@lingui/macro';
-
 import {
  Button,
  EmptyState,
@ -22,6 +20,7 @@ import { getQSConfig, parseQueryString } from '../../../util/qs';
 import ErrorDetail from '../../../components/ErrorDetail';
 import AlertModal from '../../../components/AlertModal';
 import TeamRoleListItem from './TeamRoleListItem';
+import { ToolbarAddButton } from '../../../components/PaginatedDataList';
 import UserAndTeamAccessAdd from '../../../components/UserAndTeamAccessAdd/UserAndTeamAccessAdd';

 const QS_CONFIG = getQSConfig('roles', {
@ -33,6 +32,8 @@ const QS_CONFIG = getQSConfig('roles', {
 function TeamRolesList({ me, team }) {
  const { search } = useLocation();
  const [roleToDisassociate, setRoleToDisassociate] = useState(null);
+  const [showAddModal, setShowAddModal] = useState(false);
+  const [associateError, setAssociateError] = useState(null);

  const {
    isLoading,
@ -165,10 +166,10 @@ function TeamRolesList({ me, team }) {
            additionalControls={[
              ...(canAdd
                ? [
-                    <UserAndTeamAccessAdd
-                      apiModel={TeamsAPI}
-                      onFetchData={fetchRoles}
-                      title={t`Add team permissions`}
+                    <ToolbarAddButton
+                      ouiaId="role-add-button"
+                      key="add"
+                      onClick={() => setShowAddModal(true)}
                    />,
                  ]
                : []),
@ -192,7 +193,18 @@ function TeamRolesList({ me, team }) {
          />
        )}
      />
-
+      {showAddModal && (
+        <UserAndTeamAccessAdd
+          apiModel={TeamsAPI}
+          onFetchData={() => {
+            setShowAddModal(false);
+            fetchRoles();
+          }}
+          title={t`Add team permissions`}
+          onClose={() => setShowAddModal(false)}
+          onError={err => setAssociateError(err)}
+        />
+      )}
      {roleToDisassociate && (
        <AlertModal
          aria-label={t`Disassociate role`}
@ -228,6 +240,18 @@ function TeamRolesList({ me, team }) {
          </div>
        </AlertModal>
      )}
+      {associateError && (
+        <AlertModal
+          aria-label={t`Associate role error`}
+          isOpen={associateError}
+          variant="error"
+          title={t`Error!`}
+          onClose={() => setAssociateError(null)}
+        >
+          {t`Failed to associate role`}
+          <ErrorDetail error={associateError} />
+        </AlertModal>
+      )}
      {disassociationError && (
        <AlertModal
          isOpen={disassociationError}
--- a/awx/ui_next/src/screens/User/UserRoles/UserRolesList.jsx
+++ b/awx/ui_next/src/screens/User/UserRoles/UserRolesList.jsx
@ -18,7 +18,7 @@ import PaginatedTable, {
 } from '../../../components/PaginatedTable';
 import ErrorDetail from '../../../components/ErrorDetail';
 import AlertModal from '../../../components/AlertModal';
-
+import { ToolbarAddButton } from '../../../components/PaginatedDataList';
 import DatalistToolbar from '../../../components/DataListToolbar';
 import UserRolesListItem from './UserRolesListItem';
 import UserAndTeamAccessAdd from '../../../components/UserAndTeamAccessAdd/UserAndTeamAccessAdd';
@ -34,6 +34,8 @@ const QS_CONFIG = getQSConfig('roles', {
 function UserRolesList({ user }) {
  const { search } = useLocation();
  const [roleToDisassociate, setRoleToDisassociate] = useState(null);
+  const [showAddModal, setShowAddModal] = useState(false);
+  const [associateError, setAssociateError] = useState(null);

  const {
    isLoading,
@ -178,10 +180,10 @@ function UserRolesList({ user }) {
            additionalControls={[
              ...(canAdd
                ? [
-                    <UserAndTeamAccessAdd
-                      apiModel={UsersAPI}
-                      onFetchData={fetchRoles}
-                      title={t`Add user permissions`}
+                    <ToolbarAddButton
+                      ouiaId="role-add-button"
+                      key="add"
+                      onClick={() => setShowAddModal(true)}
                    />,
                  ]
                : []),
@ -189,6 +191,18 @@ function UserRolesList({ user }) {
          />
        )}
      />
+      {showAddModal && (
+        <UserAndTeamAccessAdd
+          apiModel={UsersAPI}
+          onFetchData={() => {
+            setShowAddModal(false);
+            fetchRoles();
+          }}
+          title={t`Add user permissions`}
+          onClose={() => setShowAddModal(false)}
+          onError={err => setAssociateError(err)}
+        />
+      )}
      {roleToDisassociate && (
        <AlertModal
          aria-label={t`Disassociate role`}
@ -224,6 +238,18 @@ function UserRolesList({ user }) {
          </div>
        </AlertModal>
      )}
+      {associateError && (
+        <AlertModal
+          aria-label={t`Associate role error`}
+          isOpen={associateError}
+          variant="error"
+          title={t`Error!`}
+          onClose={() => setAssociateError(null)}
+        >
+          {t`Failed to associate role`}
+          <ErrorDetail error={associateError} />
+        </AlertModal>
+      )}
      {disassociationError && (
        <AlertModal
          isOpen={disassociationError}