feat: [AH-2849]: support evaluation details tab in version details page#83
feat: [AH-2849]: support evaluation details tab in version details page#83abhinavcode wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds an "Evaluation Details" tab and ScanDetailsPage to multiple version-detail pages, introduces mock OSS overview and vulnerabilities components, gates tab visibility by feature flag and registry type, updates localization and string types, and bumps one dependency version. Changes
Sequence DiagramsequenceDiagram
actor User
participant ScanDetailsPage
participant VersionProviderContext
participant useParentComponents
participant OssOverviewView
participant OssVulnerabilitiesView
User->>ScanDetailsPage: Select "Evaluation Details" tab
ScanDetailsPage->>VersionProviderContext: request purl
VersionProviderContext-->>ScanDetailsPage: return purl
ScanDetailsPage->>useParentComponents: request OSS view components
useParentComponents-->>ScanDetailsPage: return OssOverviewView & OssVulnerabilitiesView
ScanDetailsPage->>OssOverviewView: render with purl + onViewVulnerabilities callback
User->>ScanDetailsPage: click "Vulnerabilities" tab
ScanDetailsPage->>OssVulnerabilitiesView: render with purl
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
web/src/ar/__mocks__/components/OssOverviewView.tsx (1)
19-22: Consider exportingOssSbomOverviewViewProps.
ScanDetailsPage(and any future consumer) currently cannot directly import this type and must fall back toComponentProps<typeof OssOverviewView>to infer it. Exporting the interface makes the contract explicit and discoverable.♻️ Suggested change
-interface OssSbomOverviewViewProps { +export interface OssSbomOverviewViewProps { purl: string onViewVulnerabilities: () => void }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/__mocks__/components/OssOverviewView.tsx` around lines 19 - 22, Export the interface OssSbomOverviewViewProps so consumers can import the prop type directly; update the declaration from "interface OssSbomOverviewViewProps { ... }" to "export interface OssSbomOverviewViewProps { purl: string; onViewVulnerabilities: () => void }" and ensure any components or files that currently use ComponentProps<typeof OssOverviewView> (e.g., ScanDetailsPage) can switch to importing OssSbomOverviewViewProps for a clearer, explicit contract.web/src/ar/__mocks__/components/OssVulnerabilitiesView.tsx (1)
19-21: Consider exportingOssSbomVulnerabilitiesViewPropsfor the same reasons asOssSbomOverviewViewPropsinOssOverviewView.tsx.♻️ Suggested change
-interface OssSbomVulnerabilitiesViewProps { +export interface OssSbomVulnerabilitiesViewProps { purl: string }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/__mocks__/components/OssVulnerabilitiesView.tsx` around lines 19 - 21, The interface OssSbomVulnerabilitiesViewProps is currently declared but not exported; update its declaration to export the interface (export interface OssSbomVulnerabilitiesViewProps { purl: string }) so it can be imported like OssSbomOverviewViewProps in other modules and tests; locate the declaration of OssSbomVulnerabilitiesViewProps in the mock component file and change it to an exported interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/src/ar/pages/version-details/components/ScanDetailsPage/ScanDetailsPage.tsx`:
- Around line 38-39: Tabs in ScanDetailsPage.tsx both use the same placeholder
icon ("document"); update the tab icon props to use distinct, meaningful icons:
change the Overview tab's icon prop from "document" to "eye-open" and change the
Vulnerabilities tab's icon prop from "document" to "warning-sign" (ensure
iconProps remains unchanged); make the same replacements for the repeated tab
definition later in the file where icon="document" appears.
- Around line 29-31: The child MFE views (OssOverviewView,
OssVulnerabilitiesView) are being rendered/passed a purl when
VersionProviderContext.data may be missing or purl is an empty string; update
ScanDetailsPage to guard rendering by checking the resolved purl (from data via
VersionProviderContext) and only render or pass props to OssOverviewView and
OssVulnerabilitiesView when purl is a non-empty string (e.g., conditional render
or early return when !purl), ensuring you do not pass the empty default '' into
those components and preventing premature API calls inside the injected MFEs.
In
`@web/src/ar/pages/version-details/components/VersionDetailsTabs/constants.tsx`:
- Around line 70-78: The EVALUATION_DETAILS tab entry in constants.tsx sets
isSupportedInPublicView: true which differs from other Enterprise tabs; update
the VersionDetailsTab.EVALUATION_DETAILS config to set isSupportedInPublicView:
false (or confirm deliberate exception) so it matches the Enterprise pattern
used by SUPPLY_CHAIN, SECURITY_TESTS, and DEPLOYMENTS; modify the object with
label 'versionDetails.tabs.evaluationDetails' and featureFlag
FeatureFlags.HAR_DEPENDENCY_FIREWALL accordingly in the VersionDetailsTabs
constants.
In
`@web/src/ar/pages/version-details/components/VersionDetailsTabs/VersionDetailsTabs.tsx`:
- Line 73: Provider value for AppStoreContext is recreated every render causing
consumers like useFeatureFlags() to see changes; update the provider in App.tsx
(where AppStoreContext.Provider value is currently created as a new object
literal including appStoreData and matchPath) to compute the value with
React.useMemo so the object identity remains stable unless its real dependencies
change (e.g., appStoreData, matchPath, specific callbacks). Ensure you pass
through the existing appStoreData.featureFlags reference unchanged (or memoize
it) so VersionDetailsTabs.tsx's useFeatureFlags() and its .filter(each =>
(each.featureFlag ? featureFlags[each.featureFlag] : true)) logic only re-runs
when featureFlags actually change. Also memoize any functions included in the
provider value to avoid recreating them each render.
---
Nitpick comments:
In `@web/src/ar/__mocks__/components/OssOverviewView.tsx`:
- Around line 19-22: Export the interface OssSbomOverviewViewProps so consumers
can import the prop type directly; update the declaration from "interface
OssSbomOverviewViewProps { ... }" to "export interface OssSbomOverviewViewProps
{ purl: string; onViewVulnerabilities: () => void }" and ensure any components
or files that currently use ComponentProps<typeof OssOverviewView> (e.g.,
ScanDetailsPage) can switch to importing OssSbomOverviewViewProps for a clearer,
explicit contract.
In `@web/src/ar/__mocks__/components/OssVulnerabilitiesView.tsx`:
- Around line 19-21: The interface OssSbomVulnerabilitiesViewProps is currently
declared but not exported; update its declaration to export the interface
(export interface OssSbomVulnerabilitiesViewProps { purl: string }) so it can be
imported like OssSbomOverviewViewProps in other modules and tests; locate the
declaration of OssSbomVulnerabilitiesViewProps in the mock component file and
change it to an exported interface.
| const { data } = useContext(VersionProviderContext) | ||
| const { purl = '' } = data || {} | ||
| const { OssOverviewView, OssVulnerabilitiesView } = useParentComponents() |
There was a problem hiding this comment.
Guard against rendering child views when purl is empty/undefined.
data || {} + purl = '' means that when context data hasn't populated yet (or has no purl), both OssOverviewView and OssVulnerabilitiesView receive an empty string. This may trigger API calls with an invalid key inside the injected MFE components before the data is ready.
🛡️ Suggested guard
const { data } = useContext(VersionProviderContext)
- const { purl = '' } = data || {}
+ const { purl } = data || {}
const { OssOverviewView, OssVulnerabilitiesView } = useParentComponents()
const { getString } = useStrings()
+ if (!purl) return null // or a spinner/loading state
return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/src/ar/pages/version-details/components/ScanDetailsPage/ScanDetailsPage.tsx`
around lines 29 - 31, The child MFE views (OssOverviewView,
OssVulnerabilitiesView) are being rendered/passed a purl when
VersionProviderContext.data may be missing or purl is an empty string; update
ScanDetailsPage to guard rendering by checking the resolved purl (from data via
VersionProviderContext) and only render or pass props to OssOverviewView and
OssVulnerabilitiesView when purl is a non-empty string (e.g., conditional render
or early return when !purl), ensuring you do not pass the empty default '' into
those components and preventing premature API calls inside the injected MFEs.
| icon="document" | ||
| iconProps={{ size: 12 }} |
There was a problem hiding this comment.
Both tabs use the same "document" icon — likely a placeholder.
Overview and Vulnerabilities tabs share icon="document". Distinct icons (e.g., "eye-open" for Overview, "warning-sign" for Vulnerabilities) would better communicate tab intent.
Also applies to: 56-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/src/ar/pages/version-details/components/ScanDetailsPage/ScanDetailsPage.tsx`
around lines 38 - 39, Tabs in ScanDetailsPage.tsx both use the same placeholder
icon ("document"); update the tab icon props to use distinct, meaningful icons:
change the Overview tab's icon prop from "document" to "eye-open" and change the
Vulnerabilities tab's icon prop from "document" to "warning-sign" (ensure
iconProps remains unchanged); make the same replacements for the repeated tab
definition later in the file where icon="document" appears.
| { | ||
| label: 'versionDetails.tabs.evaluationDetails', | ||
| value: VersionDetailsTab.EVALUATION_DETAILS, | ||
| parent: Parent.Enterprise, | ||
| isSupportedInPublicView: true, | ||
| supportActions: false, | ||
| featureFlag: FeatureFlags.HAR_DEPENDENCY_FIREWALL, | ||
| registryType: RepositoryConfigType.UPSTREAM | ||
| }, |
There was a problem hiding this comment.
Confirm isSupportedInPublicView: true is intentional for the EVALUATION_DETAILS tab.
Every other Enterprise-gated tab (SUPPLY_CHAIN, SECURITY_TESTS, DEPLOYMENTS) sets isSupportedInPublicView: false. With this set to true, the tab will surface to unauthenticated users browsing a public Enterprise upstream registry (provided HAR_DEPENDENCY_FIREWALL is enabled). If policy-evaluation data is sensitive, this should be false, matching the pattern of its peers.
🔍 Comparison with peer Enterprise tabs
| Tab | parent |
isSupportedInPublicView |
|---|---|---|
SUPPLY_CHAIN |
Enterprise | false |
SECURITY_TESTS |
Enterprise | false |
DEPLOYMENTS |
Enterprise | false |
EVALUATION_DETAILS |
Enterprise | true ← differs |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/ar/pages/version-details/components/VersionDetailsTabs/constants.tsx`
around lines 70 - 78, The EVALUATION_DETAILS tab entry in constants.tsx sets
isSupportedInPublicView: true which differs from other Enterprise tabs; update
the VersionDetailsTab.EVALUATION_DETAILS config to set isSupportedInPublicView:
false (or confirm deliberate exception) so it matches the Enterprise pattern
used by SUPPLY_CHAIN, SECURITY_TESTS, and DEPLOYMENTS; modify the object with
label 'versionDetails.tabs.evaluationDetails' and featureFlag
FeatureFlags.HAR_DEPENDENCY_FIREWALL accordingly in the VersionDetailsTabs
constants.
| .filter(each => (isCurrentSessionPublic ? each.isSupportedInPublicView : true)) | ||
| .filter(each => versionType.getAllowedVersionDetailsTab().includes(each.value)) | ||
| }, [data, isCurrentSessionPublic, parent]) | ||
| .filter(each => (each.featureFlag ? featureFlags[each.featureFlag] : true)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for useFeatureFlags implementation
rg -n "useFeatureFlags" --type=ts --type=tsx -B 2 -A 15 | head -100Repository: abhinavcode/harness
Length of output: 90
🏁 Script executed:
# Also find where this hook is defined
fd -e ts -e tsx | xargs grep -l "export.*useFeatureFlags\|function useFeatureFlags\|const useFeatureFlags"Repository: abhinavcode/harness
Length of output: 155
🏁 Script executed:
# Get the full context of the VersionDetailsTabs file around line 73
cat -n web/src/ar/pages/version-details/components/VersionDetailsTabs/VersionDetailsTabs.tsx | sed -n '55,90p'Repository: abhinavcode/harness
Length of output: 2166
🏁 Script executed:
# Check the useFeatureFlags implementation in the ar hooks
cat -n web/src/ar/hooks/useFeatureFlag.tsRepository: abhinavcode/harness
Length of output: 1270
🏁 Script executed:
# Also check the main hooks version
cat -n web/src/hooks/useFeatureFlag.tsRepository: abhinavcode/harness
Length of output: 877
🏁 Script executed:
# Check the exports from ar/hooks/index.ts to see which one is used
cat -n web/src/ar/hooks/index.tsRepository: abhinavcode/harness
Length of output: 2172
🏁 Script executed:
# Find and examine the AppStoreContext implementation
fd -e ts -e tsx | xargs grep -l "AppStoreContext" | head -5Repository: abhinavcode/harness
Length of output: 312
🏁 Script executed:
# Check the AppStoreContext implementation specifically
find . -name "AppStoreContext.ts" -o -name "AppStoreContext.tsx" | xargs cat -nRepository: abhinavcode/harness
Length of output: 2090
🏁 Script executed:
# Search for where AppStoreContext is created/provided
rg -n "AppStoreContext" --type=ts --type=tsx -B 3 -A 3 | grep -A 10 "createContext\|Provider" | head -50Repository: abhinavcode/harness
Length of output: 90
🏁 Script executed:
# Search for where AppStoreContext.Provider is used
rg -n "AppStoreContext.Provider\|<AppStoreContext" --type=ts --type=tsx -B 5 -A 10 | head -80Repository: abhinavcode/harness
Length of output: 90
🏁 Script executed:
# Find files that provide/update the AppStoreContext
fd -e ts -e tsx | xargs grep -l "AppStoreContext.*Provider\|useAppStore.*update" | head -10Repository: abhinavcode/harness
Length of output: 130
🏁 Script executed:
# Search for updateAppStore to understand how featureFlags are updated
rg -n "updateAppStore\|featureFlags.*=" --type=ts --type=tsx -B 2 -A 2 | head -100Repository: abhinavcode/harness
Length of output: 90
🏁 Script executed:
# Search with proper type specification
rg "AppStoreContext.*Provider" -A 15 -B 5 | head -100Repository: abhinavcode/harness
Length of output: 4147
🏁 Script executed:
# Look at the App.tsx file directly to see how context is set up
cat -n web/src/ar/app/App.tsx | head -150Repository: abhinavcode/harness
Length of output: 6935
🏁 Script executed:
# Search for featureFlags state/update patterns
rg "featureFlags" -B 3 -A 3 | head -120Repository: abhinavcode/harness
Length of output: 9205
The AppStoreContext.Provider value is reconstructed as a new object every render, which causes unnecessary re-renders of consuming components even if featureFlags itself is stable. In web/src/ar/app/App.tsx (line 95), the provider value is created as a new object literal { ...appStoreData, matchPath, ... } on every render rather than being memoized. This means useFeatureFlags() will be seen as "changed" by React.useContext on every render, triggering the useMemo dependency in VersionDetailsTabs to recompute unnecessarily. While the actual featureFlags object from appStoreData may be stable, wrapping it in a new provider value object defeats the optimization. Consider memoizing the provider value to prevent this.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/src/ar/pages/version-details/components/VersionDetailsTabs/VersionDetailsTabs.tsx`
at line 73, Provider value for AppStoreContext is recreated every render causing
consumers like useFeatureFlags() to see changes; update the provider in App.tsx
(where AppStoreContext.Provider value is currently created as a new object
literal including appStoreData and matchPath) to compute the value with
React.useMemo so the object identity remains stable unless its real dependencies
change (e.g., appStoreData, matchPath, specific callbacks). Ensure you pass
through the existing appStoreData.featureFlags reference unchanged (or memoize
it) so VersionDetailsTabs.tsx's useFeatureFlags() and its .filter(each =>
(each.featureFlag ? featureFlags[each.featureFlag] : true)) logic only re-runs
when featureFlags actually change. Also memoize any functions included in the
provider value to avoid recreating them each render.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/src/ar/pages/version-details/MavenVersion/__tests__/__mockData__.ts (1)
52-52: Consider using a spec-conformant PURL string as the mock value.
'test'is not a valid Package URL (PURL); a more realistic value (e.g.,'pkg:maven/com.example:my-maven-project@1.0.0') would better reflect production data and catch any downstream format-sensitive logic in tests.♻️ Proposed change
- purl: 'test' + purl: 'pkg:maven/com.example:my-maven-project@1.0.0'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/version-details/MavenVersion/__tests__/__mockData__.ts` at line 52, The mock object's purl field currently uses a non-conformant value ('test'); update the purl property in web/src/ar/pages/version-details/MavenVersion/__tests__/__mockData__.ts to a spec-compliant Package URL (PURL) string (e.g., replace purl: 'test' with something like 'pkg:maven/com.example/my-maven-project@1.0.0') so tests exercise real PURL parsing/format-sensitive logic.web/src/ar/pages/version-details/DockerVersion/__tests__/__mockData__.ts (1)
91-91:purl: 'test'is a non-spec-compliant placeholder — consider a realistic valuePURL has a well-defined format (e.g.,
pkg:docker/library/maven-app@1.0.0). Using'test'is fine for tests that only check propagation/presence, but if any downstream test or component validates the PURL format, this value will silently produce wrong results. A spec-compliant stub like'pkg:docker/maven-app@1.0.0'would be more representative.♻️ Proposed improvement
- purl: 'test' + purl: 'pkg:docker/maven-app@1.0.0'Apply the same change to both
mockDockerVersionSummary(line 91) andmockDockerVersionSummaryWithoutSscaAndStoData(line 103).Also applies to: 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/version-details/DockerVersion/__tests__/__mockData__.ts` at line 91, Replace the non-spec PURL string used in the test mocks with a realistic, spec-compliant package URL: update the purl field in both mockDockerVersionSummary and mockDockerVersionSummaryWithoutSscaAndStoData from 'test' to a valid PURL such as 'pkg:docker/library/maven-app@1.0.0' so downstream validators and format-dependent tests receive representative data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/ar/pages/version-details/DockerVersion/__tests__/__mockData__.ts`:
- Line 91: Replace the non-spec PURL string used in the test mocks with a
realistic, spec-compliant package URL: update the purl field in both
mockDockerVersionSummary and mockDockerVersionSummaryWithoutSscaAndStoData from
'test' to a valid PURL such as 'pkg:docker/library/maven-app@1.0.0' so
downstream validators and format-dependent tests receive representative data.
In `@web/src/ar/pages/version-details/MavenVersion/__tests__/__mockData__.ts`:
- Line 52: The mock object's purl field currently uses a non-conformant value
('test'); update the purl property in
web/src/ar/pages/version-details/MavenVersion/__tests__/__mockData__.ts to a
spec-compliant Package URL (PURL) string (e.g., replace purl: 'test' with
something like 'pkg:maven/com.example/my-maven-project@1.0.0') so tests exercise
real PURL parsing/format-sensitive logic.
2 participants
Summary by CodeRabbit
New Features
Dependencies