dkala-api/ORDER_MODULE_REVIEW.md

Order Module Review - Business & Logical Bugs

Critical Bugs

1. SQL Column Name Mismatch (CRITICAL)

Location: src/modules/orders/providers/orders.service.ts (lines 414, 426, 470, 495)

Issue: Raw SQL queries use restaurant_id column, but the entity uses shop relationship. MikroORM generates shop_id column for @ManyToOne(() => Shop), not restaurant_id.

Impact: SQL queries will fail with "column does not exist" errors.

Code:

WHERE o.restaurant_id = ?  // ❌ Wrong column name

Fix: Should be shop_id or use MikroORM query builder instead of raw SQL.

---

2. Price Inconsistency Between Cart and Order

Location: src/modules/orders/providers/orders.service.ts (lines 365-385)

Issue: In buildOrderItemsData(), prices are fetched fresh from database (variant.price), ignoring cart prices. If prices changed between adding to cart and checkout, user pays different amount than shown.

Impact:

• User sees one price in cart, pays different price
• Potential financial discrepancies
• Poor user experience

Code:

unitPrice: variant.price || 0,  // ❌ Uses DB price, not cart price
discount: variant.product.discount || 0,  // ❌ Uses DB discount, not cart discount

Fix: Use prices from cartItem instead of fetching from database.

---

3. Cart Cleared Outside Transaction

Location: src/modules/orders/providers/orders.service.ts (lines 125-127)

Issue: Cart is cleared AFTER transaction completes but BEFORE payOrder() is called. If payOrder() fails, cart is already cleared but payment might not be initiated.

Impact:

• User loses cart items even if checkout fails
• Potential data inconsistency
• Poor user experience

Code:

const order = await this.em.transactional(async em => {
  // ... order creation
});

await this.cartService.clearCart(userId, shopId);  // ❌ Outside transaction

const { paymentUrl } = await this.paymentsService.payOrder(order.id);  // If this fails, cart is already cleared

Fix: Clear cart only after successful payment initiation, or include in transaction.

---

High Priority Bugs

4. Missing Product/Variant Availability Validation

Location: src/modules/orders/providers/orders.service.ts (lines 369-374)

Issue: No check if product/variant is still active (isActive) or available when building order items.

Impact:

• Users can order inactive/unavailable products
• Business rule violations
• Potential fulfillment issues

Code:

const variant = await this.em.findOne(Variant, { id: cartItem.productId }, { populate: ['product', 'product.shop'] });
if (!variant) throw new NotFoundException(OrderMessage.PRODUCT_NOT_FOUND);
// ❌ Missing: Check if variant.product.isActive
// ❌ Missing: Check if variant is still available

Fix: Add validation for variant.product.isActive and any availability flags.

---

5. Missing Stock/Inventory Check

Location: src/modules/orders/providers/orders.service.ts (lines 365-385)

Issue: No validation for product stock/inventory quantity before creating order.

Impact:

• Overselling products
• Orders for out-of-stock items
• Fulfillment failures

Fix: Add stock quantity validation if inventory management exists.

---

6. Race Condition in Checkout

Location: src/modules/orders/providers/orders.service.ts (lines 66-134)

Issue: Multiple users can checkout simultaneously with same cart items. No locking mechanism to prevent overselling.

Impact:

• Concurrent checkouts can oversell limited stock
• Multiple orders for same limited items
• Inventory inconsistencies

Fix: Implement row-level locking or optimistic locking for variant/product during checkout.

---

7. Status Transition Logic Inconsistency

Location: src/modules/orders/providers/orders.service.ts (lines 44-52, 282-288)

Issue: STATUS_TRANSITIONS map allows PENDING_PAYMENT → PREPARING, but canTransition() method restricts this to Cash payments only. The map doesn't reflect actual business rules.

Impact:

• Confusing code logic
• Potential for incorrect status transitions
• Maintenance issues

Code:

// Line 45: Map allows this transition
[OrderStatus.PENDING_PAYMENT]: [OrderStatus.PAID, OrderStatus.CANCELED, OrderStatus.PREPARING],

// Line 282-288: But logic restricts it
if (from == OrderStatus.PENDING_PAYMENT && to == OrderStatus.PREPARING && paymentMethod !== PaymentMethodEnum.Cash) {
  return false;  // ❌ Contradicts STATUS_TRANSITIONS map
}

Fix: Remove PREPARING from PENDING_PAYMENT transitions in the map, or document the special case clearly.

---

Medium Priority Bugs

8. Cron Job Logic Incomplete

Location: src/modules/orders/crone/order.crone.ts (lines 87-170)

Issue:

• Comment says "shipped/delivered" but only checks SHIPPED status
• Uses updatedAt which might not reflect when status was changed
• Should check history array for actual status change time

Impact:

• DELIVERED orders never auto-complete
• Orders might complete too early or too late
• Inaccurate completion timing

Code:

// Line 89: Comment mentions delivered
// run every 15 minutes to complete orders that have been in shipped/delivered statuses

// Line 96-100: But only checks SHIPPED
status: {
  $in: [OrderStatus.SHIPPED],  // ❌ Missing DELIVERED
},
updatedAt: { $lte: cutoff },  // ❌ Should check history array for status change time

Fix: Include DELIVERED status and check history array for accurate status change time.

---

9. History Array Direct Mutation

Location: src/modules/orders/providers/orders.service.ts (line 240)

Issue: History array is mutated directly. If flush fails, history might be inconsistent.

Impact:

• Potential data inconsistency
• History might not reflect actual status changes

Code:

order.history.push({ status: toStatus, changedAt: new Date(), desc: desc || null });  // ❌ Direct mutation
await this.em.persistAndFlush(order);

Fix: Create new array or use immutable approach. Consider transaction rollback handling.

---

10. Missing Validation for Zero/Negative Quantities

Location: src/modules/orders/providers/orders.service.ts (lines 93-109)

Issue: No validation that quantity is positive when creating order items.

Impact:

• Orders with zero or negative quantities
• Invalid order data
• Potential calculation errors

Fix: Add validation for quantity > 0 in buildOrderItemsData().

---

11. Payment Amount Set at Creation

Location: src/modules/orders/providers/orders.service.ts (lines 111-117)

Issue: Payment amount is set to order.total at order creation. If order total changes later, payment amount won't match.

Impact:

• Payment amount might not match order total
• Potential refund/adjustment issues

Note: This might be intentional (snapshot at creation), but should be documented.

---

12. Decimal Precision Loss

Location: src/modules/orders/entities/order.entity.ts (lines 54, 60, 63, 66, 69, 72, 75)

Issue: All decimal fields use scale: 0, meaning no decimal places. This can cause rounding issues for currency.

Impact:

• Loss of precision for fractional currency amounts
• Rounding errors
• Potential financial discrepancies

Code:

@Property({ type: 'decimal', precision: 10, scale: 0, default: 0 })  // ❌ scale: 0

Fix: Use appropriate scale (e.g., scale: 2 for currency).

---

Low Priority / Code Quality Issues

13. Inconsistent Error Handling

Location: Various locations

Issue: Some methods throw exceptions, others return null/undefined. Inconsistent error handling patterns.

---

14. Missing Transaction for Status Change

Location: src/modules/orders/providers/orders.service.ts (lines 225-256)

Issue: changeOrderStatus() doesn't use explicit transaction, though it modifies order and emits events.

Impact: If event emission fails, order status is changed but notification might not be sent.

---

15. Potential Null Reference

Location: src/modules/orders/providers/orders.service.ts (line 247)

Issue: order.user?.id || '' - if user is null, empty string is used, which might cause issues downstream.

---

Recommendations

1. Fix SQL column names immediately - This is breaking production
2. Use cart prices instead of DB prices - Critical for financial accuracy
3. Add product availability checks - Prevent ordering unavailable items
4. Implement proper transaction boundaries - Ensure data consistency
5. Add stock/inventory validation - Prevent overselling
6. Fix status transition logic - Make it consistent and clear
7. Improve decimal precision - Use appropriate scale for currency
8. Add comprehensive validation - Validate all inputs before order creation
9. Implement locking mechanism - Prevent race conditions
10. Add unit tests - Cover all edge cases and business rules