Repository navigation
[Customer Portal][Web] Enhance Deployments: Product Deletion Workflow, Add/Edit Improvements & UI Refinements - #291
Conversation
Add support for product descriptions and a delete flow for deployment products. Introduces a new DeleteProductModal component and integrates it into the product list; deletion is implemented as a PATCH setting active: false via usePatchDeploymentProduct and invalidates deployment product queries. AddProductModal and ManageProductModal now include an editable description field and pass description in requests. EditDeploymentModal and ManageProductModal were updated to perform PATCH-style updates that only send changed fields. Also update request models to include description and active, adjust UI (icons, skeletons, loading states) and minor layout refinements.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds editable description fields across product add/manage/edit flows, implements a delete-confirmation modal and patch-based deactivation for deployment products, extends request models with Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant List as "DeploymentProductList\n(manages delete state)"
participant Modal as "DeleteProductModal\n(confirmation)"
participant API as "usePatchDeploymentProduct\n(API)"
participant Cache as "Query Cache"
User->>List: Click delete on product row
List->>List: set productToDelete, open modal
List->>Modal: render modal (open=true, product)
User->>Modal: Click Confirm
Modal->>List: invoke onConfirm
List->>List: set isDeleting=true
List->>API: PATCH /deployment/products/{id} { active: false }
API-->>List: 200 OK
List->>Cache: invalidate deployment products query
Cache-->>List: query refreshed
List->>List: clear delete state, close modal
User->>List: sees updated product list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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: 5
🧹 Nitpick comments (2)
apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx (2)
166-173: Remove duplicate query invalidation after delete mutation.
usePatchDeploymentProductalready invalidates[ApiQueryKeys.DEPLOYMENT_PRODUCTS, deploymentId]on success, so the extra invalidation here can trigger redundant refetches.♻️ Proposed refactor
await patchProduct.mutateAsync({ deploymentId, productId: productToDelete.id, body: { active: false }, }); - queryClient.invalidateQueries({ - queryKey: [ApiQueryKeys.DEPLOYMENT_PRODUCTS, deploymentId], - }); setDeleteModalOpen(false); setProductToDelete(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/project-details/deployments/DeploymentProductList.tsx` around lines 166 - 173, The code is performing a redundant cache invalidation: after calling patchProduct.mutateAsync (from usePatchDeploymentProduct) the component calls queryClient.invalidateQueries([ApiQueryKeys.DEPLOYMENT_PRODUCTS, deploymentId]) even though the hook already invalidates that key on success; remove the explicit queryClient.invalidateQueries call (the call site referencing queryClient.invalidateQueries and ApiQueryKeys.DEPLOYMENT_PRODUCTS with deploymentId) so the hook’s built-in success handler manages refetches and avoid duplicate refetches.
150-154: Use full skeleton state for background refetches too.Please include
isFetchingin the loading condition so the list and count use skeletons during background refreshes as well.♻️ Proposed refactor
const { data: products = [], isLoading, + isFetching, isError, } = useGetDeploymentsProducts(deploymentId); + const showLoading = isLoading || isFetching; ... - {isLoading ? ( + {showLoading ? ( <Skeleton ... - {isLoading ? ( + {showLoading ? ( <ProductsSkeleton />Based on learnings: In the customer-portal webapp, the team prefers full skeleton loading states during background refetches (using
isLoading || isFetching) to provide clear visual feedback that the table is updating.Also applies to: 191-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx` around lines 150 - 154, In DeploymentProductList, the loading condition currently only checks isLoading from useGetDeploymentsProducts(deploymentId); update the condition to include isFetching as well (i.e., use isLoading || isFetching) so the list, count and skeleton UI render during background refetches; apply the same change to the other similar block referenced around the later product list code (the second useGetDeploymentsProducts usage between lines 191–216) so both places use isLoading || isFetching to drive the full skeleton state.
🤖 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/project-details/deployments/AddProductModal.tsx`:
- Around line 108-112: The useEffect that calls setTimeout to reset form state
is redundant because handleClose already resets the form synchronously; remove
the entire effect block (the useEffect that references open and calls
setTimeout(() => setForm(INITIAL_FORM), 0)) so you no longer schedule a deferred
setForm; keep handleClose as the single source of truth for resetting the form
(references: useEffect, setTimeout, setForm, INITIAL_FORM, handleClose).
- Around line 149-150: The numeric fields cores and tps can become NaN when
casting invalid input (e.g., whitespace) with Number(form.cores) in
AddProductModal.tsx; update the payload construction in the submit/building
function (where cores and tps are set) to parse and validate the values (e.g.,
use Number(value) or parseInt/parseFloat, then check Number.isFinite or
Number.isNaN) and only include them if they are valid numbers, otherwise set
them to undefined so the PostDeploymentProductRequest types are preserved;
locate the assignment to cores/tps in the AddProductModal component and replace
the direct Number(...) casts with a small guard that returns undefined on NaN.
In
`@apps/customer-portal/webapp/src/components/project-details/deployments/EditDeploymentModal.tsx`:
- Around line 135-141: The code currently coerces an empty form.typeKey to 0 via
Number(form.typeKey) causing an unintended body.typeKey = 0; change the logic to
first detect an empty/null/undefined form.typeKey and only convert and assign
when it's present: compute newTypeKey only when form.typeKey is non-empty (e.g.,
!== "" && != null), compare that to deployment.type?.id (originalTypeKey) and
set body.typeKey only when form.typeKey was provided and the numeric value
differs; ensure when form.typeKey is empty you leave body.typeKey untouched to
avoid sending typeKey: 0 in the PATCH.
In
`@apps/customer-portal/webapp/src/components/project-details/deployments/ManageProductModal.tsx`:
- Around line 128-132: The current logic in ManageProductModal.tsx uses
description.trim() || undefined for newDescription which prevents sending an
explicit empty string to clear an existing description; change the logic so
newDescription is description.trim() (use '' for empty) and compare against
originalDescription normalized to '' (e.g., originalDescription =
product.description || '') and, if different, set body.description =
newDescription (so an empty string will be included in the PATCH payload to
clear the description rather than omitted).
- Around line 114-126: The patch is sending invalid numeric values because
Number(cores) and Number(tps) can yield NaN/Infinity; add a small helper (e.g.,
validateFiniteNonNegative) and use it to parse/validate cores and tps before
assigning to body: call validateFiniteNonNegative(cores) and
validateFiniteNonNegative(tps) to return either a finite non-negative number or
undefined, compare those results against product.cores/product.tps, and only set
body.cores/body.tps when the validated value differs; reference the existing
cores, tps, product.cores, product.tps, and body variables in
ManageProductModal.tsx.
---
Nitpick comments:
In
`@apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx`:
- Around line 166-173: The code is performing a redundant cache invalidation:
after calling patchProduct.mutateAsync (from usePatchDeploymentProduct) the
component calls queryClient.invalidateQueries([ApiQueryKeys.DEPLOYMENT_PRODUCTS,
deploymentId]) even though the hook already invalidates that key on success;
remove the explicit queryClient.invalidateQueries call (the call site
referencing queryClient.invalidateQueries and ApiQueryKeys.DEPLOYMENT_PRODUCTS
with deploymentId) so the hook’s built-in success handler manages refetches and
avoid duplicate refetches.
- Around line 150-154: In DeploymentProductList, the loading condition currently
only checks isLoading from useGetDeploymentsProducts(deploymentId); update the
condition to include isFetching as well (i.e., use isLoading || isFetching) so
the list, count and skeleton UI render during background refetches; apply the
same change to the other similar block referenced around the later product list
code (the second useGetDeploymentsProducts usage between lines 191–216) so both
places use isLoading || isFetching to drive the full skeleton state.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/customer-portal/webapp/src/components/project-details/deployments/AddProductModal.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeleteProductModal.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsxapps/customer-portal/webapp/src/components/project-details/deployments/EditDeploymentModal.tsxapps/customer-portal/webapp/src/components/project-details/deployments/ManageProductModal.tsxapps/customer-portal/webapp/src/models/requests.ts
Introduce a validateFiniteNonNegative helper and use it when parsing product cores and TPS to ensure only finite non-negative numbers (or undefined) are PATCHed; normalize description comparison to trim both sides and avoid sending empty strings. Add an effect to reset pagination when time-tracking filters change. Remove a noisy console.error in change-requests export handling and add an eslint comment around the setState call. Add @deployments path alias to tsconfig and Vite for easier imports of the deployments components. Also include minor formatting/import cleanup.
Update deployment card, lists and edit modal to improve UX and correctness. Key changes:
- DeploymentCard: use formatProjectDateTime, show created/updated timestamps, render deployment type as a Chip, tighten layout and switch some local imports to shared deployment aliases.
- DeploymentDocumentList & DeploymentProductList: change action buttons to contained style for prominence; add product skeleton placeholder and use isFetching alongside isLoading; show combined loading state where appropriate.
- DeploymentProductList: adjust display labels ("Cores:" / "TPS:"), remove an unnecessary ErrorIndicator, and stop manual query invalidation after delete (relying on built-in cache behavior).
- EditDeploymentModal: add prevDeploymentIdRef to reset/init form only when deployment changes or modal closes, move form reset into close handling, and make typeKey handling robust to empty values to avoid sending invalid payloads.
Overall these changes improve visual consistency, loading feedback, and prevent unnecessary updates when editing deployments.
Add a parseValidNumber helper to reliably parse/validate numeric string inputs and use it for cores/tps in the add-product payload. Remove the effect that reset the add-product form on modal close. In DeleteProductModal, compute a displayName that includes the product version (handles version as object or string) and use it in the confirmation message.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentCard.tsx (1)
56-57: Consider handling empty date strings in the display.If
createdOnorupdatedOnis missing/invalid,formatProjectDateTimereturns an empty string, which would render as "Created on • Updated on " with awkward spacing. Consider providing a fallback.Optional: Add fallback for missing dates
- const createdAtStr = formatProjectDateTime(createdOn); - const updatedAtStr = formatProjectDateTime(updatedOn); + const createdAtStr = formatProjectDateTime(createdOn) || "N/A"; + const updatedAtStr = formatProjectDateTime(updatedOn) || "N/A";Also applies to: 180-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentCard.tsx` around lines 56 - 57, The UI currently displays empty strings when formatProjectDateTime(createdOn) or formatProjectDateTime(updatedOn) returns empty, causing awkward spacing; update the component to provide a clear fallback (e.g., "—", "Unknown", or "N/A") by assigning createdAtStr = formatProjectDateTime(createdOn) || 'N/A' and updatedAtStr = formatProjectDateTime(updatedOn) || 'N/A' (or alternatively handle this inside formatProjectDateTime) and use those variables where they are rendered (reference createdAtStr, updatedAtStr, formatProjectDateTime, createdOn, updatedOn and the display locations around the existing Created on / Updated on render lines including the similar block at 180-182).apps/customer-portal/webapp/src/pages/ChangeRequestsPage.tsx (1)
174-176: Consider logging the export error for debugging.Removing
console.errorreduces noise, but silently swallowing errors in the export flow may make production debugging harder. If a logger utility is available in this codebase, consider using it here to capture failed export attempts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/pages/ChangeRequestsPage.tsx` around lines 174 - 176, The export error branch in ChangeRequestsPage currently stops exporting but swallows the error when isInfiniteError is true; update the handler around the export flow to log the failure (e.g., call the project logger like logger.error or a similar existing logging utility) before or after calling setIsExporting(false) so the error and context (export id/filters/user) are recorded; reference the isInfiniteError condition and the setIsExporting call when adding the log, and include the actual error object/message in the log entry for debugging.
🤖 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/project-details/deployments/DeploymentProductList.tsx`:
- Line 384: The style value mb: 1.535 in DeploymentProductList (the component in
DeploymentProductList.tsx) looks like a likely typo; update the margin-bottom
value to the intended rounded value (e.g., mb: 1.5 or mb: 1.25) in the JSX/style
object where mb: 1.535 is set so the spacing is consistent; locate the mb: 1.535
occurrence in the render/return of the DeploymentProductList component and
replace it with the correct decimal (confirm with design tokens or surrounding
values before choosing 1.5 vs 1.25).
---
Nitpick comments:
In
`@apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentCard.tsx`:
- Around line 56-57: The UI currently displays empty strings when
formatProjectDateTime(createdOn) or formatProjectDateTime(updatedOn) returns
empty, causing awkward spacing; update the component to provide a clear fallback
(e.g., "—", "Unknown", or "N/A") by assigning createdAtStr =
formatProjectDateTime(createdOn) || 'N/A' and updatedAtStr =
formatProjectDateTime(updatedOn) || 'N/A' (or alternatively handle this inside
formatProjectDateTime) and use those variables where they are rendered
(reference createdAtStr, updatedAtStr, formatProjectDateTime, createdOn,
updatedOn and the display locations around the existing Created on / Updated on
render lines including the similar block at 180-182).
In `@apps/customer-portal/webapp/src/pages/ChangeRequestsPage.tsx`:
- Around line 174-176: The export error branch in ChangeRequestsPage currently
stops exporting but swallows the error when isInfiniteError is true; update the
handler around the export flow to log the failure (e.g., call the project logger
like logger.error or a similar existing logging utility) before or after calling
setIsExporting(false) so the error and context (export id/filters/user) are
recorded; reference the isInfiniteError condition and the setIsExporting call
when adding the log, and include the actual error object/message in the log
entry for debugging.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/customer-portal/webapp/src/components/project-details/deployments/AddProductModal.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeleteProductModal.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeploymentCard.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeploymentDocumentList.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsxapps/customer-portal/webapp/src/components/project-details/deployments/EditDeploymentModal.tsxapps/customer-portal/webapp/src/components/project-details/deployments/ManageProductModal.tsxapps/customer-portal/webapp/src/components/project-details/time-tracking/ProjectTimeTracking.tsxapps/customer-portal/webapp/src/pages/ChangeRequestsPage.tsxapps/customer-portal/webapp/tsconfig.app.jsonapps/customer-portal/webapp/vite.config.ts
✅ Files skipped from review due to trivial changes (1)
- apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentDocumentList.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/customer-portal/webapp/src/components/project-details/deployments/ManageProductModal.tsx
- apps/customer-portal/webapp/src/components/project-details/deployments/AddProductModal.tsx
Replace getFileTypeIcon with a useMemo-based fileType and inline icon rendering to simplify logic and memoize computations. Reformat several JSX elements (Skeleton and Box) for clearer props layout and import useMemo. Also adjust a minor spacing value in DeploymentProductList (mb: 1.535 -> 1.5). These changes improve readability and consistency of the document list component.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/project-details/deployments/DeploymentProductList.tsx`:
- Around line 180-182: The catch block in DeploymentProductList that currently
only console.errors on delete failure should surface a user-facing message:
update the error handling in the function handling deletion (e.g.,
handleDeleteProduct / deleteProduct call) to set a local error state (or call
your existing toast/modal helper) and display an inline error or toast/modal
when deletion fails, and ensure any UI loading state like
isDeleting/isDeletingId is cleared in the catch before finally; reference
DeploymentProductList and the deleteProduct/handleDeleteProduct flow to locate
and wire the toast/modal or error state update.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentDocumentList.tsxapps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx
Add ErrorBanner to DeploymentProductList to surface delete errors. Introduce deleteError state, clear it before attempting a delete, and set a friendly error message (including the product label when available) on failure. Render an ErrorBanner above the product list with a dismiss handler. Minor JSX restructuring (wrap in fragment) to accommodate the banner and preserve existing modals and list behavior.
cfd9fb7
into
wso2-open-operations:customer-portal-milestone-1
Description
This pull request introduces several enhancements and improvements to the deployments feature in the customer portal webapp. The most significant changes are the addition of a user-friendly product deletion workflow, improvements to the product addition modal, and UI/UX refinements for deployment and product lists. Below are the most important changes grouped by theme:
Product Deletion Workflow:
DeleteProductModalcomponent to confirm and handle the deletion (deactivation) of products from a deployment, including proper feedback and loading state. The deletion is implemented as a PATCH request settingactive: false. (apps/customer-portal/webapp/src/components/project-details/deployments/DeleteProductModal.tsx)DeploymentProductList, allowing users to delete products with confirmation and visual feedback. (apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx) [1] [2] [3]Product Addition and Management:
AddProductModalto support editing and submitting the product description, including validation for numeric fields using a newparseValidNumberhelper. The description field is now enabled and its value is submitted. (apps/customer-portal/webapp/src/components/project-details/deployments/AddProductModal.tsx) [1] [2] [3] [4]AddProductModalby removing unnecessaryuseEffectand ensuring form state resets appropriately. (apps/customer-portal/webapp/src/components/project-details/deployments/AddProductModal.tsx)Deployment and Product List UI/UX:
DeploymentCardto display both creation and update timestamps, show the deployment type as aChip, and clean up the footer by removing unused indicators. (apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentCard.tsx) [1] [2] [3] [4]apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentDocumentList.tsx,apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx) [1] [2]apps/customer-portal/webapp/src/components/project-details/deployments/DeploymentProductList.tsx) [1] [2]These changes collectively improve the usability, reliability, and maintainability of the deployments feature in the customer portal.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes