diff --git a/repositories/orderRepository.go b/repositories/orderRepository.go index 4490fe2..72d619b 100644 --- a/repositories/orderRepository.go +++ b/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 "-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 +// "-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 "-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 }