Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new product edit page and client with a full EditProductForm, sessionStorage-based duplication/prefill for the new-product flow, drag-and-drop product-line moves replacing draft editing, and an Edit action in product headers replacing status badges. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ProductLinesView
participant SessionStorage
participant Router
participant NewProductPage
participant EditProductForm
User->>ProductLinesView: Click "Duplicate" on product
ProductLinesView->>ProductLinesView: Build duplicate payload
ProductLinesView->>SessionStorage: Store payload under key
ProductLinesView->>Router: Navigate to /products/new?duplicateKey=<key>
Router->>NewProductPage: Init with searchParams
NewProductPage->>SessionStorage: getDuplicateData(duplicateKey)
SessionStorage-->>NewProductPage: Return payload (and remove key)
NewProductPage->>NewProductPage: Pre-fill form state from payload
User->>EditProductForm: View/modify pre-filled form
User->>EditProductForm: Submit -> project.updateConfig / save
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Pull request overview
This pull request refactors the payments product management functionality by removing inline editing capabilities from product cards and introducing dedicated pages for creating and editing products. The duplicate functionality has been implemented using sessionStorage for temporary data transfer.
Changes:
- Removed inline editing state management and UI from ProductCard component, making it view-only
- Created dedicated edit page at
/products/[productId]/editfor editing existing products - Added duplication functionality using sessionStorage to transfer product data to the new product creation page
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| page-client-product-lines-view.tsx | Simplified ProductCard to view-only mode, removed draft management, updated Edit/Duplicate actions to navigate to dedicated pages |
| new/page-client.tsx | Added support for initializing form from duplicated product data stored in sessionStorage |
| [productId]/edit/page.tsx | New Next.js page component for product editing route |
| [productId]/edit/page-client.tsx | New comprehensive product edit form with validation, preview, and full editing capabilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const effectiveProductLineId = productLineId && paymentsConfig.productLines[productLineId].customerType === customerType | ||
| ? productLineId | ||
| : ""; |
There was a problem hiding this comment.
The code accesses paymentsConfig.productLines[productLineId] without checking if productLineId exists in the object first. If productLineId is set to a non-existent product line ID, this will throw a runtime error when trying to access the .customerType property of undefined. Consider adding a check like productLineId in paymentsConfig.productLines before accessing the property.
| const effectiveProductLineId = productLineId && paymentsConfig.productLines[productLineId].customerType === customerType | |
| ? productLineId | |
| : ""; | |
| const effectiveProductLineId = | |
| productLineId && | |
| paymentsConfig.productLines[productLineId]?.customerType === customerType | |
| ? productLineId | |
| : ""; |
Greptile SummaryThis PR adds product editing and creation pages for the payments feature, along with improvements to the product lines view. The changes split the product management UI into separate pages for creating new products and editing existing ones, while enhancing the product lines view with better organization and inline product management capabilities. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User
participant CreatePage as Create Product Page
participant EditPage as Edit Product Page
participant Config as Project Config
participant Toast as Toast/Alert
User->>CreatePage: Select Customer Type
CreatePage->>CreatePage: Show Product Form
User->>CreatePage: Fill Form (name, prices, items)
User->>CreatePage: Click Create Button
CreatePage->>CreatePage: Validate Form
alt Validation Fails
CreatePage->>CreatePage: Show Error Messages
else Validation Passes
CreatePage->>Config: updateConfig(product)
Config-->>CreatePage: Success
CreatePage->>Toast: Show "Product created"
CreatePage->>User: Navigate to Products List
end
User->>EditPage: Open Edit Product Page
EditPage->>Config: Load Existing Product
EditPage->>EditPage: Initialize Form State
User->>EditPage: Modify Product Fields
User->>EditPage: Click Save Changes
EditPage->>EditPage: Validate Form
alt Validation Fails
EditPage->>User: Show Error Messages
else Validation Passes
EditPage->>Config: updateConfig(updated product)
Config-->>EditPage: Success
EditPage->>Toast: Show "Product updated"
EditPage->>User: Navigate to Products List
end
User->>EditPage: Click "Create Product Line"
EditPage->>EditPage: Open Create Dialog
User->>EditPage: Fill Dialog
EditPage->>Config: updateConfig(productLine)
Config-->>EditPage: Success
EditPage->>Toast: Show Success
EditPage->>EditPage: Auto-select Product Line
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (3)
-
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/new/page-client.tsx, line 570-576 (link)style: Async button handler should use
runAsynchronouslyWithAlertwrapper for consistency with other async handlers in this file and codebase standards.Context Used: Rule from
dashboard- UserunAsynchronouslyWithAlertfrom@stackframe/stack-shared/dist/utils/promisesfor async butto... (source) -
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx, line 1719-1756 (link)style: Async function
handleDeleteProductlacks error handling wrapper. Should userunAsynchronouslyWithAlertto ensure errors are shown to users.Context Used: Rule from
dashboard- UserunAsynchronouslyWithAlertfrom@stackframe/stack-shared/dist/utils/promisesfor async butto... (source) -
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx, line 1787-1811 (link)style: Async operation in
onDeleteProductLinehandler lacks error handling. Should wrap withrunAsynchronouslyWithAlertfor user feedback.Context Used: Rule from
dashboard- UserunAsynchronouslyWithAlertfrom@stackframe/stack-shared/dist/utils/promisesfor async butto... (source)
4 files reviewed, 5 comments
| const handleCreateProductLine = async (productLine: { id: string, displayName: string }) => { | ||
| await project.updateConfig({ | ||
| [`payments.productLines.${productLine.id}`]: { | ||
| displayName: productLine.displayName || null, | ||
| customerType, | ||
| }, | ||
| }); | ||
| setProductLineId(productLine.id); | ||
| }; |
There was a problem hiding this comment.
style: Async handler handleCreateProductLine should use runAsynchronouslyWithAlert instead of exposing raw promise. This ensures consistent error handling and user alerts across the application.
| const handleCreateProductLine = async (productLine: { id: string, displayName: string }) => { | |
| await project.updateConfig({ | |
| [`payments.productLines.${productLine.id}`]: { | |
| displayName: productLine.displayName || null, | |
| customerType, | |
| }, | |
| }); | |
| setProductLineId(productLine.id); | |
| }; | |
| const handleCreateProductLine = (productLine: { id: string, displayName: string }) => { | |
| runAsynchronouslyWithAlert(async () => { | |
| await project.updateConfig({ | |
| [`payments.productLines.${productLine.id}`]: { | |
| displayName: productLine.displayName || null, | |
| customerType, | |
| }, | |
| }); | |
| setProductLineId(productLine.id); | |
| }); | |
| }; |
Context Used: Rule from dashboard - Use runAsynchronouslyWithAlert from @stackframe/stack-shared/dist/utils/promises for async butto... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx
Line: 161:169
Comment:
**style:** Async handler `handleCreateProductLine` should use `runAsynchronouslyWithAlert` instead of exposing raw promise. This ensures consistent error handling and user alerts across the application.
```suggestion
const handleCreateProductLine = (productLine: { id: string, displayName: string }) => {
runAsynchronouslyWithAlert(async () => {
await project.updateConfig({
[`payments.productLines.${productLine.id}`]: {
displayName: productLine.displayName || null,
customerType,
},
});
setProductLineId(productLine.id);
});
};
```
**Context Used:** Rule from `dashboard` - Use `runAsynchronouslyWithAlert` from `@stackframe/stack-shared/dist/utils/promises` for async butto... ([source](https://app.greptile.com/review/custom-context?memory=5e671275-7493-402a-93a8-969537ec4d63))
How can I resolve this? If you propose a fix, please make it concise.| <Button | ||
| onClick={handleSave} | ||
| disabled={!canSave || isSaving} | ||
| > | ||
| {isSaving ? "Saving..." : "Save Changes"} | ||
| </Button> |
There was a problem hiding this comment.
style: Async button handler handleSave should be wrapped with runAsynchronouslyWithAlert for consistent error handling. Current manual try/catch pattern should be replaced with the utility.
| <Button | |
| onClick={handleSave} | |
| disabled={!canSave || isSaving} | |
| > | |
| {isSaving ? "Saving..." : "Save Changes"} | |
| </Button> | |
| <Button | |
| onClick={() => runAsynchronouslyWithAlert(handleSave)} | |
| disabled={!canSave || isSaving} | |
| > | |
| {isSaving ? "Saving..." : "Save Changes"} | |
| </Button> |
Context Used: Rule from dashboard - Use runAsynchronouslyWithAlert from @stackframe/stack-shared/dist/utils/promises for async butto... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx
Line: 281:286
Comment:
**style:** Async button handler `handleSave` should be wrapped with `runAsynchronouslyWithAlert` for consistent error handling. Current manual try/catch pattern should be replaced with the utility.
```suggestion
<Button
onClick={() => runAsynchronouslyWithAlert(handleSave)}
disabled={!canSave || isSaving}
>
{isSaving ? "Saving..." : "Save Changes"}
</Button>
```
**Context Used:** Rule from `dashboard` - Use `runAsynchronouslyWithAlert` from `@stackframe/stack-shared/dist/utils/promises` for async butto... ([source](https://app.greptile.com/review/custom-context?memory=5e671275-7493-402a-93a8-969537ec4d63))
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx:
- Around line 692-700: The onSave async callback passed to ItemDialog should be
wrapped with runAsynchronouslyWithAlert to surface errors to users; replace the
inline async handler used in the ItemDialog prop (the function that calls
project.updateConfig and toast) with a call to runAsynchronouslyWithAlert(() =>
/* same async logic */) so any thrown errors are caught and shown, referencing
the ItemDialog prop onSave, project.updateConfig, toast, and
runAsynchronouslyWithAlert.
🧹 Nitpick comments (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/new/page-client.tsx (2)
178-191: Type assertion without validation could lead to subtle bugs.The
as Productassertion assumes the parsed data matches theProducttype. If the sessionStorage data is malformed (e.g., from a version mismatch after deployment), this could cause runtime errors when accessing expected properties.Consider adding basic shape validation or using a safer fallback pattern.
🔧 Optional: Add minimal shape validation
function getDuplicateData(key: string | null): Product | null { if (!key) return null; try { const data = sessionStorage.getItem(key); if (data) { sessionStorage.removeItem(key); // Clean up after reading - return JSON.parse(data) as Product; + const parsed = JSON.parse(data); + // Basic shape validation - ensure required fields exist + if (parsed && typeof parsed === 'object' && 'customerType' in parsed) { + return parsed as Product; + } } } catch { // Ignore parsing errors } return null; }
222-228: Confusing boolean logic for add-on detection.The condition
duplicateData?.isAddOnTo !== false && duplicateData?.isAddOnTo !== undefinedessentially checks ifisAddOnTois truthy (an object). The subsequent access pattern is correct but the intent could be clearer.♻️ Clearer intent expression
- const duplicateIsAddOn = duplicateData?.isAddOnTo !== false && duplicateData?.isAddOnTo !== undefined; - const duplicateIsAddOnTo = duplicateIsAddOn && duplicateData.isAddOnTo - ? Object.keys(duplicateData.isAddOnTo as Record<string, boolean>) - : []; + const duplicateIsAddOn = duplicateData?.isAddOnTo && duplicateData.isAddOnTo !== false; + const duplicateIsAddOnTo = duplicateIsAddOn + ? Object.keys(duplicateData.isAddOnTo as Record<string, boolean>) + : [];apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx (1)
84-116: Significant code duplication with new product page.The form initialization logic (lines 96-115) and much of the form structure is duplicated from
new/page-client.tsx. Consider extracting shared logic into a custom hook or shared component.Potential extraction targets:
- Form state initialization and parsing logic
- Validation logic (
validateForm)- Included items management functions (
addIncludedItem,editIncludedItem,removeIncludedItem)- The entire Options section UI (stackable, server-only, add-on, free trial, product line)
This would reduce maintenance burden and ensure consistency between create and edit flows.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx (1)
736-761: Consider removing no-op callbacks for cleaner API.
onSaveandonRemoveare passed as no-op functions since this is now view-only. IfProductPriceRowsupports an optionalreadOnlymode, these callbacks could be omitted entirely.
| <ItemDialog | ||
| open={showNewItemDialog} | ||
| onOpenChange={setShowNewItemDialog} | ||
| onSave={async (item) => { | ||
| await project.updateConfig({ [`payments.items.${item.id}`]: { displayName: item.displayName, customerType: item.customerType } }); | ||
| toast({ title: "Item created" }); | ||
| }} | ||
| existingItemIds={Object.keys(paymentsConfig.items)} | ||
| forceCustomerType={customerType} |
There was a problem hiding this comment.
Async callback in ItemDialog onSave needs error handling.
Same issue as handleSave - the async function passed to onSave should be wrapped with runAsynchronouslyWithAlert to ensure errors are properly displayed to users.
🐛 Recommended fix
<ItemDialog
open={showNewItemDialog}
onOpenChange={setShowNewItemDialog}
- onSave={async (item) => {
- await project.updateConfig({ [`payments.items.${item.id}`]: { displayName: item.displayName, customerType: item.customerType } });
- toast({ title: "Item created" });
- }}
+ onSave={(item) => runAsynchronouslyWithAlert(async () => {
+ await project.updateConfig({ [`payments.items.${item.id}`]: { displayName: item.displayName, customerType: item.customerType } });
+ toast({ title: "Item created" });
+ })}
existingItemIds={Object.keys(paymentsConfig.items)}
forceCustomerType={customerType}
/>📝 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.
| <ItemDialog | |
| open={showNewItemDialog} | |
| onOpenChange={setShowNewItemDialog} | |
| onSave={async (item) => { | |
| await project.updateConfig({ [`payments.items.${item.id}`]: { displayName: item.displayName, customerType: item.customerType } }); | |
| toast({ title: "Item created" }); | |
| }} | |
| existingItemIds={Object.keys(paymentsConfig.items)} | |
| forceCustomerType={customerType} | |
| <ItemDialog | |
| open={showNewItemDialog} | |
| onOpenChange={setShowNewItemDialog} | |
| onSave={(item) => runAsynchronouslyWithAlert(async () => { | |
| await project.updateConfig({ [`payments.items.${item.id}`]: { displayName: item.displayName, customerType: item.customerType } }); | |
| toast({ title: "Item created" }); | |
| })} | |
| existingItemIds={Object.keys(paymentsConfig.items)} | |
| forceCustomerType={customerType} |
🤖 Prompt for AI Agents
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx
around lines 692 - 700, The onSave async callback passed to ItemDialog should be
wrapped with runAsynchronouslyWithAlert to surface errors to users; replace the
inline async handler used in the ItemDialog prop (the function that calls
project.updateConfig and toast) with a call to runAsynchronouslyWithAlert(() =>
/* same async logic */) so any thrown errors are caught and shown, referencing
the ItemDialog prop onSave, project.updateConfig, toast, and
runAsynchronouslyWithAlert.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx (1)
1070-1093: Wrap clipboard writes withrunAsynchronously.Clipboard writes can reject (permissions/HTTP). Wrapping them avoids unhandled rejections and uses the shared async handler.
As per coding guidelines, ...
🛠️ Suggested fix
- onClick={async () => { - await navigator.clipboard.writeText(`const url = await ${customerType}.createCheckoutUrl({ productId: "${id}" });\nwindow.open(url, "_blank");`); - toast({ title: "Copied to clipboard" }); - }} + onClick={() => + runAsynchronously((async () => { + await navigator.clipboard.writeText( + `const url = await ${customerType}.createCheckoutUrl({ productId: "${id}" });\nwindow.open(url, "_blank");` + ); + toast({ title: "Copied to clipboard" }); + })()) + } ... - onClick={async () => { - const prompt = generateComprehensivePrompt(); - await navigator.clipboard.writeText(prompt); - toast({ title: "Prompt copied to clipboard" }); - }} + onClick={() => + runAsynchronously((async () => { + const prompt = generateComprehensivePrompt(); + await navigator.clipboard.writeText(prompt); + toast({ title: "Prompt copied to clipboard" }); + })()) + }
🤖 Fix all issues with AI agents
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx:
- Around line 1341-1367: The code assumes
paymentsConfig.productLines[targetProductLineId] exists and uses toast for
blocked moves; update the validation to first verify the target product line
object exists (check paymentsConfig.productLines[targetProductLineId] !==
undefined) before reading customerType or displayName, and if it's missing
(deleted mid-drag) show a blocking alert and return; likewise, when customerType
mismatch between paymentsConfig.productLines[targetProductLineId].customerType
and draggedProduct.customerType, replace the toast with a blocking alert and
return; keep setIsMovingProduct(true) and the await
onMoveProduct(draggedProductId, targetProductLineId) only after these existence
and compatibility checks.
🧹 Nitpick comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx (2)
86-93: Preferinterfacefor the new prop/config shapes.These object-shaped
typealiases should beinterfacefor consistency in TSX.As per coding guidelines, ...
♻️ Example conversion (apply similarly to the other new prop types)
-type ProductEditableInputProps = { +interface ProductEditableInputProps { value: string, onUpdate?: (value: string) => void, readOnly?: boolean, placeholder?: string, inputClassName?: string, transform?: (value: string) => string, -}; +}Also applies to: 438-457, 483-489, 1132-1150
985-1002: UseurlStringfor the new navigation URLs.The new edit/duplicate routes should use
urlStringinstead of template strings to keep URL construction consistent and safe.As per coding guidelines, ...
♻️ Suggested change
- onClick={() => router.push(`/projects/${projectId}/payments/products/${id}/edit`)} + onClick={() => router.push(urlString`/projects/${projectId}/payments/products/${id}/edit`)} ... - router.push(`/projects/${projectId}/payments/products/new?duplicate=${duplicateKey}`); + router.push(urlString`/projects/${projectId}/payments/products/new?duplicate=${duplicateKey}`);
| // Get the target product line's customer type | ||
| const targetCustomerType = targetProductLineId | ||
| ? paymentsConfig.productLines[targetProductLineId].customerType | ||
| : undefined; | ||
|
|
||
| // Validate customer type compatibility: | ||
| // - Can always drop to "No product line" | ||
| // - Can drop to a product line if it has the same customer type as the product | ||
| if (targetProductLineId && targetCustomerType !== draggedProduct.customerType) { | ||
| toast({ | ||
| title: "Cannot move product", | ||
| description: `This product has customer type "${draggedProduct.customerType}" but the target product line is for "${targetCustomerType}" customers.`, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Show loading state and update the product's productLineId | ||
| setIsMovingProduct(true); | ||
| try { | ||
| await onMoveProduct(draggedProductId, targetProductLineId); | ||
|
|
||
| toast({ | ||
| title: "Product moved", | ||
| description: targetProductLineId | ||
| ? `Moved to "${paymentsConfig.productLines[targetProductLineId].displayName || targetProductLineId}"` | ||
| : "Removed from product line", | ||
| }); |
There was a problem hiding this comment.
Harden drag-drop target validation and use alerts for blocked moves.
paymentsConfig.productLines[targetProductLineId] is assumed to exist; if the product line is deleted mid-drag this will throw. Also, the incompatible drop is a blocked action, so use a blocking alert instead of toast.
As per coding guidelines, ...
🛠️ Suggested fix
- const targetCustomerType = targetProductLineId
- ? paymentsConfig.productLines[targetProductLineId].customerType
- : undefined;
+ const targetProductLine = targetProductLineId
+ ? paymentsConfig.productLines[targetProductLineId]
+ : undefined;
+ if (targetProductLineId && !targetProductLine) {
+ alert("Target product line no longer exists.");
+ return;
+ }
+ const targetCustomerType = targetProductLine?.customerType;
...
- toast({
- title: "Cannot move product",
- description: `This product has customer type "${draggedProduct.customerType}" but the target product line is for "${targetCustomerType}" customers.`,
- });
+ alert(
+ `Cannot move product. This product has customer type "${draggedProduct.customerType}" but the target product line is for "${targetCustomerType}" customers.`
+ );
...
- description: targetProductLineId
- ? `Moved to "${paymentsConfig.productLines[targetProductLineId].displayName || targetProductLineId}"`
+ description: targetProductLineId
+ ? `Moved to "${targetProductLine?.displayName || targetProductLineId}"`🤖 Prompt for AI Agents
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx
around lines 1341 - 1367, The code assumes
paymentsConfig.productLines[targetProductLineId] exists and uses toast for
blocked moves; update the validation to first verify the target product line
object exists (check paymentsConfig.productLines[targetProductLineId] !==
undefined) before reading customerType or displayName, and if it's missing
(deleted mid-drag) show a blocking alert and return; likewise, when customerType
mismatch between paymentsConfig.productLines[targetProductLineId].customerType
and draggedProduct.customerType, replace the toast with a blocking alert and
return; keep setIsMovingProduct(true) and the await
onMoveProduct(draggedProductId, targetProductLineId) only after these existence
and compatibility checks.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| const existingIsAddOn = existingProduct.isAddOnTo !== false; | ||
| const existingIsAddOnTo = existingIsAddOn |
There was a problem hiding this comment.
| const existingIsAddOn = existingProduct.isAddOnTo !== false; | |
| const existingIsAddOnTo = existingIsAddOn | |
| const existingIsAddOn = existingProduct.isAddOnTo !== false && existingProduct.isAddOnTo !== undefined; | |
| const existingIsAddOnTo = existingIsAddOn && existingProduct.isAddOnTo |
The isAddOnTo field can be undefined according to the schema, but line 96 only checks if it's not false, causing a runtime error when trying to call Object.keys() on undefined.
View Details
Analysis
Missing undefined check for isAddOnTo field causes TypeError
What fails: EditProductForm in apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx crashes when loading a product with an undefined isAddOnTo field.
How to reproduce:
- Create a product where the
isAddOnTofield is undefined (which is valid according to the product schema - see schema definition) - Navigate to edit that product
- The edit page fails with:
TypeError: Cannot convert undefined or null to object
What happens: The code at line 96-99 checks existingProduct.isAddOnTo !== false, which evaluates to true when isAddOnTo is undefined (because undefined !== false). It then attempts to call Object.keys(undefined) which throws a TypeError.
Expected behavior: The page should load successfully and treat undefined isAddOnTo as "not an add-on" (same behavior as false), based on the schema definition which allows isAddOnTo to be:
false(literal false, not an add-on)Record<string, true>(an add-on with parent products)undefined(optional field)
Root cause: The code pattern was missing the explicit undefined check that exists in the parallel pattern at apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/new/page-client.tsx line 223, which correctly uses: duplicateData?.isAddOnTo !== false && duplicateData?.isAddOnTo !== undefined
Fix applied: Updated the condition to include an explicit undefined check, matching the safe pattern already used in the new product form.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const currentProduct = paymentsConfig.products[productId]; | ||
|
|
||
| // Update the entire product object with the new productLineId | ||
| // Using undefined instead of null to properly clear the value | ||
| await project.updateConfig({ | ||
| [`payments.products.${productId}`]: { | ||
| ...currentProduct, |
There was a problem hiding this comment.
Missing null check for currentProduct before spreading it in the onMoveProduct callback. If a product is deleted between when the drag starts and when it ends, this will cause a runtime error.
View Details
📝 Patch Details
diff --git a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx
index 265edf30..ed493ea2 100644
--- a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx
+++ b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx
@@ -2023,6 +2023,7 @@ export default function PageClient({ createDraftRequestId, draftCustomerType = '
paymentsConfig={paymentsConfig}
onMoveProduct={async (productId, targetProductLineId) => {
const currentProduct = paymentsConfig.products[productId];
+ if (!currentProduct) return;
// Update the entire product object with the new productLineId
// Using undefined instead of null to properly clear the value
Analysis
Missing null check for currentProduct in onMoveProduct callback
What fails: The onMoveProduct callback in page-client-product-lines-view.tsx spreads currentProduct without checking if it exists. If a product is deleted between when the drag starts and when it ends (race condition), currentProduct will be undefined, causing updateConfig to fail validation.
How to reproduce:
- Start dragging a product to move it to a different product line
- Simultaneously delete that product via another operation
- Complete the drag and drop before the deletion propagates
- The
updateConfigcall receives an incomplete product object with onlyproductLineIdset, missing required fields likedisplayName,customerType,prices, etc.
Result: Schema validation fails because required fields are missing from the product update. This results in an error being thrown to the user.
Expected: The callback should check if the product exists before attempting to update it, following the defensive programming pattern already used in handleDragEnd (line 1327), where if (!draggedProduct) return; guards against this scenario.
Fixed by: Adding if (!currentProduct) return; check at line 2026, matching the pattern used elsewhere in the same component for handling potentially deleted products during drag-and-drop operations.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| })); | ||
|
|
||
| // Validate that the selected productLineId matches the current customerType | ||
| const effectiveProductLineId = productLineId && paymentsConfig.productLines[productLineId].customerType === customerType |
There was a problem hiding this comment.
| const effectiveProductLineId = productLineId && paymentsConfig.productLines[productLineId].customerType === customerType | |
| const effectiveProductLineId = productLineId && productLineId in paymentsConfig.productLines && paymentsConfig.productLines[productLineId].customerType === customerType |
Missing existence check when accessing paymentsConfig.productLines[productLineId] could cause a runtime error if the product line is deleted while the edit form is loaded.
View Details
Analysis
Missing existence check for productLineId in edit form allows runtime error
What fails: ProductLineId existence check in EditProductForm() at line 144 of apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx accesses paymentsConfig.productLines[productLineId].customerType without verifying the key exists in the object.
How to reproduce:
- Edit an existing product
- Set
productLineIdto a value that doesn't exist inpaymentsConfig.productLines - Trigger the form to re-evaluate the
effectiveProductLineIdvariable
Result: Throws TypeError: "Cannot read properties of undefined (reading 'customerType')" at runtime because paymentsConfig.productLines[productLineId] returns undefined when the key doesn't exist.
Expected: Should safely return empty string when productLineId doesn't exist, matching the pattern used in new/page-client.tsx line 211: const validProductLineId = urlProductLineId && urlProductLineId in paymentsConfig.productLines ? urlProductLineId : null;
Fix: Added in operator check before accessing nested property to verify the key exists in the Record type, preventing access to undefined.customerType.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| // Get the target product line's customer type | ||
| const targetCustomerType = targetProductLineId | ||
| ? paymentsConfig.productLines[targetProductLineId].customerType | ||
| : undefined; |
There was a problem hiding this comment.
Drag-drop handler crashes on deleted product line
Medium Severity
The handleDragEnd function accesses paymentsConfig.productLines[targetProductLineId].customerType and .displayName without verifying the product line exists. If a product line is deleted while a user is actively dragging (e.g., by another browser tab or concurrent API call), this will crash when the user drops on the now-deleted product line.
Additional Locations (1)
1 participant
Note
Delivers major UX for managing products and product lines.
Edit Productpage: full form with validation and live preview to managedisplayName, pricing (free/include-by-default), included items (add/edit/remove),serverOnly,stackable, add-ons, free trials, andproductLineId; dialogs to create product lines/itemsproductLineId; add-product CTAs; duplication now stores data insessionStorageand opens create flowsessionStorage; respects URL params; add-on selectors now show all existing productsWritten by Cursor Bugbot for commit d81999d. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.