From c2b34346626cb22bb5942eae1d98d36859c6d3ed Mon Sep 17 00:00:00 2001 From: Alfred Rosenthal Date: Fri, 29 Sep 2023 10:19:18 -0700 Subject: [PATCH 1/5] fixed approval flow --- .../admin/users/AccessRequestList.tsx | 17 +++++++-------- app/src/hooks/api/useAdminApi.ts | 21 ++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/app/src/features/admin/users/AccessRequestList.tsx b/app/src/features/admin/users/AccessRequestList.tsx index a2822cb51c..2e148b9cd9 100644 --- a/app/src/features/admin/users/AccessRequestList.tsx +++ b/app/src/features/admin/users/AccessRequestList.tsx @@ -97,15 +97,14 @@ const AccessRequestList: React.FC = (props) => { setActiveReviewDialog({ open: false, request: null }); try { - await biohubApi.admin.approveAccessRequest( - updatedRequest.id, - updatedRequest.data.userGuid, - updatedRequest.data.username, - updatedRequest.data.identitySource, - updatedRequest.data.email, - updatedRequest.data.displayName, - (values.system_role && [values.system_role]) || [] - ); + await biohubApi.admin.approveAccessRequest(updatedRequest.id, { + userGuid: updatedRequest.data.userGuid, + userIdentifier: updatedRequest.data.username, + identitySource: updatedRequest.data.identitySource, + email: updatedRequest.data.email, + displayName: updatedRequest.data.displayName, + roleIds: (values.system_role && [values.system_role]) || [] + }); try { await biohubApi.admin.sendGCNotification( diff --git a/app/src/hooks/api/useAdminApi.ts b/app/src/hooks/api/useAdminApi.ts index f1269a3856..5355c0deb0 100644 --- a/app/src/hooks/api/useAdminApi.ts +++ b/app/src/hooks/api/useAdminApi.ts @@ -58,20 +58,17 @@ const useAdminApi = (axios: AxiosInstance) => { const approveAccessRequest = async ( administrativeActivityId: number, - userGuid: string | null, - userIdentifier: string, - identitySource: string, - displayName: string, - email: string, - roleIds: number[] = [] + userData: { + userGuid: string | null; + userIdentifier: string; + identitySource: string; + email: string; + displayName: string; + roleIds: number[]; + } ): Promise => { const { data } = await axios.put(`/api/administrative-activity/system-access/${administrativeActivityId}/approve`, { - userGuid, - userIdentifier, - identitySource, - displayName, - email, - roleIds: roleIds + ...userData }); return data; From 16d592123bd0f3c5f7e3610c7ee24f101691d450 Mon Sep 17 00:00:00 2001 From: Alfred Rosenthal Date: Wed, 4 Oct 2023 10:32:02 -0700 Subject: [PATCH 2/5] fixed spelling mistake, updated activity fetch to be guid --- api/src/paths/administrative-activity.ts | 11 +++++------ app/src/hooks/useKeycloakWrapper.tsx | 2 +- app/src/interfaces/useAdminApi.interface.ts | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/api/src/paths/administrative-activity.ts b/api/src/paths/administrative-activity.ts index a3f7ff1d0a..2db83ff03a 100644 --- a/api/src/paths/administrative-activity.ts +++ b/api/src/paths/administrative-activity.ts @@ -87,9 +87,9 @@ GET.apiDoc = { 'application/json': { schema: { type: 'object', - required: ['has_pending_acccess_request', 'has_one_or_more_project_roles'], + required: ['has_pending_access_request', 'has_one_or_more_project_roles'], properties: { - has_pending_acccess_request: { + has_pending_access_request: { type: 'boolean' }, has_one_or_more_project_roles: { @@ -166,10 +166,9 @@ export function getAdministrativeActivityStanding(): RequestHandler { const connection = getAPIUserDBConnection(); try { - // TODO Update to use user guid instead of identifier - const userIdentifier = getUserGuid(req['keycloak_token']); + const userGUID = getUserGuid(req['keycloak_token']); - if (!userIdentifier) { + if (!userGUID) { throw new HTTP400('Failed to identify user'); } @@ -177,7 +176,7 @@ export function getAdministrativeActivityStanding(): RequestHandler { const administrativeActivityService = new AdministrativeActivityService(connection); - const response = await administrativeActivityService.getAdministrativeActivityStanding(userIdentifier); + const response = await administrativeActivityService.getAdministrativeActivityStanding(userGUID); await connection.commit(); diff --git a/app/src/hooks/useKeycloakWrapper.tsx b/app/src/hooks/useKeycloakWrapper.tsx index 0042055c8f..e3a9d10d71 100644 --- a/app/src/hooks/useKeycloakWrapper.tsx +++ b/app/src/hooks/useKeycloakWrapper.tsx @@ -319,7 +319,7 @@ function useKeycloakWrapper(): IKeycloakWrapper { systemRoles: getSystemRoles(), hasSystemRole, isSystemUser, - hasAccessRequest: !!administrativeActivityStandingDataLoader.data?.has_pending_acccess_request, + hasAccessRequest: !!administrativeActivityStandingDataLoader.data?.has_pending_access_request, hasOneOrMoreProjectRoles: !!administrativeActivityStandingDataLoader.data?.has_one_or_more_project_roles, getUserIdentifier, getUserGuid, diff --git a/app/src/interfaces/useAdminApi.interface.ts b/app/src/interfaces/useAdminApi.interface.ts index 6af45ce947..dfda377fa7 100644 --- a/app/src/interfaces/useAdminApi.interface.ts +++ b/app/src/interfaces/useAdminApi.interface.ts @@ -34,7 +34,7 @@ export interface IGetAccessRequestsListResponse { } export interface IGetAdministrativeActivityStanding { - has_pending_acccess_request: boolean; + has_pending_access_request: boolean; has_one_or_more_project_roles: boolean; } From 08ec66d3a728156d5aeaaa3700b805c7c3e9ddf9 Mon Sep 17 00:00:00 2001 From: Alfred Rosenthal Date: Wed, 4 Oct 2023 10:32:19 -0700 Subject: [PATCH 3/5] fixed SQL to look for guid vs user identifier --- .../repositories/administrative-activity-repository.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/src/repositories/administrative-activity-repository.ts b/api/src/repositories/administrative-activity-repository.ts index 5c4e47cd13..495042a0c2 100644 --- a/api/src/repositories/administrative-activity-repository.ts +++ b/api/src/repositories/administrative-activity-repository.ts @@ -9,7 +9,7 @@ import { jsonSchema } from '../zod-schema/json'; import { BaseRepository } from './base-repository'; export const IAdministrativeActivityStanding = z.object({ - has_pending_acccess_request: z.boolean(), + has_pending_access_request: z.boolean(), has_one_or_more_project_roles: z.boolean() }); @@ -182,7 +182,7 @@ export class AdministrativeActivityRepository extends BaseRepository { * @return {*} {(Promise)} * @memberof AdministrativeActivityRepository */ - async getAdministrativeActivityStanding(userIdentifier: string): Promise { + async getAdministrativeActivityStanding(userGUID: string): Promise { const sqlStatement = SQL` WITH administrative_activity_with_status @@ -191,7 +191,7 @@ export class AdministrativeActivityRepository extends BaseRepository { CASE WHEN COUNT(*) > 0 THEN TRUE ELSE FALSE - END AS has_pending_acccess_request + END AS has_pending_access_request FROM administrative_activity aa LEFT OUTER JOIN @@ -199,7 +199,7 @@ export class AdministrativeActivityRepository extends BaseRepository { ON aa.administrative_activity_status_type_id = aast.administrative_activity_status_type_id WHERE - (aa.data -> 'username')::text = '"' || ${userIdentifier} || '"' + (aa.data -> 'userGuid')::text = '"' || ${userGUID} || '"' AND aast.name = 'Pending' ), @@ -217,7 +217,7 @@ export class AdministrativeActivityRepository extends BaseRepository { ON pp.system_user_id = su.system_user_id WHERE - su.user_identifier = ${userIdentifier} + su.user_guid = ${userGUID} ) SELECT * FROM From 15f6d28ca2e239c61d5474686a2f9fddc9cf7758 Mon Sep 17 00:00:00 2001 From: Alfred Rosenthal Date: Wed, 4 Oct 2023 11:02:17 -0700 Subject: [PATCH 4/5] fixed tests --- api/src/paths/administrative-activity.test.ts | 2 +- .../administrative-activity-service.test.ts | 2 +- .../admin/users/AccessRequestList.test.tsx | 17 ++++++++--------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/api/src/paths/administrative-activity.test.ts b/api/src/paths/administrative-activity.test.ts index cdb381b727..97b5e212ba 100644 --- a/api/src/paths/administrative-activity.test.ts +++ b/api/src/paths/administrative-activity.test.ts @@ -109,7 +109,7 @@ describe('getAdministrativeActivityStanding', () => { sinon.stub(db, 'getAPIUserDBConnection').returns(dbConnectionObj); const mockResponse: IAdministrativeActivityStanding = { - has_pending_acccess_request: true, + has_pending_access_request: true, has_one_or_more_project_roles: true }; diff --git a/api/src/services/administrative-activity-service.test.ts b/api/src/services/administrative-activity-service.test.ts index 1a7ead543e..0f570b87cf 100644 --- a/api/src/services/administrative-activity-service.test.ts +++ b/api/src/services/administrative-activity-service.test.ts @@ -98,7 +98,7 @@ describe('AdministrativeActivityService', () => { const dbConnection = getMockDBConnection(); const mockRepoResponse = { - has_pending_acccess_request: false, + has_pending_access_request: false, has_one_or_more_project_roles: false }; const repoStub = sinon diff --git a/app/src/features/admin/users/AccessRequestList.test.tsx b/app/src/features/admin/users/AccessRequestList.test.tsx index 2a64cad211..67b93462b7 100644 --- a/app/src/features/admin/users/AccessRequestList.test.tsx +++ b/app/src/features/admin/users/AccessRequestList.test.tsx @@ -223,15 +223,14 @@ describe('AccessRequestList', () => { await waitFor(() => { expect(refresh).toHaveBeenCalledTimes(1); expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledTimes(1); - expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledWith( - 1, - 'aaaa', - 'testusername', - SYSTEM_IDENTITY_SOURCE.IDIR, - 'email@email.com', - 'test user', - [2] - ); + expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledWith(1, { + displayName: 'test user', + email: 'email@email.com', + identitySource: 'IDIR', + roleIds: [2], + userGuid: 'aaaa', + userIdentifier: 'testusername' + }); }); }); From 51aebfcf719fd4a95848f96e817e6683414892ec Mon Sep 17 00:00:00 2001 From: Alfred Rosenthal Date: Thu, 5 Oct 2023 08:57:14 -0700 Subject: [PATCH 5/5] updating JSDOCS --- .../repositories/administrative-activity-repository.ts | 4 ++-- api/src/services/administrative-activity-service.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/src/repositories/administrative-activity-repository.ts b/api/src/repositories/administrative-activity-repository.ts index 495042a0c2..0a0dd1dd4c 100644 --- a/api/src/repositories/administrative-activity-repository.ts +++ b/api/src/repositories/administrative-activity-repository.ts @@ -176,9 +176,9 @@ export class AdministrativeActivityRepository extends BaseRepository { } /** - * SQL query to count pending records in the administrative_activity table. + * SQL query to count pending records in the administrative_activity table for a given user GUID * - * @param {string} userIdentifier + * @param {string} userGUID * @return {*} {(Promise)} * @memberof AdministrativeActivityRepository */ diff --git a/api/src/services/administrative-activity-service.ts b/api/src/services/administrative-activity-service.ts index d0f27e8e7c..e1306e090c 100644 --- a/api/src/services/administrative-activity-service.ts +++ b/api/src/services/administrative-activity-service.ts @@ -88,14 +88,14 @@ export class AdministrativeActivityService extends DBService { } /** - * Fetch an existing administrative activity record for a user, based on their user identifier. + * Fetch an existing administrative activity record for a user, based on their user GUID. * - * @param {string} userIdentifier + * @param {string} userGUID * @return {*} {(Promise)} * @memberof AdministrativeActivityService */ - async getAdministrativeActivityStanding(userIdentifier: string): Promise { - return this.administrativeActivityRepository.getAdministrativeActivityStanding(userIdentifier); + async getAdministrativeActivityStanding(userGUID: string): Promise { + return this.administrativeActivityRepository.getAdministrativeActivityStanding(userGUID); } /**