Skip to content

Commit

Permalink
Merge pull request #910 from jay-hodgson/SWC-6810
Browse files Browse the repository at this point in the history 
SWC-6810
  • Loading branch information
@jay-hodgson
jay-hodgson authored Apr 29, 2024
2 parents 1a49e5e + 681ac4e commit f56db8a
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 20 deletions.
2 changes: 1 addition & 1 deletion packages/synapse-react-client/package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "synapse-react-client",
"version": "3.2.12",
"version": "3.2.13",
"private": false,
"main": "./dist/index.js",
"module": "./dist/index.mjs",
Expand Down
7 changes: 6 additions & 1 deletion ...rUpdateAccessRequirementWizard/CreateOrUpdateAccessRequirementWizard.integration.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,17 @@ describe('CreateOrUpdateAccessRequirementWizard', () => {
const arName = 'new self-sign ar'
const ar: Pick<
SelfSignAccessRequirement,
'concreteType' | 'subjectIds' | 'accessType' | 'name'
| 'concreteType'
| 'subjectIds'
| 'accessType'
| 'name'
| 'subjectsDefinedByAnnotations'
> = {
concreteType: SELF_SIGN_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE,
subjectIds: [entitySubject],
accessType: ACCESS_TYPE.DOWNLOAD,
name: arName,
subjectsDefinedByAnnotations: false,
}

// step 1: common AR fields
Expand Down
6 changes: 3 additions & 3 deletions ...omponents/CreateOrUpdateAccessRequirementWizard/CreateOrUpdateAccessRequirementWizard.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
export type CreateOrUpdateAccessRequirementWizardProps = {
open: boolean
onCancel: () => void
onComplete: () => void
onComplete: (accessRequirementID?: string) => void // return the (possibly new) Access Requirement ID
} & Pick<
SetAccessRequirementCommonFieldsProps,
'subject' | 'accessRequirementId'
Expand Down Expand Up @@ -96,7 +96,7 @@ export const CreateOrUpdateAccessRequirementWizard: React.FunctionComponent<
accessRequirementId={accessRequirementId!}
onSaveComplete={saveSuccessful => {
if (saveSuccessful) {
onComplete()
onComplete(accessRequirementId)
}
setIsLoading(false)
}}
Expand All @@ -108,7 +108,7 @@ export const CreateOrUpdateAccessRequirementWizard: React.FunctionComponent<
ref={setBasicArFieldsRef}
accessRequirementId={accessRequirementId!}
onSave={() => {
onComplete()
onComplete(accessRequirementId)
setIsLoading(false)
}}
onError={() => setIsLoading(false)}
Expand Down
108 changes: 102 additions & 6 deletions ...src/components/SetAccessRequirementCommonFields/SetAccessRequirementCommonFields.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
mockACTAccessRequirement,
mockManagedACTAccessRequirement,
mockSelfSignAccessRequirement,
mockSelfSignAnnotationBasedSubjectsAccessRequirement,
mockTeamSelfSignAccessRequirement,
mockToUAccessRequirement,
} from '../../mocks/mockAccessRequirements'
Expand Down Expand Up @@ -82,10 +83,17 @@ function renderComponent(props: SetAccessRequirementCommonFieldsProps) {
async function setUp(props: SetAccessRequirementCommonFieldsProps) {
const user = userEvent.setup()
const { ref, component } = renderComponent(props)

const nameInput = await screen.findByRole('textbox', { name: 'Name' })
return {
ref,
component,
user,
nameInput,
}
}

return { ref, component, user, nameInput }
async function findSubjectsDefinedByAnnotationsCheckbox() {
return await screen.findByRole('checkbox')
}

function getRadioButtons() {
Expand All @@ -106,10 +114,12 @@ describe('SetAccessRequirementCommonFields', () => {

test('creates a new managed entity AR', async () => {
const { ref, user, nameInput } = await setUp(newEntityArProps)

const subjectsDefinedByAnnotationsCheckbox =
await findSubjectsDefinedByAnnotationsCheckbox()
await waitFor(() => {
expect(screen.getByText(MOCK_FILE_NAME)).toBeVisible()
expect(nameInput).toHaveValue('')
expect(subjectsDefinedByAnnotationsCheckbox).not.toBeChecked()
})

const radioButtons = getRadioButtons()
Expand All @@ -128,12 +138,68 @@ describe('SetAccessRequirementCommonFields', () => {
await waitFor(() => {
const managedAr: Pick<
ManagedACTAccessRequirement,
'concreteType' | 'name' | 'subjectIds' | 'accessType'
| 'concreteType'
| 'name'
| 'subjectIds'
| 'accessType'
| 'subjectsDefinedByAnnotations'
> = {
concreteType: MANAGED_ACT_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE,
name: name,
subjectIds: [entitySubject],
accessType: ACCESS_TYPE.DOWNLOAD,
subjectsDefinedByAnnotations: false,
}

expect(createAccessRequirementSpy).toHaveBeenCalledWith(
MOCK_ACCESS_TOKEN,
managedAr,
)
expect(updateAccessRequirementSpy).not.toHaveBeenCalled()

expect(onSave).toHaveBeenCalledTimes(1)
expect(onSave).toHaveBeenLastCalledWith(
MOCK_NEWLY_CREATED_AR_ID.toString(),
MANAGED_ACT_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE,
)
expect(onError).not.toHaveBeenCalled()
})
})

test('creates a new managed entity AR with subjects defined by annotation', async () => {
const { ref, user, nameInput } = await setUp(newEntityArProps)
const subjectsDefinedByAnnotationsCheckbox =
await findSubjectsDefinedByAnnotationsCheckbox()
await waitFor(() => {
expect(subjectsDefinedByAnnotationsCheckbox).not.toBeChecked()
})

const name = 'some name'
await user.type(nameInput, name)
await user.click(subjectsDefinedByAnnotationsCheckbox)

expect(subjectsDefinedByAnnotationsCheckbox).toBeChecked()
expect(nameInput).toHaveValue(name)
expect(onSave).not.toHaveBeenCalled()
expect(onError).not.toHaveBeenCalled()

// parent calls save
ref.current?.save()

await waitFor(() => {
const managedAr: Pick<
ManagedACTAccessRequirement,
| 'concreteType'
| 'name'
| 'subjectIds'
| 'accessType'
| 'subjectsDefinedByAnnotations'
> = {
concreteType: MANAGED_ACT_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE,
name: name,
subjectIds: [],
accessType: ACCESS_TYPE.DOWNLOAD,
subjectsDefinedByAnnotations: true,
}

expect(createAccessRequirementSpy).toHaveBeenCalledWith(
Expand All @@ -150,6 +216,26 @@ describe('SetAccessRequirementCommonFields', () => {
expect(onError).not.toHaveBeenCalled()
})
})
test('updates an existing self-sign team AR with subjects defined by annotation', async () => {
const { nameInput } = await setUp({
accessRequirementId:
mockSelfSignAnnotationBasedSubjectsAccessRequirement.id.toString(),
onSave,
onError,
})

const subjectsDefinedByAnnotationsCheckbox =
await findSubjectsDefinedByAnnotationsCheckbox()

await waitFor(() => {
expect(nameInput).toHaveValue(
mockSelfSignAnnotationBasedSubjectsAccessRequirement.name,
)
// cannot select access type for existing AR
expect(screen.queryByRole('radiogroup')).toBeNull()
expect(subjectsDefinedByAnnotationsCheckbox).toBeChecked()
})
})

test('creates a new self-sign entity AR', async () => {
const { ref, user, nameInput } = await setUp(newEntityArProps)
Expand All @@ -176,12 +262,17 @@ describe('SetAccessRequirementCommonFields', () => {
await waitFor(() => {
const selfSignAr: Pick<
SelfSignAccessRequirement,
'concreteType' | 'name' | 'subjectIds' | 'accessType'
| 'concreteType'
| 'name'
| 'subjectIds'
| 'accessType'
| 'subjectsDefinedByAnnotations'
> = {
concreteType: SELF_SIGN_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE,
name: name,
subjectIds: [entitySubject],
accessType: ACCESS_TYPE.DOWNLOAD,
subjectsDefinedByAnnotations: false,
}

expect(createAccessRequirementSpy).toHaveBeenCalledWith(
Expand Down Expand Up @@ -243,12 +334,17 @@ describe('SetAccessRequirementCommonFields', () => {
await waitFor(() => {
const selfSignAr: Pick<
SelfSignAccessRequirement,
'concreteType' | 'name' | 'subjectIds' | 'accessType'
| 'concreteType'
| 'name'
| 'subjectIds'
| 'accessType'
| 'subjectsDefinedByAnnotations'
> = {
concreteType: SELF_SIGN_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE,
name: name,
subjectIds: [teamSubject],
accessType: ACCESS_TYPE.PARTICIPATE,
subjectsDefinedByAnnotations: false,
}

expect(createAccessRequirementSpy).toHaveBeenCalledWith(
Expand Down
58 changes: 49 additions & 9 deletions ...ient/src/components/SetAccessRequirementCommonFields/SetAccessRequirementCommonFields.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { SynapseClientError } from '../../utils/SynapseClientError'
import EntitySubjectsSelector from '../EntitySubjectsSelector'
import TeamSubjectsSelector from '../TeamSubjectsSelector'
import { RadioGroup } from '../widgets/RadioGroup'
import { Checkbox } from '../widgets/Checkbox'

export const EMPTY_SUBJECT_LIST_ERROR_MESSAGE =
'Please select at least one resource for this Access Requirement to be associated with.'
Expand All @@ -50,15 +51,15 @@ const headerProps: Partial<TypographyProps> = {
fontWeight: 700,
}

function getAccessType(subject: RestrictableObjectDescriptor) {
switch (subject.type) {
function getAccessType(subjectType: RestrictableObjectType) {
switch (subjectType) {
case RestrictableObjectType.ENTITY:
return ACCESS_TYPE.DOWNLOAD
case RestrictableObjectType.TEAM:
return ACCESS_TYPE.PARTICIPATE
default:
throw new Error(
`RestrictableObjectType ${subject.type} does not have an access type specified.`,
`RestrictableObjectType ${subjectType} does not have an access type specified.`,
)
}
}
Expand Down Expand Up @@ -97,6 +98,8 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
const [hasPendingSubjects, setHasPendingSubjects] = useState<boolean>(false)
const [subjectsError, setSubjectsError] = useState<string | null>(null)
const [name, setName] = useState<string>('')
const [subjectsDefinedByAnnotations, setSubjectsDefinedByAnnotations] =
useState<boolean>(false)
const [arType, setArType] = useState<ACCESS_REQUIREMENT_CONCRETE_TYPE>(
subject?.type === RestrictableObjectType.TEAM
? SELF_SIGN_ACCESS_REQUIREMENT_CONCRETE_TYPE_VALUE
Expand Down Expand Up @@ -137,6 +140,9 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
setArType(accessRequirement.concreteType)
setName(accessRequirement.name)
setSubjects(accessRequirement.subjectIds)
setSubjectsDefinedByAnnotations(
accessRequirement.subjectsDefinedByAnnotations,
)
}
}, [accessRequirement])

Expand All @@ -150,8 +156,11 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
return initialSubjects[0].type
}
}
if (subjectsDefinedByAnnotations) {
return RestrictableObjectType.ENTITY
}
return null
}, [subject, accessRequirement])
}, [subject, accessRequirement, subjectsDefinedByAnnotations])

const selectorContent = useMemo(() => {
if (!subjectsType) return <></>
Expand All @@ -166,6 +175,9 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
setHasPendingSubjects(value.trim() !== '')
}

if (subjectsDefinedByAnnotations) {
return <></>
}
switch (subjectsType) {
case RestrictableObjectType.TEAM:
return (
Expand All @@ -189,7 +201,7 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
)
return <></>
}
}, [subjectsType, subjects])
}, [subjectsType, subjects, subjectsDefinedByAnnotations])

useImperativeHandle(
ref,
Expand All @@ -198,8 +210,9 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
save() {
const isStillLoading =
(isEditing && !accessRequirement) || !subjectsType
const hasSubjectsError = hasPendingSubjects || subjects.length === 0

const hasSubjectsError =
!subjectsDefinedByAnnotations &&
(hasPendingSubjects || subjects.length === 0)
if (isStillLoading || hasSubjectsError) {
if (hasSubjectsError && !isStillLoading) {
if (hasPendingSubjects) {
Expand All @@ -213,12 +226,14 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
return
}

const newAccessType = getAccessType(subjectsType)
if (!isEditing) {
const newAr: Partial<AccessRequirement> = {
concreteType: arType,
subjectIds: subjects,
name: name,
accessType: getAccessType(subjects[0]),
accessType: newAccessType,
subjectsDefinedByAnnotations: subjectsDefinedByAnnotations,
}
createAccessRequirement(newAr)
}
Expand All @@ -228,7 +243,8 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
...accessRequirement,
subjectIds: subjects,
name: name,
accessType: getAccessType(subjects[0]),
accessType: newAccessType,
subjectsDefinedByAnnotations: subjectsDefinedByAnnotations,
})
}
},
Expand All @@ -245,6 +261,7 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
onError,
createAccessRequirement,
updateAccessRequirement,
subjectsDefinedByAnnotations,
],
)

Expand All @@ -271,6 +288,29 @@ export const SetAccessRequirementCommonFields = React.forwardRef(
return (
<>
<Typography {...headerProps}>Resources</Typography>
{subjectsType !== RestrictableObjectType.TEAM && (
<>
<Checkbox
label="Associated entities should be defined by annotations (DUO)"
checked={subjectsDefinedByAnnotations}
onChange={() => {
setSubjectsError(null)
// if we are switching from DUO to manually defining the subjects (and the AR has been retrieved), then
// reset to the AR subject IDs or the original subject ID. Otherwise, clear out the subjects.
let newSubjectIds: RestrictableObjectDescriptor[] = []
if (subjectsDefinedByAnnotations) {
if (accessRequirement) {
newSubjectIds = accessRequirement.subjectIds
} else if (subject) {
newSubjectIds = [subject]
}
}
setSubjects(newSubjectIds)
setSubjectsDefinedByAnnotations(!subjectsDefinedByAnnotations)
}}
/>
</>
)}
{selectorContent}
{subjectsError && <Alert severity="error">{subjectsError}</Alert>}
<Stack direction="row" gap={1} alignItems="center" mb={1} mt={2}>
Expand Down
Loading

0 comments on commit f56db8a

Please sign in to comment.