Open
Conversation
Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com>
Contributor
Reviewer's GuideAdds a new AI models listing feature, including backend OpenAPI endpoint wiring, React query hook, table controls, toolbar, routing, and sidebar navigation entry for a paginated, filterable models list view. Sequence diagram for loading the models list viewsequenceDiagram
actor User
participant SidebarApp
participant Router
participant ModelList
participant ModelSearchProvider
participant ReactQuery
participant listAllModels
participant TrustdAPI
participant ModelToolbar
participant ModelTable
User->>SidebarApp: Click Models NavLink
SidebarApp->>Router: Navigate to path Paths.models
Router->>ModelList: Render route element
ModelList->>ModelSearchProvider: Render with children
ModelSearchProvider->>ReactQuery: useFetchAllModels(params)
ReactQuery->>listAllModels: listAllModels(client, query)
listAllModels->>TrustdAPI: GET /api/v2/sbom/models?q,sort,offset,limit
TrustdAPI-->>listAllModels: 200 PaginatedResults_SbomModel
listAllModels-->>ReactQuery: AxiosResponse
ReactQuery-->>ModelSearchProvider: data items and total
ModelSearchProvider-->>ModelToolbar: Provide ModelSearchContext
ModelSearchProvider-->>ModelTable: Provide ModelSearchContext
ModelToolbar->>ModelToolbar: Render filters and top pagination
ModelTable->>ModelTable: Render table rows from context
User->>ModelToolbar: Change filter or page
ModelToolbar->>ModelSearchProvider: Update table control state
ModelSearchProvider->>ReactQuery: Refetch useFetchAllModels
ReactQuery->>listAllModels: listAllModels(client, updatedQuery)
listAllModels->>TrustdAPI: GET /api/v2/sbom/models with new params
TrustdAPI-->>listAllModels: 200 PaginatedResults_SbomModel
listAllModels-->>ReactQuery: AxiosResponse
ReactQuery-->>ModelSearchProvider: Updated data
ModelSearchProvider-->>ModelTable: Update tableControls and totalItemCount
ModelTable->>ModelTable: Re-render rows
Class diagram for new models list components and query hookclassDiagram
class ModelList {
+ReactNode children
+render()
}
class ModelSearchProvider {
+ReactNode children
+tableControlState
+ModelSearchContext Provider
+render()
}
class ModelSearchContext {
<<context>>
+tableControls ITableControls
+totalItemCount number
+isFetching boolean
+fetchError AxiosError
}
class ModelTable {
+render()
}
class ModelToolbar {
+boolean showFilters
+render()
}
class useFetchAllModels {
<<hook>>
+useFetchAllModels(params HubRequestParams, disableQuery boolean)
}
class ITableControls {
<<interface>>
+numRenderedColumns number
+currentPageItems SbomModel[]
+propHelpers
}
class SbomModel {
+string id
+string name
+object properties
}
class getModelProperties {
+getModelProperties(properties object) SbomModelProperties
}
class SbomModelProperties {
+string suppliedBy
+string licenses
}
class TablePersistenceKeyPrefixes {
+string models
}
class Paths {
+string models
}
class listAllModels {
+listAllModels(client object, query object)
}
class ReactQueryUseQuery {
<<hook>>
+useQuery(options object)
}
class SimplePagination {
+string idPrefix
+boolean isTop
+object paginationProps
}
class FilterToolbar {
+render()
}
class TableControlsHelpers {
+getHubRequestParams(state object) HubRequestParams
+useTableControlState(options object)
+useTableControlProps(options object)
+requestParamsQuery(params HubRequestParams) object
}
ModelList --> ModelSearchProvider : uses
ModelList --> ModelToolbar : uses
ModelList --> ModelTable : uses
ModelSearchProvider --> ModelSearchContext : provides
ModelToolbar --> ModelSearchContext : consumes
ModelTable --> ModelSearchContext : consumes
ModelSearchProvider --> useFetchAllModels : calls
useFetchAllModels --> ReactQueryUseQuery : uses
useFetchAllModels --> listAllModels : calls
ModelTable --> SbomModel : renders
ModelTable --> getModelProperties : calls
getModelProperties --> SbomModelProperties : returns
ModelToolbar --> FilterToolbar : uses
ModelToolbar --> SimplePagination : uses
ModelTable --> SimplePagination : uses
ModelSearchProvider --> TableControlsHelpers : uses
TablePersistenceKeyPrefixes <.. ModelSearchProvider : models key
Paths <.. SidebarApp : models route entry
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The model list page depends on
getModelPropertiesfrom the sbom details drawer, which creates a cross-page coupling; consider moving this helper into a shared utility module so the model list is not tied to the sbom details implementation. - In
ModelSearchProvider, thetableNameis singular ("model") while the persistence key prefix is plural (TablePersistenceKeyPrefixes.models); if other tables follow a naming convention it may be worth aligning this to keep persisted state keys predictable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The model list page depends on `getModelProperties` from the sbom details drawer, which creates a cross-page coupling; consider moving this helper into a shared utility module so the model list is not tied to the sbom details implementation.
- In `ModelSearchProvider`, the `tableName` is singular (`"model"`) while the persistence key prefix is plural (`TablePersistenceKeyPrefixes.models`); if other tables follow a naming convention it may be worth aligning this to keep persisted state keys predictable.
## Individual Comments
### Comment 1
<location path="client/src/app/pages/model-list/model-context.tsx" line_range="32" />
<code_context>
+ fetchError: AxiosError | null;
+}
+
+const contextDefaultValue = {} as IModelSearchContext;
+
+export const ModelSearchContext =
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid casting an empty object as the full context type
Casting `{}` as `IModelSearchContext` bypasses type safety and could cause runtime errors if the context is used outside a provider. Prefer either:
- Initializing with a concrete default value (no-op functions, empty arrays, etc.), or
- Leaving it possibly `undefined` and throwing in a custom hook when used without a provider.
This preserves type checking and causes incorrect usage to fail fast.
Suggested implementation:
```typescript
const contextDefaultValue: IModelSearchContext | undefined = undefined;
export const ModelSearchContext =
React.createContext<IModelSearchContext | undefined>(contextDefaultValue);
export const useModelSearchContext = (): IModelSearchContext => {
const context = React.useContext(ModelSearchContext);
if (!context) {
throw new Error(
"useModelSearchContext must be used within a ModelSearchProvider"
);
}
return context;
};
interface IModelProvider {
```
1. Find all usages of `React.useContext(ModelSearchContext)` (or `useContext(ModelSearchContext)`) and replace them with `useModelSearchContext()`.
2. Ensure `React` is imported in this file with a namespace import (`import * as React from "react";`) or otherwise has `React.useContext` available; if not, add/import `useContext` and adjust the hook accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| fetchError: AxiosError | null; | ||
| } | ||
|
|
||
| const contextDefaultValue = {} as IModelSearchContext; |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Avoid casting an empty object as the full context type
Casting {} as IModelSearchContext bypasses type safety and could cause runtime errors if the context is used outside a provider. Prefer either:
- Initializing with a concrete default value (no-op functions, empty arrays, etc.), or
- Leaving it possibly
undefinedand throwing in a custom hook when used without a provider.
This preserves type checking and causes incorrect usage to fail fast.
Suggested implementation:
const contextDefaultValue: IModelSearchContext | undefined = undefined;
export const ModelSearchContext =
React.createContext<IModelSearchContext | undefined>(contextDefaultValue);
export const useModelSearchContext = (): IModelSearchContext => {
const context = React.useContext(ModelSearchContext);
if (!context) {
throw new Error(
"useModelSearchContext must be used within a ModelSearchProvider"
);
}
return context;
};
interface IModelProvider {- Find all usages of
React.useContext(ModelSearchContext)(oruseContext(ModelSearchContext)) and replace them withuseModelSearchContext(). - Ensure
Reactis imported in this file with a namespace import (import * as React from "react";) or otherwise hasReact.useContextavailable; if not, add/importuseContextand adjust the hook accordingly.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #995 +/- ##
==========================================
- Coverage 66.91% 66.90% -0.01%
==========================================
Files 221 221
Lines 3887 3889 +2
Branches 903 904 +1
==========================================
+ Hits 2601 2602 +1
Misses 945 945
- Partials 341 342 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: https://redhat.atlassian.net/browse/TC-4111
Counter part of guacsec/trustify#2325 (review)
Summary by Sourcery
Add a new Models listing view backed by a models API endpoint and integrate it into the application navigation and routing.
New Features: