Repository navigation
[Customer Portal][FE][Web] Add All updates viewing page + Specific updates viewing page + Update data PDF Generation and Table Export - #235
Conversation
Introduce usePostUpdateLevelsSearch hook to call POST /updates/levels/search and expose update level search results; add route for UpdateLevelDetailsPage under updates/pending/level/:levelKey. Add jspdf and jspdf-autotable to package.json (pnpm lock updated). Improve usePostCreateDeployment: handle 409 conflict with a clear error message and switch cache invalidation to refetchQueries to refresh deployments data.
Introduce a new AllUpdatesTab component to search and display update levels (uses useGetRecommendedUpdateLevels and usePostUpdateLevelsSearch hooks), plus UpdateLevelsReportModal to preview/download a PDF report (generateUpdateLevelsReportPdf). Includes unit tests for both components using vitest/testing-library and simple mocks for the API hooks. Also includes a trivial newline fix in apps/customer-portal/webapp/package.json.
Add a new update levels report utility and tests, and make related UI tweaks: - Add apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts: implements getUpdateLevelsReportData (structures/counts/formatting for an Update Levels Report) and generateUpdateLevelsReportPdf (PDF generation using jspdf + jspdf-autotable). - Add unit tests apps/customer-portal/webapp/src/utils/__tests__/updateLevelsReportPdf.test.ts verifying output shape, date formatting, and error on empty data. - Update apps/customer-portal/webapp/src/pages/UpdatesPage.tsx: replace placeholder Typography content with AllUpdatesTab, remove unused Typography import and background prefetch call to useGetProductUpdateLevels. - Update apps/customer-portal/webapp/src/utils/updates.ts: add getUpdateTypeChipColor helper to map update types to StatCardColor tokens (security -> error, default -> success). These changes add report generation support and clean up the Updates page integration.
Introduce a new UpdateLevelDetailsPage for viewing full update description entries per level and filtering by security/regular updates. Add API models for the POST /updates/levels/search flow: UpdateLevelsSearchRequest, UpdateLevelsSearchResponse, UpdateLevelEntry, UpdateDescriptionLevel and SecurityAdvisory. Update PendingUpdatesPage to build the search request from URL params, call usePostUpdateLevelsSearch, compute level ranges from the response, and navigate to the new detail route; PendingUpdatesList props were adjusted (data, isError, onView). Update AppLayout route detection to treat the new /updates/pending/level/:levelKey route as a details-style page.
Refactor PendingUpdatesList to accept the UpdateLevelsSearchResponse shape, handle error and empty states, and render update type chips with theme-aware colors. Add an onView callback for the Details action and replace the old pendingRows/recommendedItem logic with sorted entries and counts. Update unit tests to use the new response fixture, error/empty expectations, and vi mocks. Also improve UpdateProductGrid to show error/empty states and include starting/ending update level params when navigating. Add UPDATE_LEVELS_SEARCH to ApiQueryKeys.
📝 WalkthroughWalkthroughAdds a searchable Update Levels feature: new POST /updates/levels/search hook and models, filter/report UI (AllUpdatesTab, UpdateLevelsReportModal), level details page, PDF report generation using jspdf + autotable, routing for level details, refactors pending updates to consume the search response, and accompanying tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as AllUpdatesTab / DetailsPage
participant Client as AuthApiClient
participant Backend
participant Modal as Report Modal
participant PDF as PDF Generator
User->>UI: Select filters / click "Search Update Levels"
UI->>Client: POST /updates/levels/search (usePostUpdateLevelsSearch)
Client->>Backend: POST /updates/levels/search
Backend-->>Client: UpdateLevelsSearchResponse (map of levels)
Client-->>UI: Return search data
UI-->>User: Render results (cards/list)
User->>UI: Click "View Report"
UI->>Modal: Open with reportData
Modal-->>User: Show report preview
User->>Modal: Click "Download PDF"
Modal->>PDF: generateUpdateLevelsReportPdf(reportData)
PDF->>PDF: Build PDF via jspdf + autotable
PDF-->>User: Trigger file download
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (10)
apps/customer-portal/webapp/src/components/updates/pending-updates/__tests__/PendingUpdatesList.test.tsx (1)
116-123: Missing test foronViewcallback invocation.The test verifies that "View" buttons render, but doesn't test that clicking them calls
onViewwith the correctlevelKey. This is the key user interaction for this component.Example test to add
import { fireEvent } from "@testing-library/react"; it("calls onView with the correct levelKey when View is clicked", () => { const onView = vi.fn(); render(<PendingUpdatesList data={mockData} isError={false} onView={onView} />); const viewButtons = screen.getAllByText("View"); fireEvent.click(viewButtons[0]); expect(onView).toHaveBeenCalledWith("7"); fireEvent.click(viewButtons[1]); expect(onView).toHaveBeenCalledWith("8"); });🤖 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 116 - 123, Add a test in PendingUpdatesList.test.tsx that verifies the onView callback is invoked with the correct levelKey when a "View" button is clicked: render the PendingUpdatesList with a vi.fn() spy for onView, import fireEvent from `@testing-library/react`, locate the "View" buttons (e.g., via screen.getAllByText("View")), fireEvent.click on each button, and assert the spy was called with "7" for the first and "8" for the second; reference the PendingUpdatesList component and the mockData used in the existing tests.apps/customer-portal/webapp/src/utils/__tests__/updateLevelsReportPdf.test.ts (1)
91-103: Date formatting test assertions are very loose.The regex checks (matches any month abbreviation, any digit, any 4-digit year) would pass for nearly any date-like string. Consider asserting the expected formatted value directly once the timezone handling in
formatReleaseDateis settled (e.g., if switched to UTC:expect(result.tableRows[0].releaseDate).toBe("Jan 22, 2024")).🤖 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__/updateLevelsReportPdf.test.ts` around lines 91 - 103, The test uses loose regexes to check releaseDate; replace them with an exact equality assertion against the expected formatted string from formatReleaseDate (e.g., expect(result.tableRows[0].releaseDate).toBe("Jan 22, 2024")) after confirming timezone behavior, by updating the test that calls getUpdateLevelsReportData to assert the precise value returned by formatReleaseDate for the known mockData release timestamp instead of the three loose regex matches.apps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsx (2)
165-178: Inconsistent indentation on theChipelement.The
<Chip>and its props are indented at a different level than the parent<Box>. This makes the JSX harder to read and maintain.Proposed fix
<Box sx={{ display: "flex", justifyContent: "center", width: "100%", }} > - <Chip - label={ - entry.updateType.charAt(0).toUpperCase() + entry.updateType.slice(1) - } - size="small" - sx={{ - height: 22, - fontSize: "0.72rem", - fontWeight: 600, - bgcolor: alpha(theme.palette[chipColor].main, 0.15), - color: theme.palette[chipColor].dark, - border: `1px solid ${alpha(theme.palette[chipColor].main, 0.35)}`, - }} - /> + <Chip + label={ + entry.updateType.charAt(0).toUpperCase() + entry.updateType.slice(1) + } + size="small" + sx={{ + height: 22, + fontSize: "0.72rem", + fontWeight: 600, + bgcolor: alpha(theme.palette[chipColor].main, 0.15), + color: theme.palette[chipColor].dark, + border: `1px solid ${alpha(theme.palette[chipColor].main, 0.35)}`, + }} + /> </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/updates/pending-updates/PendingUpdatesList.tsx` around lines 165 - 178, The JSX for the Chip inside PendingUpdatesList.tsx is indented inconsistently relative to its parent Box; fix by aligning the <Chip ... /> element and all its props to the same indentation level as the other children of the Box in the PendingUpdatesList component so the opening tag, props (label, size, sx, etc.), and closing slash align vertically with sibling elements and match the file's existing JSX indentation style.
58-72: Error state shows only an icon with no explanatory message.When
isErroris true, the user sees only an SVG icon with no text. Consider adding a brief message (e.g., "Failed to load pending updates") so the user understands the state, consistent with theEmptyStatecomponent that includes adescriptionprop.Proposed improvement
<Box sx={{ display: "flex", justifyContent: "center", alignItems: "center", flexDirection: "column", py: 5, }} > <ErrorStateIcon style={{ width: 200, height: "auto" }} /> + <Typography variant="body2" color="text.secondary" sx={{ mt: 2 }}> + Failed to load pending updates. Please try again. + </Typography> </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/updates/pending-updates/PendingUpdatesList.tsx` around lines 58 - 72, The error view in PendingUpdatesList.tsx currently returns only an ErrorStateIcon when isError is true; update the isError return to include a brief explanatory message (e.g., "Failed to load pending updates") alongside the ErrorStateIcon so users understand the problem—either use the existing EmptyState component with a description prop or add a Typography/label next to ErrorStateIcon within the same Box; modify the JSX in the isError branch (referencing isError, ErrorStateIcon, and the Box wrapper) to render both the icon and the descriptive text.apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts (2)
98-104:mixedCountis hardcoded to0with no explanation.If "mixed" is a valid update type that isn't yet supported, add a comment or TODO. If it's never expected, consider removing it from the interface to avoid misleading report consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts` around lines 98 - 104, mixedCount is hardcoded to 0 in updateLevelsReportPdf.ts which is misleading; update the logic around the mixedCount declaration (currently "const mixedCount = 0") to either compute it from entries (e.g., count entries where e.updateType === "mixed") or, if "mixed" is not a valid type, remove mixedCount and any references to it from the reporting code and interface; if you intend to support it later, add a clear TODO comment next to the mixedCount declaration and a unit test covering mixed-type entries.
112-126:appliedis hardcoded to"No"for every row.If applied status isn't yet available from the data source, consider adding a TODO comment to clarify this is a placeholder. Otherwise, the report always shows "No" for the "Applied" column, which may be inaccurate for levels that have already been applied.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts` around lines 112 - 126, The generated rows set applied to the literal "No" inside entries.map when building tableRows, which hides real state; update the mapping in the function that builds tableRows (the entries.map callback / UpdateLevelsReportTableRow construction) to populate applied from the source (e.g., use entry.applied or infer from entry.updateDescriptionLevels/status fields) instead of the hardcoded "No", and if the data truly isn't available yet add a TODO comment next to the applied field assignment indicating it's a placeholder and use a neutral value like "N/A" or undefined until the real property is added.apps/customer-portal/webapp/src/components/updates/all-updates/__tests__/AllUpdatesTab.test.tsx (1)
52-58:useParamsis not mocked —projectIdwill beundefined.The
AllUpdatesTabcomponent usesuseParamsto extractprojectIdfor navigation. WithMemoryRouterand no route definition,useParamsreturns{}. This is fine for the current rendering-only tests, but will cause issues if you add tests for thehandleViewnavigation path. Consider setting up a route path inMemoryRouterif you plan to expand coverage.Example setup with route params
import { MemoryRouter, Route, Routes } from "react-router"; render( <MemoryRouter initialEntries={["/projects/test-project-id/updates"]}> <Routes> <Route path="/projects/:projectId/updates" element={<AllUpdatesTab />} /> </Routes> </MemoryRouter>, );🤖 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/all-updates/__tests__/AllUpdatesTab.test.tsx` around lines 52 - 58, Tests render AllUpdatesTab without providing route params so useParams returns {} and projectId is undefined; fix by ensuring useParams provides a projectId for navigation tests—either wrap the render in a MemoryRouter with a matching Route (use MemoryRouter initialEntries like "/projects/test-project-id/updates" and Routes/Route with path "/projects/:projectId/updates" rendering <AllUpdatesTab />) or mock react-router's useParams to return { projectId: "test-project-id" }; this will allow handleView/navigation logic in AllUpdatesTab to receive a valid projectId.apps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsx (1)
108-116:searchRequestmemo is a redundant pass-through ofsearchParams.The
useMemoon Lines 108–116 just shallow-copiessearchParamsfields, producing an identical object. You could passsearchParamsdirectly tousePostUpdateLevelsSearch:- const searchRequest = useMemo(() => { - if (!searchParams) return null; - return { - productName: searchParams.productName, - productVersion: searchParams.productVersion, - startingUpdateLevel: searchParams.startingUpdateLevel, - endingUpdateLevel: searchParams.endingUpdateLevel, - }; - }, [searchParams]); - - const { data: searchData, isLoading: isSearchLoading, isError: isSearchError } = usePostUpdateLevelsSearch(searchRequest); + const { data: searchData, isLoading: isSearchLoading, isError: isSearchError } = usePostUpdateLevelsSearch(searchParams);The
searchParamsstate already has the right shape (matchingUpdateLevelsSearchRequest), so the intermediate memo adds indirection without value.🤖 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/all-updates/AllUpdatesTab.tsx` around lines 108 - 116, The current useMemo creates a redundant copy named searchRequest from searchParams; remove the useMemo and stop creating searchRequest, instead pass searchParams directly into usePostUpdateLevelsSearch (or its caller) — ensure any null/undefined checks previously guarding searchRequest are applied to searchParams (e.g., if (!searchParams) return null) and update references in AllUpdatesTab (searchRequest → searchParams) so the component uses the existing state directly.apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx (1)
386-399: Error and "level not found" cases share the same state — no message distinguishes them.When
isErroris true (API failure) vs.!entry(level key not in response), the user sees the sameErrorStateIconwith no explanatory text. Consider adding a brief message to help the user (especially for the "not found" case):Suggested improvement
) : isError || !entry ? ( <Box sx={{ display: "flex", justifyContent: "center", alignItems: "center", flexDirection: "column", py: 5, }} > <ErrorStateIcon style={{ width: 200, height: "auto" }} /> + <Typography variant="body2" color="text.secondary" sx={{ mt: 2 }}> + {isError + ? "Failed to load update level details. Please try again." + : "Update level not found."} + </Typography> </Box>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx` around lines 386 - 399, The current JSX branch that renders PendingUpdatesListSkeleton / ErrorStateIcon conflates API errors and missing level data; update the conditional in UpdateLevelDetailsPage.tsx (the ternary that checks isLoading ? ... : isError || !entry ? ...) so it renders distinct UIs: when isError is true render ErrorStateIcon plus a short error message (e.g., "Failed to load updates. Please try again.") and when !entry render ErrorStateIcon plus a clear "Level not found" message and optional action (back/refresh). Locate the block referencing isLoading, isError, entry, PendingUpdatesListSkeleton and ErrorStateIcon and replace the combined branch with separate checks for isError and !entry to display the appropriate user-facing text.apps/customer-portal/webapp/src/components/updates/all-updates/UpdateLevelsReportModal.tsx (1)
61-63: Empty<Dialog>rendered whenreportDatais null.When
reportDataisnullandopenistrue, the user sees a blank dialog. Current callers guard against this (e.g.,handleViewReportinAllUpdatesTabbails early), but the component itself doesn't prevent it. Consider returningnull(or keepingopen={false}) when there's no data, to make the component self-defensive:Suggested hardening
if (!reportData) { - return <Dialog open={open} onClose={onClose} />; + return null; }🤖 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/all-updates/UpdateLevelsReportModal.tsx` around lines 61 - 63, UpdateLevelsReportModal currently renders an empty Dialog when reportData is null; make the component self-defensive by returning null (or rendering the Dialog with open={false}) whenever reportData is falsy so no blank modal appears. Update the render logic inside UpdateLevelsReportModal to check reportData before returning the Dialog (e.g., in the component body where it currently does if (!reportData) return <Dialog open={open} onClose={onClose} />) and replace that with return null (or change to <Dialog open={false} onClose={onClose} />) so callers no longer need to guard against a blank modal. Ensure you reference the existing props open and onClose and keep the rest of the component unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer-portal/webapp/package.json`:
- Around line 29-30: The dependency constraint for jspdf in package.json is
pinned to the vulnerable 2.x series; update the "jspdf" version specifier from
"^2.5.2" to "^4.2.0" (or latest 4.x) in package.json, run your package manager
to update the lockfile (npm install / yarn install), ensure jspdf-autotable
compatibility (upgrade it if needed) and run the app/test suite to verify no
regressions from the breaking changes in the jspdf 4.x release.
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/__tests__/UpdateLevelsReportModal.test.tsx`:
- Around line 61-71: UpdateLevelsReportModal currently renders an empty Dialog
when reportData is null, causing a blank modal overlay; modify
UpdateLevelsReportModal to early-return null (or alternatively render <Dialog
open={false} onClose={onClose} ...>) whenever reportData is null so no empty
dialog appears while data is loading, and keep the prop names open, reportData
and onClose intact so existing callers/tests like the test in
UpdateLevelsReportModal.test.tsx still work.
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsx`:
- Around line 174-184: The handler handleSearch currently treats 0 as falsy by
checking "!start || !end", which blocks valid level 0 selections; change the
guard to explicitly validate numeric presence instead of falsy-ness — e.g.
ensure productName and productVersion are present and that start and end are
valid numbers (use Number.isFinite or isNaN checks) and non-negative, and only
then call setSearchParams with startingUpdateLevel and endingUpdateLevel; update
the validation in handleSearch (and consider referencing filter.startLevel /
filter.endLevel and startLevelOptions) so selecting 0 is accepted.
In `@apps/customer-portal/webapp/src/pages/PendingUpdatesPage.tsx`:
- Around line 39-52: The useMemo for searchRequest treats startingUpdateLevel
and endingUpdateLevel as falsy (via !startingUpdateLevel), which rejects
legitimate 0 values; update the condition in the useMemo to explicitly check the
raw search params and the numeric parse for validity: ensure productName and
productBaseVersion remain checked for truthiness, but replace the
!startingUpdateLevel/!endingUpdateLevel checks with explicit checks that
searchParams.get("startingUpdateLevel") and
searchParams.get("endingUpdateLevel") are not null (or use searchParams.has) and
that the parsed Numbers (startingUpdateLevel and endingUpdateLevel) are valid
numbers (e.g., not NaN) so 0 is accepted; adjust the return null logic inside
the useMemo accordingly (referencing searchRequest, startingUpdateLevel,
endingUpdateLevel, productName, productBaseVersion, and searchParams).
In `@apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx`:
- Around line 300-308: The guard in the useMemo that builds searchRequest
wrongly treats zero as falsy (using !startingUpdateLevel || !endingUpdateLevel),
so update levels of 0 get rejected; instead check presence of the query params
before converting to Number (e.g. test searchParams.get("startingUpdateLevel")
!== null and searchParams.get("endingUpdateLevel") !== null) or explicitly
compare to undefined/null, and keep startingUpdateLevel and endingUpdateLevel as
the numeric values used in the returned object in the useMemo that defines
searchRequest (refer to startingUpdateLevel, endingUpdateLevel, and the useMemo
creating searchRequest).
- Around line 155-187: The bugFixes map uses each fix string directly as an
anchor href (variable fix in the JSX) which can allow javascript: or data: URIs;
add a URL-scheme validation helper (e.g., isSafeUrl(url) that returns true only
for /^https?:\/\//i) and use it when rendering: for each fix, if isSafeUrl(fix)
render the existing <Box component="a" href={fix} ...>, otherwise render the
value as plain text (or a non-clickable element) to avoid creating an unsafe
link; update the mapping/filter logic around bugFixes and the fix variable
accordingly so only validated URLs become anchors.
In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts`:
- Around line 58-68: formatReleaseDate currently constructs a Date and uses
local-time methods (getMonth/getDate/getFullYear), causing inconsistent output
across timezones; update the function (formatReleaseDate) to use the UTC
equivalents (getUTCMonth, getUTCDate, getUTCFullYear) so the formatted release
date is consistent for all users (also confirm ts is milliseconds vs seconds
before changing units).
- Around line 203-211: The footer is being written inside didDrawPage using
doc.getNumberOfPages(), which yields the current page count so each page shows
"Page X of X" incorrectly; after calling autoTable() (or after all drawing
completes) iterate doc.getNumberOfPages() and add the footer to every page using
doc.setPage(pageIndex) so you can write "Page {i} of {total}" correctly, or
implement jsPDF's placeholder pattern with doc.putTotalPages() (use didDrawPage
only to write "Page {current} of {total}" placeholder and call
doc.putTotalPages(total) after generating the document); update references in
the file to didDrawPage, autoTable(), doc.getNumberOfPages(), and
doc.putTotalPages() accordingly.
In `@apps/customer-portal/webapp/src/utils/updates.ts`:
- Around line 25-35: The JSDoc for getUpdateTypeChipColor is incorrect: it says
non-security/"regular" maps to "info" (blue) but the function returns "success"
(green); update the JSDoc comment above getUpdateTypeChipColor to describe the
actual behavior (e.g., state that "security" -> "error" (red) and all other
types -> "success" (green) and adjust the `@returns` description to StatCardColor
"success") so the documentation matches the implementation.
---
Nitpick comments:
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/__tests__/AllUpdatesTab.test.tsx`:
- Around line 52-58: Tests render AllUpdatesTab without providing route params
so useParams returns {} and projectId is undefined; fix by ensuring useParams
provides a projectId for navigation tests—either wrap the render in a
MemoryRouter with a matching Route (use MemoryRouter initialEntries like
"/projects/test-project-id/updates" and Routes/Route with path
"/projects/:projectId/updates" rendering <AllUpdatesTab />) or mock
react-router's useParams to return { projectId: "test-project-id" }; this will
allow handleView/navigation logic in AllUpdatesTab to receive a valid projectId.
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsx`:
- Around line 108-116: The current useMemo creates a redundant copy named
searchRequest from searchParams; remove the useMemo and stop creating
searchRequest, instead pass searchParams directly into usePostUpdateLevelsSearch
(or its caller) — ensure any null/undefined checks previously guarding
searchRequest are applied to searchParams (e.g., if (!searchParams) return null)
and update references in AllUpdatesTab (searchRequest → searchParams) so the
component uses the existing state directly.
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/UpdateLevelsReportModal.tsx`:
- Around line 61-63: UpdateLevelsReportModal currently renders an empty Dialog
when reportData is null; make the component self-defensive by returning null (or
rendering the Dialog with open={false}) whenever reportData is falsy so no blank
modal appears. Update the render logic inside UpdateLevelsReportModal to check
reportData before returning the Dialog (e.g., in the component body where it
currently does if (!reportData) return <Dialog open={open} onClose={onClose} />)
and replace that with return null (or change to <Dialog open={false}
onClose={onClose} />) so callers no longer need to guard against a blank modal.
Ensure you reference the existing props open and onClose and keep the rest of
the component unchanged.
In
`@apps/customer-portal/webapp/src/components/updates/pending-updates/__tests__/PendingUpdatesList.test.tsx`:
- Around line 116-123: Add a test in PendingUpdatesList.test.tsx that verifies
the onView callback is invoked with the correct levelKey when a "View" button is
clicked: render the PendingUpdatesList with a vi.fn() spy for onView, import
fireEvent from `@testing-library/react`, locate the "View" buttons (e.g., via
screen.getAllByText("View")), fireEvent.click on each button, and assert the spy
was called with "7" for the first and "8" for the second; reference the
PendingUpdatesList component and the mockData used in the existing tests.
In
`@apps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsx`:
- Around line 165-178: The JSX for the Chip inside PendingUpdatesList.tsx is
indented inconsistently relative to its parent Box; fix by aligning the <Chip
... /> element and all its props to the same indentation level as the other
children of the Box in the PendingUpdatesList component so the opening tag,
props (label, size, sx, etc.), and closing slash align vertically with sibling
elements and match the file's existing JSX indentation style.
- Around line 58-72: The error view in PendingUpdatesList.tsx currently returns
only an ErrorStateIcon when isError is true; update the isError return to
include a brief explanatory message (e.g., "Failed to load pending updates")
alongside the ErrorStateIcon so users understand the problem—either use the
existing EmptyState component with a description prop or add a Typography/label
next to ErrorStateIcon within the same Box; modify the JSX in the isError branch
(referencing isError, ErrorStateIcon, and the Box wrapper) to render both the
icon and the descriptive text.
In `@apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx`:
- Around line 386-399: The current JSX branch that renders
PendingUpdatesListSkeleton / ErrorStateIcon conflates API errors and missing
level data; update the conditional in UpdateLevelDetailsPage.tsx (the ternary
that checks isLoading ? ... : isError || !entry ? ...) so it renders distinct
UIs: when isError is true render ErrorStateIcon plus a short error message
(e.g., "Failed to load updates. Please try again.") and when !entry render
ErrorStateIcon plus a clear "Level not found" message and optional action
(back/refresh). Locate the block referencing isLoading, isError, entry,
PendingUpdatesListSkeleton and ErrorStateIcon and replace the combined branch
with separate checks for isError and !entry to display the appropriate
user-facing text.
In
`@apps/customer-portal/webapp/src/utils/__tests__/updateLevelsReportPdf.test.ts`:
- Around line 91-103: The test uses loose regexes to check releaseDate; replace
them with an exact equality assertion against the expected formatted string from
formatReleaseDate (e.g., expect(result.tableRows[0].releaseDate).toBe("Jan 22,
2024")) after confirming timezone behavior, by updating the test that calls
getUpdateLevelsReportData to assert the precise value returned by
formatReleaseDate for the known mockData release timestamp instead of the three
loose regex matches.
In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts`:
- Around line 98-104: mixedCount is hardcoded to 0 in updateLevelsReportPdf.ts
which is misleading; update the logic around the mixedCount declaration
(currently "const mixedCount = 0") to either compute it from entries (e.g.,
count entries where e.updateType === "mixed") or, if "mixed" is not a valid
type, remove mixedCount and any references to it from the reporting code and
interface; if you intend to support it later, add a clear TODO comment next to
the mixedCount declaration and a unit test covering mixed-type entries.
- Around line 112-126: The generated rows set applied to the literal "No" inside
entries.map when building tableRows, which hides real state; update the mapping
in the function that builds tableRows (the entries.map callback /
UpdateLevelsReportTableRow construction) to populate applied from the source
(e.g., use entry.applied or infer from entry.updateDescriptionLevels/status
fields) instead of the hardcoded "No", and if the data truly isn't available yet
add a TODO comment next to the applied field assignment indicating it's a
placeholder and use a neutral value like "N/A" or undefined until the real
property is added.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/customer-portal/webapp/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
apps/customer-portal/webapp/package.jsonapps/customer-portal/webapp/src/App.tsxapps/customer-portal/webapp/src/api/usePostCreateDeployment.tsapps/customer-portal/webapp/src/api/usePostUpdateLevelsSearch.tsapps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsxapps/customer-portal/webapp/src/components/updates/all-updates/UpdateLevelsReportModal.tsxapps/customer-portal/webapp/src/components/updates/all-updates/__tests__/AllUpdatesTab.test.tsxapps/customer-portal/webapp/src/components/updates/all-updates/__tests__/UpdateLevelsReportModal.test.tsxapps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsxapps/customer-portal/webapp/src/components/updates/pending-updates/__tests__/PendingUpdatesList.test.tsxapps/customer-portal/webapp/src/components/updates/update-cards/UpdateProductGrid.tsxapps/customer-portal/webapp/src/constants/apiConstants.tsapps/customer-portal/webapp/src/layouts/AppLayout.tsxapps/customer-portal/webapp/src/models/requests.tsapps/customer-portal/webapp/src/models/responses.tsapps/customer-portal/webapp/src/pages/PendingUpdatesPage.tsxapps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsxapps/customer-portal/webapp/src/pages/UpdatesPage.tsxapps/customer-portal/webapp/src/utils/__tests__/updateLevelsReportPdf.test.tsapps/customer-portal/webapp/src/utils/updateLevelsReportPdf.tsapps/customer-portal/webapp/src/utils/updates.ts
Enhance validation, safety, and UX across update-related components and utilities: - AllUpdatesTab: strengthen input validation for start/end levels (empty, non-numeric, negative, ordering) and compute canSearch reliably. - UpdateLevelsReportModal: avoid rendering an empty Dialog when no reportData (return null instead). - Tests: mock useParams in AllUpdatesTab tests and update updateLevelsReportPdf test to assert UTC-formatted dates; add a view-button interaction test in PendingUpdatesList tests. - PendingUpdatesList: show an error message when loading fails and fix Chip rendering/formatting; add onView click handling test. - PendingUpdatesPage & UpdateLevelDetailsPage: robustly handle search params (distinguish missing params vs default 0, guard against NaN) and update useMemo deps. - UpdateLevelDetailsPage: add isSafeUrl helper and render non-http(s) links as plain text; improve UI states to differentiate loading/error/not-found with messages. - updateLevelsReportPdf: format release dates using UTC, count "mixed" entries properly, set applied to "N/A" (placeholder), and render page footers by iterating pages (ensures correct page numbering). Updated test names/expectations accordingly. - updates.ts: update docstring to map security->error and other types->success (green) for chip color. These changes fix validation edge cases, improve accessibility/safety for external links, provide clearer UI error states, and ensure deterministic PDF date formatting and pagination.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts (2)
58-68: UTC date formatting correctly applied.Past review comment addressed:
formatReleaseDatenow usesgetUTCMonth,getUTCDate, andgetUTCFullYear, ensuring timezone-consistent output across all users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts` around lines 58 - 68, formatReleaseDate already uses UTC getters (getUTCMonth, getUTCDate, getUTCFullYear) so no change is required; keep the current implementation in function formatReleaseDate to ensure timezone-consistent formatting across users.
206-215: Footer page numbering now correct.Past review comment addressed: footer text is stamped via post-processing loop after
autoTable()completes, sogetNumberOfPages()reflects the final total and each page correctly reads "Page i of N".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts` around lines 206 - 215, Footer stamping must occur after the autoTable pagination completes so page counts are final; ensure the post-processing loop uses doc.getNumberOfPages(), iterates pages with doc.setPage(i), sets font with doc.setFontSize(8), and writes the footer text via doc.text using reportData.productName and reportData.productVersion to produce "Update Levels Report Page i of N" on each page—move or keep this block after any autoTable() calls so the totalPages value is correct.apps/customer-portal/webapp/src/pages/PendingUpdatesPage.tsx (1)
44-68: Falsy-on-zero fix correctly applied.Replacing
!startingUpdateLevelwith explicitstartParam === nullandNumber.isNaN()checks addresses the prior review comment and handles0as a valid update level.🤖 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 44 - 68, The previous falsy-on-zero bug in PendingUpdatesPage's searchRequest has to be fixed by replacing any checks like !startingUpdateLevel/!endingUpdateLevel with explicit null/NaN checks: return null if startParam === null or endParam === null or Number.isNaN(startingUpdateLevel) or Number.isNaN(endingUpdateLevel); ensure the useMemo for searchRequest (variable name searchRequest) uses productName, productBaseVersion, startParam, endParam, startingUpdateLevel and endingUpdateLevel in its dependency array and update any other code paths that still rely on falsy checks for startingUpdateLevel/endingUpdateLevel to use the explicit checks instead.
🧹 Nitpick comments (4)
apps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsx (2)
108-116:searchRequestuseMemo is an unnecessary copy ofsearchParams.The local
searchParamsstate already has the same shape asUpdateLevelsSearchRequest. Passing it directly tousePostUpdateLevelsSearcheliminates the intermediary.♻️ Proposed simplification
-const searchRequest = useMemo(() => { - if (!searchParams) return null; - return { - productName: searchParams.productName, - productVersion: searchParams.productVersion, - startingUpdateLevel: searchParams.startingUpdateLevel, - endingUpdateLevel: searchParams.endingUpdateLevel, - }; -}, [searchParams]); - -const { data: searchData, isLoading: isSearchLoading, isError: isSearchError } = usePostUpdateLevelsSearch(searchRequest); +const { data: searchData, isLoading: isSearchLoading, isError: isSearchError } = usePostUpdateLevelsSearch(searchParams);🤖 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/all-updates/AllUpdatesTab.tsx` around lines 108 - 116, The memoized searchRequest is redundant because searchParams already matches UpdateLevelsSearchRequest; remove the useMemo and pass searchParams directly into usePostUpdateLevelsSearch (or wherever searchRequest was used) in AllUpdatesTab (replace references to searchRequest with searchParams), keeping the existing null/undefined guard (i.e., only call/use the hook when searchParams is present) so behavior and types remain unchanged.
174-195: Validation logic is duplicated betweenhandleSearchandcanSearch.Both perform identical checks. If the validation rules change, both sites need updating. Consider extracting a shared
isValidFilterhelper to keep them in sync.♻️ Suggested refactor
+function isValidFilter( + filter: AllUpdatesTabFilterState, +): filter is AllUpdatesTabFilterState & { startLevel: string; endLevel: string } { + const start = Number(filter.startLevel); + const end = Number(filter.endLevel); + return ( + !!filter.productName && + !!filter.productVersion && + filter.startLevel !== "" && + filter.endLevel !== "" && + Number.isFinite(start) && + Number.isFinite(end) && + start >= 0 && + end >= 0 && + start <= end + ); +} const handleSearch = useCallback(() => { - const start = Number(filter.startLevel); - const end = Number(filter.endLevel); - if ( - !filter.productName || ... - ) return; + if (!isValidFilter(filter)) return; + const start = Number(filter.startLevel); + const end = Number(filter.endLevel); setSearchParams({ ... }); }, [filter]); -const startNum = Number(filter.startLevel); -const endNum = Number(filter.endLevel); -const canSearch = - !!filter.productName && ... && startNum <= endNum; +const canSearch = isValidFilter(filter);Also applies to: 231-242
🤖 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/all-updates/AllUpdatesTab.tsx` around lines 174 - 195, Extract the duplicated validation into a single helper like isValidFilter(filter) that performs the Number conversion for start/end and all checks (productName, productVersion, startLevel/endLevel non-empty, finite numbers, non-negative, start <= end); replace the inline checks in handleSearch and in canSearch to call isValidFilter(filter) and, in handleSearch, after isValidFilter returns true convert start/end to Number and call setSearchParams with startingUpdateLevel/endingUpdateLevel; ensure any useCallback/useMemo deps (e.g., handleSearch) include filter or the new helper as needed.apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx (2)
361-365:filterButtonscan be a module-level constant.This array doesn't depend on component state or props, so it's recreated on every render unnecessarily.
♻️ Proposed fix
+const FILTER_BUTTONS: { key: FilterType; label: string }[] = [ + { key: "all", label: "All" }, + { key: "security", label: "Security" }, + { key: "regular", label: "Regular" }, +]; + export default function UpdateLevelDetailsPage(): JSX.Element { ... - const filterButtons: { key: FilterType; label: string }[] = [ - { key: "all", label: "All" }, - { key: "security", label: "Security" }, - { key: "regular", label: "Regular" }, - ]; + // use FILTER_BUTTONS directly in JSX🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx` around lines 361 - 365, Move the filterButtons array out of the component and declare it as a module-level constant to avoid recreating it on each render; locate the current in-component declaration named filterButtons (typed as { key: FilterType; label: string }[]) inside UpdateLevelDetailsPage and extract it to top-level scope in the file, keeping the same type annotation and values ("all","security","regular") and then reference the module-level filterButtons inside the component where it’s used.
44-63: JSDoc block forparseJsonStringArrayis misplaced aboveisSafeUrl.The multi-line JSDoc at lines 44–50 describes
parseJsonStringArraybut is positioned immediately beforeisSafeUrl.parseJsonStringArray(line 56) ends up with no docstring.♻️ Proposed fix
-/** - * Parses a JSON-stringified array string into a plain string array. - * Returns empty array on parse failure. - * - * `@param` {string} raw - The raw JSON string. - * `@returns` {string[]} Parsed string items. - */ /** Returns true only for http/https URLs to prevent javascript: or data: URIs. */ function isSafeUrl(url: string): boolean { return /^https?:\/\//i.test(url.trim()); } +/** + * Parses a JSON-stringified array string into a plain string array. + * Returns empty array on parse failure. + * + * `@param` {string} raw - The raw JSON string. + * `@returns` {string[]} Parsed string items. + */ function parseJsonStringArray(raw: string): string[] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx` around lines 44 - 63, The JSDoc for parseJsonStringArray is placed above isSafeUrl; move the multi-line JSDoc block so it directly precedes the parseJsonStringArray function declaration (or duplicate it above parseJsonStringArray and remove/replace the misplaced block above isSafeUrl) ensuring parseJsonStringArray has its descriptive JSDoc and isSafeUrl keeps its single-line comment; update the comment placement around the functions isSafeUrl and parseJsonStringArray so each function has the correct documentation block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/UpdateLevelsReportModal.tsx`:
- Around line 51-63: The component UpdateLevelsReportModal currently declares
its return type as JSX.Element but returns null when reportData is missing;
update the function signature (export default function
UpdateLevelsReportModal(...): JSX.Element | null) so the return type allows null
(or use React.ReactElement | null) to satisfy strict TypeScript and keep the
existing early-return behavior.
In
`@apps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsx`:
- Around line 79-80: The summary calculation ignores entries with updateType ===
"mixed", causing counts to not add up; update the logic that computes
securityCount and regularCount (references: securityCount, regularCount,
entries, updateType) and the JSX summary that uses them so it accounts for mixed
entries—either compute mixedCount = entries.filter(([, e]) => e.updateType ===
"mixed").length and include it in the displayed sentence or derive regularCount
as entries.length - securityCount - mixedCount so the total matches
entries.length; update all places using the old counts (including the JSX around
line ranges ~79 and ~90-94) to include the mixed case.
---
Duplicate comments:
In `@apps/customer-portal/webapp/src/pages/PendingUpdatesPage.tsx`:
- Around line 44-68: The previous falsy-on-zero bug in PendingUpdatesPage's
searchRequest has to be fixed by replacing any checks like
!startingUpdateLevel/!endingUpdateLevel with explicit null/NaN checks: return
null if startParam === null or endParam === null or
Number.isNaN(startingUpdateLevel) or Number.isNaN(endingUpdateLevel); ensure the
useMemo for searchRequest (variable name searchRequest) uses productName,
productBaseVersion, startParam, endParam, startingUpdateLevel and
endingUpdateLevel in its dependency array and update any other code paths that
still rely on falsy checks for startingUpdateLevel/endingUpdateLevel to use the
explicit checks instead.
In `@apps/customer-portal/webapp/src/utils/updateLevelsReportPdf.ts`:
- Around line 58-68: formatReleaseDate already uses UTC getters (getUTCMonth,
getUTCDate, getUTCFullYear) so no change is required; keep the current
implementation in function formatReleaseDate to ensure timezone-consistent
formatting across users.
- Around line 206-215: Footer stamping must occur after the autoTable pagination
completes so page counts are final; ensure the post-processing loop uses
doc.getNumberOfPages(), iterates pages with doc.setPage(i), sets font with
doc.setFontSize(8), and writes the footer text via doc.text using
reportData.productName and reportData.productVersion to produce "Update Levels
Report Page i of N" on each page—move or keep this block after any autoTable()
calls so the totalPages value is correct.
---
Nitpick comments:
In
`@apps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsx`:
- Around line 108-116: The memoized searchRequest is redundant because
searchParams already matches UpdateLevelsSearchRequest; remove the useMemo and
pass searchParams directly into usePostUpdateLevelsSearch (or wherever
searchRequest was used) in AllUpdatesTab (replace references to searchRequest
with searchParams), keeping the existing null/undefined guard (i.e., only
call/use the hook when searchParams is present) so behavior and types remain
unchanged.
- Around line 174-195: Extract the duplicated validation into a single helper
like isValidFilter(filter) that performs the Number conversion for start/end and
all checks (productName, productVersion, startLevel/endLevel non-empty, finite
numbers, non-negative, start <= end); replace the inline checks in handleSearch
and in canSearch to call isValidFilter(filter) and, in handleSearch, after
isValidFilter returns true convert start/end to Number and call setSearchParams
with startingUpdateLevel/endingUpdateLevel; ensure any useCallback/useMemo deps
(e.g., handleSearch) include filter or the new helper as needed.
In `@apps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsx`:
- Around line 361-365: Move the filterButtons array out of the component and
declare it as a module-level constant to avoid recreating it on each render;
locate the current in-component declaration named filterButtons (typed as { key:
FilterType; label: string }[]) inside UpdateLevelDetailsPage and extract it to
top-level scope in the file, keeping the same type annotation and values
("all","security","regular") and then reference the module-level filterButtons
inside the component where it’s used.
- Around line 44-63: The JSDoc for parseJsonStringArray is placed above
isSafeUrl; move the multi-line JSDoc block so it directly precedes the
parseJsonStringArray function declaration (or duplicate it above
parseJsonStringArray and remove/replace the misplaced block above isSafeUrl)
ensuring parseJsonStringArray has its descriptive JSDoc and isSafeUrl keeps its
single-line comment; update the comment placement around the functions isSafeUrl
and parseJsonStringArray so each function has the correct documentation block.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/customer-portal/webapp/src/components/updates/all-updates/AllUpdatesTab.tsxapps/customer-portal/webapp/src/components/updates/all-updates/UpdateLevelsReportModal.tsxapps/customer-portal/webapp/src/components/updates/all-updates/__tests__/AllUpdatesTab.test.tsxapps/customer-portal/webapp/src/components/updates/pending-updates/PendingUpdatesList.tsxapps/customer-portal/webapp/src/components/updates/pending-updates/__tests__/PendingUpdatesList.test.tsxapps/customer-portal/webapp/src/pages/PendingUpdatesPage.tsxapps/customer-portal/webapp/src/pages/UpdateLevelDetailsPage.tsxapps/customer-portal/webapp/src/utils/__tests__/updateLevelsReportPdf.test.tsapps/customer-portal/webapp/src/utils/updateLevelsReportPdf.tsapps/customer-portal/webapp/src/utils/updates.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/customer-portal/webapp/src/utils/updates.ts
- apps/customer-portal/webapp/src/utils/tests/updateLevelsReportPdf.test.ts
Bump jspdf to ^4.2.0 and adapt code accordingly. Add isValidFilter helper and simplify search request logic in AllUpdatesTab; use the helper for validation and canSearch logic. Make UpdateLevelsReportModal return null when reportData is missing and update its doc comment. Show mixed update counts in PendingUpdatesList and adjust tests. Move FILTER_BUTTONS and consolidate isSafeUrl in UpdateLevelDetailsPage. Remove an obsolete TODO comment in updateLevelsReportPdf.
85323bd
into
wso2-open-operations:customer-portal-milestone-1
Description
This pull request adds support for generating PDFs and working with tables in the customer portal webapp by introducing the
jspdfandjspdf-autotablelibraries. To support these features, a number of new dependencies and their transitive dependencies have been added to the project. No application code changes are present in this PR; all changes are related to dependency management.Dependency Additions and Updates:
Main PDF and Table Libraries:
jspdfandjspdf-autotabletopackage.jsonand locked their versions inpnpm-lock.yamlto enable PDF generation and table support in the webapp. [1] [2] [3] [4]Direct and Transitive Dependencies for PDF Generation:
jspdfand related libraries, includingfast-png,fflate,pako,canvg,html2canvas,css-line-break,svg-pathdata,text-segmentation,stackblur-canvas,rgbcolor,performance-now,raf,regenerator-runtime,iobuffer,utrie, andbase64-arraybuffer. These support image processing, table rendering, and PDF features. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17]Type Declarations:
@types/pakoand@types/raf, to support TypeScript development. [1] [2] [3]Snapshot and Lockfile Updates:
pnpm-lock.yamlsnapshots section to reflect all new and updated dependencies, ensuring reproducible builds. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]No changes were made to application logic; this PR is purely about adding and locking dependencies to enable future PDF and table export features.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
UI/UX Improvements