Show user permissions and user audit trail #602
Conversation
…show-user-permissions-and-audits
| ...user, | ||
| isSuperUser: userIsSuperUser, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
We're not using this function at all, this was a mistake from the last ticket
| const userList = await User.query() | ||
| .where('deleted', 0) | ||
| .orderBy('email') | ||
| .withGraphFetched('userToParticipantRoles'); |
There was a problem hiding this comment.
pulling all the roles the user has as well
There was a problem hiding this comment.
do we still need to do this when we're loading all of this on loading the dialog?
| @@ -3,7 +3,7 @@ import { Response } from 'express'; | |||
| import { GetParticipantAuditTrail } from '../../services/auditTrailService'; | |||
| import { UserParticipantRequest } from '../../services/participantsService'; | |||
|
|
|||
| export async function handleGetAuditTrail(req: UserParticipantRequest, res: Response) { | |||
| export async function handleGetParticipantAuditTrail(req: UserParticipantRequest, res: Response) { | |||
There was a problem hiding this comment.
renamed to be more clear since we have a regular get audit trail now too
| > | ||
| <span>This user does not belong to any participant.</span> | ||
| </TableNoDataPlaceholder> | ||
| )} |
There was a problem hiding this comment.
this case above should not happen but good to have just in case
| <td className='role-name'>{roleName ?? 'Cannot find participant role'}</td> | ||
| </tr> | ||
| ); | ||
| } |
There was a problem hiding this comment.
not having participantName or roleName should not happen but good to include just in case
| </div> | ||
| )} | ||
| </Dialog> | ||
| ); |
There was a problem hiding this comment.
i dont think there's a reason to list out every single participant if the user is UID2Support or SuperUser
| @@ -43,3 +43,8 @@ export const GetParticipantAuditTrail = async (participant: Participant) => { | |||
| ); | |||
| return auditTrail; | |||
| }; | |||
|
|
|||
| export const GetAuditTrail = async () => { | |||
There was a problem hiding this comment.
I think we should be filtering this by user on the server and behind the Superuser role. I also wonder if we should maybe put this in managementService with the other stuff. Personally I like having everything that should only be accessible to Superusers in the same place to prevent someone from using those functions in another way. What do you think?
| @@ -3,7 +3,7 @@ import { Response } from 'express'; | |||
| import { GetParticipantAuditTrail } from '../../services/auditTrailService'; | |||
| import { UserParticipantRequest } from '../../services/participantsService'; | |||
|
|
|||
| export async function handleGetAuditTrail(req: UserParticipantRequest, res: Response) { | |||
There was a problem hiding this comment.
maybe handleGetUserAuditTrail to make it even more clear?
src/web/screens/manageUsers.tsx
Outdated
| import { RouteErrorBoundary } from '../utils/RouteErrorBoundary'; | ||
| import { PortalRoute } from './routeUtils'; | ||
|
|
||
| const loader = () => { | ||
| const userList = GetAllUsers(); | ||
| return defer({ userList }); | ||
| const participantsList = GetAllParticipants(); |
There was a problem hiding this comment.
I think we should probably just load these two on demand when the dialog opens. Maybe create a different endpoint in managementRouter to get the user's participant/role info rather than loading everything and putting it together in the UI. I like keeping as much sensitive info as possible hidden on the server.
ashleysmithTTD
deleted the
ans-UID2-5008-show-user-permissions-and-audits
branch
Test Plan:
-Test that viewing a UID2 support person does not show all participants
-Test that viewing a user with only Admin and Operations shows their participants. Test a user with a single participant and with multiple participants (make sure there is a dateApproved column not null when testing)
-Test that viewing a user with no audit logs works
-Test viewing your own user that probably has a lot of audit logs that scrolling and paging works