diff --git a/repositories/orderOutlet_test.go b/repositories/orderOutlet_test.go new file mode 100644 index 0000000..9c8206e --- /dev/null +++ b/repositories/orderOutlet_test.go @@ -0,0 +1,62 @@ +package repositories + +import ( + "strings" + "testing" +) + +// The block that stopped orders being placed: no outlet on the order. +// +// Stock is per-outlet, so a missing locationid read as zero available and the +// order was refused as "insufficient stock" — for Cadbury Dairy Milk at Suriya +// Peelamedu, which had 25 on the shelf. These pin the message on the actual +// cause. +func TestAnOrderWithNoOutletIsRefusedForTheRightReason(t *testing.T) { + err := assertOutletNamed([]stockLine{ + {Productid: 7086, Productname: "Cadbury Dairy Milk 100g", Locationid: 0, Quantity: 1}, + }) + if err == nil { + t.Fatal("an order with no outlet was accepted") + } + if strings.Contains(strings.ToLower(err.Error()), "insufficient stock") { + t.Errorf("still blames the stock: %v", err) + } + if !strings.Contains(err.Error(), "locationid") { + t.Errorf("does not say what to send: %v", err) + } + if !strings.Contains(err.Error(), "Cadbury Dairy Milk 100g") { + t.Errorf("does not name the line: %v", err) + } +} + +// One good line does not excuse a bad one — a mixed order would deduct part of +// the basket at a real outlet and the rest at nowhere. +func TestOneLineWithoutAnOutletFailsTheWholeOrder(t *testing.T) { + err := assertOutletNamed([]stockLine{ + {Productid: 7086, Productname: "Dairy Milk", Locationid: 1170}, + {Productid: 7085, Productname: "5 Star", Locationid: 0}, + }) + if err == nil { + t.Fatal("a mixed order was accepted") + } + if !strings.Contains(err.Error(), "5 Star") { + t.Errorf("blamed the wrong line: %v", err) + } +} + +func TestAnOrderThatNamesItsOutletPasses(t *testing.T) { + if err := assertOutletNamed([]stockLine{ + {Productid: 7086, Productname: "Dairy Milk", Locationid: 1170}, + }); err != nil { + t.Errorf("a valid order was refused: %v", err) + } +} + +// An unnamed product still has to produce a usable message — the id is all +// there is to go on. +func TestAnUnnamedLineIsIdentifiedById(t *testing.T) { + err := assertOutletNamed([]stockLine{{Productid: 7086, Locationid: 0}}) + if err == nil || !strings.Contains(err.Error(), "7086") { + t.Errorf("want the product id in the message, got: %v", err) + } +} diff --git a/repositories/orderRepository.go b/repositories/orderRepository.go index 89221e6..63d32ff 100644 --- a/repositories/orderRepository.go +++ b/repositories/orderRepository.go @@ -1522,6 +1522,23 @@ func (r *orderRepository) createOrderTx(tx *gorm.DB, data models.Orders) (models }) } + // An order has to name the outlet it is placed at, and one that did not was + // rejected as though the shop were empty. + // + // Stock is held per outlet: availableStock filters `locationid = ?`, so with + // 0 it matches no ledger row and returns 0 for every product. Each line then + // failed the check below with "insufficient stock ... available 0" — for a + // product sitting on the shelf with 25 of them. The message named the wrong + // thing entirely and sent people hunting for stock that was already there, + // which is the most expensive kind of error: confidently wrong. + // + // Checked per line, because a line may carry its own outlet and otherwise + // falls back to the header's. + if err := assertOutletNamed(lines); err != nil { + tx.Rollback() + return models.Orders{}, err + } + if err := lockStockRows(tx, data.Tenantid, lines); err != nil { tx.Rollback() return models.Orders{}, err diff --git a/repositories/stockLedger.go b/repositories/stockLedger.go index 8269e61..82d2596 100644 --- a/repositories/stockLedger.go +++ b/repositories/stockLedger.go @@ -180,3 +180,31 @@ func legacyOrderQty(quantity float64) int { } return q } + +// assertOutletNamed refuses an order that does not say which shop it is for. +// +// Stock is held per outlet — availableStock filters `locationid = ?` — so a +// line with no outlet matches no ledger row and reads as zero available. Every +// such order was then refused as "insufficient stock ... available 0", naming +// the shelf as the problem when the shelf was full and the request was what was +// incomplete. A merchant reading that goes and checks their stock, finds it, +// and has nowhere else to look. +// +// Per line rather than per order: a line may carry its own outlet, and only +// falls back to the header's when it does not. +func assertOutletNamed(lines []stockLine) error { + for _, line := range lines { + if line.Locationid != 0 { + continue + } + name := line.Productname + if name == "" { + name = fmt.Sprintf("ID %d", line.Productid) + } + return fmt.Errorf( + "this order names no outlet, so there is no shelf to sell '%s' from: "+ + "send locationid on the order, or on each item", name, + ) + } + return nil +}