Skip to content

Commit

Permalink
SIMSBIOHUB-593: Add constraint for a user to have only one role per P…
Browse files Browse the repository at this point in the history 
…roject (#1307)

* Add database constraint to enforce one role per user per project
* Add project member role icons to form control

---------

Co-authored-by: Nick Phura <nickphura@gmail.com>
  • Loading branch information
@mauberti-bc @NickPhura
mauberti-bc and NickPhura authored Jun 21, 2024
1 parent a3cb8a7 commit e4a401e
Show file tree
Hide file tree
Showing 10 changed files with 322 additions and 54 deletions.
4 changes: 3 additions & 1 deletion api/src/repositories/code-repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,9 @@ export class CodeRepository extends BaseRepository {
project_role_id as id,
name
FROM project_role
WHERE record_end_date is null;
WHERE record_end_date is null
ORDER BY
CASE WHEN name = 'Coordinator' THEN 0 ELSE 1 END;
`;

const response = await this.connection.sql(sqlStatement, ICode);
Expand Down
123 changes: 119 additions & 4 deletions api/src/services/project-participation-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ describe('ProjectParticipationService', () => {
});
});

describe('doProjectParticipantsHaveARole', () => {
describe('_doProjectParticipantsHaveARole', () => {
it('should return true if one project user has a specified role', () => {
const projectUsers: PostParticipantData[] = [
{
Expand All @@ -962,7 +962,7 @@ describe('ProjectParticipationService', () => {
const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service.doProjectParticipantsHaveARole(projectUsers, PROJECT_ROLE.COLLABORATOR);
const result = service._doProjectParticipantsHaveARole(projectUsers, PROJECT_ROLE.COLLABORATOR);

expect(result).to.be.true;
});
Expand All @@ -984,7 +984,7 @@ describe('ProjectParticipationService', () => {
const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service.doProjectParticipantsHaveARole(projectUsers, PROJECT_ROLE.COLLABORATOR);
const result = service._doProjectParticipantsHaveARole(projectUsers, PROJECT_ROLE.COLLABORATOR);

expect(result).to.be.true;
});
Expand All @@ -1006,7 +1006,97 @@ describe('ProjectParticipationService', () => {
const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service.doProjectParticipantsHaveARole(projectUsers, PROJECT_ROLE.COLLABORATOR);
const result = service._doProjectParticipantsHaveARole(projectUsers, PROJECT_ROLE.COLLABORATOR);

expect(result).to.be.false;
});
});

describe('_doProjectParticipantsHaveOneRole', () => {
it('should return true if one project user has one specified role', () => {
const projectUsers: PostParticipantData[] = [
{
project_participation_id: 23,
system_user_id: 22,
project_role_names: [PROJECT_ROLE.COLLABORATOR]
}
];

const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service._doProjectParticipantsHaveOneRole(projectUsers);

expect(result).to.be.true;
});

it('should return true if multiple project users have one specified role', () => {
const projectUsers: PostParticipantData[] = [
{
project_participation_id: 12,
system_user_id: 11,
project_role_names: [PROJECT_ROLE.COLLABORATOR]
},
{
project_participation_id: 23,
system_user_id: 22,
project_role_names: [PROJECT_ROLE.OBSERVER]
}
];

const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service._doProjectParticipantsHaveOneRole(projectUsers);

expect(result).to.be.true;
});

it('should return false if a participant has multiple specified role', () => {
const projectUsers: PostParticipantData[] = [
{
project_participation_id: 12,
system_user_id: 11,
project_role_names: [PROJECT_ROLE.COORDINATOR]
},
{
project_participation_id: 23,
system_user_id: 22,
project_role_names: [PROJECT_ROLE.OBSERVER]
},
{
project_participation_id: 23,
system_user_id: 22,
project_role_names: [PROJECT_ROLE.COLLABORATOR]
}
];

const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service._doProjectParticipantsHaveOneRole(projectUsers);

expect(result).to.be.false;
});

it('should return false if a participant has multiple specified roles in the same record', () => {
const projectUsers: PostParticipantData[] = [
{
project_participation_id: 12,
system_user_id: 11,
project_role_names: [PROJECT_ROLE.COORDINATOR]
},
{
project_participation_id: 23,
system_user_id: 22,
project_role_names: [PROJECT_ROLE.OBSERVER, PROJECT_ROLE.COLLABORATOR]
}
];

const dbConnection = getMockDBConnection();
const service = new ProjectParticipationService(dbConnection);

const result = service._doProjectParticipantsHaveOneRole(projectUsers);

expect(result).to.be.false;
});
Expand Down Expand Up @@ -1058,6 +1148,10 @@ describe('ProjectParticipationService', () => {
project_participation_id: 12,
project_role_names: [PROJECT_ROLE.COORDINATOR] // Existing user to be updated
},
{
system_user_id: 33,
project_role_names: [PROJECT_ROLE.COLLABORATOR] // Existing user to be unaffected
},
{
system_user_id: 44,
project_role_names: [PROJECT_ROLE.OBSERVER] // New user
Expand Down Expand Up @@ -1086,6 +1180,25 @@ describe('ProjectParticipationService', () => {
user_guid: '123-456-789-1',
user_identifier: 'testuser1'
},
{
project_participation_id: 6, // Existing user to be unaffected
project_id: 1,
system_user_id: 33,
project_role_ids: [2],
project_role_names: [PROJECT_ROLE.COLLABORATOR],
project_role_permissions: ['Permission1'],
agency: null,
display_name: 'test user 1',
email: 'email@email.com',
family_name: 'lname',
given_name: 'fname',
identity_source: SYSTEM_IDENTITY_SOURCE.IDIR,
record_end_date: null,
role_ids: [2],
role_names: [SYSTEM_ROLE.PROJECT_CREATOR],
user_guid: '123-456-789-1',
user_identifier: 'testuser1'
},
{
project_participation_id: 23, // Existing user to be removed
project_id: 1,
Expand Down Expand Up @@ -1121,6 +1234,8 @@ describe('ProjectParticipationService', () => {
expect(getProjectParticipantsStub).to.have.been.calledOnceWith(projectId);
expect(deleteProjectParticipationRecordStub).to.have.been.calledWith(1, 23);
expect(updateProjectParticipationRoleStub).to.have.been.calledOnceWith(12, PROJECT_ROLE.COORDINATOR);
expect(updateProjectParticipationRoleStub).to.not.have.been.calledWith(6, PROJECT_ROLE.COLLABORATOR);
expect(postProjectParticipantStub).to.not.have.been.calledWith(projectId, 6, PROJECT_ROLE.COLLABORATOR);
expect(postProjectParticipantStub).to.have.been.calledOnceWith(projectId, 44, PROJECT_ROLE.OBSERVER);
});
});
Expand Down
122 changes: 100 additions & 22 deletions api/src/services/project-participation-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,50 +314,128 @@ export class ProjectParticipationService extends DBService {
return true;
}

doProjectParticipantsHaveARole(participants: PostParticipantData[], roleToCheck: PROJECT_ROLE): boolean {
/**
* Internal function for validating that all Project members have a role
*
* @param {PostParticipantData[]} participants
* @param {PROJECT_ROLE} roleToCheck
* @return {*} {boolean}
* @memberof ProjectParticipationService
*/
_doProjectParticipantsHaveARole(participants: PostParticipantData[], roleToCheck: PROJECT_ROLE): boolean {
return participants.some((item) => item.project_role_names.some((role) => role === roleToCheck));
}

async upsertProjectParticipantData(projectId: number, participants: PostParticipantData[]): Promise<void> {
if (!this.doProjectParticipantsHaveARole(participants, PROJECT_ROLE.COORDINATOR)) {
/**
* Internal function for validating that all project participants have one unique role.
*
* @param {PostParticipantData[]} participants
* @return {*} {boolean}
* @memberof ProjectParticipationService
*/
_doProjectParticipantsHaveOneRole(participants: PostParticipantData[]): boolean {
// Map of system_user_id to set of unique role names
const participantUniqueRoles = new Map<number, Set<string>>();

for (const participant of participants) {
const system_user_id = participant.system_user_id;
const project_role_names = participant.project_role_names;

// Get the set of unique role names, or initialize a new set if the user is not in the map
const uniqueRoleNamesForParticipant = participantUniqueRoles.get(system_user_id) ?? new Set<string>();

for (const role of project_role_names) {
// Add the role names to the set, converting to lowercase to ensure case-insensitive comparison
uniqueRoleNamesForParticipant.add(role.toLowerCase());
}

// Update the map with the new set of unique role names
participantUniqueRoles.set(system_user_id, uniqueRoleNamesForParticipant);
}

// Returns true if all participants have one unique role
return Array.from(participantUniqueRoles.values()).every((roleNames) => roleNames.size === 1);
}

/**
* Updates existing participants, deletes those participants not in the incoming list, and inserts new participants.
*
* @param {number} projectId
* @param {PostParticipantData[]} incomingParticipants
* @return {*} {Promise<void>}
* @throws ApiGeneralError If no participant has a coordinator role or if any participant has multiple roles.
* @memberof ProjectParticipationService
*/
async upsertProjectParticipantData(projectId: number, incomingParticipants: PostParticipantData[]): Promise<void> {
// Confirm that at least one participant has a coordinator role
if (!this._doProjectParticipantsHaveARole(incomingParticipants, PROJECT_ROLE.COORDINATOR)) {
throw new ApiGeneralError(
`Projects require that at least one participant has a ${PROJECT_ROLE.COORDINATOR} role.`
);
}

// all actions to take
const promises: Promise<any>[] = [];
// Check for multiple roles for any participant
if (!this._doProjectParticipantsHaveOneRole(incomingParticipants)) {
throw new ApiGeneralError(
'Users can only have one role per Project but multiple roles were specified for at least one user.'
);
}

// get the existing participants for a project
// Fetch existing participants for the project
const existingParticipants = await this.projectParticipationRepository.getProjectParticipants(projectId);

// Compare incoming with existing to find any outliers to delete
// Prepare promises for all database operations
const promises: Promise<any>[] = [];

// Identify participants to delete
const participantsToDelete = existingParticipants.filter(
(item) => !participants.find((incoming) => incoming.system_user_id === item.system_user_id)
(existingParticipant) =>
!incomingParticipants.some(
(incomingParticipant) => incomingParticipant.system_user_id === existingParticipant.system_user_id
)
);

// delete
participantsToDelete.forEach((item) => {
// Delete participants not present in the incoming payload
participantsToDelete.forEach((participantToDelete) => {
promises.push(
this.projectParticipationRepository.deleteProjectParticipationRecord(projectId, item.project_participation_id)
this.projectParticipationRepository.deleteProjectParticipationRecord(
projectId,
participantToDelete.project_participation_id
)
);
});

participants.forEach((item) => {
if (item.project_participation_id) {
// Upsert participants based on conditions
incomingParticipants.forEach((incomingParticipant) => {
const existingParticipant = existingParticipants.find(
(existingParticipant) => existingParticipant.system_user_id === incomingParticipant.system_user_id
);

if (existingParticipant) {
// Update existing participant's role
if (
!existingParticipant.project_role_names.some((existingRole) =>
incomingParticipant.project_role_names.includes(existingRole as PROJECT_ROLE)
)
) {
promises.push(
this.projectParticipationRepository.updateProjectParticipationRole(
incomingParticipant.project_participation_id ?? existingParticipant.project_participation_id,
incomingParticipant.project_role_names[0]
)
);
}
} else if (!existingParticipant) {
// Insert new participant if the user does not already exist in the project, otherwise triggers database constraint error
promises.push(
this.projectParticipationRepository.updateProjectParticipationRole(
item.project_participation_id,
item.project_role_names[0]
this.projectParticipationRepository.postProjectParticipant(
projectId,
incomingParticipant.system_user_id,
incomingParticipant.project_role_names[0]
)
);
} else {
this.projectParticipationRepository.postProjectParticipant(
projectId,
item.system_user_id,
item.project_role_names[0]
);
}
// If the participant already exists with the desired role, do nothing
});

await Promise.all(promises);
Expand Down
22 changes: 20 additions & 2 deletions app/src/components/user/UserRoleSelector.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { mdiClose } from '@mdi/js';
import Icon from '@mdi/react';
import Box from '@mui/material/Box';
import { grey } from '@mui/material/colors';
import grey from '@mui/material/colors/grey';
import IconButton from '@mui/material/IconButton';
import MenuItem from '@mui/material/MenuItem';
import Paper from '@mui/material/Paper';
import Select from '@mui/material/Select';
import Typography from '@mui/material/Typography';
import { PROJECT_ROLE_ICONS } from 'constants/roles';
import { ICode } from 'interfaces/useCodesApi.interface';
import { IGetProjectParticipant } from 'interfaces/useProjectApi.interface';
import { IGetSurveyParticipant } from 'interfaces/useSurveyApi.interface';
Expand Down Expand Up @@ -64,11 +66,27 @@ const UserRoleSelector: React.FC<IUserRoleSelectorProps> = (props) => {
if (!selected) {
return props.label;
}
return selected;
return (
<Typography alignItems="center" display="flex">
{selected}
{PROJECT_ROLE_ICONS[selected] && (
<>
&nbsp;
<Icon path={PROJECT_ROLE_ICONS[selected] ?? ''} size={0.75} color={grey[600]} />
</>
)}
</Typography>
);
}}>
{roles.map((item) => (
<MenuItem key={item.id} value={item.name}>
{item.name}
{PROJECT_ROLE_ICONS[item.name] && (
<>
&nbsp;
<Icon path={PROJECT_ROLE_ICONS[item.name] ?? ''} size={0.75} color={grey[600]} />
</>
)}
</MenuItem>
))}
</Select>
Expand Down
13 changes: 13 additions & 0 deletions app/src/constants/roles.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { mdiAccountEdit, mdiCrown } from '@mdi/js';

/**
* System level roles.
*
Expand Down Expand Up @@ -33,3 +35,14 @@ export enum PROJECT_PERMISSION {
COLLABORATOR = 'Collaborator',
OBSERVER = 'Observer'
}

/**
* Project role icons
*
* @export
*/
export const PROJECT_ROLE_ICONS: Record<string, string | undefined> = {
Coordinator: mdiCrown,
Collaborator: mdiAccountEdit,
Observer: undefined
};
Loading

0 comments on commit e4a401e

Please sign in to comment.