Repository navigation
Add security vulnerabilities table - #199
Conversation
Introduce ProductVulnerabilitiesTable and ProductVulnerabilitiesList components under components/security. ProductVulnerabilitiesTable is a container that handles data fetching (usePostProductVulnerabilitiesSearch, useGetVulnerabilitiesMetaData), debounced search, severity filters, pagination, and maps API responses to a paginated shape. ProductVulnerabilitiesList renders the table UI with loading skeleton, error and empty states, severity/type chips (with color helper), clickable rows, and TablePagination. These components enable searchable, filterable, and paginated display of product vulnerabilities and surface hooks for total-record and error handling.
Introduce two new components for the Product Vulnerabilities UI: ProductVulnerabilitiesTableHeader and ProductVulnerabilitiesTableSkeleton. The header provides title/description, a search field with optional filter icon, and integrates ActiveFilters for managing applied filters. The skeleton component renders configurable placeholder rows for table loading states. Both use @wso2/oxygen-ui primitives and expose props for search, filter callbacks, and rows-per-page.
Introduce a new SecurityPage component for the customer portal webapp. The page displays SecurityStats and a TabBar with three tabs (Product Vulnerabilities, Security Advisories, Component Analysis), renders corresponding components, and manages active tab state. It also tracks vulnerability total records and error state, and navigates to vulnerability detail routes using projectId from route params.
Introduce apps/customer-portal/webapp/src/utils/vulnerabilities.ts with two helper functions: getVulnerabilitySeverityColor and getVulnerabilityStatusColor. Both normalize input and map common severity (Critical/High/Medium/Low) and status (In Progress/Open/Resolved) labels to Oxygen UI color paths, returning a default 'text.secondary' for unknown or missing values.
Introduce a generic React hook useDebouncedValue in apps/customer-portal/webapp/src/hooks. The hook accepts a value and a delay (ms) and returns a debounced value, using a ref-managed timeout that is cleared on updates and cleanup to avoid leaks. Useful for reducing rapid re-renders or throttling API calls. File includes Apache-2.0 license header.
Introduce GenericSubCountCard.tsx: a reusable TypeScript React stat card that displays an icon, value, and label with an optional footer. Uses @wso2/oxygen-ui Box, Card, and Typography, accepts props for label, value, icon, optional color (defaults to "primary.main"), and footerContent, and is exported as the default component.
Import SecurityPage and VulnerabilityDetailsPage and add dedicated security-center subroutes under project routes: index -> SecurityPage and :vulnerabilityId -> VulnerabilityDetailsPage. Minor routing reformatting/nesting cleanup to accommodate the new pages and maintain existing protected routes and fallbacks.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR adds a Security Center feature to the customer portal by restructuring routing with nested project-scoped pages, introducing a SecurityPage with tabbed vulnerability tracking, creating reusable UI components for displaying product vulnerabilities with pagination and filtering, and adding utility functions for vulnerability severity and status color mapping. Changes
Sequence DiagramsequenceDiagram
actor User
participant SecurityPage
participant ProductVulnerabilitiesTable
participant API as Backend API
participant ProductVulnerabilitiesList
participant Router
User->>SecurityPage: Navigate to /security-center
SecurityPage->>ProductVulnerabilitiesTable: Render with callbacks
ProductVulnerabilitiesTable->>API: POST /products/vulnerabilities/search (debounced)
API-->>ProductVulnerabilitiesTable: Return vulnerabilities + total records
ProductVulnerabilitiesTable->>ProductVulnerabilitiesList: Pass fetched data
ProductVulnerabilitiesList->>User: Display paginated vulnerability table
User->>ProductVulnerabilitiesList: Click on vulnerability row
ProductVulnerabilitiesList->>SecurityPage: Trigger onVulnerabilityClick callback
SecurityPage->>Router: Navigate to /security-center/:vulnerabilityId
Router->>User: Render VulnerabilityDetailsPage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 |
|
@coderabbitai Review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a comprehensive Security Center feature to the customer portal, including a vulnerabilities table, and enhances the Updates section with a pending updates detail page. The changes enable users to view, search, and filter product vulnerabilities, as well as drill down into specific pending update levels for products.
Changes:
- Added Security Center page with vulnerability management table and tab navigation for future advisories and component analysis features
- Implemented pending updates detail page showing granular update level information with security/regular classification
- Updated product update levels model from kebab-case to camelCase field names for TypeScript convention consistency
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/vulnerabilities.ts |
New utility for mapping vulnerability severity and status to UI colors |
src/utils/updates.ts |
Added function to compute pending update levels with security/regular classification |
src/hooks/useDebouncedValue.ts |
New custom hook for debouncing search input values |
src/pages/SecurityPage.tsx |
New security center page with tabbed interface for vulnerabilities, advisories, and components |
src/pages/PendingUpdatesPage.tsx |
New detail page displaying pending updates for a specific product version |
src/components/security/* |
New vulnerability table components with search, filtering, and pagination |
src/components/updates/pending-updates/* |
New components for displaying pending update levels in table format |
src/components/common/GenericSubCountCard.tsx |
Reusable stat card component for displaying counts with icons |
src/api/usePostProductVulnerabilitiesSearch.ts |
New hook for searching vulnerabilities using POST request |
src/api/useGetVulnerabilitiesMetaData.ts |
New hook for fetching vulnerability filter metadata |
src/api/useGetProductVulnerability.ts |
New hook for fetching individual vulnerability details |
src/models/responses.ts |
Updated field names to camelCase; added vulnerability-related interfaces |
src/App.tsx |
Added security-center and pending updates routes |
src/components/updates/update-cards/* |
Updated to support navigation to pending updates page |
Comments suppressed due to low confidence (1)
apps/customer-portal/webapp/src/components/updates/update-cards/UpdateProductCard.tsx:117
- The button should be disabled when onViewPendingUpdates is undefined to prevent users from clicking a non-functional button. Currently, if projectId is not available (see UpdateProductGrid.tsx line 82-90), the button will render but won't respond to clicks, creating a poor user experience. Add the disabled prop:
disabled={!onViewPendingUpdates}
<Button
fullWidth
variant="outlined"
color="warning"
startIcon={<FileText size={16} />}
onClick={onViewPendingUpdates}
sx={{
textTransform: "none",
}}
>
View {pendingLevels} Pending Updates
</Button>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
All comments resolved in : #201 |
The merge-base changed after approval.
4c007c4 to
aa20cc6
Compare
7dc22b1
into
wso2-open-operations:customer-portal-milestone-1
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/customer-portal/webapp/src/components/dashboard/charts/OutstandingIncidentsChart.tsx (1)
151-152:⚠️ Potential issue | 🟡 MinorStale
entityNameafter title rename.
entityName="outstanding cases"(line 151) does not match the updated title "Outstanding Engagements", producing inconsistent copy in the error state.🔧 Proposed fix
- <ErrorIndicator entityName="outstanding cases" /> + <ErrorIndicator entityName="outstanding engagements" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/dashboard/charts/OutstandingIncidentsChart.tsx` around lines 151 - 152, The ErrorIndicator usage in OutstandingIncidentsChart.tsx still passes entityName="outstanding cases" which is inconsistent with the renamed chart title "Outstanding Engagements"; update the prop on the ErrorIndicator component (the ErrorIndicator component invocation in OutstandingIncidentsChart) to use a matching entityName value such as "outstanding engagements" so the error state copy aligns with the displayed title.apps/customer-portal/webapp/src/components/dashboard/charts/ActiveCasesChart.tsx (1)
146-147:⚠️ Potential issue | 🟡 MinorStale
entityNameafter title rename.
entityName="active cases"in theErrorIndicator(line 146) no longer matches the updated component title "Active Engagements". Error state messaging will be inconsistent with the card heading.🔧 Proposed fix
- <ErrorIndicator entityName="active cases" /> + <ErrorIndicator entityName="active engagements" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/dashboard/charts/ActiveCasesChart.tsx` around lines 146 - 147, Update the ErrorIndicator's entityName prop to match the renamed title: replace the stale entityName="active cases" used in the ActiveCasesChart component with the correct label (e.g., "active engagements" or "Active Engagements" to match the card heading) so error messages align with the card heading; ensure the change is applied where ErrorIndicator is rendered and that casing/spacing matches other UI text for consistency.apps/customer-portal/webapp/src/components/dashboard/charts/CasesTrendChart.tsx (1)
128-130:⚠️ Potential issue | 🟡 MinorSkeleton legend count mismatch with actual data entries.
The skeleton renders 4 placeholders (
[1, 2, 3, 4]), butCASES_TREND_CHART_DATAcontains 5 severity levels. TheChartLegendwill render 5 items when data loads, causing a layout shift.Proposed fix
- {[1, 2, 3, 4].map((i) => ( + {[1, 2, 3, 4, 5].map((i) => ( <Skeleton key={i} variant="rounded" width={60} height={20} /> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/dashboard/charts/CasesTrendChart.tsx` around lines 128 - 130, The skeleton legend currently renders 4 placeholders ([1,2,3,4]) which mismatches CASES_TREND_CHART_DATA's 5 severity items and causes a layout shift; update the placeholder count to match the real data by deriving the skeleton items from CASES_TREND_CHART_DATA.length (or use CASES_TREND_CHART_DATA.map) so the Skeleton loop in CasesTrendChart.tsx produces the same number of legend placeholders as ChartLegend when data loads.apps/customer-portal/webapp/src/constants/dashboardConstants.ts (1)
45-73:⚠️ Potential issue | 🟡 MinorTooltip text still references "cases" after labels were renamed to "Engagements".
The
labelfields were updated (e.g., "Total Engagements", "Active Engagements"), but thetooltipTextfields still say "cases" (e.g., "Total number of cases reported…", "Currently active and unresolved cases"). This creates an inconsistent user-facing experience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/constants/dashboardConstants.ts` around lines 45 - 73, The tooltipText values in DASHBOARD_STATS still refer to "cases" while the labels use "Engagements"; update the tooltipText for the entries with id "totalCases", "openCases", "resolvedCases", and "avgResponseTime" in the DASHBOARD_STATS array to consistently use "engagements" (e.g., "Total number of engagements reported for this project", "Currently active and unresolved engagements", "Successfully closed and resolved engagements", "Average time taken to first respond to an engagement") ensuring text and casing match the label terminology.
🧹 Nitpick comments (29)
apps/customer-portal/webapp/src/hooks/useDebouncedValue.ts (1)
26-43:useRefis unnecessary — use a local timeout variable instead.React guarantees the previous effect's cleanup runs before the new effect body executes, so lines 29-31 are always a no-op (the timeout was already cleared by the cleanup on line 40). Storing the ID in a ref adds complexity with no benefit here.
The canonical, idiomatic pattern:
♻️ Simplified implementation
-import { useState, useEffect, useRef } from "react"; +import { useState, useEffect } from "react"; export function useDebouncedValue<T>(value: T, delayMs: number): T { const [debouncedValue, setDebouncedValue] = useState<T>(value); - const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); useEffect(() => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - - timeoutRef.current = setTimeout(() => { + const timeout = setTimeout(() => { setDebouncedValue(value); - timeoutRef.current = null; }, delayMs); - return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - }; + return () => clearTimeout(timeout); }, [value, delayMs]); return debouncedValue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/hooks/useDebouncedValue.ts` around lines 26 - 43, The timeoutRef useRef and its checks are unnecessary because React runs the previous effect's cleanup before the next effect, so remove timeoutRef and the useRef import, then use a local variable inside the useEffect (e.g., let timeout: ReturnType<typeof setTimeout> | null) to hold the timer ID, assign it with setTimeout which calls setDebouncedValue(value) and sets the local to null, and clearTimeout(timeout) in the return cleanup; keep the same dependency array ([value, delayMs]) and reference the setDebouncedValue call and timeout handling inside the useEffect function.apps/customer-portal/webapp/src/components/common/rich-text-editor/__tests__/Editor.test.tsx (2)
30-30: Move theErrorBannerProviderimport to the top-level import block.This import is placed after the
vi.mock(...)block instead of alongside the other imports.ErrorBannerProvideris not mocked and has no ordering dependency on the mock, so there is no reason to place it here. Vitest hoistsvi.mockcalls before imports anyway, so it is purely a style inconsistency.♻️ Proposed fix
import { render, screen, waitFor } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { ThemeProvider, createTheme } from "@wso2/oxygen-ui"; import Editor from "@components/common/rich-text-editor/Editor"; +import { ErrorBannerProvider } from "@context/error-banner/ErrorBannerContext"; vi.mock("@hooks/useLogger", () => ({ useLogger: () => ({ ... }), })); - -import { ErrorBannerProvider } from "@context/error-banner/ErrorBannerContext";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/common/rich-text-editor/__tests__/Editor.test.tsx` at line 30, The import for ErrorBannerProvider should be moved out of its current position after the vi.mock(...) block and placed with the other top-level imports in Editor.test.tsx; locate the line importing ErrorBannerProvider from "@context/error-banner/ErrorBannerContext" and relocate it into the main import block above the vi.mock calls (it is not mocked and has no ordering dependency), keeping vi.mock(...) where it is since Vitest hoists mocks.
64-88: Avoid duplicating provider boilerplate — userenderEditorfor the initial render.The new test manually repeats the
ThemeProvider + ErrorBannerProvider + Editortree verbatim, even thoughrenderEditoralready encapsulates exactly that wrapper and returns the full RTL result (includingrerender). The initialrender(...)call on lines 65–71 can be replaced withrenderEditor(...). Forrerender, a small local helper avoids the repeated JSX:♻️ Proposed refactor
- it("updates content when value prop changes from empty", async () => { - const { rerender } = render( - <ThemeProvider theme={createTheme()}> - <ErrorBannerProvider> - <Editor value="" onChange={vi.fn()} /> - </ErrorBannerProvider> - </ThemeProvider>, - ); - - expect(screen.getByTestId("case-description-editor")).toBeInTheDocument(); - - rerender( - <ThemeProvider theme={createTheme()}> - <ErrorBannerProvider> - <Editor value="<p>AI Generated Content</p>" onChange={vi.fn()} /> - </ErrorBannerProvider> - </ThemeProvider>, - ); + it("updates content when value prop changes from empty", async () => { + const onChange = vi.fn(); + const { rerender } = renderEditor({ value: "", onChange }); + + expect(screen.getByTestId("case-description-editor")).toBeInTheDocument(); + + rerender( + <ThemeProvider theme={createTheme()}> + <ErrorBannerProvider> + <Editor value="<p>AI Generated Content</p>" onChange={onChange} /> + </ErrorBannerProvider> + </ThemeProvider>, + ); await waitFor(() => { expect(screen.getByTestId("case-description-editor")).toHaveTextContent( "AI Generated Content", ); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/common/rich-text-editor/__tests__/Editor.test.tsx` around lines 64 - 88, The test duplicates the ThemeProvider+ErrorBannerProvider+Editor render tree instead of using the existing helper; replace the initial render(...) call with renderEditor(...) (the helper that returns RTL utilities including rerender) and for the subsequent rerender call use a small local helper (e.g., rerenderWithValue or call the returned rerender from renderEditor) to re-render the Editor with value "<p>AI Generated Content</p>" so you avoid repeating ThemeProvider and ErrorBannerProvider JSX while keeping assertions against screen.getByTestId("case-description-editor") intact.apps/customer-portal/webapp/src/components/common/GenericSubCountCard.tsx (1)
22-22:valueprop type has redundant members — simplify toReactNode
ReactNodealready includesstring,number,null,undefined,boolean,ReactElement, etc. The explicitstring | numbermembers are subsumed byReactNodeand add no additional type safety.♻️ Proposed simplification
- value: string | number | ReactNode; + value: ReactNode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/common/GenericSubCountCard.tsx` at line 22, The prop type for value in the GenericSubCountCard component is overly broad because string and number are already included in ReactNode; update the props/interface that currently declares "value: string | number | ReactNode" (look for the GenericSubCountCardProps or the component props type) to just "value: ReactNode", ensure ReactNode is imported from 'react' if not already, and run the TypeScript typecheck to confirm no call sites need adjustment.apps/customer-portal/webapp/package.json (1)
10-11: LGTM — newtestscript is standard and the package versions are valid.
vitestat^4.0.18is the current npm latest, andzodat^4.3.6is also the current npm latest. All three version constraints are confirmed valid.One optional follow-up: consider splitting the script into two entries for clarity:
- "test": "vitest" + "test": "vitest run", + "test:watch": "vitest"
vitest(no flags) enters watch mode locally and auto-detectsCI=trueto run once in pipelines, so the single-script form works fine. The split is purely ergonomic —npm testalways does a one-shot run regardless of environment.Also,
@testing-library/user-eventis absent. If any of the new component tests simulate realistic user input (typing, clicking), consider adding it alongside the existing RTL packages.+ "@testing-library/user-event": "^14.x.x",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/package.json` around lines 10 - 11, Update package.json to add ergonomic test scripts and optionally include user-event: replace the current single "test" script that runs vitest with two scripts (e.g., "test": "vitest --run" for CI/one-shot and "test:watch": "vitest" for local watch), keeping "preview" unchanged; also add "@testing-library/user-event" to devDependencies so component tests can simulate realistic user interactions alongside the existing testing-library packages. Ensure you update the "scripts" section entries ("test" and add "test:watch") and add the "@testing-library/user-event" entry under devDependencies with an appropriate semver.apps/customer-portal/webapp/src/components/common/rich-text-editor/Editor.tsx (1)
71-86: Optional: skip theeditor.update()call when content is already non-emptyWith
initialHtmlin the dependency array, every parentvalueprop update (including onChange-driven re-renders on each keystroke) triggerseditor.update(). When the editor already has content the update is a no-op for Lexical, but the call itself incurs scheduling overhead on every keystroke. A cheapeditor.read()-based pre-check can eliminate those no-op updates:♻️ Proposed optimisation
useEffect(() => { if (initialHtml) { + let isEmpty = true; + editor.read(() => { + const root = $getRoot(); + isEmpty = root.getChildrenSize() === 0 || + root.getTextContent().trim() === ""; + }); + if (!isEmpty) return; + editor.update(() => { const root = $getRoot(); - const currentContent = root.getTextContent(); - - if (currentContent.trim() === "") { - const parser = new DOMParser(); - const dom = parser.parseFromString(initialHtml, "text/html"); - const nodes = $generateNodesFromDOM(editor, dom); - root.clear(); - root.append(...nodes); - } + const parser = new DOMParser(); + const dom = parser.parseFromString(initialHtml, "text/html"); + const nodes = $generateNodesFromDOM(editor, dom); + root.clear(); + root.append(...nodes); }); } }, [editor, initialHtml]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/common/rich-text-editor/Editor.tsx` around lines 71 - 86, The effect currently calls editor.update on every change whenever initialHtml is set, causing unnecessary scheduling; change it to first call editor.read to inspect the current root (via $getRoot().getTextContent()) and only call editor.update to parse/insert nodes (using DOMParser, $generateNodesFromDOM, root.clear(), root.append(...nodes)) when the content is empty (trim() === ""); this avoids the noop update overhead by performing a cheap read pre-check before invoking update.apps/customer-portal/webapp/src/components/dashboard/cases-table/CasesList.tsx (1)
167-176: Minor indentation inconsistency inside<TableCell>.The
<Chip>on lines 168–175 is indented with extra leading spaces relative to its parent<TableCell>. Other cells in this file align their content at one level inside the<TableCell>. This is cosmetic only.🧹 Suggested formatting fix
<TableCell> - <Chip - label={row.caseTypes?.label || "--"} - size="small" - variant="outlined" - sx={{ - fontWeight: 500, - }} - /> + <Chip + label={row.caseTypes?.label || "--"} + size="small" + variant="outlined" + sx={{ + fontWeight: 500, + }} + /> </TableCell>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/dashboard/cases-table/CasesList.tsx` around lines 167 - 176, Indentation inside the TableCell for the Chip is inconsistent with other cells; in CasesList.tsx adjust the spacing so the <Chip ... /> JSX is indented one level inside the TableCell (match the indentation style used elsewhere in this file) to remove the extra leading spaces around the Chip element and keep formatting consistent for the TableCell/Chip block.apps/customer-portal/webapp/src/components/security/ProductVulnerabilitiesTableHeader.tsx (1)
108-152: Consider adding anaria-labelto the search field for accessibility.The
TextFieldrelies solely onplaceholdertext for identification. Screen readers benefit from an explicitaria-labelor associated<label>element.♿ Proposed fix
<TextField sx={{ width: "100%", "& .MuiInputBase-root": { pr: 0.5, }, }} + aria-label="Search CVE or component" value={searchValue} onChange={handleSearchChange} placeholder="Search CVE or component"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/security/ProductVulnerabilitiesTableHeader.tsx` around lines 108 - 152, The search TextField in ProductVulnerabilitiesTableHeader lacks an explicit accessible name; update the TextField (component TextField in ProductVulnerabilitiesTableHeader) to include an aria-label (or add a visible/hidden label prop) that describes the input (e.g., "Search CVE or component") so screen readers don't rely solely on the placeholder; ensure the aria-label is added alongside existing props (value, onChange, placeholder, slotProps) and preserved for the start/end adornments and handlers like handleSearchChange and onFilterIconClick.apps/customer-portal/webapp/src/utils/caseCreation.ts (1)
17-18: Consolidate duplicate imports from the same module.Two separate import statements pull from
@models/responses. Merge them into one.🧹 Proposed fix
-import type { DeploymentProductItem } from "@models/responses"; -import type { CaseClassificationResponse } from "@models/responses"; +import type { DeploymentProductItem, CaseClassificationResponse } from "@models/responses";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/caseCreation.ts` around lines 17 - 18, Merge the two duplicate import statements from the same module by combining the type names into a single import from "@models/responses"; specifically replace the separate imports of DeploymentProductItem and CaseClassificationResponse with one consolidated import that lists both types together to remove redundancy.apps/customer-portal/webapp/src/utils/updates.ts (1)
176-189: Redundant sort and guard condition ingetPendingUpdateLevels.The
forloop already iterates levels in ascending order and pushes only those present inupdateLevelSet, sopendingLevelsis already sorted whensort()is called on line 182 — this call is always a no-op.Separately, the
securityCount > 0guard on line 189 is redundant: whensecurityCount === 0,index < 0is alwaysfalseregardless, so the condition already reduces toindex < securityCount.♻️ Proposed cleanup
- pendingLevels.sort((a, b) => a - b); - const securityCount = recommended.availableSecurityUpdatesCount; return pendingLevels.map((level, index) => ({ updateLevel: level, - updateType: - index < securityCount && securityCount > 0 ? "security" : "regular", + updateType: index < securityCount ? "security" : "regular", }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/updates.ts` around lines 176 - 189, The pendingLevels array in getPendingUpdateLevels is built in ascending order by iterating level from endingUpdateLevel + 1 to recommendedUpdateLevel and filtering with updateLevelSet, so the pendingLevels.sort(...) call is redundant and should be removed; also simplify the updateType assignment by dropping the unnecessary securityCount > 0 guard and use index < securityCount directly (securityCount is from recommended.availableSecurityUpdatesCount) when mapping pendingLevels to objects.apps/customer-portal/webapp/src/utils/__tests__/updates.test.ts (1)
143-198: Consider adding an edge case whereavailableSecurityUpdatesCountexceeds pending levels count.The current tests cover all main paths, but there's no test for when
availableSecurityUpdatesCount > pendingLevels.length(e.g., API reports 15 security updates but only 5 levels are pending). In that scenario all pending rows would be labelled"security"— worth asserting explicitly to lock in that behavior.✅ Suggested additional test
it("labels all rows as security when securityCount exceeds pending count", () => { const overRecommended = createUpdateLevelItem({ productName: "wso2am", productBaseVersion: "4.2.0", endingUpdateLevel: 11, recommendedUpdateLevel: 22, availableSecurityUpdatesCount: 20, // more than 11 pending levels }); const rows = getPendingUpdateLevels(overRecommended, productLevels); expect(rows.every((r) => r.updateType === "security")).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/__tests__/updates.test.ts` around lines 143 - 198, Add a test case to apps/customer-portal/webapp/src/utils/__tests__/updates.test.ts that constructs a RecommendedUpdateLevelItem via createUpdateLevelItem with availableSecurityUpdatesCount larger than the number of pending levels (e.g., set availableSecurityUpdatesCount to 20), call getPendingUpdateLevels(recommended, productLevels), and assert that every returned row has updateType === "security" to ensure all pending levels are labeled security when the security count exceeds pending levels.apps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsx (1)
48-49: Double-pass filter forsecurityCount/regularCountis a micro-inefficiency.Both counts could be derived in a single pass. This is negligible at current list sizes but worth noting.
♻️ Proposed cleanup
- const securityCount = pendingRows.filter((r) => r.updateType === "security").length; - const regularCount = pendingRows.filter((r) => r.updateType === "regular").length; + const securityCount = pendingRows.reduce( + (n, r) => n + (r.updateType === "security" ? 1 : 0), 0, + ); + const regularCount = pendingRows.length - securityCount;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsx` around lines 48 - 49, The code computes securityCount and regularCount by filtering pendingRows twice; change this to a single pass over pendingRows (e.g., use Array.prototype.reduce or a single for-loop in PendingUpdatesList) to accumulate both counts in one traversal, updating the variables securityCount and regularCount accordingly so the rest of the component uses those computed values.apps/customer-portal/webapp/src/components/security/ProductVulnerabilitiesList.tsx (1)
131-143:getVulnerabilitySeverityColoris called twice per row with the same input.Cache the result in a variable to avoid the double computation on every row render.
♻️ Proposed refactor
- <Chip - label={severityLabel} - size="small" - variant="outlined" - sx={{ - color: getVulnerabilitySeverityColor(severityLabel), - borderColor: getVulnerabilitySeverityColor( - severityLabel, - ), - fontWeight: 500, - }} - /> + {(() => { + const severityColor = getVulnerabilitySeverityColor(severityLabel); + return ( + <Chip + label={severityLabel} + size="small" + variant="outlined" + sx={{ + color: severityColor, + borderColor: severityColor, + fontWeight: 500, + }} + /> + ); + })()}Alternatively, compute
severityColoralongsideseverityLabelat the top of the row'sreturn:const severityLabel = row.severity?.label ?? "--"; + const severityColor = getVulnerabilitySeverityColor(severityLabel); return ( ... - color: getVulnerabilitySeverityColor(severityLabel), - borderColor: getVulnerabilitySeverityColor(severityLabel), + color: severityColor, + borderColor: severityColor,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/security/ProductVulnerabilitiesList.tsx` around lines 131 - 143, The code calls getVulnerabilitySeverityColor(severityLabel) twice when rendering the Chip; compute it once, store it in a local variable (e.g., severityColor) alongside severityLabel at the top of the row render, and then use that single variable for both color and borderColor in the Chip sx prop (referencing getVulnerabilitySeverityColor, severityLabel, severityColor, and the Chip component).apps/customer-portal/webapp/src/pages/__tests__/CreateCasePage.test.tsx (2)
130-132: SharedQueryClientcauses cross-test cache pollution — create a fresh instance per test.The
queryClientat line 130 is shared across all tests. React Query's in-memory cache persists between tests, so query results resolved in test 1 may be served as cached data in test 2, masking real failures.♻️ Proposed fix
-const queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false } }, -}); describe("CreateCasePage", () => { + let queryClient: QueryClient; + + beforeEach(() => { + queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + }); + + afterEach(() => { + queryClient.clear(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/__tests__/CreateCasePage.test.tsx` around lines 130 - 132, The shared QueryClient instance named queryClient (with defaultOptions: { queries: { retry: false } }) is reused across tests and causes React Query cache pollution; fix it by creating a fresh QueryClient for each test (e.g., instantiate a new QueryClient in a beforeEach or inside each test / render helper) and use that instance when rendering the component or wrapping with QueryClientProvider so caches don't persist between tests.
150-221: Movevi.mockcalls to module level — placing them inside the test body is misleading and fragile.Vitest hoists
vi.mockcalls to before imports via a static transform. Placing them inside anit()body afterrender()(1) makes the code appear sequential when it isn't, (2) applies these mocks globally to all tests in the file implicitly, and (3) will confuse future maintainers. These should be at module level alongside the existing top-level mocks (lines 26–126).♻️ Proposed structure
-// (remove from inside it() body) +// At module level, alongside the other vi.mock calls above: +vi.mock("../../api/useGetProjectDetails", () => ({ + default: vi.fn(() => ({ + data: { id: "123", name: "WSO2 Super App", key: "WSA" }, + isLoading: false, + error: null, + })), +})); + +vi.mock("../../api/useGetCasesFilters", () => ({ ... })); +// ... etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/__tests__/CreateCasePage.test.tsx` around lines 150 - 221, The vi.mock calls for hooks like useGetProjectDetails, useGetCasesFilters, useMockConfig, useGetProjectDeployments, useGetDeploymentsProducts, usePostCase, and usePostAttachments should be moved out of the test body to the module scope (alongside the other top-level mocks) so they run before imports/rendering; locate the vi.mock blocks inside CreateCasePage.test.tsx and relocate them to the file top-level (before any imports or before the render call) to ensure they are applied consistently and not misleadingly scoped to a single it() block.apps/customer-portal/webapp/src/components/updates/pending-updates/__tests__/PendingUpdatesList.test.tsx (1)
43-87: UsetoBeInTheDocument()instead of.toBeDefined()for DOM element assertions.
getByTexteither returns an element or throws, soexpect(...).toBeDefined()is vacuous — it never produces a meaningful failure. PrefertoBeInTheDocument()(via@testing-library/jest-dom) for clarity and intent.♻️ Example for one assertion (apply consistently)
- expect(screen.getByText("No pending updates found for this product and version.")).toBeDefined(); + expect(screen.getByText("No pending updates found for this product and version.")).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/updates/pending-updates/__tests__/PendingUpdatesList.test.tsx` around lines 43 - 87, Replace vacuous expect(...).toBeDefined() DOM assertions in the PendingUpdatesList tests with proper jest-dom assertions: use expect(screen.getByText(...)).toBeInTheDocument() for single-element checks (e.g., the empty-state message, summary line, column headers, specific row text "12","13","14"), and replace expect(screen.getAllByText(...).length).toBeGreaterThan(0) / .toBe(3) with expect(screen.getAllByText(...)).toHaveLength(n) or expect(screen.getAllByText(...).length).toBeGreaterThan(0) converted to expect(screen.getAllByText(...)).toHaveLength(expect.any(Number) > 0) style (prefer toHaveLength for exact counts and toHaveLength >0 where needed) so all assertions use toBeInTheDocument or toHaveLength instead of toBeDefined; ensure `@testing-library/jest-dom` is available in the test setup.apps/customer-portal/webapp/src/components/updates/update-cards/UpdateProductGrid.tsx (1)
79-92: Silent click whenprojectIdis absent — consider disabling the button.When
projectIdisundefined,onViewPendingUpdatesisundefined.UpdateProductCardrenders the "View Pending Updates" button unconditionally withonClick={onViewPendingUpdates}, so clicking it silently does nothing. This is currently harmless sinceUpdatesPagealways suppliesprojectId, but the component could be reused elsewhere.Consider threading a
disabledflag toUpdateProductCardwhen no handler is available, or omitting the button entirely:♻️ Suggested guard in UpdateProductCard
- onClick={onViewPendingUpdates} + onClick={onViewPendingUpdates} + disabled={!onViewPendingUpdates}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/updates/update-cards/UpdateProductGrid.tsx` around lines 79 - 92, The UpdateProductGrid passes onViewPendingUpdates as undefined when projectId is absent, causing UpdateProductCard's unconditional button to be a silent no-op; update the call site to pass a disabled flag (e.g., disabled={!projectId}) or only render onViewPendingUpdates when projectId exists, and update consumers accordingly: in UpdateProductGrid set onViewPendingUpdates to the navigate callback only if projectId is defined and also pass a disabled prop (or omit the prop) so UpdateProductCard can render the button disabled or hide it; reference UpdateProductGrid, UpdateProductCard, onViewPendingUpdates and projectId when making the change.apps/customer-portal/webapp/src/App.tsx (1)
37-38: Inconsistent import path style.These two imports use relative paths (
"./pages/...") while every other page import in this file uses the@pages/alias. Align for consistency.Proposed fix
-import SecurityPage from "./pages/SecurityPage"; -import VulnerabilityDetailsPage from "./pages/VulnerabilityDetailsPage"; +import SecurityPage from "@pages/SecurityPage"; +import VulnerabilityDetailsPage from "@pages/VulnerabilityDetailsPage";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/App.tsx` around lines 37 - 38, The two imports for SecurityPage and VulnerabilityDetailsPage use relative paths; update their import statements so they use the project alias like the other pages (i.e., import SecurityPage from "@pages/SecurityPage" and VulnerabilityDetailsPage from "@pages/VulnerabilityDetailsPage") to align with the rest of App.tsx and ensure consistent module resolution.apps/customer-portal/webapp/src/api/useGetVulnerabilitiesMetaData.ts (1)
51-55: Error message omits the HTTP status code.The sibling hook
useGetProductVulnerability.tsincludes bothresponse.statusandresponse.statusTextin its error message, which makes debugging easier. Consider aligning.Proposed fix
if (!response.ok) { throw new Error( - `Error fetching vulnerabilities metadata: ${response.statusText}`, + `Error fetching vulnerabilities metadata: ${response.status} ${response.statusText}`, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/api/useGetVulnerabilitiesMetaData.ts` around lines 51 - 55, The error thrown in useGetVulnerabilitiesMetaData omits the HTTP status code; update the error construction inside the response.ok check in useGetVulnerabilitiesMetaData to include both response.status and response.statusText (matching the sibling hook useGetProductVulnerability.ts) so the thrown Error contains the numeric status and the status text for easier debugging.apps/customer-portal/webapp/src/components/dashboard/charts/ChartLayout.tsx (1)
52-61: Stale JSDoc — parameter names and types no longer match the interface.The
@paramblock still referencesprops.outstandingIncidents(should beoutstandingCases) andCasesTrendData[](type no longer exists). Consider updating or removing the per-param docs to stay in sync withChartLayoutProps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/dashboard/charts/ChartLayout.tsx` around lines 52 - 61, Update the stale JSDoc for the ChartLayout component so it matches the current ChartLayoutProps: replace references to props.outstandingIncidents with outstandingCases, remove or update the nonexistent CasesTrendData type (use the correct type from ChartLayoutProps or omit the per-param type), and ensure the listed params and types mirror ChartLayoutProps (or remove the per-param `@param` block entirely and keep a high-level description). Target the JSDoc immediately above the ChartLayout component definition to make the names/types consistent.apps/customer-portal/webapp/src/pages/SecurityPage.tsx (2)
42-58: Statictabsarray is recreated on every render.
tabshas no dependency on props or state. Extract it to module scope to avoid unnecessary allocations.Proposed fix
+const SECURITY_TABS = [ + { + id: "vulnerabilities", + label: "Product Vulnerabilities", + icon: Siren, + }, + { + id: "advisories", + label: "Security Advisories", + icon: ShieldAlert, + }, + { + id: "components", + label: "Component Analysis", + icon: Package, + }, +]; + const SecurityPage = (): JSX.Element => { const navigate = useNavigate(); const { projectId } = useParams<{ projectId: string }>(); const [activeTab, setActiveTab] = useState("vulnerabilities"); ... - const tabs = [ - { - id: "vulnerabilities", - label: "Product Vulnerabilities", - icon: Siren, - }, - ... - ]; ... <TabBar - tabs={tabs} + tabs={SECURITY_TABS} activeTab={activeTab} onTabChange={setActiveTab} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/SecurityPage.tsx` around lines 42 - 58, The tabs array defined as const tabs = [...] inside SecurityPage is recreated on every render; move this constant to module scope (outside the SecurityPage component function) so it is allocated once, keeping the same shape and identifiers (id, label, icon) and names (tabs, and referenced icons Siren, ShieldAlert, Package) so existing references inside SecurityPage remain unchanged.
60-87:SecurityStatsonly reflects vulnerabilities tab data — may be confusing on other tabs.
totalRecordsandisErrorare sourced fromProductVulnerabilitiesTablecallbacks. When the user switches to "Advisories" or "Components", the stats header still displays stale vulnerability data (orundefinedon first visit). If the stats are vulnerabilities-specific, consider hiding or labeling them accordingly; otherwise, each tab should feed its own stats.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/SecurityPage.tsx` around lines 60 - 87, SecurityStats is currently fed only by ProductVulnerabilitiesTable (vulnerabilityTotalRecords, vulnerabilitiesError) so it shows stale/undefined data when activeTab !== "vulnerabilities"; update the component so stats reflect the active tab by either (A) hiding SecurityStats unless activeTab === "vulnerabilities" or (B) adding per-tab stats state and callbacks: create state for advisoriesTotalRecords/advisoriesError and componentsTotalRecords/componentsError and pass their setter callbacks to SecurityAdvisoriesTable and ComponentAnalysis, then pass the correct pair to SecurityStats based on activeTab (use activeTab to select which totalRecords/isError to render); alternatively, when switching tabs via setActiveTab, reset vulnerabilityTotalRecords/vulnerabilitiesError to avoid stale values.apps/customer-portal/webapp/src/pages/__tests__/NoveraChatPage.test.tsx (1)
241-280: Test ends without asserting the classification outcome.The test verifies the loading spinner appears when "Create Case" is clicked while products are loading, which is good. However, line 279 fires a change event as a re-render trigger but never asserts the final navigation or that classification actually proceeded after products finished loading. This makes the test name ("should wait for products to load before classifying case") only partially verified — it confirms waiting but not the subsequent classification.
Consider adding a
waitForassertion after the trigger to confirm the loading spinner disappears or that navigation occurs onceisLoadingflips tofalse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/__tests__/NoveraChatPage.test.tsx` around lines 241 - 280, The test currently stops after firing a change event and never asserts that classification proceeded; after the final fireEvent.change(input, { target: { value: "Trigger update" } }) call in the "should wait for products to load before classifying case" test, add a waitFor (or use findBy*) that asserts the loading spinner (getByTestId("circular-progress")) disappears or that navigation/classification happened (e.g., check for expected post-classification text via findByText or that the route changed after NoveraChatPage completes classification) so the test verifies the outcome once useAllDeploymentProductsMock transitions to isLoading: false.apps/customer-portal/webapp/src/api/useGetProductVulnerability.ts (1)
42-68: Minor inconsistency:try/catchwrapping differs from sibling hook.This hook wraps the entire
queryFnbody intry/catchfor error logging, whileuseGetVulnerabilitiesMetaData.tsdoes not. Both approaches work (react-query surfaces thrown errors either way), but aligning the pattern across sibling hooks would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/api/useGetProductVulnerability.ts` around lines 42 - 68, The hook currently wraps the entire queryFn in a try/catch for logging, which differs from sibling hooks; remove the try/catch in useGetProductVulnerability's queryFn so errors are allowed to bubble to react-query (keep existing throws like the baseUrl check and non-ok response), keep the logger.debug calls for response.status and data but delete the logger.error and rethrow in the catch; this aligns the pattern with useGetVulnerabilitiesMetaData while preserving the same thrown errors from fetchFn/response handling.apps/customer-portal/webapp/src/pages/CreateCasePage.tsx (2)
130-171: Duplicated inline type forclassificationResponse— extract to a shared interface.The same shape is defined inline at lines 130–143 (for
locationState) and again at lines 147–159 (for the state initializer). Consider extracting this to a named interface (or reusingCaseClassificationResponsefrom the models) to reduce duplication and improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/CreateCasePage.tsx` around lines 130 - 171, Extract the duplicated inline type used for locationState and the useState initializer into a single named interface (e.g., CaseClassificationResponse) or import the existing CaseClassificationResponse from your models, then replace the inline type annotations on locationState and the useState generic for classificationResponse (and keep using setClassificationResponse) to reference that shared interface; ensure the STORAGE_KEY usage and JSON.parse error handling remain unchanged.
164-170: Usesconsole.errorinstead of theloggeravailable in scope.Other error paths in this file use
showErroror structured logging. Thiscatchblock uses rawconsole.error, which bypasses the LoggerProvider.♻️ Suggested fix
Since
useLoggeris not currently imported in this file, and adding it may be more than desired, at minimum consider consistency. If the logger is added:+ const logger = useLogger(); ... - console.error("Failed to parse stored classification data", e); + logger.error("Failed to parse stored classification data", e);Similarly for lines 181–184.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/CreateCasePage.tsx` around lines 164 - 170, The catch block that reads sessionStorage (around the function that retrieves stored classification using STORAGE_KEY) uses console.error, which bypasses the app LoggerProvider; replace that console.error usage with the project logger (useLogger) or the existing showError utility for consistency: import and call useLogger (or obtain the existing logger used elsewhere in CreateCasePage) and log a structured error message including the exception and context ("Failed to parse stored classification data" and STORAGE_KEY), and mirror the same change for the other similar catch at lines 181–184 so all storage parse errors use the unified logger/showError instead of console.error.apps/customer-portal/webapp/src/pages/DashboardPage.tsx (1)
232-247: Fallback objects after||are unreachable —useMemoalways returns a value.
activeCases,outstandingCases, andcasesTrendare always defined (nevernull/undefined/0) sinceuseMemoalways returns an object or array. The|| { ... }/|| []fallbacks are dead code.♻️ Suggested cleanup
- outstandingCases={outstandingCases || { - low: 0, - medium: 0, - high: 0, - critical: 0, - catastrophic: 0, - total: 0, - }} - activeCases={activeCases || { - workInProgress: 0, - waitingOnClient: 0, - waitingOnWso2: 0, - total: 0, - }} - casesTrend={casesTrend || []} + outstandingCases={outstandingCases} + activeCases={activeCases} + casesTrend={casesTrend}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/DashboardPage.tsx` around lines 232 - 247, The fallback objects passed into ChartLayout are dead code because useMemo always returns defined values; remove the unnecessary `|| { ... }` and `|| []` fallbacks for the props outstandingCases, activeCases, and casesTrend (references: ChartLayout props and the variables activeCases, outstandingCases, casesTrend produced by useMemo) so the component receives the memoized objects/array directly; if there are concerns about typing, tighten the useMemo return types instead of keeping the fallback expressions.apps/customer-portal/webapp/src/components/security/ProductVulnerabilitiesTable.tsx (1)
91-99:onErrorandonTotalRecordsChangeinuseEffectdeps may cause render loops if callers pass unstable references.If the parent component passes inline arrow functions for
onErrororonTotalRecordsChange, these effects re-fire every render. Consider documenting that these callbacks should be stable (wrapped inuseCallback) or guard against it internally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/security/ProductVulnerabilitiesTable.tsx` around lines 91 - 99, The effects currently depend on onError and onTotalRecordsChange which can cause re-renders if callers pass unstable inline callbacks; change the implementation to avoid calling those callbacks as deps: keep refs for the latest onError and onTotalRecordsChange (update the refs in a separate effect) and then in the existing effects only depend on data?.totalRecords and isError, using the refs to invoke the callbacks; alternatively, add internal guards that track the previous totalRecords and previous isError via useRef and only call the callback when the actual value changed (useEffect for data.totalRecords -> call latest onTotalRecordsChangeRef.current when value differs, and likewise for isError -> onErrorRef.current).apps/customer-portal/backend/modules/types/types.bal (1)
634-661: Add pagination toCallRequestsResponseto match request contract and established patterns.The TODO on line 660 flags missing pagination. Request types like
CallRequestSearchPayloadalready includeentity:Pagination, butCallRequestsResponselacks it. Multiple response types in the same file (CaseSearchResponse,CommentsResponse,ProductVulnerabilitySearchResponse) follow the pattern of includingint totalRecordsand*entity:Pagination. Adding pagination before clients stabilize on the current shape ensures consistency.Proposed fix
# Call requests response. public type CallRequestsResponse record {| # List of call requests CallRequest[] callRequests; - // TODO: Remove after adding pagination + # Total records count + int totalRecords; + *entity:Pagination; |};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/types/types.bal` around lines 634 - 661, CallRequestsResponse is missing pagination fields; update the CallRequestsResponse record to include int totalRecords and the optional entity:Pagination (matching the pattern used by CallRequestSearchPayload and other responses like CaseSearchResponse, CommentsResponse, and ProductVulnerabilitySearchResponse). Add a field int totalRecords and a field *entity:Pagination pagination (or similarly named pagination) to CallRequestsResponse so clients receive total count and pagination metadata consistent with the established response shape.
| public isolated function validateDeploymentUpdatePayload(DeploymentUpdatePayload payload) returns string? { | ||
| boolean hasDeploymentFields = payload.name !is () && payload.typeKey !is (); | ||
|
|
||
| // Check if payload has at least one field | ||
| if !hasDeploymentFields && payload.active is () { | ||
| return "At least one field (name, typeKey, or active) must be provided for update."; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the utils.bal file and examine the relevant code section
fd "utils.bal" --type f | grep entityRepository: wso2-open-operations/cs-tools
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Examine the validateDeploymentUpdatePayload function and surrounding context
cat -n apps/customer-portal/backend/modules/entity/utils.bal | sed -n '110,140p'Repository: wso2-open-operations/cs-tools
Length of output: 1317
🏁 Script executed:
#!/bin/bash
# Find the DeploymentUpdatePayload type definition
rg -n "type DeploymentUpdatePayload" --type=bal -A 10Repository: wso2-open-operations/cs-tools
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check how validateDeploymentUpdatePayload is called in the codebase
rg -n "validateDeploymentUpdatePayload" --type=bal -B 3 -A 3Repository: wso2-open-operations/cs-tools
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Search for DeploymentUpdatePayload type definition
rg -n "type DeploymentUpdatePayload" -A 10Repository: wso2-open-operations/cs-tools
Length of output: 978
🏁 Script executed:
#!/bin/bash
# Search for all usages of validateDeploymentUpdatePayload
rg -n "validateDeploymentUpdatePayload" -B 2 -A 2Repository: wso2-open-operations/cs-tools
Length of output: 1488
🏁 Script executed:
#!/bin/bash
# Search for DeploymentUpdatePayload in types.bal
find . -name "types.bal" -path "*/entity/*" -exec cat -n {} \;Repository: wso2-open-operations/cs-tools
Length of output: 29680
Fix logic operator: use || instead of && to allow any deployment field.
On line 115, hasDeploymentFields uses AND, which requires both name and typeKey to be non-nil. However, the error message says "At least one field (name, typeKey, or active) must be provided"—implying OR logic. A user who provides only name (with typeKey nil) will incorrectly receive an error despite providing a valid field.
Fix
- boolean hasDeploymentFields = payload.name !is () && payload.typeKey !is ();
+ boolean hasDeploymentFields = payload.name !is () || payload.typeKey !is ();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public isolated function validateDeploymentUpdatePayload(DeploymentUpdatePayload payload) returns string? { | |
| boolean hasDeploymentFields = payload.name !is () || payload.typeKey !is (); | |
| // Check if payload has at least one field | |
| if !hasDeploymentFields && payload.active is () { | |
| return "At least one field (name, typeKey, or active) must be provided for update."; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/backend/modules/entity/utils.bal` around lines 114 -
120, The boolean logic in validateDeploymentUpdatePayload is inverted: change
how hasDeploymentFields is computed so it becomes true when any of payload.name
or payload.typeKey is non-nil (use OR instead of AND), then keep the existing
check that also considers payload.active; update the hasDeploymentFields
expression (referencing variable hasDeploymentFields in function
validateDeploymentUpdatePayload) to use the OR operator so a single provided
field (name or typeKey) is accepted and the error message behavior matches the
intent.
| # Get products. | ||
| # | ||
| # + return - List of products or an error | ||
| resource function get products(http:RequestContext ctx, int? offset, int? 'limit) returns entity:ProductsResponse|http:InternalServerError { | ||
| authorization:UserInfoPayload|error userInfo = ctx.getWithType(authorization:HEADER_USER_INFO); | ||
| if userInfo is error { | ||
| return <http:InternalServerError>{ | ||
| body: { | ||
| message: ERR_MSG_USER_INFO_HEADER_NOT_FOUND | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| entity:ProductsResponse|error response = entity:getProducts(userInfo.idToken, {}); // TODO: Handle pagination | ||
| if response is error { | ||
| string customError = "Failed to retrieve products."; | ||
| log:printError(customError, response); | ||
| return <http:InternalServerError>{ | ||
| body: { | ||
| message: customError | ||
| } | ||
| }; | ||
| } | ||
| return response; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find ProductSearchPayload type definition
ast-grep --pattern 'type ProductSearchPayload record {
$$$
}'Repository: wso2-open-operations/cs-tools
Length of output: 55
🏁 Script executed:
# Search for entity:getProducts function to understand its signature and contract
rg -A 10 'function getProducts' --type balRepository: wso2-open-operations/cs-tools
Length of output: 100
🏁 Script executed:
# Check the entity module structure to understand the payload types
fd -e bal . integrations/customer-service/modules/entity | head -20Repository: wso2-open-operations/cs-tools
Length of output: 204
🏁 Script executed:
# Look at how pagination is used in other endpoints in the same file
rg -B 5 -A 10 'resource function get' apps/customer-portal/backend/service.bal | head -100Repository: wso2-open-operations/cs-tools
Length of output: 3924
🏁 Script executed:
# List Ballerina files in the repository
find . -name "*.bal" -type f | head -20Repository: wso2-open-operations/cs-tools
Length of output: 1252
🏁 Script executed:
# Search for products endpoint without type restrictions
rg -i "products" --max-count=20 -B 2 -A 8Repository: wso2-open-operations/cs-tools
Length of output: 50386
🏁 Script executed:
# Search for getProducts function definition
rg "getProducts" --max-count=20 -B 2 -A 15Repository: wso2-open-operations/cs-tools
Length of output: 2031
🏁 Script executed:
# Find ProductSearchPayload or similar payload types
rg "ProductSearchPayload|ProductsResponse" --max-count=20 -B 2 -A 5Repository: wso2-open-operations/cs-tools
Length of output: 9728
🏁 Script executed:
# Find Pagination type definition
rg "type Pagination" --max-count=5 -B 1 -A 5 apps/customer-portal/backend/modules/entity/Repository: wso2-open-operations/cs-tools
Length of output: 618
🏁 Script executed:
# Check entity types.bal more carefully for Pagination
cat apps/customer-portal/backend/modules/entity/types.bal | grep -A 10 "Pagination"Repository: wso2-open-operations/cs-tools
Length of output: 2937
🏁 Script executed:
# Verify DEFAULT_OFFSET constant
rg "DEFAULT_OFFSET" apps/customer-portal/backend/modules/entity/constants.bal -A 2Repository: wso2-open-operations/cs-tools
Length of output: 125
Pagination parameters offset and limit are accepted but silently ignored.
The endpoint declares int? offset, int? 'limit as query parameters but passes an empty payload {} to entity:getProducts, causing all requests to use default pagination (offset=0, limit=20) regardless of caller input. This breaks the API contract and can lead to performance issues on large product catalogs.
Either forward the parameters to the payload or remove them from the signature until the TODO is addressed.
Proposed fix
- resource function get products(http:RequestContext ctx, int? offset, int? 'limit) returns entity:ProductsResponse|http:InternalServerError {
+ resource function get products(http:RequestContext ctx, int? offset, int? 'limit) returns entity:ProductsResponse|http:InternalServerError {
authorization:UserInfoPayload|error userInfo = ctx.getWithType(authorization:HEADER_USER_INFO);
if userInfo is error {
return <http:InternalServerError>{
body: {
message: ERR_MSG_USER_INFO_HEADER_NOT_FOUND
}
};
}
- entity:ProductsResponse|error response = entity:getProducts(userInfo.idToken, {}); // TODO: Handle pagination
+ entity:ProductsResponse|error response = entity:getProducts(userInfo.idToken, {
+ pagination: {offset: offset ?: 0, 'limit: 'limit ?: 20}
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Get products. | |
| # | |
| # + return - List of products or an error | |
| resource function get products(http:RequestContext ctx, int? offset, int? 'limit) returns entity:ProductsResponse|http:InternalServerError { | |
| authorization:UserInfoPayload|error userInfo = ctx.getWithType(authorization:HEADER_USER_INFO); | |
| if userInfo is error { | |
| return <http:InternalServerError>{ | |
| body: { | |
| message: ERR_MSG_USER_INFO_HEADER_NOT_FOUND | |
| } | |
| }; | |
| } | |
| entity:ProductsResponse|error response = entity:getProducts(userInfo.idToken, { | |
| pagination: {offset: offset ?: 0, 'limit: 'limit ?: 20} | |
| }); | |
| if response is error { | |
| string customError = "Failed to retrieve products."; | |
| log:printError(customError, response); | |
| return <http:InternalServerError>{ | |
| body: { | |
| message: customError | |
| } | |
| }; | |
| } | |
| return response; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/backend/service.bal` around lines 1232 - 1256, The
resource function get products currently accepts offset and 'limit but passes an
empty payload to entity:getProducts, so the parameters are ignored; update get
products to construct and pass a pagination payload (e.g., include offset and
limit only when non-nil or use defaults) to entity:getProducts (referencing
entity:getProducts and the resource function get products) so caller-supplied
offset and 'limit are honored, or if you intend to disable pagination remove the
offset and 'limit parameters from the get products signature and related docs
instead; ensure nullable ints are handled before passing to entity:getProducts.
| string? validationError = entity:validateCallRequestUpdatePayload(payload); | ||
| if validationError is string { | ||
| log:printWarn(validationError); | ||
| return <http:BadRequest>{ | ||
| body: { | ||
| message: validationError | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| string|error? validateUtcTimesError = entity:validateUtcTimes(payload.utcTimes); | ||
| if validateUtcTimesError is string { | ||
| log:printWarn(validateUtcTimesError); | ||
| return <http:BadRequest>{ | ||
| body: { | ||
| message: validateUtcTimesError | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| if validateUtcTimesError is error { | ||
| string customError = "Failed to validate UTC times for call request update."; | ||
| log:printError(customError, validateUtcTimesError); | ||
| return <http:InternalServerError>{ | ||
| body: { | ||
| message: customError | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| string? validateCallRequestUpdatePayload = entity:validateCallRequestUpdatePayload(payload); | ||
| if validateCallRequestUpdatePayload is string { | ||
| log:printWarn(validateCallRequestUpdatePayload); | ||
| return <http:BadRequest>{ | ||
| body: { | ||
| message: validateCallRequestUpdatePayload | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Duplicate call to validateCallRequestUpdatePayload — the validation at lines 1812–1820 is redundant.
entity:validateCallRequestUpdatePayload(payload) is invoked at line 1782 and then again at line 1812 with the exact same payload. The second invocation is dead code that will never trigger a BadRequest (since the first check already returns early on validation failure).
Remove the duplicate block:
Proposed fix
if validateUtcTimesError is error {
string customError = "Failed to validate UTC times for call request update.";
log:printError(customError, validateUtcTimesError);
return <http:InternalServerError>{
body: {
message: customError
}
};
}
- string? validateCallRequestUpdatePayload = entity:validateCallRequestUpdatePayload(payload);
- if validateCallRequestUpdatePayload is string {
- log:printWarn(validateCallRequestUpdatePayload);
- return <http:BadRequest>{
- body: {
- message: validateCallRequestUpdatePayload
- }
- };
- }
-
entity:CallRequestUpdateResponse|error response = entity:updateCallRequest(userInfo.idToken, callRequestId,
payload);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| string? validationError = entity:validateCallRequestUpdatePayload(payload); | |
| if validationError is string { | |
| log:printWarn(validationError); | |
| return <http:BadRequest>{ | |
| body: { | |
| message: validationError | |
| } | |
| }; | |
| } | |
| string|error? validateUtcTimesError = entity:validateUtcTimes(payload.utcTimes); | |
| if validateUtcTimesError is string { | |
| log:printWarn(validateUtcTimesError); | |
| return <http:BadRequest>{ | |
| body: { | |
| message: validateUtcTimesError | |
| } | |
| }; | |
| } | |
| if validateUtcTimesError is error { | |
| string customError = "Failed to validate UTC times for call request update."; | |
| log:printError(customError, validateUtcTimesError); | |
| return <http:InternalServerError>{ | |
| body: { | |
| message: customError | |
| } | |
| }; | |
| } | |
| entity:CallRequestUpdateResponse|error response = entity:updateCallRequest(userInfo.idToken, callRequestId, | |
| payload); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/backend/service.bal` around lines 1782 - 1820, There are
two consecutive calls to entity:validateCallRequestUpdatePayload(payload) —
remove the redundant second block that declares
validateCallRequestUpdatePayload, checks it, logs the warn and returns
BadRequest; keep the original validation at the top (validationError) and ensure
no leftover unused variable names remain (remove the second
validateCallRequestUpdatePayload variable and its if-check/log/return).
| <Box sx={{ display: "flex", alignItems: "flex-start", gap: 2 }}> | ||
| <Box sx={{ color }}>{icon}</Box> | ||
| <Box sx={{ flex: 1 }}> | ||
| <Typography variant="h4">{value}</Typography> |
There was a problem hiding this comment.
Rendering arbitrary ReactNode inside <h4> risks invalid HTML
Typography variant="h4" renders to <h4>. If a caller passes a ReactNode containing block-level elements (div, p, section, etc.) as value, the result is invalid HTML (block element nested inside a heading). Since the type explicitly allows ReactNode, this is easy to trigger accidentally.
Consider restricting value back to string | number if complex nodes are not intended, or changing the container to a Box (renders as div) when value is not a primitive.
🛡️ Safer container for arbitrary ReactNode values
- <Typography variant="h4">{value}</Typography>
+ {typeof value === "string" || typeof value === "number" ? (
+ <Typography variant="h4">{value}</Typography>
+ ) : (
+ <Box>{value}</Box>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Typography variant="h4">{value}</Typography> | |
| {typeof value === "string" || typeof value === "number" ? ( | |
| <Typography variant="h4">{value}</Typography> | |
| ) : ( | |
| <Box>{value}</Box> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/webapp/src/components/common/GenericSubCountCard.tsx` at
line 52, The component GenericSubCountCard currently renders the prop value
directly inside <Typography variant="h4"> which outputs an <h4>, risking invalid
HTML if callers pass a ReactNode containing block elements; update
GenericSubCountCard to restrict the value prop type to string | number (instead
of ReactNode) OR render conditionally: if typeof value is 'string' or 'number'
keep <Typography variant="h4">{value}</Typography>, otherwise wrap the node in a
neutral container (e.g., Box/div) instead of the h4; adjust the value prop type
and any callers accordingly and ensure propTypes/TS types for value and the
rendering logic in GenericSubCountCard reflect this change.
| useEffect(() => { | ||
| if (isFirstRender && initialHtml) { | ||
| if (initialHtml) { | ||
| editor.update(() => { | ||
| const root = $getRoot(); | ||
| // Only set initial value if the editor is empty to avoid overwriting user updates | ||
| if (root.getTextContent() === "") { | ||
| const currentContent = root.getTextContent(); | ||
|
|
||
| if (currentContent.trim() === "") { | ||
| const parser = new DOMParser(); | ||
| const dom = parser.parseFromString(initialHtml, "text/html"); | ||
| const nodes = $generateNodesFromDOM(editor, dom); | ||
| root.clear(); | ||
| root.append(...nodes); | ||
| } | ||
| }); | ||
| setIsFirstRender(false); | ||
| } | ||
| }, [editor, initialHtml, isFirstRender]); | ||
| }, [editor, initialHtml]); |
There was a problem hiding this comment.
getTextContent() guard misses image-only (non-text) editor content
root.getTextContent() only aggregates text nodes. If the user has inserted content composed solely of non-text Lexical nodes (e.g., ImageNodes), the check currentContent.trim() === "" will evaluate to true even though the editor is non-empty, causing initialHtml to be re-applied and overwriting the user's content.
A more reliable guard is to check node count rather than text content:
🛡️ Proposed fix
- const currentContent = root.getTextContent();
-
- if (currentContent.trim() === "") {
+ const hasContent = root.getChildrenSize() > 0 &&
+ !(root.getChildrenSize() === 1 &&
+ root.getFirstChild()?.getTextContent?.() === "");
+
+ if (!hasContent) {
const parser = new DOMParser();
const dom = parser.parseFromString(initialHtml, "text/html");
const nodes = $generateNodesFromDOM(editor, dom);
root.clear();
root.append(...nodes);
}Alternatively, check whether the root has a single empty paragraph (Lexical's default blank-slate state) instead of relying on text content:
- const currentContent = root.getTextContent();
-
- if (currentContent.trim() === "") {
+ const children = root.getChildren();
+ const isEffectivelyEmpty =
+ children.length === 0 ||
+ (children.length === 1 &&
+ children[0].getTextContent().trim() === "" &&
+ children[0].getChildrenSize?.() === 0);
+
+ if (isEffectivelyEmpty) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (initialHtml) { | |
| editor.update(() => { | |
| const root = $getRoot(); | |
| const hasContent = root.getChildrenSize() > 0 && | |
| !(root.getChildrenSize() === 1 && | |
| root.getFirstChild()?.getTextContent?.() === ""); | |
| if (!hasContent) { | |
| const parser = new DOMParser(); | |
| const dom = parser.parseFromString(initialHtml, "text/html"); | |
| const nodes = $generateNodesFromDOM(editor, dom); | |
| root.clear(); | |
| root.append(...nodes); | |
| } | |
| }); | |
| } | |
| }, [editor, initialHtml]); |
| useEffect(() => { | |
| if (initialHtml) { | |
| editor.update(() => { | |
| const root = $getRoot(); | |
| const children = root.getChildren(); | |
| const isEffectivelyEmpty = | |
| children.length === 0 || | |
| (children.length === 1 && | |
| children[0].getTextContent().trim() === "" && | |
| children[0].getChildrenSize?.() === 0); | |
| if (isEffectivelyEmpty) { | |
| const parser = new DOMParser(); | |
| const dom = parser.parseFromString(initialHtml, "text/html"); | |
| const nodes = $generateNodesFromDOM(editor, dom); | |
| root.clear(); | |
| root.append(...nodes); | |
| } | |
| }); | |
| } | |
| }, [editor, initialHtml]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/customer-portal/webapp/src/components/common/rich-text-editor/Editor.tsx`
around lines 71 - 86, The current guard uses root.getTextContent() inside
editor.update() which treats image-only content as empty and causes initialHtml
to overwrite non-text nodes; instead, detect an actually-empty editor by
checking the root's node structure (e.g., root.getChildrenSize() === 0 or the
root contains a single empty paragraph node) before applying initialHtml. Update
the condition in the editor.update() block (around $getRoot(),
root.getTextContent(), $generateNodesFromDOM, root.clear(),
root.append(...nodes)) to use a node-count or single-empty-paragraph check
rather than trimming getTextContent().
| if (chatHistory && hasEnvProducts) { | ||
| try { | ||
| const classificationResponse = await classifyCase({ | ||
| chatHistory, | ||
| envProducts, | ||
| region: "EU", | ||
| tier: "Tier 1", | ||
| }); |
There was a problem hiding this comment.
Hardcoded region and tier will produce incorrect classifications for non-EU / non-Tier-1 customers.
These values should be derived from the project or deployment context. If this is temporary for demo purposes, consider adding a TODO comment.
if (chatHistory && hasEnvProducts) {
try {
const classificationResponse = await classifyCase({
chatHistory,
envProducts,
- region: "EU",
- tier: "Tier 1",
+ region: "EU", // TODO: derive from project/deployment context
+ tier: "Tier 1", // TODO: derive from project/deployment context
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (chatHistory && hasEnvProducts) { | |
| try { | |
| const classificationResponse = await classifyCase({ | |
| chatHistory, | |
| envProducts, | |
| region: "EU", // TODO: derive from project/deployment context | |
| tier: "Tier 1", // TODO: derive from project/deployment context | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/webapp/src/pages/NoveraChatPage.tsx` around lines 96 -
103, The call to classifyCase in NoveraChatPage is passing hardcoded region:
"EU" and tier: "Tier 1", which will misclassify non-EU/non-Tier-1 customers;
update the code to derive region and tier dynamically (e.g., from the
project/deployment context, user/project metadata, props on NoveraChatPage, or
environment/config values) and pass those computed values into classifyCase({
chatHistory, envProducts, region, tier }); if this is only temporary for demos,
add a clear TODO comment above the call indicating the hardcoded values and the
plan to replace them with dynamic resolution.
| const handleCreateCase = useCallback(() => { | ||
| setIsCreateCaseLoading(true); | ||
|
|
||
| if (isAllProductsLoading) { | ||
| setIsWaitingForClassification(true); | ||
| } else { | ||
| performClassification(); | ||
| } | ||
| }, [isAllProductsLoading, performClassification]); | ||
|
|
||
| useEffect(() => { | ||
| if (isWaitingForClassification && !isAllProductsLoading) { | ||
| setIsWaitingForClassification(false); | ||
| performClassification(); | ||
| } | ||
| }, [isWaitingForClassification, isAllProductsLoading, performClassification]); |
There was a problem hiding this comment.
If isAllProductsLoading never resolves to false, loading state is stuck indefinitely.
When handleCreateCase sets isWaitingForClassification = true (line 127), the cleanup depends on isAllProductsLoading eventually becoming false to trigger the useEffect at line 133. If the products query is disabled, errors without transitioning isLoading, or never fires, isCreateCaseLoading remains true forever with no timeout or fallback.
Consider adding a timeout or checking the error state of the products query to avoid an indefinitely stuck loading indicator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/webapp/src/pages/NoveraChatPage.tsx` around lines 123 -
138, The loading flow can get stuck if isAllProductsLoading never becomes false;
update handleCreateCase/useEffect to add a fallback that clears
isCreateCaseLoading and triggers performClassification after a timeout or when
the products query errors. Specifically, in the logic around handleCreateCase,
set a timer (e.g., 10s) when setting isWaitingForClassification and store its id
so you can clear it; expand the useEffect watching isWaitingForClassification to
also check a products error flag (or productsQuery.isError) and to clear the
timeout and reset isWaitingForClassification/isCreateCaseLoading before calling
performClassification; ensure any timers are cleaned up in a cleanup function to
avoid leaks and reference the existing symbols handleCreateCase,
isAllProductsLoading, isWaitingForClassification, performClassification, and
isCreateCaseLoading.
There was a problem hiding this comment.
No error state handling — a failed API call shows the skeleton indefinitely.
If useGetRecommendedUpdateLevels or useGetProductUpdateLevels errors out, recommendedData remains undefined and isLoading stays true, resulting in a permanent skeleton with no user feedback.
Consider destructuring isError from both hooks and rendering an error message when either fails.
🛡️ Suggested approach
- const { data: recommendedData } = useGetRecommendedUpdateLevels();
- const { data: productLevelsData, isLoading: isProductLevelsLoading } =
- useGetProductUpdateLevels();
+ const { data: recommendedData, isError: isRecommendedError } = useGetRecommendedUpdateLevels();
+ const { data: productLevelsData, isLoading: isProductLevelsLoading, isError: isProductLevelsError } =
+ useGetProductUpdateLevels();
// ... existing code ...
+ if (isRecommendedError || isProductLevelsError) {
+ return (
+ <Box sx={{ py: 4 }}>
+ <Typography variant="body1" color="error">
+ Failed to load update data. Please try again later.
+ </Typography>
+ <Button startIcon={<ArrowLeft size={16} />} onClick={handleBack} sx={{ mt: 2 }}>
+ Back to Updates
+ </Button>
+ </Box>
+ );
+ }Also applies to: 95-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/webapp/src/pages/PendingUpdatesPage.tsx` around lines 41
- 43, Destructure isError (and optionally error) from both
useGetRecommendedUpdateLevels and useGetProductUpdateLevels hooks (in
PendingUpdatesPage where recommendedData and productLevelsData are fetched) and
update the render logic to show an error state when either isError is true
instead of leaving the skeleton visible; ensure you handle the combined cases
(both loading, one error, both error) so the skeleton only shows while both are
loading and a clear error message/component is rendered when a call fails.
| const normalized = severity?.toLowerCase() || ""; | ||
| if (normalized === "critical") return "error.main"; | ||
| if (normalized === "high") return "warning.main"; | ||
| if (normalized === "medium") return "text.disabled"; |
There was a problem hiding this comment.
"medium" severity mapped to "text.disabled" — semantically incorrect color token.
text.disabled is a very-low-opacity grey (rgba(0,0,0,0.26)) intended to signal that "a component or element isn't interactive." Applying it to medium-severity indicators makes a medium-risk vulnerability indistinguishable from a disabled/inactive UI element, which will confuse users. Consider a token that carries actual visual weight, such as "text.secondary" (the current default fallback) or "warning.light".
🎨 Proposed fix
- if (normalized === "medium") return "text.disabled";
+ if (normalized === "medium") return "warning.light";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (normalized === "medium") return "text.disabled"; | |
| if (normalized === "medium") return "warning.light"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/webapp/src/utils/vulnerabilities.ts` at line 26, The
mapping for medium severity incorrectly returns the disabled color token; in the
severity-to-color logic (where the variable normalized is checked, e.g., the
branch that currently does `if (normalized === "medium") return
"text.disabled";`) change the returned token to a semantically appropriate
visible token such as "text.secondary" (or "warning.light") so medium
vulnerabilities are visually distinct from disabled UI elements; update the
branch that checks normalized === "medium" to return the chosen token and ensure
any tests or usages expecting "text.disabled" are adjusted accordingly.
| if (normalized.includes("progress")) return "warning.main"; | ||
| if (normalized.includes("open")) return "info.main"; | ||
| if (normalized.includes("resolved")) return "success.main"; |
There was a problem hiding this comment.
includes("open") can produce false-positive matches for statuses like "Reopened".
If the API ever returns a status such as "Reopened" or "Reopen Requested", normalized.includes("open") will match it and return "info.main" instead of falling through to the default. The exact-match pattern used by getVulnerabilitySeverityColor is safer; prefer it here too, or widen the match to anchor the token (e.g. === "open" for exact, or /(^|\s)open(\s|$)/.test()).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/webapp/src/utils/vulnerabilities.ts` around lines 38 -
40, The current checks in getVulnerabilitySeverityColor use
normalized.includes("open") which can false-positive on values like "reopened";
change the check in the function to match tokens exactly (e.g., use normalized
=== "open" or a word-boundary regex like /\bopen\b/.test(normalized)) and update
the other similar includes checks (e.g., "progress", "resolved") if they should
be exact matches so the function returns the correct color for exact statuses.
Summary by CodeRabbit