SIMSBIOHUB-869: Improve UX for Team–Policy Assignments (#334)

* paginated server-side data grid

* simplify team policy container

* dedupe code

* cleanup

* improve consistency

* remove edit

* code smells

* pr comments

* improve team-policy assignment dialog

* multiselect policies

* search

* clean up

* remove unnecessary types

Author: mauberti-bc

api/src/services/access-policy/team-policy-service.test.ts

@@ -30,6 +30,7 @@ describe('TeamPolicyService', () => {
         policy_id: '33333333-3333-3333-3333-333333333333'
       };
 
+      const getExistingStub = sinon.stub(TeamPolicyRepository.prototype, 'getPoliciesByTeamId').resolves([]);
       const stub = sinon.stub(TeamPolicyRepository.prototype, 'insertTeamPolicy').resolves(mockTeamPolicy);
 
       const input: CreateTeamPolicy = {
@@ -39,9 +40,44 @@ describe('TeamPolicyService', () => {
 
       const result = await service.createTeamPolicy(input);
 
+      expect(getExistingStub).to.have.been.calledWith('22222222-2222-2222-2222-222222222222', {
+        policyIds: ['33333333-3333-3333-3333-333333333333']
+      });
       expect(stub).to.have.been.calledWith(input);
       expect(result).to.eql(mockTeamPolicy);
     });
+
+    it('should return existing team policy and skip insert when association already exists', async () => {
+      const existingTeamPolicy: TeamPolicyDetails = {
+        team_policy_id: '11111111-1111-1111-1111-111111111111',
+        team_id: '22222222-2222-2222-2222-222222222222',
+        policy_id: '33333333-3333-3333-3333-333333333333',
+        team_name: 'Team A',
+        policy_name: 'Policy A'
+      };
+
+      const getExistingStub = sinon
+        .stub(TeamPolicyRepository.prototype, 'getPoliciesByTeamId')
+        .resolves([existingTeamPolicy]);
+      const insertStub = sinon.stub(TeamPolicyRepository.prototype, 'insertTeamPolicy');
+
+      const input: CreateTeamPolicy = {
+        team_id: '22222222-2222-2222-2222-222222222222',
+        policy_id: '33333333-3333-3333-3333-333333333333'
+      };
+
+      const result = await service.createTeamPolicy(input);
+
+      expect(getExistingStub).to.have.been.calledWith('22222222-2222-2222-2222-222222222222', {
+        policyIds: ['33333333-3333-3333-3333-333333333333']
+      });
+      expect(insertStub).to.not.have.been.called;
+      expect(result).to.eql({
+        team_policy_id: '11111111-1111-1111-1111-111111111111',
+        team_id: '22222222-2222-2222-2222-222222222222',
+        policy_id: '33333333-3333-3333-3333-333333333333'
+      });
+    });
   });
 
   describe('getTeamPolicy', () => {

api/src/services/access-policy/team-policy-service.ts

@@ -20,7 +20,20 @@ export class TeamPolicyService extends DBService {
    * @return {Promise<TeamPolicy>} - The created team policy record.
    * @memberof TeamPolicyService
    */
-  createTeamPolicy(teamPolicyData: CreateTeamPolicy): Promise<TeamPolicy> {
+  async createTeamPolicy(teamPolicyData: CreateTeamPolicy): Promise<TeamPolicy> {
+    const existingPolicies = await this.teamPolicyRepository.getPoliciesByTeamId(teamPolicyData.team_id, {
+      policyIds: [teamPolicyData.policy_id]
+    });
+
+    if (existingPolicies.length > 0) {
+      const existingPolicy = existingPolicies[0];
+      return {
+        team_policy_id: existingPolicy.team_policy_id,
+        team_id: existingPolicy.team_id,
+        policy_id: existingPolicy.policy_id
+      };
+    }
+
     return this.teamPolicyRepository.insertTeamPolicy(teamPolicyData);
   }
 
@@ -34,6 +47,7 @@ export class TeamPolicyService extends DBService {
    */
   async createTeamPolicies(teamId: string, policyIds: string[]): Promise<TeamPolicy[]> {
     const uniquePolicyIds = [...new Set(policyIds)];
+
     if (!uniquePolicyIds.length) {
       return [];
     }
@@ -46,7 +60,9 @@ export class TeamPolicyService extends DBService {
     const policyIdsToCreate = uniquePolicyIds.filter((policyId) => !existingPolicyIds.has(policyId));
 
     return Promise.all(
-      policyIdsToCreate.map((policyId) => this.createTeamPolicy({ team_id: teamId, policy_id: policyId }))
+      policyIdsToCreate.map((policyId) =>
+        this.teamPolicyRepository.insertTeamPolicy({ team_id: teamId, policy_id: policyId })
+      )
     );
   }
 

app/src/components/data-grid/ServerPaginatedDataGrid.tsx

@@ -0,0 +1,91 @@
+import {
+  GridColDef,
+  GridPaginationModel,
+  GridRowId,
+  GridRowSelectionModel,
+  GridSortModel,
+  GridValidRowModel
+} from '@mui/x-data-grid';
+import CustomDataGrid from './CustomDataGrid';
+
+interface IServerPaginatedDataGridProps<T extends GridValidRowModel> {
+  rows: T[];
+  columns: GridColDef<T>[];
+  getRowId: (row: T) => GridRowId;
+  dataTestId: string;
+  noRowsMessage: string;
+  rowCount: number;
+  paginationModel: GridPaginationModel;
+  setPaginationModel: (model: GridPaginationModel) => void;
+  sortModel: GridSortModel;
+  setSortModel: (model: GridSortModel) => void;
+  onRowClick?: (row: T) => void;
+  rowSelectionModel?: GridRowSelectionModel;
+  onRowSelectionModelChange?: (model: GridRowSelectionModel) => void;
+  checkboxSelection?: boolean;
+  disableMultipleRowSelection?: boolean;
+}
+
+/**
+ * Reusable server-side paginated data grid wrapper.
+ *
+ * Encapsulates common server pagination/sorting wiring for `CustomDataGrid`
+ * and exposes strongly-typed row behavior.
+ *
+ * @template T
+ * @param {IServerPaginatedDataGridProps<T>} props
+ * @returns {JSX.Element}
+ */
+export const ServerPaginatedDataGrid = <T extends GridValidRowModel>({
+  rows,
+  columns,
+  getRowId,
+  dataTestId,
+  noRowsMessage,
+  rowCount,
+  paginationModel,
+  setPaginationModel,
+  sortModel,
+  setSortModel,
+  onRowClick,
+  rowSelectionModel,
+  onRowSelectionModelChange,
+  checkboxSelection,
+  disableMultipleRowSelection
+}: IServerPaginatedDataGridProps<T>) => {
+  return (
+    <CustomDataGrid
+      data-testid={dataTestId}
+      rows={rows}
+      columns={columns}
+      getRowId={getRowId}
+      paginationMode="server"
+      paginationModel={paginationModel}
+      onPaginationModelChange={setPaginationModel}
+      pageSizeOptions={[10, 25, 50]}
+      sortingMode="server"
+      sortingOrder={['asc', 'desc']}
+      sortModel={sortModel}
+      onSortModelChange={setSortModel}
+      rowCount={rowCount}
+      noRowsMessage={noRowsMessage}
+      localeText={{ noRowsLabel: noRowsMessage }}
+      disableRowSelectionOnClick
+      disableColumnSelector
+      checkboxSelection={checkboxSelection}
+      disableMultipleRowSelection={disableMultipleRowSelection}
+      rowSelectionModel={rowSelectionModel}
+      onRowSelectionModelChange={onRowSelectionModelChange}
+      onRowClick={(params) => {
+        onRowClick?.(params.row as T);
+      }}
+      sx={{
+        border: 'none',
+        '& .MuiDataGrid-columnHeaderTitle': {
+          fontWeight: 700,
+          textTransform: 'uppercase'
+        }
+      }}
+    />
+  );
+};

app/src/components/dialog/EditDialog.test.tsx

@@ -1,5 +1,5 @@
 import { fireEvent, waitFor } from '@testing-library/react';
-import EditDialog from 'components/dialog/EditDialog';
+import { EditDialog } from 'components/dialog/EditDialog';
 import CustomTextFieldFormik from 'components/fields/CustomTextFieldFormik';
 import { useFormikContext } from 'formik';
 import { render } from 'test-helpers/test-utils';
@@ -31,19 +31,21 @@ const handleOnCancel = vi.fn();
 const renderContainer = ({
   testFieldValue,
   dialogError,
-  open = true
+  open,
+  isLoading
 }: {
   testFieldValue: string;
   dialogError?: string;
   open?: boolean;
+  isLoading?: boolean;
 }) => {
   return render(
     <div id="root">
       <EditDialog
-        isLoading={false}
+        isLoading={isLoading}
         dialogTitle="This is dialog title"
         dialogError={dialogError || undefined}
-        open={open}
+        open={open ?? false}
         component={{
           element: <SampleFormikForm />,
           initialValues: { testField: testFieldValue },
@@ -58,7 +60,7 @@ const renderContainer = ({
 
 describe('EditDialog', () => {
   it('renders component and data values', () => {
-    const { getByTestId, getByText } = renderContainer({ testFieldValue: 'this is a test' });
+    const { getByTestId, getByText } = renderContainer({ testFieldValue: 'this is a test', open: true });
 
     expect(getByTestId('testField')).toBeVisible();
     expect(getByText('this is a test')).toBeVisible();
@@ -67,21 +69,22 @@ describe('EditDialog', () => {
   it('matches snapshot when open, with error message', () => {
     const { getByTestId, getByText } = renderContainer({
       testFieldValue: 'this is a test',
-      dialogError: 'This is an error'
+      dialogError: 'This is an error',
+      open: true
     });
 
     expect(getByTestId('testField')).toBeVisible();
     expect(getByText('This is an error')).toBeVisible();
   });
 
   it('calls the onSave prop when `Save Changes` button is clicked', async () => {
-    const { findByText, getByLabelText } = renderContainer({ testFieldValue: 'initial value' });
+    const { getByTestId, getByLabelText } = renderContainer({ testFieldValue: 'initial value', open: true });
 
     const textField = await getByLabelText('Test Field', { exact: false });
 
     fireEvent.change(textField, { target: { value: 'updated value' } });
 
-    const saveChangesButton = await findByText('Save Changes', { exact: false });
+    const saveChangesButton = getByTestId('edit-dialog-save-button');
 
     fireEvent.click(saveChangesButton);
 
@@ -93,8 +96,19 @@ describe('EditDialog', () => {
     });
   });
 
+  it('hides save label text while loading', () => {
+    const { getByTestId, queryByText } = renderContainer({
+      testFieldValue: 'this is a test',
+      open: true,
+      isLoading: true
+    });
+
+    expect(getByTestId('edit-dialog-save-button')).toBeVisible();
+    expect(queryByText('Save Changes', { exact: false })).toBeNull();
+  });
+
   it('calls the onCancel prop when `Cancel` button is clicked', async () => {
-    const { findByText } = renderContainer({ testFieldValue: 'this is a test' });
+    const { findByText } = renderContainer({ testFieldValue: 'this is a test', open: true });
 
     const cancelButton = await findByText('Cancel', { exact: false });
 

app/src/components/dialog/EditDialog.tsx

@@ -3,6 +3,7 @@ import Dialog from '@mui/material/Dialog';
 import DialogActions from '@mui/material/DialogActions';
 import DialogContent from '@mui/material/DialogContent';
 import DialogTitle from '@mui/material/DialogTitle';
+import { LoadingGuard } from 'components/loading/LoadingGuard';
 import { Formik, FormikValues } from 'formik';
 
 export interface IEditDialogComponentProps<T> {
@@ -64,7 +65,7 @@ export interface IEditDialogProps<T> {
   /**
    * Prop to track if the dialog should be in a 'loading' state
    */
-  isLoading: boolean;
+  isLoading?: boolean;
 }
 
 /**
@@ -91,19 +92,21 @@ export const EditDialog = <T extends FormikValues>(props: React.PropsWithChildre
         props.onSave(values);
       }}>
       {(formikProps) => (
-        <Dialog open={props.open} aria-labelledby="edit-dialog-title" aria-describedby="edit-dialog-description">
+        <Dialog
+          open={props.open ?? false}
+          aria-labelledby="edit-dialog-title"
+          aria-describedby="edit-dialog-description">
           <DialogTitle id="edit-dialog-title">{props.dialogTitle}</DialogTitle>
           <DialogContent>{props.component.element}</DialogContent>
           <DialogActions>
             <Button
               loading={props.isLoading}
-              disabled={!formikProps.isValid}
               onClick={formikProps.submitForm}
               color="primary"
               variant="contained"
               autoFocus
               data-testid="edit-dialog-save-button">
-              {props.dialogSaveButtonLabel || 'Save Changes'}
+              <LoadingGuard isLoading={props.isLoading}>{props.dialogSaveButtonLabel || 'Save Changes'}</LoadingGuard>
             </Button>
             <Button onClick={props.onCancel} color="primary" variant="outlined" data-testid="edit-dialog-cancel-button">
               Cancel
@@ -115,5 +118,3 @@ export const EditDialog = <T extends FormikValues>(props: React.PropsWithChildre
     </Formik>
   );
 };
-
-export default EditDialog;

app/src/components/fields/CustomMultiAutocompleteFormik.test.tsx

@@ -1,7 +1,7 @@
 import { Formik } from 'formik';
 import { render } from 'test-helpers/test-utils';
 import { sortAutocompleteOptions } from 'utils/autocomplete';
-import CustomMultiAutocompleteFormik from './CustomMultiAutocompleteFormik';
+import { CustomMultiAutocompleteFormik } from './CustomMultiAutocompleteFormik';
 import { ICustomMultiAutocompleteOption } from './CustomMultiAutocomplete';
 
 describe('CustomMultiAutocompleteFormik', () => {