Skip to content

Conversation

@rafiqul4
Copy link
Collaborator

@rafiqul4 rafiqul4 commented Feb 2, 2026

bangladisit fixed

Copilot AI review requested due to automatic review settings February 2, 2026 01:33
@github-project-automation github-project-automation bot moved this to Backlog in StormCom Feb 2, 2026
@vercel
Copy link

vercel bot commented Feb 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stormcomui Ready Ready Preview, Comment Feb 3, 2026 10:16am

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements Bangladesh-specific localization and courier delivery features for the e-commerce platform. The changes include currency conversion from USD to BDT (Bangladeshi Taka), implementation of location-based courier pricing (inside/outside Dhaka), analytics API fixes, and various code quality improvements.

Changes:

  • Currency localization: Converted all USD/$ references to BDT/৳ throughout the application
  • Courier delivery system: Added location-based delivery pricing with inside/outside Dhaka options
  • Analytics improvements: Fixed API response formats, improved customer metrics calculations, and added comprehensive e2e tests
  • Code quality: Added abort controllers for API requests, improved error handling, and cleaned up TypeScript/ESLint issues

Reviewed changes

Copilot reviewed 76 out of 93 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
src/lib/utils.ts Added BDT currency formatting functions, renamed from Dollars to Taka with backward compatibility alias
src/lib/stores/cart-store.ts Added delivery location state and courier pricing calculations to cart
src/lib/services/*.ts Updated default currency to BDT, fixed analytics customer metrics logic, added transaction timeouts
src/components/**/*.tsx Updated all currency displays from $ to ৳, improved analytics dashboard state management
src/app/store/[slug]/**/*.tsx Integrated courier pricing UI in product pages and checkout flow
src/app/api/**/*.ts Fixed analytics API response formats, improved error messages, added date range handling
prisma/schema.prisma Added courierPriceInsideDhaka and courierPriceOutsideDhaka fields to Product model
e2e/analytics.spec.ts Added comprehensive e2e test suite for analytics dashboard
package-lock.json Dependency lock file updates
test-results/* Playwright test artifacts (binary files)
Comments suppressed due to low confidence (1)

src/components/storefront/discount-banner.tsx:48

  • The localStorage access in wasBannerDismissed is not wrapped in a typeof window check, but the function is called during the initial state setup (lines 65-68) which happens during SSR. While the check is now deferred to useEffect, the function itself should still be defensive about window access to prevent issues if called elsewhere. Add a typeof window === 'undefined' check at the start of the function.

Comment on lines 58 to 81
console.log('[UPDATE PRODUCT] Starting update for productId:', id);
console.log('[UPDATE PRODUCT] storeId from body:', storeId);
console.log('[UPDATE PRODUCT] variants count:', body.variants?.length || 0);

if (!storeId) {
throw new Error('storeId is required');
}

// Verify user has access to this store
const hasStoreAccess = await verifyStoreAccess(storeId);
console.log('[UPDATE PRODUCT] hasStoreAccess:', hasStoreAccess);

if (!hasStoreAccess) {
throw new Error('Access denied. You do not have permission to modify products in this store.');
}

const productService = ProductService.getInstance();
const product = await productService.updateProduct(
id,
storeId,
body
);

return createSuccessResponse(product);
try {
const productService = ProductService.getInstance();
const product = await productService.updateProduct(
id,
storeId,
body
);

console.log('[UPDATE PRODUCT] Success! Updated product:', product?.name);
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Console.log statements are being added for debugging (lines 58-60, 68, 82). These should be removed before merging to production, or replaced with a proper logging framework. Console logs in production can expose sensitive information and degrade performance.

Copilot uses AI. Check for mistakes.
Comment on lines 225 to 238
getCourierTotal: () => {
const { items, deliveryLocation } = get();
if (!deliveryLocation || items.length === 0) return 0;

// Single delivery charge per order - take the maximum courier price among all items
const maxCourierPrice = Math.max(
...items.map((item) => {
return deliveryLocation === 'inside_dhaka'
? (item.courierPriceInsideDhaka ?? 0)
: (item.courierPriceOutsideDhaka ?? 0);
})
);

return maxCourierPrice;
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The courier pricing calculation takes the maximum price among all items, but this may not be the correct business logic. Typically, courier charges are either: (1) per-item and summed, (2) weight-based, or (3) a single flat rate. Taking the maximum could lead to undercharging. For example, if Item A has ৳50 courier and Item B has ৳100 courier, the total charged is ৳100, but the seller might expect ৳150. This needs clarification from business requirements.

Copilot uses AI. Check for mistakes.
Comment on lines 400 to 423
{hasCourierPricing && items.length > 0 && (
<span className="ml-2 text-primary font-medium">
(Courier: ৳{items.reduce((sum, item) => sum + (item.courierPriceInsideDhaka ?? 0) * item.quantity, 0).toFixed(2)})
</span>
)}
</p>
</div>
</div>
</Label>
</div>

<div className={`flex items-center space-x-3 p-4 rounded-lg border-2 transition-colors cursor-pointer ${
selectedDeliveryLocation === "outside_dhaka" ? "border-primary bg-primary/5" : "border-muted"
}`}>
<RadioGroupItem value="outside_dhaka" id="outside_dhaka" />
<Label htmlFor="outside_dhaka" className="flex-1 cursor-pointer">
<div className="flex items-center gap-3">
<Truck className="h-5 w-5 text-primary" />
<div>
<p className="font-medium">Outside Dhaka</p>
<p className="text-sm text-muted-foreground">
Delivery outside Dhaka city
{hasCourierPricing && items.length > 0 && (
<span className="ml-2 text-primary font-medium">
(Courier: ৳{items.reduce((sum, item) => sum + (item.courierPriceOutsideDhaka ?? 0) * item.quantity, 0).toFixed(2)})
</span>
)}
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the checkout page, the courier price displayed in the delivery location selection (lines 402, 424) multiplies by quantity, but the getCourierTotal function in cart-store takes the maximum per-order charge, not per-item. This creates an inconsistency where the UI shows one amount but the final total calculates differently. Either the display should not multiply by quantity, or the getCourierTotal logic should sum per-item charges.

Copilot uses AI. Check for mistakes.
Comment on lines 494 to 504
// Retention: customers from previous period who also ordered in current period
let retainedCustomers = 0;
if (previousPeriodCustomerIds.length > 0) {
const retained = customerIdsInPeriod.filter(id => previousPeriodCustomerIds.includes(id));
retainedCustomers = retained.length;
}

// Calculate retention rate
const customerRetentionRate = previousPeriodCustomerIds.length > 0
? (retainedCustomers / previousPeriodCustomerIds.length) * 100
: (totalCustomers > 0 ? 100 : 0); // If no previous period, assume 100% retention if we have customers
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retention rate calculation has been fixed, but the logic for retainedCustomers counts customers who appear in both periods. However, returningCustomers (line 450) counts customers who had orders BEFORE the current period. These are different metrics - a customer could be "returning" (ordered before) but not "retained" (didn't order in previous period). Clarify the business definitions and ensure the metrics align correctly.

Copilot uses AI. Check for mistakes.
<CardContent className="space-y-4">
<RadioGroup
value={selectedDeliveryLocation}
onValueChange={(value) => setValue("deliveryLocation", value as "inside_dhaka" | "outside_dhaka")}
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting state directly within setState is not recommended. The form value should already be controlled by the react-hook-form, so calling setValue here is redundant and could cause issues. The RadioGroup's onValueChange should be sufficient to update the form state without needing an explicit setValue call.

Copilot uses AI. Check for mistakes.
Comment on lines 63 to 64
// Delivery Location (Required for courier pricing)
deliveryLocation: z.enum(["inside_dhaka", "outside_dhaka"]).describe("Please select a delivery location"),
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delivery location is required for checkout (line 64), but the cart store initializes it as null and only sets it when the user selects it in checkout. If a user adds items to cart, then navigates away and comes back, the delivery location is persisted (line 266), but getCourierTotal() returns 0 if deliveryLocation is null (line 227). This could lead to incorrect totals being displayed. Consider either making deliveryLocation required earlier in the flow or handling the null state more explicitly in the UI.

Copilot uses AI. Check for mistakes.
Comment on lines 265 to 269
}, {
// Extended timeout for slow database connections (remote PostgreSQL in dev)
maxWait: 10000, // Max wait to acquire connection (10s)
timeout: 30000, // Transaction timeout (30s) - handles slow queries
});
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transaction timeout is increased to 30 seconds (line 268) to handle slow database connections, but this is a very long timeout for a database operation. While it may be necessary for development with remote PostgreSQL, in production this could mask performance issues or cause requests to hang. Consider making this configurable via environment variable (e.g., DB_TRANSACTION_TIMEOUT) so it can be reduced in production while remaining high in development.

Copilot uses AI. Check for mistakes.
Comment on lines 240 to 241
// React Compiler note: disable the incompatible-library check for useReactTable
// eslint-disable-next-line react-hooks/incompatible-library
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 60 says to disable the incompatible-library check, but the eslint-disable comment was removed on line 241. This creates confusion - either keep the comment and the disable directive, or remove both. The incompatible-library warning exists because React Compiler may not work properly with TanStack Table's API patterns.

Copilot uses AI. Check for mistakes.
Comment on lines 484 to 515
create: validatedData.variants.map((variant, index) => {
// Auto-generate options from variant name if options is empty or default
let options: string;
if (typeof variant.options === 'string') {
const parsed = JSON.parse(variant.options || '{}');
if (Object.keys(parsed).length === 0) {
// Empty options - generate from variant name
options = JSON.stringify({ variant: variant.name });
} else {
options = variant.options;
}
} else if (variant.options && Object.keys(variant.options).length > 0) {
options = JSON.stringify(variant.options);
} else {
// No options provided - generate from variant name
options = JSON.stringify({ variant: variant.name });
}

return {
name: variant.name,
sku: variant.sku,
barcode: variant.barcode,
price: variant.price,
compareAtPrice: variant.compareAtPrice,
inventoryQty: variant.inventoryQty ?? 0,
lowStockThreshold: variant.lowStockThreshold ?? 5,
weight: variant.weight,
image: variant.image,
options,
isDefault: variant.isDefault ?? (index === 0), // First variant is default if not specified
};
}),
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variant options auto-generation logic (lines 486-500, 714-731, 755-772) is duplicated across three locations: create variants, update variants, and create new variants during update. This violates DRY principles. Extract this logic into a helper function like generateVariantOptions(variant) to improve maintainability and ensure consistency.

Copilot uses AI. Check for mistakes.
Comment on lines 59 to 62
useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setIsMounted(true);
}, []);
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The eslint-disable-next-line react-hooks/set-state-in-effect comment on line 60 is disabling a valid warning. Setting state directly in a useEffect without dependencies can cause infinite loops. While this particular case is safe (empty deps array, runs once), the pattern is discouraged. Consider using useState(false) and avoiding the useEffect entirely, or restructure to check isMounted in a different way.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants