From 80618f5f00d15f8f093cf08288905106ffc137bd Mon Sep 17 00:00:00 2001 From: araddcc002 Date: Tue, 17 Dec 2024 13:03:05 +0000 Subject: [PATCH] removed getEntitiesLookup code and replaced reaction display to use single end point --- backend/src/connectors/authentication/Base.ts | 3 - backend/src/routes.ts | 2 - .../routes/v2/entities/getEntitiesLookup.ts | 50 ---------------- backend/src/services/specification.ts | 8 --- .../getEntitiesLookup.spec.ts.snap | 57 ------------------- .../routes/entities/getEntitiesLookup.spec.ts | 38 ------------- frontend/actions/user.ts | 21 +------ frontend/src/reviews/ReactionDisplay.tsx | 45 ++++++++------- 8 files changed, 26 insertions(+), 198 deletions(-) delete mode 100644 backend/src/routes/v2/entities/getEntitiesLookup.ts delete mode 100644 backend/test/routes/entities/__snapshots__/getEntitiesLookup.spec.ts.snap delete mode 100644 backend/test/routes/entities/getEntitiesLookup.spec.ts diff --git a/backend/src/connectors/authentication/Base.ts b/backend/src/connectors/authentication/Base.ts index 36568401a..45a2ffb9d 100644 --- a/backend/src/connectors/authentication/Base.ts +++ b/backend/src/connectors/authentication/Base.ts @@ -22,9 +22,6 @@ export abstract class BaseAuthenticationConnector { abstract getEntities(user: UserInterface): Promise> abstract getUserInformation(userEntity: string): Promise abstract getEntityMembers(entity: string): Promise> - async getMultipleUsersInformation(entities: string[]): Promise { - return Promise.all(entities.map(async (entity) => await this.getUserInformation(entity))) - } async getUserInformationList(entity: string): Promise { const entities = await this.getEntityMembers(entity) diff --git a/backend/src/routes.ts b/backend/src/routes.ts index 3524de749..3ac492758 100644 --- a/backend/src/routes.ts +++ b/backend/src/routes.ts @@ -9,7 +9,6 @@ import { requestId } from './routes/middleware/requestId.js' import { getDockerRegistryAuth } from './routes/v1/registryAuth.js' import { getCurrentUser } from './routes/v2/entities/getCurrentUser.js' import { getEntities } from './routes/v2/entities/getEntities.js' -import { getEntitiesLookup } from './routes/v2/entities/getEntitiesLookup.js' import { getEntityLookup } from './routes/v2/entities/getEntityLookup.js' import { getFilescanningInfo } from './routes/v2/filescanning/getFilescanningInfo.js' import { putFileScan } from './routes/v2/filescanning/putFileScan.js' @@ -181,7 +180,6 @@ server.get('/api/v2/model/:modelId/permissions/mine', ...getModelCurrentUserPerm server.get('/api/v2/entities', ...getEntities) server.get('/api/v2/entities/me', ...getCurrentUser) server.get('/api/v2/entity/:dn/lookup', ...getEntityLookup) -server.get('/api/v2/entities/lookup', ...getEntitiesLookup) server.get('/api/v2/config/ui', ...getUiConfig) diff --git a/backend/src/routes/v2/entities/getEntitiesLookup.ts b/backend/src/routes/v2/entities/getEntitiesLookup.ts deleted file mode 100644 index ac62b93f1..000000000 --- a/backend/src/routes/v2/entities/getEntitiesLookup.ts +++ /dev/null @@ -1,50 +0,0 @@ -import bodyParser from 'body-parser' -import { Request, Response } from 'express' -import { z } from 'zod' - -import { UserInformation } from '../../../connectors/authentication/Base.js' -import authentication from '../../../connectors/authentication/index.js' -import { registerPath, UserInformationSchemaList } from '../../../services/specification.js' -import { toEntity } from '../../../utils/entity.js' -import { coerceArray, parse } from '../../../utils/validate.js' - -export const getEntitiesLookupSchema = z.object({ - query: z.object({ - dnList: coerceArray(z.array(z.string())), - }), -}) - -registerPath({ - method: 'get', - path: '/api/v2/entities/lookup', - tags: ['user'], - description: 'Get information about a list of entities', - schema: getEntitiesLookupSchema, - responses: { - 200: { - description: 'Information about the provided entities.', - content: { - 'application/json': { - schema: z.object({ entities: UserInformationSchemaList }), - }, - }, - }, - }, -}) - -interface GetEntitiesLookup { - entities: UserInformation[] -} - -export const getEntitiesLookup = [ - bodyParser.json(), - async (req: Request, res: Response) => { - const { - query: { dnList }, - } = parse(req, getEntitiesLookupSchema) - const dnListArray = dnList.map((dnListMember) => toEntity('user', dnListMember)) - const informationList = await authentication.getMultipleUsersInformation(dnListArray) - - return res.json({ entities: informationList }) - }, -] diff --git a/backend/src/services/specification.ts b/backend/src/services/specification.ts index 1814def0e..f7045853f 100644 --- a/backend/src/services/specification.ts +++ b/backend/src/services/specification.ts @@ -304,11 +304,3 @@ export const UserInformationSchema = z.object({ name: z.string().optional().openapi({ example: 'Joe Bloggs' }), organisation: z.string().optional().openapi({ example: 'Acme Corp' }), }) - -export const UserInformationSchemaList = z - .object({ - email: z.string().optional().openapi({ example: 'user@example.com' }), - name: z.string().optional().openapi({ example: 'Joe Bloggs' }), - organisation: z.string().optional().openapi({ example: 'Acme Corp' }), - }) - .array() diff --git a/backend/test/routes/entities/__snapshots__/getEntitiesLookup.spec.ts.snap b/backend/test/routes/entities/__snapshots__/getEntitiesLookup.spec.ts.snap deleted file mode 100644 index 8799ccf26..000000000 --- a/backend/test/routes/entities/__snapshots__/getEntitiesLookup.spec.ts.snap +++ /dev/null @@ -1,57 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[`routes > entities > getEntitiesLookup > 200 > ok 1`] = ` -[ - [ - [ - "user:userdn", - "user:userdn", - ], - ], -] -`; - -exports[`routes > entities > getEntitiesLookup > 200 > ok 2`] = ` -{ - "entities": [ - { - "email": "joeb@example.com", - "name": "Joe Bloggs", - "organisation": "Acme Corp", - }, - { - "email": "janeb@example.com", - "name": "Jane Bloggs", - "organisation": "Acme Corp", - }, - ], -} -`; - -exports[`routes > entities > getEntityLookup > 200 > ok 1`] = ` -[ - [ - [ - "user:userdn", - "user:userdn", - ], - ], -] -`; - -exports[`routes > entities > getEntityLookup > 200 > ok 2`] = ` -{ - "entities": [ - { - "email": "joeb@example.com", - "name": "Joe Bloggs", - "organisation": "Acme Corp", - }, - { - "email": "janeb@example.com", - "name": "Jane Bloggs", - "organisation": "Acme Corp", - }, - ], -} -`; diff --git a/backend/test/routes/entities/getEntitiesLookup.spec.ts b/backend/test/routes/entities/getEntitiesLookup.spec.ts deleted file mode 100644 index 8468a3d56..000000000 --- a/backend/test/routes/entities/getEntitiesLookup.spec.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { describe, expect, test, vi } from 'vitest' - -import { testGet } from '../../testUtils/routes.js' - -vi.mock('../../../src/utils/user.js') - -const authenticationMocks = vi.hoisted(() => ({ - authenticationMiddleware: vi.fn(() => []), - getMultipleUsersInformation: vi.fn(() => [ - { - email: `joeb@example.com`, - name: 'Joe Bloggs', - organisation: 'Acme Corp', - }, - { - email: `janeb@example.com`, - name: 'Jane Bloggs', - organisation: 'Acme Corp', - }, - ]), -})) -vi.mock('../../../src/connectors/authentication/index.js', async () => ({ - default: authenticationMocks, -})) - -vi.mock('../../../src/utils/entity.js', async () => ({ - toEntity: vi.fn(() => 'user:userdn'), -})) - -describe('routes > entities > getEntitiesLookup', () => { - test('200 > ok', async () => { - const res = await testGet(`/api/v2/entities/lookup?dnList=user1&dnList=user2`) - - expect(res.statusCode).toBe(200) - expect(authenticationMocks.getMultipleUsersInformation.mock.calls).matchSnapshot() - expect(res.body).matchSnapshot() - }) -}) diff --git a/frontend/actions/user.ts b/frontend/actions/user.ts index 53d7517a3..72bd4d233 100644 --- a/frontend/actions/user.ts +++ b/frontend/actions/user.ts @@ -63,25 +63,8 @@ export function useGetUserInformation(dn: string) { } } -interface MultipleUserInformationResponse { - entities: UserInformation[] -} - -export function useGetMultipleUserInformation(dnList: string[]) { - const queryParams = { - ...(dnList.length > 0 && { dnList }), - } - const { data, isLoading, error, mutate } = useSWR( - `/api/v2/entities/lookup?${qs.stringify(queryParams)}`, - fetcher, - ) - - return { - mutateUserInformation: mutate, - userInformation: data?.entities || [], - isUserInformationLoading: isLoading, - isUserInformationError: error, - } +export function getUserInformation(dn: string) { + return fetch(`/api/v2/entity/${dn}/lookup`, { method: 'get' }) } interface GetUserTokensResponse { diff --git a/frontend/src/reviews/ReactionDisplay.tsx b/frontend/src/reviews/ReactionDisplay.tsx index 37b072c00..1ea9bc6e6 100644 --- a/frontend/src/reviews/ReactionDisplay.tsx +++ b/frontend/src/reviews/ReactionDisplay.tsx @@ -1,8 +1,6 @@ import { Button, Tooltip } from '@mui/material' -import { useGetMultipleUserInformation } from 'actions/user' -import { ReactNode, useMemo } from 'react' -import Loading from 'src/common/Loading' -import MessageAlert from 'src/MessageAlert' +import { getUserInformation } from 'actions/user' +import { ReactNode, useEffect, useState } from 'react' import { ReactionKindKeys } from 'types/types' import { plural } from 'utils/stringUtils' @@ -14,32 +12,37 @@ type ReactionDisplayProps = { } export default function ReactionDisplay({ kind, icon, users, onReactionClick }: ReactionDisplayProps) { - const { userInformation, isUserInformationLoading, isUserInformationError } = useGetMultipleUserInformation(users) - const title = useMemo(() => { - return userInformation && userInformation.length > 3 - ? `${userInformation - .slice(0, 3) - .map((user) => user.name) - .join(', ')}, and ${userInformation.length - 3} others` - : `${userInformation.map((user) => user.name).join(', ')}` - }, [userInformation]) + const [usersToDisplay, setUsersToDisplay] = useState('') - if (isUserInformationError) { - return - } + useEffect(() => { + async function fetchData() { + if (!usersToDisplay) { + setUsersToDisplay( + `${users + .slice(0, 3) + .map(async (user) => { + const response = await getUserInformation(user) + if (response.ok) { + const responseBody = await response.json() + return responseBody.entity.name + } + }) + .join(', ')} ${users.length > 3 ? ` and ${users.length - 3} others` : ''}`, + ) + } + } + fetchData() + }, [users, usersToDisplay]) - if (isUserInformationLoading) { - return - } return ( - + )