Support user invites for multiple participants #516
Conversation
- Allow for existing user to be added to a brand new participant - Allow for existing user to be added to an existing participant - New email template for existing user being invited
| const existingParticipant = await Participant.query().findOne( | ||
| 'name', | ||
| participantRequest.participantName | ||
| ); | ||
| if (existingParticipant) { | ||
| errorMessage = 'Duplicate participant name'; |
There was a problem hiding this comment.
These two cases are now valid, so should not error
There was a problem hiding this comment.
If this isn't an error message anymore, are we doing anything with the strings that get returned (and does the function care or use the different strings being returned)? Should it just be a boolean function instead?
There was a problem hiding this comment.
the method still returns an error message if there's an error or null if there is no error, so the functionality is still the same as before. If we change it to return a boolean then we'd lose the error message. I'm inclined to leave as-is unless you feel strongly? Perhaps my inital change/comment was confusing.
| const participant = await User.transaction(async (trx) => { | ||
| let user = await findUserByEmail(parsedUser.email); | ||
| if (!user) { | ||
| user = await User.query(trx).insertAndFetch(parsedUser); |
There was a problem hiding this comment.
Only insert/update the user if they don't exist already. Otherwise leave them as-is (don't update their name/jobFunction)
| participant!.id | ||
| ); | ||
| await sendInviteEmailToNewUser(kcAdminClient, newUser); | ||
| await inviteUserToParticipant(userPartial, participant!, traceId); |
There was a problem hiding this comment.
This new method handles both cases (existing user & new user)
| @@ -6,7 +6,7 @@ import { AuditAction, AuditTrailEvents } from '../../entities/AuditTrail'; | |||
| import { | |||
| Participant, | |||
| ParticipantCreationPartial, | |||
| ParticipantStatus, | |||
| ParticipantStatus | |||
There was a problem hiding this comment.
Same comment as the other PR with the difference in comma formatting
|
|
||
| export async function getParticipantUsers(req: ParticipantRequest, res: Response) { | ||
| const { participant } = req; | ||
| const users = await getAllUserFromParticipant(participant!); | ||
| return res.status(200).json(users); | ||
| } | ||
|
|
||
| const invitationParser = z.object({ | ||
| export const UserInvitationParser = z.object({ | ||
| firstName: z.string(), | ||
| lastName: z.string(), | ||
| email: z.string(), |
There was a problem hiding this comment.
nit: Is the convention normally that the first word is capitalized? I see it changed from the first letter being lowercase to upper case
There was a problem hiding this comment.
good point, I'd say most of them are actually camelCase:
Looking at the zod docs, it looks like they use PascalCase for objects and camelCase for primitives.
ChatGPT argues that PascalCase should be used for models or types that are intended to be reused across the application, while camelCase should be used for schemas that are defined within a specific, narrow scope, like a single function. I tend to agree with that.
Thinking about this more, it probably doesn't make the most sense to include Parser in the name either, since it's just a ZodObject which has a parse method on it.
I've actioned this for this instance - I'm happy to apply that convention more broadly in a separate PR /ticket if we think it's the right move.
thanks!
There was a problem hiding this comment.
Definitely not urgent, but we could make a separate continuous improvement, quick win ticket for changing this at some point
…portal into ajy-UID2-3878-Support-user-invites-for-multiple-participants
alex-yau-ttd
deleted the
ajy-UID2-3878-Support-user-invites-for-multiple-participants
branch
Inviting to existing participant
chrome_DK0L5AQHWy.mp4
Inviting to new participant
chrome_oDuMMnTmYu.mp4
New email
Future work
Support user deletion in the multiple-participant access world.