Merge pull request #1102 from nickgros/SWC-6800d

packages/synapse-react-client/src/components/AccessRequirementAclEditor/AccessRequirementAclEditor.tsx

@@ -69,6 +69,7 @@ export const AccessRequirementAclEditor = React.forwardRef(
       addResourceAccessItem,
       updateResourceAccessItem,
       removeResourceAccessItem,
+      resetDirtyState,
     } = useUpdateAcl({
       onChange: () => setError(null),
       onError: setError,
@@ -77,6 +78,7 @@ export const AccessRequirementAclEditor = React.forwardRef(
     // set resourceAccessList when the fetched acl changes
     useEffect(() => {
       if (originalAcl) {
+        resetDirtyState()
         setResourceAccessList(originalAcl.resourceAccess)
       }
     }, [originalAcl, setResourceAccessList])
@@ -164,13 +166,15 @@ export const AccessRequirementAclEditor = React.forwardRef(
           resourceAccessList={resourceAccessList}
           availablePermissionLevels={availablePermissionLevels}
           isLoading={isLoadingOriginalAcl}
-          isInEditMode={true}
+          canEdit={true}
           emptyText={EMPTY_RESOURCE_ACCESS_LIST_TEXT}
           onAddPrincipalToAcl={id =>
             addResourceAccessItem(id, [ACCESS_TYPE.REVIEW_SUBMISSIONS])
           }
           updateResourceAccessItem={updateResourceAccessItem}
           removeResourceAccessItem={removeResourceAccessItem}
+          showAddRemovePublicButton={false}
+          showNotifyCheckbox={false}
         />
         {error && <Alert severity="error">{error}</Alert>}
       </Stack>

packages/synapse-react-client/src/components/AclEditor/AclEditor.stories.tsx

@@ -17,8 +17,10 @@ const meta: Meta<AclEditorProps> = {
     updateResourceAccessItem: fn(),
     removeResourceAccessItem: fn(),
     isLoading: false,
-    isInEditMode: true,
+    canEdit: true,
     emptyText: 'No permissions have been granted.',
+    showAddRemovePublicButton: true,
+    showNotifyCheckbox: true,
   },
 }
 export default meta

packages/synapse-react-client/src/components/AclEditor/AclEditor.test.tsx

@@ -1,9 +1,15 @@
-import { render, screen, waitFor } from '@testing-library/react'
+import { render, screen, waitFor, within } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import React from 'react'
 import { server } from '../../mocks/msw/server'
 import { createWrapper } from '../../testutils/TestingLibraryUtils'
-import { AclEditor, AclEditorProps } from './AclEditor'
+import {
+  AclEditor,
+  AclEditorProps,
+  ADD_PUBLIC_PRINCIPALS_BUTTON_TEXT,
+  NOTIFY_NEW_ACL_USERS_CHECKBOX_LABEL,
+  REMOVE_PUBLIC_PRINCIPALS_BUTTON_TEXT,
+} from './AclEditor'
 import {
   MOCK_TEAM_ID,
   MOCK_TEAM_ID_2,
@@ -20,9 +26,16 @@ import { PermissionLevel } from '../../utils/PermissionLevelToAccessType'
 import {
   addUserToAcl,
   confirmItem,
+  queryForAddUserCombobox,
   removeItem,
   updatePermissionLevel,
 } from './AclEditorTestUtils'
+import {
+  ANONYMOUS_PRINCIPAL_ID,
+  AUTHENTICATED_PRINCIPAL_ID,
+  PUBLIC_PRINCIPAL_ID,
+  PUBLIC_PRINCIPAL_IDS,
+} from '../../utils/SynapseConstants'
 
 const DEFAULT_RESOURCE_ACCESS: ResourceAccess[] = [
   {
@@ -59,10 +72,12 @@ const defaultProps: AclEditorProps = {
   availablePermissionLevels: DEFAULT_AVAILABLE_PERMISSION_LEVELS,
   isLoading: false,
   emptyText: DEFAULT_EMPTY_TEXT,
-  isInEditMode: true,
+  canEdit: true,
   onAddPrincipalToAcl: mockAddResourceAccessItem,
   updateResourceAccessItem: mockUpdateResourceAccessItem,
   removeResourceAccessItem: mockRemoveResourceAccessItem,
+  showNotifyCheckbox: false,
+  showAddRemovePublicButton: false,
 }
 
 function renderComponent(props: AclEditorProps) {
@@ -81,10 +96,16 @@ async function setUp(
   const user = userEvent.setup()
   const { component } = renderComponent(props)
 
+  const numberOfPublicPrincipalsInResourceAccess =
+    props.resourceAccessList.filter(ra =>
+      PUBLIC_PRINCIPAL_IDS.includes(ra.principalId),
+    ).length
+
   // wait for UserOrTeamBadge(s) to finish loading
   await waitFor(() => {
     expect(screen.queryAllByRole('link')).toHaveLength(
-      expectedResourceAccessItems,
+      // public principals do not get a link, so subtract that amount from the expected count
+      expectedResourceAccessItems - numberOfPublicPrincipalsInResourceAccess,
     )
   })
 
@@ -137,9 +158,7 @@ describe('AclEditor', () => {
 
     await addUserToAcl(user, mockUserData2.userProfile!.userName)
 
-    expect(mockAddResourceAccessItem).toHaveBeenCalledWith(
-      String(mockUserData2.id),
-    )
+    expect(mockAddResourceAccessItem).toHaveBeenCalledWith(mockUserData2.id)
   })
 
   it('Handles updating the permissions of a user or team', async () => {
@@ -158,8 +177,8 @@ describe('AclEditor', () => {
     expect(mockRemoveResourceAccessItem).toHaveBeenCalledWith(MOCK_TEAM_ID)
   })
 
-  it('does not show edit controls when isInEditMode is false', async () => {
-    const { itemRows } = await setUp({ isInEditMode: false })
+  it('does not show edit controls when canEdit is false', async () => {
+    const { itemRows } = await setUp({ canEdit: false })
 
     // Data should still be present
     confirmItem(itemRows[0], mockTeamData.name, 'Can Review')
@@ -174,4 +193,183 @@ describe('AclEditor', () => {
     expect(screen.queryByRole('combobox')).not.toBeInTheDocument()
     expect(screen.queryByRole('button')).not.toBeInTheDocument()
   })
+
+  it('allows customizing which entries are editable by passing a function to canEdit', async () => {
+    function allowEditingOnlyTeam1(resourceAccess: ResourceAccess) {
+      return resourceAccess.principalId === MOCK_TEAM_ID
+    }
+
+    const { itemRows } = await setUp({
+      canEdit: allowEditingOnlyTeam1,
+      canRemoveEntry: true,
+    })
+
+    // Data should still be present
+    confirmItem(itemRows[0], mockTeamData.name, 'Can Review')
+    confirmItem(itemRows[1], mockTeamData2.name, 'Exempt Eligible')
+    confirmItem(
+      itemRows[2],
+      `@${mockUserData1.userProfile?.userName}`,
+      'Can Review & Exempt Eligible',
+    )
+
+    // team 1 is editable and deletable
+    within(itemRows[0]).getByRole('combobox')
+    within(itemRows[0]).getByRole('button')
+    // team2 and user1 are deletable but not editable
+    expect(within(itemRows[1]).queryByRole('combobox')).not.toBeInTheDocument()
+    within(itemRows[1]).getByRole('button')
+    expect(within(itemRows[2]).queryByRole('combobox')).not.toBeInTheDocument()
+    within(itemRows[2]).getByRole('button')
+
+    // Users can still be added
+    expect(queryForAddUserCombobox()).toBeInTheDocument()
+  })
+
+  it('respects the value of canRemoveEntry (boolean)', async () => {
+    const { itemRows } = await setUp({
+      canEdit: true,
+      canRemoveEntry: false,
+    })
+
+    // Data should still be present
+    confirmItem(itemRows[0], mockTeamData.name, 'Can Review')
+    confirmItem(itemRows[1], mockTeamData2.name, 'Exempt Eligible')
+    confirmItem(
+      itemRows[2],
+      `@${mockUserData1.userProfile?.userName}`,
+      'Can Review & Exempt Eligible',
+    )
+
+    // all rows are editable but not deletable
+    within(itemRows[0]).getByRole('combobox')
+    expect(within(itemRows[0]).queryByRole('button')).not.toBeInTheDocument()
+    within(itemRows[1]).getByRole('combobox')
+    expect(within(itemRows[1]).queryByRole('button')).not.toBeInTheDocument()
+    within(itemRows[2]).getByRole('combobox')
+    expect(within(itemRows[2]).queryByRole('button')).not.toBeInTheDocument()
+
+    // Users can still be added
+    expect(queryForAddUserCombobox()).toBeInTheDocument()
+  })
+
+  it('respects the value of canRemoveEntry (function)', async () => {
+    function allowRemovingOnlyTeam1(resourceAccess: ResourceAccess) {
+      return resourceAccess.principalId === MOCK_TEAM_ID
+    }
+
+    const { itemRows } = await setUp({
+      canEdit: true,
+      canRemoveEntry: allowRemovingOnlyTeam1,
+    })
+
+    // Data should still be present
+    confirmItem(itemRows[0], mockTeamData.name, 'Can Review')
+    confirmItem(itemRows[1], mockTeamData2.name, 'Exempt Eligible')
+    confirmItem(
+      itemRows[2],
+      `@${mockUserData1.userProfile?.userName}`,
+      'Can Review & Exempt Eligible',
+    )
+
+    // team 1 is editable and deletable
+    within(itemRows[0]).getByRole('combobox')
+    within(itemRows[0]).getByRole('button')
+    // team2 and user1 are editable but not deletable
+    within(itemRows[1]).getByRole('combobox')
+    expect(within(itemRows[1]).queryByRole('button')).not.toBeInTheDocument()
+    within(itemRows[2]).getByRole('combobox')
+    expect(within(itemRows[2]).queryByRole('button')).not.toBeInTheDocument()
+
+    // Users can still be added
+    expect(queryForAddUserCombobox()).toBeInTheDocument()
+  })
+
+  it('allows overriding a displayed permission value', async () => {
+    const overrideText = 'Can take over the world'
+    function displayedPermissionLevelOverride(resourceAccess: ResourceAccess) {
+      if (resourceAccess.principalId === MOCK_TEAM_ID) {
+        return overrideText
+      }
+      return undefined
+    }
+    const { itemRows } = await setUp({
+      canEdit: false,
+      displayedPermissionLevelOverride,
+    })
+
+    // Team 1 gets the override
+    confirmItem(itemRows[0], mockTeamData.name, overrideText)
+
+    // Remaining teams are unchanged
+    confirmItem(itemRows[1], mockTeamData2.name, 'Exempt Eligible')
+    confirmItem(
+      itemRows[2],
+      `@${mockUserData1.userProfile?.userName}`,
+      'Can Review & Exempt Eligible',
+    )
+  })
+
+  it('shows a checkbox to notify users', async () => {
+    const onCheckboxChange = jest.fn()
+    const { user } = await setUp({
+      showNotifyCheckbox: true,
+      notifyCheckboxValue: true,
+      onNotifyCheckboxChange: onCheckboxChange,
+    })
+
+    const checkbox = screen.getByLabelText(NOTIFY_NEW_ACL_USERS_CHECKBOX_LABEL)
+    expect(checkbox).toHaveAttribute('value', 'true')
+
+    await user.click(checkbox)
+
+    expect(onCheckboxChange).toHaveBeenLastCalledWith(false)
+  })
+
+  it('shows a button to add public', async () => {
+    const { user } = await setUp({
+      showAddRemovePublicButton: true,
+    })
+
+    const makePublicButton = screen.getByRole('button', {
+      name: ADD_PUBLIC_PRINCIPALS_BUTTON_TEXT,
+    })
+    await user.click(makePublicButton)
+
+    expect(mockAddResourceAccessItem).toHaveBeenCalledWith(PUBLIC_PRINCIPAL_ID)
+    expect(mockAddResourceAccessItem).toHaveBeenCalledWith(
+      AUTHENTICATED_PRINCIPAL_ID,
+    )
+  })
+
+  it('shows a button to remove public if any public principals are in the ACL', async () => {
+    const { user } = await setUp({
+      resourceAccessList: [
+        {
+          principalId: PUBLIC_PRINCIPAL_ID,
+          accessType: [ACCESS_TYPE.REVIEW_SUBMISSIONS],
+        },
+        {
+          principalId: AUTHENTICATED_PRINCIPAL_ID,
+          accessType: [ACCESS_TYPE.REVIEW_SUBMISSIONS],
+        },
+      ],
+      showAddRemovePublicButton: true,
+    })
+
+    const removePublicAccessButton = screen.getByRole('button', {
+      name: REMOVE_PUBLIC_PRINCIPALS_BUTTON_TEXT,
+    })
+    await user.click(removePublicAccessButton)
+
+    expect(mockRemoveResourceAccessItem).toHaveBeenCalledWith(
+      PUBLIC_PRINCIPAL_ID,
+    )
+    expect(mockRemoveResourceAccessItem).toHaveBeenCalledWith(
+      AUTHENTICATED_PRINCIPAL_ID,
+    )
+    expect(mockRemoveResourceAccessItem).toHaveBeenCalledWith(
+      ANONYMOUS_PRINCIPAL_ID,
+    )
+  })
 })