Address code review: replace CSV parser with papaparse, improve type safety and CUID validation

Co-authored-by: syed-reza98 <71028588+syed-reza98@users.noreply.github.com>

Author: Copilot

docs/api/PRODUCTS-API.md

@@ -356,7 +356,7 @@ Soft deletes a product by setting `deletedAt` timestamp. The product is also arc
 POST /api/products/import
 ```
 
-Bulk imports products from a CSV file.
+Bulk imports products from a CSV file using [papaparse](https://www.papaparse.com/) for robust CSV parsing that handles edge cases like multiline quoted fields, different line endings, and escaped characters.
 
 #### Request
 
@@ -365,7 +365,7 @@ Bulk imports products from a CSV file.
 | Field | Type | Required | Description |
 |-------|------|----------|-------------|
 | `file` | File | Yes | CSV file (max 10MB) |
-| `storeId` | string | Yes | Store ID |
+| `storeId` | string | Yes | Store ID (verified against user's organization membership) |
 
 #### CSV Format
 
@@ -380,15 +380,18 @@ Optional columns:
 - `brandId`
 - `inventoryQty`
 - `status`
-- `images` (comma-separated URLs)
+- `images` (comma-separated URLs or JSON array)
 
 **Example CSV:**
 ```csv
 name,sku,price,description,categoryId,inventoryQty,status
 "Product 1",SKU001,29.99,"Description here",clxcat123,100,DRAFT
 "Product 2",SKU002,49.99,"Another product",clxcat456,50,ACTIVE
+"Product with ""quotes""",SKU003,19.99,"Description with, comma",clxcat789,25,DRAFT
 ```
 
+> **Note:** The CSV parser handles quoted fields, escaped quotes (doubled `""`), and commas within quotes correctly.
+
 #### Response
 
 ```json
@@ -599,6 +602,13 @@ Variant options are stored as JSON:
 
 ## Changelog
 
+### v1.1.0 (Code Review Improvements)
+- **CSV Import**: Replaced custom CSV parser with [papaparse](https://www.papaparse.com/) library for robust handling of edge cases (multiline quoted fields, different line endings, escaped characters)
+- **Type Safety**: Improved Zod error handling using proper `ZodIssue` type
+- **Store ID Validation**: Enhanced CUID format validation for store IDs with support for both CUID and CUID2 formats
+- **Documentation**: Added transaction guarantee documentation for variant/attribute updates
+- **Migration**: Added documentation for Product_sku_idx index removal (replaced by multi-tenant composite unique constraint)
+
 ### v1.0.0 (Initial Release)
 - Product CRUD operations
 - Variant management (0-100 variants per product)

package-lock.json

@@ -64,6 +64,7 @@
     "next-auth": "^4.24.13",
     "next-themes": "^0.4.6",
     "nodemailer": "^7.0.10",
+    "papaparse": "^5.5.3",
     "prisma": "^6.19.0",
     "react": "19.2.0",
     "react-dom": "19.2.0",
@@ -78,6 +79,7 @@
   "devDependencies": {
     "@tailwindcss/postcss": "^4",
     "@types/node": "^20",
+    "@types/papaparse": "^5.5.0",
     "@types/react": "^19",
     "@types/react-dom": "^19",
     "babel-plugin-react-compiler": "1.0.0",

package.json

@@ -1,2 +1,5 @@
--- DropIndex
+-- Note: Product_sku_idx index was dropped because multi-tenant SKU uniqueness
+-- is now enforced via @@unique([storeId, sku]) constraint on the Product model.
+-- This ensures SKUs are unique per store, not globally, which is the correct
+-- behavior for a multi-tenant e-commerce platform.
 DROP INDEX "Product_sku_idx";

prisma/migrations/20251126180812_init/migration.sql

@@ -6,7 +6,8 @@ import { getServerSession } from 'next-auth/next';
 import { authOptions } from '@/lib/auth';
 import { verifyStoreAccess } from '@/lib/get-current-user';
 import { ProductService } from '@/lib/services/product.service';
-import { z } from 'zod';
+import { z, ZodIssue } from 'zod';
+import Papa from 'papaparse';
 
 // Zod schema for CSV record validation
 const csvRecordSchema = z.object({
@@ -29,64 +30,6 @@ const csvRecordSchema = z.object({
   images: z.string().optional(),
 }).passthrough(); // Allow additional columns
 
-// Simple CSV parser (handles quoted fields and commas within quotes)
-function parseCSV(text: string): Array<Record<string, string>> {
-  const lines = text.split('\n').filter(line => line.trim());
-  if (lines.length < 2) {
-    return [];
-  }
-
-  // Parse header
-  const headers = parseCSVLine(lines[0]);
-  
-  // Parse data rows
-  const records: Array<Record<string, string>> = [];
-  for (let i = 1; i < lines.length; i++) {
-    const values = parseCSVLine(lines[i]);
-    if (values.length > 0) {
-      const record: Record<string, string> = {};
-      headers.forEach((header, index) => {
-        record[header.trim()] = values[index]?.trim() || '';
-      });
-      records.push(record);
-    }
-  }
-
-  return records;
-}
-
-// Parse a single CSV line, handling quoted fields
-function parseCSVLine(line: string): string[] {
-  const result: string[] = [];
-  let current = '';
-  let inQuotes = false;
-
-  for (let i = 0; i < line.length; i++) {
-    const char = line[i];
-    
-    if (char === '"') {
-      if (inQuotes && line[i + 1] === '"') {
-        // Escaped quote
-        current += '"';
-        i++;
-      } else {
-        // Toggle quote mode
-        inQuotes = !inQuotes;
-      }
-    } else if (char === ',' && !inQuotes) {
-      result.push(current);
-      current = '';
-    } else {
-      current += char;
-    }
-  }
-  
-  // Add last field
-  result.push(current);
-  
-  return result;
-}
-
 // POST /api/products/import - Bulk import products from CSV
 export async function POST(request: NextRequest) {
   try {
@@ -136,9 +79,27 @@ export async function POST(request: NextRequest) {
       );
     }
 
-    // Read and parse CSV
+    // Read and parse CSV using papaparse (handles edge cases like multiline quoted fields, different line endings, escaped characters)
     const text = await file.text();
-    const records = parseCSV(text);
+    const parseResult = Papa.parse<Record<string, string>>(text, {
+      header: true,
+      skipEmptyLines: true,
+      transformHeader: (header: string) => header.trim(),
+      transform: (value: string) => value.trim(),
+    });
+
+    // Check for parsing errors
+    if (parseResult.errors && parseResult.errors.length > 0) {
+      const criticalErrors = parseResult.errors.filter(e => e.type === 'Quotes' || e.type === 'FieldMismatch');
+      if (criticalErrors.length > 0) {
+        return NextResponse.json({
+          error: 'CSV parsing failed',
+          details: criticalErrors.map(e => ({ row: e.row, message: e.message })),
+        }, { status: 400 });
+      }
+    }
+
+    const records = parseResult.data;
 
     if (records.length === 0) {
       return NextResponse.json(
@@ -178,10 +139,8 @@ export async function POST(request: NextRequest) {
       if (result.success) {
         validatedRecords.push(result.data);
       } else {
-        // Zod 4 uses issues instead of errors
-        const errorMessages = result.error.issues 
-          ? result.error.issues.map((issue: { message: string }) => issue.message).join(', ')
-          : 'Validation failed';
+        // Use proper ZodIssue type from Zod for type-safe error handling
+        const errorMessages = result.error.issues.map((issue: ZodIssue) => issue.message).join(', ');
         validationErrors.push({
           row: i + 1,
           error: errorMessages,

src/app/api/products/import/route.ts

@@ -92,19 +92,19 @@ export async function POST(request: NextRequest) {
 
     const body = await request.json();
 
-    // Derive storeId from authenticated user's session or context
-    // Security: storeId is NOT accepted from client input to prevent tenant spoofing
-    // If user has a default store, use it; otherwise, look up storeId via membership/org
-    const storeId = session.user.storeId; // Replace with correct property or lookup
+    // Get storeId from request body
+    // Security: storeId is accepted from client but VERIFIED against user's organization membership
+    // via verifyStoreAccess() to prevent tenant spoofing
+    const storeId = body.storeId as string | undefined;
 
     if (!storeId) {
       return NextResponse.json(
-        { error: 'No store associated with your account. Cannot create product.' },
+        { error: 'storeId is required' },
         { status: 400 }
       );
     }
 
-    // Verify user has access to this store
+    // Verify user has access to this store (prevents tenant spoofing)
     const hasStoreAccess = await verifyStoreAccess(storeId);
     if (!hasStoreAccess) {
       return NextResponse.json(

src/app/api/products/route.ts

@@ -24,9 +24,25 @@ const ALLOWED_TYPES = [
 ];
 
 // Validate storeId to prevent directory traversal attacks
+// Primary validation: CUID format (25 chars, starts with 'c', lowercase alphanumeric)
+// Fallback validation: general safe alphanumeric for backward compatibility
 function isValidStoreId(storeId: string): boolean {
-  // storeId should be alphanumeric with optional hyphens (cuid format)
-  return /^[a-zA-Z0-9-_]+$/.test(storeId) && storeId.length <= 100;
+  // Standard CUID format: exactly 25 chars, starts with 'c', lowercase alphanumeric
+  const cuidRegex = /^c[a-z0-9]{24}$/;
+  
+  // CUID2 format: 21-24 lowercase alphanumeric characters
+  const cuid2Regex = /^[a-z0-9]{21,24}$/;
+  
+  // Fallback: safe alphanumeric pattern for legacy/custom IDs
+  // Allows lowercase, digits, hyphens, and underscores (10-50 chars)
+  // Does NOT allow uppercase, dots, slashes, or other special characters to prevent path traversal
+  const safeIdRegex = /^[a-z0-9_-]{10,50}$/;
+  
+  return (
+    cuidRegex.test(storeId) ||
+    cuid2Regex.test(storeId) ||
+    safeIdRegex.test(storeId)
+  );
 }
 
 // Generate a unique filename

src/app/api/products/upload/route.ts

@@ -649,7 +649,14 @@ export class ProductService {
         throw new Error(`Duplicate variant SKUs in request: ${[...new Set(duplicates)].join(', ')}`);
       }
 
-      // Use a transaction to handle variant and attribute updates atomically
+      /**
+       * Transactional Guarantee:
+       * Variant and attribute updates for a product are performed within a single transaction to ensure atomicity.
+       * This means that either all changes to variants and attributes are applied together, or none are applied if any part fails.
+       * This prevents inconsistent product state (e.g., variants updated but attributes not, or vice versa) which could lead to data integrity issues.
+       * If any operation within the transaction fails (such as a constraint violation or a database error), all changes are rolled back.
+       * Keeping these updates atomic is critical for e-commerce correctness, especially in multi-tenant environments.
+       */
       await prisma.$transaction(async (tx) => {
         // Delete variants that are no longer in the list
         if (variantIdsToDelete.length > 0) {
@@ -1483,8 +1490,9 @@ export class ProductService {
             else if (statusUpper === 'ARCHIVED') status = ProductStatus.ARCHIVED;
           }
 
-          // Build product data - schema will apply defaults for missing optional fields
-          const productData = {
+          // Build product data object with all required fields explicitly defined
+          // The createProductSchema has defaults, but we provide them here for type safety
+          const productData: CreateProductData = {
             name: record.name,
             sku: record.sku,
             price,
@@ -1494,9 +1502,13 @@ export class ProductService {
             inventoryQty: isNaN(inventoryQty) ? 0 : inventoryQty,
             status,
             images,
+            // Provide defaults for fields that have schema defaults
+            trackInventory: true,
+            lowStockThreshold: 5,
+            isFeatured: false,
           };
 
-          await this.createProduct(storeId, productData as CreateProductData);
+          await this.createProduct(storeId, productData);
           return { success: true, row: rowNumber };
         } catch (error) {
           return {