Allocate order numbers atomically instead of read-then-increment

Order ids were duplicating in production: 160 distinct (tenant, orderid) pairs
are shared by more than one order, worst of them "1135-1" on 108 orders, and
every order tenant 1147 has ever placed is numbered "1147-1".

getSequenceno read MAX(seqno)+1 and updateSeqno incremented, both against
r.db rather than the order's transaction and separated by the whole order
insert. Two concurrent orders therefore read the same number before either
wrote, and an order that rolled back still consumed one. Three further
defects made it worse:

  - A NULL orderseqno made COALESCE(MAX(orderseqno) + 1, 1) evaluate
    NULL + 1 = NULL and fall through to a hardcoded "<tenantid>-1". The
    increment then computed NULL + 1 = NULL too, so the counter could never
    leave NULL and every subsequent order reused that same id.

  - Tenants with several ordersequences rows (tenant 1135 has ~25) hit a
    GROUP BY returning multiple rows, of which Scan kept the first
    arbitrarily, while the increment updated all of them.

  - A tenant with no row at all fell back to "<tenantid>-1" indefinitely,
    because nothing ever created one.

nextSequenceNo replaces both functions with a single UPDATE ... RETURNING run
inside the caller's transaction, so the counter row stays locked until the
order commits and concurrent orders queue rather than collide. A NULL seeds
from the tenant's existing order count — at least as high as any number
already issued, so recovery cannot reissue a used id — the counter is pinned
to the tenant's lowest sequenceid so reads and writes address one row, and a
missing row is created on first use.

Verified against production data in rolled-back transactions: tenant 1147
(NULL) now yields 1147-9, 1147-10, ...; tenant 1135 (NULL plus duplicate rows)
1135-356 onward; tenant 916 keeps its 916-2024115209 subprefix format; an
unknown tenant creates its row and starts at 1. Eight concurrent allocations
produced eight distinct ids. Two real orders through the API returned 1147-9
and 1147-10, then were cancelled with stock restoring to its baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

repositories/orderRepository.go

@@ -1091,40 +1091,35 @@ func (r *orderRepository) UpdateOrder(order *models.Orders) error {
 	return tx.Commit().Error
 }
 
-func (r *orderRepository) getSequenceno(tid int, prefix string) string {
-	type SeqResult struct {
-		Orderseqno string
-	}
-
-	var q1 string
-	// Formats the ID as tenantid-subprefix+seqno (e.g., 908-20245189)
-	switch prefix {
-	case "ORD":
-		q1 = `SELECT CONCAT(tenantid, '-', 
-		             CASE WHEN subprefix IS NULL OR CAST(subprefix AS TEXT) IN ('0', '0.0', '') THEN '' ELSE CAST(subprefix AS TEXT) END, 
-		             COALESCE(MAX(orderseqno) + 1, 1)) AS orderseqno
-		      FROM ordersequences WHERE tenantid = ?
-		      GROUP BY tenantid, subprefix`
-	case "INV":
-		q1 = `SELECT CONCAT(tenantid, '-', 
-		             CASE WHEN subprefix IS NULL OR CAST(subprefix AS TEXT) IN ('0', '0.0', '') THEN '' ELSE CAST(subprefix AS TEXT) END, 
-		             COALESCE(MAX(invoiceseqno) + 1, 1)) AS orderseqno
-		      FROM ordersequences WHERE tenantid = ?
-		      GROUP BY tenantid, subprefix`
-	}
-
-	var result SeqResult
-	r.db.Raw(q1, tid).Scan(&result)
-
-	// Fallback if no row exists in the database
-	if result.Orderseqno == "" {
-		return fmt.Sprintf("%d-1", tid)
-	}
-
-	return result.Orderseqno
-}
-
-func (r *orderRepository) updateSeqno(tid int, prefix string) error {
+// nextSequenceNo claims the next order (or invoice) number for a tenant and
+// returns it formatted as tenantid-subprefix+seqno (e.g. 916-2024115209).
+//
+// It runs inside the caller's transaction and both reads and increments the
+// counter in a single UPDATE ... RETURNING, which is what makes the number
+// unique. The previous implementation split this into getSequenceno (read
+// MAX+1) and updateSeqno (increment), both on r.db rather than the order's
+// transaction, so two concurrent orders read the same value before either
+// wrote, and a rolled-back order still consumed a number.
+//
+// Three further defects that produced duplicate ids in production:
+//
+//   - A NULL orderseqno made COALESCE(MAX(orderseqno) + 1, 1) evaluate
+//     NULL + 1 = NULL, falling through to a hardcoded "<tenantid>-1"; the
+//     increment then computed NULL + 1 = NULL as well, so the counter could
+//     never leave NULL. Every order such a tenant ever placed was numbered
+//     "<tenantid>-1" — 108 orders share "1135-1" today. A NULL is now seeded
+//     from the tenant's existing order count, which is at least as high as any
+//     number already handed out, so recovery never reissues a used id.
+//
+//   - Tenants with more than one ordersequences row (tenant 1135 has ~25) hit
+//     a GROUP BY that returned several rows, of which Scan silently kept the
+//     first, while the increment updated every row. The counter is now pinned
+//     to the tenant's lowest sequenceid, so reads and writes address the same
+//     row whatever duplicates exist.
+//
+//   - A tenant with no row at all fell back to "<tenantid>-1" forever, since
+//     nothing created one. The row is now created on first use.
+func nextSequenceNo(tx *gorm.DB, tid int, prefix string) (string, error) {
 	var field string
 	switch prefix {
 	case "ORD":
@@ -1132,26 +1127,42 @@ func (r *orderRepository) updateSeqno(tid int, prefix string) error {
 	case "INV":
 		field = "invoiceseqno"
 	default:
-		return fmt.Errorf("invalid prefix: %s", prefix)
+		return "", fmt.Errorf("invalid prefix: %s", prefix)
 	}
 
-	// 🛠️ Improved: Check if row exists, if not, create it
-	var count int64
-	r.db.Table("ordersequences").Where("tenantid = ?", tid).Count(&count)
+	// field is not user input — it comes from the switch above.
+	formatted := fmt.Sprintf(`CONCAT(tenantid, '-',
+		CASE WHEN subprefix IS NULL OR CAST(subprefix AS TEXT) IN ('0', '0.0', '')
+		     THEN '' ELSE CAST(subprefix AS TEXT) END,
+		%s)`, field)
 
-	if count == 0 {
-		newSeq := map[string]interface{}{
-			"tenantid":   tid,
-			field:        1,
-			"subprefix":  nil, // Use NULL so CONCAT ignores it or set default here
-		}
-		return r.db.Table("ordersequences").Create(&newSeq).Error
+	var seq string
+	err := tx.Raw(fmt.Sprintf(`
+		UPDATE ordersequences
+		SET %s = COALESCE(%s, (SELECT COUNT(*) FROM orders WHERE tenantid = ?)) + 1,
+		    updated = NOW()
+		WHERE sequenceid = (SELECT MIN(sequenceid) FROM ordersequences WHERE tenantid = ?)
+		RETURNING %s`, field, field, formatted), tid, tid).Scan(&seq).Error
+	if err != nil {
+		return "", err
+	}
+	if seq != "" {
+		return seq, nil
 	}
 
-	// If it exists, perform the update
-	return r.db.Table("ordersequences").
-		Where("tenantid = ?", tid).
-		Update(field, gorm.Expr(fmt.Sprintf("%s + 1", field))).Error
+	// No counter row for this tenant yet — create one, seeded past whatever
+	// their existing orders already used.
+	err = tx.Raw(fmt.Sprintf(`
+		INSERT INTO ordersequences (tenantid, %s, created, updated)
+		VALUES (?, (SELECT COUNT(*) FROM orders WHERE tenantid = ?) + 1, NOW(), NOW())
+		RETURNING %s`, field, formatted), tid, tid).Scan(&seq).Error
+	if err != nil {
+		return "", err
+	}
+	if seq == "" {
+		return "", fmt.Errorf("could not allocate %s sequence for tenant %d", prefix, tid)
+	}
+	return seq, nil
 }
 
 func (r *orderRepository) CreateOrder(data models.Orders) (models.Orders, error) {
@@ -1244,7 +1255,15 @@ func (r *orderRepository) CreateOrder(data models.Orders) (models.Orders, error)
 	}
 
 	// 🛠️ Step 2: Create Order Header
-	data.Orderid = r.getSequenceno(data.Tenantid, "ORD")
+	// Claimed inside tx so the row lock on the counter holds until commit:
+	// concurrent orders queue for it instead of reading the same number, and a
+	// rollback releases it rather than burning it.
+	orderid, err := nextSequenceNo(tx, data.Tenantid, "ORD")
+	if err != nil {
+		tx.Rollback()
+		return models.Orders{}, fmt.Errorf("failed to allocate order number: %w", err)
+	}
+	data.Orderid = orderid
 
 	if err := tx.Create(&data).Error; err != nil {
 		tx.Rollback()
@@ -1290,10 +1309,6 @@ func (r *orderRepository) CreateOrder(data models.Orders) (models.Orders, error)
 		syncProductLocationStatus(tx, data.Tenantid, itemLocID, item.Productid)
 	}
 
-	if err := r.updateSeqno(data.Tenantid, "ORD"); err != nil {
-		log.Println("updateSeqno error:", err)
-	}
-
 	if err := tx.Commit().Error; err != nil {
 		return models.Orders{}, err
 	}

repositories/productRepository.go

@@ -88,12 +88,29 @@ func (r *productRepository) GetProductSubCategory(categoryID, tenantID int) ([]m
 func (r *productRepository) GetProductCount(tenantid, categoryid, subcategory int, approve string) ([]models.Productcount, error) {
 	var data []models.Productcount
 
+	// available/outofstock are counted from the ledger, not from
+	// products.productstatus. That column is a lifecycle field ("Active" /
+	// "Inactive") that a bug in the stock-receipt path used to overwrite with
+	// availability values, so counting it returned near-nonsense: of 6245
+	// products it matched 'available' on 136 and 'outofstock' on 12, with the
+	// rest — the real answer — invisible under "Active".
+	//
+	// A product counts as available when it holds positive stock at any one of
+	// the tenant's outlets, which is the only sensible tenant-wide reading of a
+	// quantity that is really per-outlet. total = available + outofstock.
 	baseQuery := `
-		SELECT 
+		SELECT
 			COUNT(*) AS total,
-			SUM(CASE WHEN a.productstatus = 'available' THEN 1 ELSE 0 END) AS available,
-			SUM(CASE WHEN a.productstatus = 'outofstock' THEN 1 ELSE 0 END) AS outofstock
+			SUM(CASE WHEN COALESCE(s.balance, 0) > 0 THEN 1 ELSE 0 END) AS available,
+			SUM(CASE WHEN COALESCE(s.balance, 0) <= 0 THEN 1 ELSE 0 END) AS outofstock
 		FROM products a
+		LEFT JOIN (
+			SELECT productid, tenantid,
+				SUM(CASE WHEN LOWER(stocktype) = 'in'  THEN quantity ELSE 0 END) -
+				SUM(CASE WHEN LOWER(stocktype) = 'out' THEN quantity ELSE 0 END) AS balance
+			FROM productstocks
+			GROUP BY productid, tenantid
+		) s ON s.productid = a.productid AND s.tenantid = a.tenantid
 		WHERE 1 = 1
 	`
 

services/productService.go

@@ -76,17 +76,9 @@ func (s *productService) CreateProductStock(stocks []models.Productstock) error
 		return err
 	}
 
-	idMap := make(map[int]struct{})
-	var productIDs []int
 	locMap := make(map[models.ProductLocationRef]struct{})
 	var locRefs []models.ProductLocationRef
 	for _, stk := range stocks {
-		if stk.Productid > 0 {
-			if _, exists := idMap[stk.Productid]; !exists {
-				idMap[stk.Productid] = struct{}{}
-				productIDs = append(productIDs, stk.Productid)
-			}
-		}
 		// Every entry gets synced, "in" and "out" alike: the status is now
 		// derived from the resulting balance rather than assumed from the
 		// direction of the movement, so an "out" that empties a location
@@ -101,12 +93,16 @@ func (s *productService) CreateProductStock(stocks []models.Productstock) error
 		}
 	}
 
-	if len(productIDs) > 0 {
-		if err := s.repo.UpdateProductStatus(productIDs, "available"); err != nil {
-			return err
-		}
-	}
-
+	// products.productstatus is deliberately NOT touched here. It is a
+	// per-product lifecycle field holding "Active"/"Inactive", and receiving
+	// stock used to overwrite it with "available" — an availability value in a
+	// lifecycle column, which is how 136 products ended up reading "available"
+	// and 12 "outofstock" with their real lifecycle state destroyed.
+	//
+	// Availability is a per-outlet fact and belongs to productlocations.status,
+	// which SyncProductLocationStatus derives from the ledger below. A single
+	// column on products cannot express it anyway: the same product can be
+	// stocked at one outlet and empty at another.
 	if len(locRefs) > 0 {
 		if err := s.repo.SyncProductLocationStatus(locRefs); err != nil {
 			return err