Repository navigation
[Customer Portal][FE][Web] Add Comprehensive Security Feature Tests for Vulnerability API Hooks and UI Components - #762
Conversation
- Implement tests for `useGetProductVulnerabilities` to verify URL encoding and error handling for missing base URL. - Create tests for `useGetProductVulnerability` to ensure successful data fetching, handling of signed-out state, and error reporting for backend failures. - Add tests for `useGetVulnerabilitiesMetaData` to check metadata fetching and error handling for non-ok responses. - Utilize React Testing Library and Vitest for testing framework and mocking dependencies.
- Introduce tests for `usePostProductVulnerabilitiesSearch` to validate search request handling and error response parsing. - Create tests for `ProductVulnerabilitiesTableHeader` to ensure search callbacks and filter actions function correctly. - Update `SecurityStats` tests to remove unnecessary mocks and verify rendering of stat cards with accurate labels. - Utilize React Testing Library and Vitest for comprehensive testing coverage.
- Introduce tests for `SecurityPage` to validate rendering of stats, tab content, and back navigation functionality. - Implement tests for `VulnerabilityDetailsPage` to ensure loader visibility during data fetching, error handling on request failure, and navigation back to the security center. - Add utility tests for vulnerability severity and status color mappings to ensure correct theme color application. - Utilize React Testing Library and Vitest for comprehensive testing coverage.
…ities utilities - Introduce tests for `VulnerabilityDetailsContent` to validate loading state, content rendering, and back navigation functionality. - Add tests for utility functions in `productVulnerabilitiesTable` to ensure accurate counting of active filters and proper formatting of clear-filter labels. - Implement tests for security page utilities to verify parsing of query parameters and fallback mechanisms. - Utilize React Testing Library and Vitest for comprehensive testing coverage.
📝 WalkthroughWalkthroughThis PR adds comprehensive unit test coverage for the security vulnerabilities feature in the customer portal. It includes tests for four data-fetching hooks, five UI components and pages, and three utility functions, exercising request handling, state management, error conditions, and user interactions across the feature. ChangesSecurity Features Test Suite
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/customer-portal/webapp/src/features/security/utils/__tests__/securityPage.test.ts (1)
30-53: ⚡ Quick winAdd explicit nullish/empty query-param fallback cases.
Current tests validate invalid strings, but not
undefined,null(if typed loosely at call sites), or"". Adding those assertions will harden parser behavior against real URL parsing edge cases.Proposed test additions
it("parses tab query param with fallback", () => { expect(parseSecurityTabQueryParam(SecurityTabId.COMPONENTS)).toBe( SecurityTabId.COMPONENTS, ); expect(parseSecurityTabQueryParam("invalid")).toBe( SecurityTabId.VULNERABILITIES, ); + expect(parseSecurityTabQueryParam("")).toBe(SecurityTabId.VULNERABILITIES); + expect(parseSecurityTabQueryParam(undefined as unknown as string)).toBe( + SecurityTabId.VULNERABILITIES, + ); }); it("parses report view mode with fallback to all", () => { expect(parseSecurityReportViewMode(SecurityReportViewMode.MY)).toBe( SecurityReportViewMode.MY, ); expect(parseSecurityReportViewMode("unknown")).toBe(SecurityReportViewMode.ALL); + expect(parseSecurityReportViewMode("")).toBe(SecurityReportViewMode.ALL); + expect(parseSecurityReportViewMode(undefined as unknown as string)).toBe(SecurityReportViewMode.ALL); }); it("parses case sort field with fallback to createdOn", () => { expect(parseSecurityReportCaseSortField(SecurityReportCaseSortField.state)).toBe( SecurityReportCaseSortField.state, ); expect(parseSecurityReportCaseSortField("bad")).toBe( SecurityReportCaseSortField.createdOn, ); + expect(parseSecurityReportCaseSortField("")).toBe( + SecurityReportCaseSortField.createdOn, + ); + expect(parseSecurityReportCaseSortField(undefined as unknown as string)).toBe( + SecurityReportCaseSortField.createdOn, + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/customer-portal/webapp/src/features/security/utils/__tests__/securityPage.test.ts` around lines 30 - 53, Add explicit tests asserting that parseSecurityTabQueryParam, parseSecurityReportViewMode, and parseSecurityReportCaseSortField return their respective fallbacks when given undefined, null, or the empty string; specifically call parseSecurityTabQueryParam(undefined), parseSecurityTabQueryParam(null), and parseSecurityTabQueryParam(""), expecting SecurityTabId.VULNERABILITIES, call parseSecurityReportViewMode(undefined/null/"") expecting SecurityReportViewMode.ALL, and call parseSecurityReportCaseSortField(undefined/null/"") expecting SecurityReportCaseSortField.createdOn so the parsers are validated against nullish/empty query-param inputs.apps/customer-portal/webapp/src/features/security/utils/__tests__/vulnerabilities.test.ts (1)
30-34: ⚡ Quick winAvoid locking in brittle raw-label matching for status colors.
This test currently codifies strict text matching only. Consider adding normalization/resilience cases (e.g., extra spaces/case variants) or migrating to stable status IDs in the utility contract, so minor backend label drift doesn’t silently degrade UI coloring.
Based on learnings, avoid deriving UI behavior from raw backend status labels without normalization or stable IDs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/customer-portal/webapp/src/features/security/utils/__tests__/vulnerabilities.test.ts` around lines 30 - 34, The test tightly asserts raw-label matching for getVulnerabilityStatusColor; update the test and implementation to be resilient by normalizing input (trim and toLowerCase) and by covering variants: add assertions for " In Progress ", "IN PROGRESS", and mixed-case/trimmed versions mapping to "warning.main", plus a fallback/unknown label case; ensure getVulnerabilityStatusColor performs the normalization before lookup (or switches to using a stable status ID mapping) so minor backend label drift or casing/whitespace differences won’t break the color mapping.apps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsx (4)
61-65: ⚡ Quick winVerify that the error banner receives the correct error message.
The test confirms
showErroris called but doesn't verify the specific error constant. According to the source,showErrorshould be called withVULNERABILITY_DETAILS_PAGE_LOAD_ERROR. Asserting the argument ensures the correct message is displayed to users.🔍 Suggested change
First, import the constant at the top of the file:
import { VULNERABILITY_DETAILS_PAGE_LOAD_ERROR } from "`@features/security/constants/errorMessages`";Then update the assertion:
render(<VulnerabilityDetailsPage />); - expect(mockShowError).toHaveBeenCalled(); + expect(mockShowError).toHaveBeenCalledWith(VULNERABILITY_DETAILS_PAGE_LOAD_ERROR);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsx` around lines 61 - 65, The test currently only checks that mockShowError was called; import the VULNERABILITY_DETAILS_PAGE_LOAD_ERROR constant and update the assertion to verify mockShowError was called with that exact constant (keep the test setup using mockHook returning { data: undefined, isLoading: false, isError: true } and rendering <VulnerabilityDetailsPage />), i.e., replace the existance-only check on mockShowError with an expectation that mockShowError was invoked with VULNERABILITY_DETAILS_PAGE_LOAD_ERROR to ensure the correct error message is passed.
40-42: ⚡ Quick winVerify that the hook is called with the correct vulnerability ID.
The mock forwards arguments to
mockHook, but none of the tests verify thatuseGetProductVulnerabilityis called with"v-1"(thevulnerabilityIdfrom the mockeduseParams). Consider adding an assertion in one or more tests to confirm the hook receives the expected ID.🧪 Example assertion to add
In any test, after rendering:
expect(mockHook).toHaveBeenCalledWith("v-1");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsx` around lines 40 - 42, The test mock for useGetProductVulnerability forwards args to mockHook but no assertion checks it received the route vulnerabilityId; after rendering the component in the test(s) add an assertion like expect(mockHook).toHaveBeenCalledWith("v-1") to verify useGetProductVulnerability was invoked with the mocked useParams value "v-1" (reference symbols: useGetProductVulnerability, mockHook, "v-1", useParams).
55-59: ⚡ Quick winUse more specific assertions to improve test clarity.
The assertion
toHaveBeenCalled()only verifies thatshowLoaderwas invoked at least once. Consider usingtoHaveBeenCalledTimes(1)to confirm it's called exactly once, which better captures the expected behavior and helps catch regressions.♻️ Suggested change
render(<VulnerabilityDetailsPage />); - expect(mockShowLoader).toHaveBeenCalled(); + expect(mockShowLoader).toHaveBeenCalledTimes(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsx` around lines 55 - 59, Test currently uses a loose assertion for loader invocation; change the assertion in the "shows loader while vulnerability details are loading" test to assert the exact call count by replacing expect(mockShowLoader).toHaveBeenCalled() with expect(mockShowLoader).toHaveBeenCalledTimes(1) so that mockHook (returning { data: undefined, isLoading: true, isError: false }) and the VulnerabilityDetailsPage rendering are verified to call mockShowLoader exactly once.
50-75: ⚡ Quick winConsider adding tests for error suppression and successful data loading.
The current tests cover the main flows, but additional test cases would improve coverage:
Error suppression: The source code includes
hasShownErrorReflogic to prevent duplicate error banners. Consider adding a test that verifiesshowErroris called only once even if the component re-renders whileisErrorremains true.Successful load and loader hide: When data loads successfully,
showSkeletonsbecomesfalseandhideLoader()should be called. Consider adding a test that verifies this transition.Props verification: Consider asserting that
VulnerabilityDetailsContentreceives the correct props (data,isLoading,isError,onBack) for at least one test case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsx` around lines 50 - 75, Add three tests: (1) Error suppression: render VulnerabilityDetailsPage with mockHook returning { data: undefined, isLoading: false, isError: true }, assert mockShowError is called once, then re-render or update the component while isError stays true and assert mockShowError is not called again to validate hasShownErrorRef behavior; (2) Successful load hides loader: mockHook returns { data: { id: "1" }, isLoading: false, isError: false }, render and assert mockHideLoader (or mockShowLoader toggles) and that showSkeletons becomes false (via the UI or by spying mockHideLoader/mockShowLoader); (3) Props verification: spy or mock VulnerabilityDetailsContent and render with mockHook returning valid data, then assert VulnerabilityDetailsContent was called with props data, isLoading, isError and an onBack callback (verify callback triggers mockNavigate when invoked).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsx`:
- Around line 61-65: The test currently only checks that mockShowError was
called; import the VULNERABILITY_DETAILS_PAGE_LOAD_ERROR constant and update the
assertion to verify mockShowError was called with that exact constant (keep the
test setup using mockHook returning { data: undefined, isLoading: false,
isError: true } and rendering <VulnerabilityDetailsPage />), i.e., replace the
existance-only check on mockShowError with an expectation that mockShowError was
invoked with VULNERABILITY_DETAILS_PAGE_LOAD_ERROR to ensure the correct error
message is passed.
- Around line 40-42: The test mock for useGetProductVulnerability forwards args
to mockHook but no assertion checks it received the route vulnerabilityId; after
rendering the component in the test(s) add an assertion like
expect(mockHook).toHaveBeenCalledWith("v-1") to verify
useGetProductVulnerability was invoked with the mocked useParams value "v-1"
(reference symbols: useGetProductVulnerability, mockHook, "v-1", useParams).
- Around line 55-59: Test currently uses a loose assertion for loader
invocation; change the assertion in the "shows loader while vulnerability
details are loading" test to assert the exact call count by replacing
expect(mockShowLoader).toHaveBeenCalled() with
expect(mockShowLoader).toHaveBeenCalledTimes(1) so that mockHook (returning {
data: undefined, isLoading: true, isError: false }) and the
VulnerabilityDetailsPage rendering are verified to call mockShowLoader exactly
once.
- Around line 50-75: Add three tests: (1) Error suppression: render
VulnerabilityDetailsPage with mockHook returning { data: undefined, isLoading:
false, isError: true }, assert mockShowError is called once, then re-render or
update the component while isError stays true and assert mockShowError is not
called again to validate hasShownErrorRef behavior; (2) Successful load hides
loader: mockHook returns { data: { id: "1" }, isLoading: false, isError: false
}, render and assert mockHideLoader (or mockShowLoader toggles) and that
showSkeletons becomes false (via the UI or by spying
mockHideLoader/mockShowLoader); (3) Props verification: spy or mock
VulnerabilityDetailsContent and render with mockHook returning valid data, then
assert VulnerabilityDetailsContent was called with props data, isLoading,
isError and an onBack callback (verify callback triggers mockNavigate when
invoked).
In
`@apps/customer-portal/webapp/src/features/security/utils/__tests__/securityPage.test.ts`:
- Around line 30-53: Add explicit tests asserting that
parseSecurityTabQueryParam, parseSecurityReportViewMode, and
parseSecurityReportCaseSortField return their respective fallbacks when given
undefined, null, or the empty string; specifically call
parseSecurityTabQueryParam(undefined), parseSecurityTabQueryParam(null), and
parseSecurityTabQueryParam(""), expecting SecurityTabId.VULNERABILITIES, call
parseSecurityReportViewMode(undefined/null/"") expecting
SecurityReportViewMode.ALL, and call
parseSecurityReportCaseSortField(undefined/null/"") expecting
SecurityReportCaseSortField.createdOn so the parsers are validated against
nullish/empty query-param inputs.
In
`@apps/customer-portal/webapp/src/features/security/utils/__tests__/vulnerabilities.test.ts`:
- Around line 30-34: The test tightly asserts raw-label matching for
getVulnerabilityStatusColor; update the test and implementation to be resilient
by normalizing input (trim and toLowerCase) and by covering variants: add
assertions for " In Progress ", "IN PROGRESS", and mixed-case/trimmed versions
mapping to "warning.main", plus a fallback/unknown label case; ensure
getVulnerabilityStatusColor performs the normalization before lookup (or
switches to using a stable status ID mapping) so minor backend label drift or
casing/whitespace differences won’t break the color mapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e360c86b-dcff-4923-af45-a45f20f75dcc
📒 Files selected for processing (12)
apps/customer-portal/webapp/src/features/security/api/__tests__/useGetProductVulnerabilities.test.tsxapps/customer-portal/webapp/src/features/security/api/__tests__/useGetProductVulnerability.test.tsxapps/customer-portal/webapp/src/features/security/api/__tests__/useGetVulnerabilitiesMetaData.test.tsxapps/customer-portal/webapp/src/features/security/api/__tests__/usePostProductVulnerabilitiesSearch.test.tsxapps/customer-portal/webapp/src/features/security/components/__tests__/ProductVulnerabilitiesTableHeader.test.tsxapps/customer-portal/webapp/src/features/security/components/__tests__/SecurityStats.test.tsxapps/customer-portal/webapp/src/features/security/components/__tests__/VulnerabilityDetailsContent.test.tsxapps/customer-portal/webapp/src/features/security/pages/__tests__/SecurityPage.test.tsxapps/customer-portal/webapp/src/features/security/pages/__tests__/VulnerabilityDetailsPage.test.tsxapps/customer-portal/webapp/src/features/security/utils/__tests__/productVulnerabilitiesTable.test.tsapps/customer-portal/webapp/src/features/security/utils/__tests__/securityPage.test.tsapps/customer-portal/webapp/src/features/security/utils/__tests__/vulnerabilities.test.ts
This pull request adds comprehensive unit tests for the Security feature in the customer portal webapp, covering API hooks and UI components related to product vulnerabilities. The tests ensure correct API integration, error handling, and user interface behavior, improving the reliability and maintainability of the security features.
New API hook tests
useGetProductVulnerabilities,useGetProductVulnerability,useGetVulnerabilitiesMetaData, andusePostProductVulnerabilitiesSearchto verify correct API requests, error handling, and authentication logic. [1] [2] [3] [4]New component tests
ProductVulnerabilitiesTableHeaderto verify search input, clear button, and filter label behavior.VulnerabilityDetailsContentto check loading state, content rendering, and back navigation callback.Updates to existing tests
SecurityStatstests to match UI label changes (e.g., "Outstanding Security Reports") and refactored loading state handling to mock the correct API hook. [1] [2] [3]Summary by CodeRabbit