Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added a reviewing filter to the marketplace #1922

Merged
merged 6 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions backend/src/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { patchModel } from './routes/v2/model/patchModel.js'
import { postModel } from './routes/v2/model/postModel.js'
import { postRequestExportToS3 } from './routes/v2/model/postRequestExport.js'
import { postRequestImportFromS3 } from './routes/v2/model/postRequestImport.js'
import { getAllModelReviewRoles } from './routes/v2/model/roles/getAllModelReviewRoles.js'
import { getModelCurrentUserRoles } from './routes/v2/model/roles/getModelCurrentUserRoles.js'
import { getModelRoles } from './routes/v2/model/roles/getModelRoles.js'
import { deleteWebhook } from './routes/v2/model/webhook/deleteWebhook.js'
Expand Down Expand Up @@ -178,6 +179,8 @@ server.get('/api/v2/model/:modelId/roles', ...getModelRoles)
server.get('/api/v2/model/:modelId/roles/mine', ...getModelCurrentUserRoles)
server.get('/api/v2/model/:modelId/permissions/mine', ...getModelCurrentUserPermissions)

server.get('/api/v2/roles/review', ...getAllModelReviewRoles)

server.get('/api/v2/entities', ...getEntities)
server.get('/api/v2/entities/me', ...getCurrentUser)
server.get('/api/v2/entity/:dn/lookup', ...getEntityLookup)
Expand Down
3 changes: 1 addition & 2 deletions backend/src/routes/v2/model/getModelsSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import audit from '../../../connectors/audit/index.js'
import { EntryKind, EntryKindKeys } from '../../../models/Model.js'
import { searchModels } from '../../../services/model.js'
import { registerPath } from '../../../services/specification.js'
import { GetModelFilters } from '../../../types/enums.js'
import { coerceArray, parse, strictCoerceBoolean } from '../../../utils/validate.js'

export const getModelsSearchSchema = z.object({
Expand All @@ -16,7 +15,7 @@ export const getModelsSearchSchema = z.object({
kind: z.string(z.nativeEnum(EntryKind)).optional(),
task: z.string().optional(),
libraries: coerceArray(z.array(z.string()).optional().default([])),
filters: coerceArray(z.array(z.nativeEnum(GetModelFilters)).optional().default([])),
filters: coerceArray(z.array(z.string()).optional().default([])),
search: z.string().optional().default(''),
allowTemplating: strictCoerceBoolean(z.boolean().optional()),
schemaId: z.string().optional(),
Expand Down
32 changes: 32 additions & 0 deletions backend/src/routes/v2/model/roles/getAllModelReviewRoles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import bodyParser from 'body-parser'
import { Request, Response } from 'express'

import { Role, RoleKind } from '../../../../types/types.js'

interface GetModelCurrentUserRolesResponse {
roles: Array<Role>
}

export const getAllModelReviewRoles = [
bodyParser.json(),
async (_req: Request, res: Response<GetModelCurrentUserRolesResponse>) => {
return res.json({
roles: [
{
id: 'msro',
name: 'Model Senior Responsible Officer',
short: 'MSRO',
kind: RoleKind.SCHEMA,
description: 'This role is specified by the schema in accordance with its policy.',
},
{
id: 'mtr',
name: 'Model Technical Reviewer',
short: 'MTR',
kind: RoleKind.SCHEMA,
description: 'This role is specified by the schema in accordance with its policy.',
},
],
})
},
]
33 changes: 15 additions & 18 deletions backend/src/services/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import ModelModel, { CollaboratorEntry, EntryKindKeys } from '../models/Model.js
import Model, { ModelInterface } from '../models/Model.js'
import ModelCardRevisionModel, { ModelCardRevisionDoc } from '../models/ModelCardRevision.js'
import { UserInterface } from '../models/User.js'
import { GetModelCardVersionOptions, GetModelCardVersionOptionsKeys, GetModelFiltersKeys } from '../types/enums.js'
import { GetModelCardVersionOptions, GetModelCardVersionOptionsKeys } from '../types/enums.js'
import { EntityKind, EntryUserPermissions } from '../types/types.js'
import { isValidatorResultError } from '../types/ValidatorResultError.js'
import { fromEntity, toEntity } from '../utils/entity.js'
Expand Down Expand Up @@ -109,7 +109,7 @@ export async function searchModels(
user: UserInterface,
kind: EntryKindKeys,
libraries: Array<string>,
filters: Array<GetModelFiltersKeys>,
filters: Array<string>,
search: string,
task?: string,
allowTemplating?: boolean,
Expand Down Expand Up @@ -145,22 +145,19 @@ export async function searchModels(
query['settings.allowTemplating'] = true
}

for (const filter of filters) {
// This switch statement is here to ensure we always handle all filters in the 'GetModelFilterKeys'
// enum. Eslint will throw an error if we are not exhaustively matching all the enum options,
// which makes it far harder to forget.
// The 'Unexpected filter' should never be reached, as we have guaranteed type consistency provided
// by TypeScript.
switch (filter) {
case 'mine':
query.collaborators = {
$elemMatch: {
entity: { $in: await authentication.getEntities(user) },
},
}
break
default:
throw BadReq('Unexpected filter', { filter })
if (filters.length > 0) {
if (filters.includes('mine')) {
query.collaborators = {
$elemMatch: {
entity: { $in: await authentication.getEntities(user) },
},
}
} else {
query.collaborators = {
$elemMatch: {
...(filters.length > 0 && { roles: { $elemMatch: { $in: filters } } }),
},
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions backend/src/services/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const requiredRoles = {
accessRequest: ['msro'],
}

export const allReviewRoles = [...new Set(requiredRoles.release.concat(requiredRoles.accessRequest))]

export async function findReviews(
user: UserInterface,
mine: boolean,
Expand Down
6 changes: 0 additions & 6 deletions backend/src/types/enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ export const SchemaKind = {
} as const
export type SchemaKindKeys = (typeof SchemaKind)[keyof typeof SchemaKind]

export const GetModelFilters = {
Mine: 'mine',
} as const

export type GetModelFiltersKeys = (typeof GetModelFilters)[keyof typeof GetModelFilters]

export const ReviewKind = {
Release: 'release',
Access: 'access',
Expand Down
7 changes: 0 additions & 7 deletions backend/test/services/model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,6 @@ describe('services > model', () => {
await searchModels(user, 'model', [], [], '', 'task')
})

test('searchModels > bad filter', async () => {
const user: any = { dn: 'test' }
modelMocks.sort.mockResolvedValueOnce([])

expect(() => searchModels(user, 'model', [], ['asdf' as any], '')).rejects.toThrowError()
})

test('getModelCardRevision > should throw NotFound if modelCard does not exist', async () => {
const mockUser = { dn: 'testUser' } as any
const mockModelId = '123'
Expand Down
16 changes: 16 additions & 0 deletions frontend/actions/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,19 @@ export async function postModelExportToS3(id: string, modelExport: ModelExportRe
body: JSON.stringify(modelExport),
})
}

export function useGetAllModelReviewRoles() {
const { data, isLoading, error, mutate } = useSWR<
{
roles: Role[]
},
ErrorInfo
>('/api/v2/roles/review', fetcher)

return {
mutateModelRoles: mutate,
modelRoles: data ? data.roles : emptyRolesList,
isModelRolesLoading: isLoading,
isModelRolesError: error,
}
}
76 changes: 51 additions & 25 deletions frontend/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ import {
Tabs,
} from '@mui/material/'
import { useTheme } from '@mui/material/styles'
import { useListModels } from 'actions/model'
import { useGetAllModelReviewRoles, useListModels } from 'actions/model'
import Link from 'next/link'
import { useRouter } from 'next/router'
import React, { ChangeEvent, Fragment, useCallback, useEffect, useState } from 'react'
import React, { ChangeEvent, useCallback, useEffect, useState } from 'react'
import ChipSelector from 'src/common/ChipSelector'
import Loading from 'src/common/Loading'
import Title from 'src/common/Title'
import ErrorWrapper from 'src/errors/ErrorWrapper'
import useDebounce from 'src/hooks/useDebounce'
import EntryList from 'src/marketplace/EntryList'
import { EntryKind, EntryKindKeys } from 'types/types'
Expand All @@ -30,20 +31,21 @@ interface KeyAndLabel {
label: string
}

const searchFilterTypeLabels: KeyAndLabel[] = [{ key: 'mine', label: 'Mine' }]
const defaultRoleOptions: KeyAndLabel[] = [{ key: 'mine', label: 'Any role' }]

export default function Marketplace() {
// TODO - fetch model tags from API
const [filter, setFilter] = useState('')
const [selectedLibraries, setSelectedLibraries] = useState<string[]>([])
const [selectedTask, setSelectedTask] = useState('')
const [selectedTypes, setSelectedTypes] = useState<string[]>([])
const [selectedRoles, setSelectedRoles] = useState<string[]>([])
const [roleOptions, setRoleOptions] = useState<KeyAndLabel[]>(defaultRoleOptions)
const [selectedTab, setSelectedTab] = useState<EntryKindKeys>(EntryKind.MODEL)
const debouncedFilter = useDebounce(filter, 250)

const { models, isModelsError, isModelsLoading } = useListModels(
EntryKind.MODEL,
selectedTypes,
selectedRoles,
selectedTask,
selectedLibraries,
debouncedFilter,
Expand All @@ -53,7 +55,9 @@ export default function Marketplace() {
models: dataCards,
isModelsError: isDataCardsError,
isModelsLoading: isDataCardsLoading,
} = useListModels(EntryKind.DATA_CARD, selectedTypes, selectedTask, selectedLibraries, debouncedFilter)
} = useListModels(EntryKind.DATA_CARD, selectedRoles, selectedTask, selectedLibraries, debouncedFilter)

const { modelRoles, isModelRolesLoading, isModelRolesError } = useGetAllModelReviewRoles()

const theme = useTheme()
const router = useRouter()
Expand All @@ -74,20 +78,23 @@ export default function Marketplace() {
}
}, [filterFromQuery, taskFromQuery, librariesFromQuery])

const handleSelectedTypesOnChange = useCallback((selected: string[]) => {
if (selected.length > 0) {
const types: string[] = []
selected.forEach((value) => {
const typeToAdd = searchFilterTypeLabels.find((typeLabel) => typeLabel.label === value)
if (typeToAdd && typeToAdd.label) {
types.push(typeToAdd.key)
}
})
setSelectedTypes(types)
} else {
setSelectedTypes([])
}
}, [])
const handleSelectedRolesOnChange = useCallback(
(selectedFilters: string[]) => {
if (selectedFilters.length > 0) {
const filters: string[] = []
selectedFilters.forEach((selectedFilter) => {
const roleFilter = roleOptions.find((roleOption) => roleOption.label === selectedFilter)
if (roleFilter) {
filters.push(roleFilter.key)
}
})
setSelectedRoles(filters)
} else {
setSelectedRoles([])
}
},
[roleOptions],
)

const updateQueryParams = useCallback(
(key: string, value: string | string[]) => {
Expand Down Expand Up @@ -133,6 +140,25 @@ export default function Marketplace() {
router.replace('/', undefined, { shallow: true })
}

useEffect(() => {
if (modelRoles) {
setRoleOptions([
...defaultRoleOptions,
...modelRoles.map((role) => {
return { key: role.id, label: `${role.short}` }
}),
])
}
}, [modelRoles])

if (isModelRolesError) {
return <ErrorWrapper message={isModelRolesError.info.message} />
}

if (isModelRolesLoading) {
return <Loading />
}

return (
<>
<Title text='Marketplace' />
Expand Down Expand Up @@ -204,12 +230,12 @@ export default function Marketplace() {
</Box>
<Box>
<ChipSelector
label='Other'
label='My Roles'
multiple
options={[...searchFilterTypeLabels.map((type) => type.label)]}
onChange={handleSelectedTypesOnChange}
selectedChips={searchFilterTypeLabels
.filter((label) => selectedTypes.includes(label.key))
options={roleOptions.map((role) => role.label)}
onChange={handleSelectedRolesOnChange}
selectedChips={roleOptions
.filter((label) => selectedRoles.includes(label.key))
.map((type) => type.label)}
size='small'
/>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/common/ChipSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default function ChipSelector({

return (
<>
{label && <Typography component='h2' variant='h6'>{`${label}`}</Typography>}
{label && <Typography component='h2' variant='h6' marginBottom={1}>{`${label}`}</Typography>}
{!expanded && allOptions.slice(0, expandThreshold)}
{expanded && allOptions}
{options.length > expandThreshold && (
Expand Down
Loading