changes

controllers/productController.go

@@ -574,19 +574,27 @@ func (ctl *ProductController) ImportCatalogueProduct(c *fiber.Ctx) error {
 	}
 
 	for _, req := range data {
-		// `categoryid` is deliberately NOT required.
+		// `categoryid` IS required, and the comment that used to sit here
+		// argued the opposite. It said an unclassified product is "visibly
+		// unfinished" and therefore harmless, and that importing "reaches no
+		// shop and cannot be sold".
 		//
-		// Importing adds a product to the admin catalogue and nothing else — it
+		// Both halves were wrong, and a sweep of all 262 tenants measured it:
-		// reaches no shop and cannot be sold — so there is nothing yet that
+		// 7 products across 3 tenants sat with categoryid 0, and 6 outlets were
-		// depends on it being classified. The console matches the tenant's own
+		// serving shoppers an EMPTY shop while their consoles listed stock.
-		// category against the catalogue's where one lines up and sends 0 where
+		// Nothing was visibly unfinished — `getlocationproducts` does not filter
-		// none does, and the admin corrects it there.
+		// on category, so the product looked entirely normal to the merchant.
+		// And import DOES reach a shop: it writes productlocations.
 		//
-		// Requiring it forced the console to send *something*, and what it sent
+		// The state was also unrecoverable. GetProductsBySubcategory rejects
-		// was the tenant's first category regardless of the product. That is
+		// categoryid 0 outright, `UpdateProduct` writes only
-		// worse than uncategorised: an unclassified product is visibly
+		// productlocations.status, and re-import corrected pricing alone — so
-		// unfinished, while a wrongly classified one looks done and is only
+		// no request could put it right. Re-import now repairs the category
-		// found by someone browsing the wrong aisle.
+		// (productService.ImportCatalogueProduct), and this refuses to create
+		// the state in the first place.
+		//
+		// A wrongly filed product remains the better failure: it is findable,
+		// and it is fixable by re-importing. An unfiled one was neither.
 		// Named individually rather than as one list of five.
 		//
 		// The old message recited every required field whichever one was
@@ -607,6 +615,9 @@ func (ctl *ProductController) ImportCatalogueProduct(c *fiber.Ctx) error {
 		if req.Catalogueid == 0 {
 			missing = append(missing, "catalogueid")
 		}
+		if req.Categoryid == 0 {
+			missing = append(missing, "categoryid")
+		}
 		if len(missing) > 0 {
 			return c.Status(http.StatusBadRequest).JSON(fiber.Map{
 				"code":    http.StatusBadRequest,

repositories/productRepository.go

@@ -23,6 +23,7 @@ type ProductRepository interface {
 	CreateProductStock(stocks []models.Productstock) error
 	UpdateProductStatus(productIDs []int, status string) error
 	SyncProductLocationStatus(refs []models.ProductLocationRef) error
+	EnsureProductLocation(refs []models.ProductLocationRef) error
 	CreateProduct(product models.Products) error
 	UpdateProduct(product models.Products) error
 	DeleteProduct(productID int) error
@@ -49,6 +50,7 @@ type ProductRepository interface {
 	GetImportedCatalogueRefs(tenantid int, brand string) ([]models.ImportedCatalogueRef, error)
 	GetTenantCategories(tenantid int) ([]models.TenantCategory, error)
 	UpdateProductPricing(productid int, retailprice, productcost, taxpercent float64) error
+	UpdateProductCategory(productid, categoryid, subcategoryid int) error
 }
 
 type productRepository struct {
@@ -299,6 +301,49 @@ func (r *productRepository) CreateProductStock(stocks []models.Productstock) err
 	return r.db.Table("productstocks").Create(&stocks).Error
 }
 
+// EnsureProductLocation puts a product on an outlet's shelf if it is not there
+// already, and leaves it completely alone if it is.
+//
+// Receiving stock used to write the ledger and nothing else. The ledger is not
+// what the customer app reads: GetProducts joins productlocations and then
+// filters `pl.locationid = ?`, so a product with no row for that outlet is
+// dropped by the join no matter how much stock arrived. A shop could request a
+// product, have it approved, receive it, watch the stock rise in the console —
+// and the product was still not for sale, with nothing anywhere saying why.
+//
+// SyncProductLocationStatus below cannot cover this: it is an UPDATE, so with
+// no row to update it succeeds having changed nothing.
+//
+// INSERT ... SELECT ... WHERE NOT EXISTS rather than an upsert, deliberately.
+// An upsert here would carry a price, and the only price this code could supply
+// is the master retailprice — which would overwrite an outlet's own, carefully
+// different, price every time a delivery arrived. Existing rows must not be
+// touched; this only ever creates the missing one.
+//
+// The seeded status is "outofstock" because it is true at the instant of the
+// insert. The caller runs SyncProductLocationStatus immediately after, which
+// derives the real value from the ledger, so a genuine receipt corrects it in
+// the same call.
+func (r *productRepository) EnsureProductLocation(refs []models.ProductLocationRef) error {
+	for _, ref := range refs {
+		if err := r.db.Exec(`
+			INSERT INTO productlocations (tenantid, locationid, productid, price, status)
+			SELECT ?, ?, ?, COALESCE(p.retailprice, 0), 'outofstock'
+			FROM products p
+			WHERE p.productid = ? AND p.tenantid = ?
+			  AND NOT EXISTS (
+				SELECT 1 FROM productlocations pl
+				WHERE pl.tenantid = ? AND pl.locationid = ? AND pl.productid = ?
+			  )`,
+			ref.Tenantid, ref.Locationid, ref.Productid,
+			ref.Productid, ref.Tenantid,
+			ref.Tenantid, ref.Locationid, ref.Productid).Error; err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // SyncProductLocationStatus recomputes productlocations.status for each ref
 // from the productstocks ledger: "available" when the live SUM(in)-SUM(out)
 // balance is positive, "outofstock" when it is not.
@@ -1124,6 +1169,31 @@ func (r *productRepository) GetTenantCategories(tenantid int) ([]models.TenantCa
 
 // UpdateProductPricing updates only the pricing fields on a product
 // snapshot, used when a catalogue product is re-imported with new pricing.
+// UpdateProductCategory files a product under a category after the fact.
+//
+// It exists because nothing else could. `UpdateProduct` writes only
+// productlocations.status despite its name, and re-importing a product the
+// tenant already has took the `existing != nil` branch, which corrected the
+// pricing and left the category as it was. So a product imported with
+// categoryid 0 was permanently invisible to the customer app — the endpoint it
+// browses rejects categoryid 0 outright — with no API able to repair it. Seven
+// products across three tenants were in that state, six outlets showing an
+// empty shop to shoppers while the console listed their stock.
+//
+// A zero is never written. Callers pass whatever the import request carried,
+// and a request that omits the category must not erase one that is already
+// correct — the guard belongs here rather than in each caller.
+func (r *productRepository) UpdateProductCategory(productid, categoryid, subcategoryid int) error {
+	if categoryid <= 0 {
+		return nil
+	}
+	updates := map[string]interface{}{"categoryid": categoryid}
+	if subcategoryid > 0 {
+		updates["subcategoryid"] = subcategoryid
+	}
+	return r.db.Table("products").Where("productid = ?", productid).Updates(updates).Error
+}
+
 func (r *productRepository) UpdateProductPricing(productid int, retailprice, productcost, taxpercent float64) error {
 	return r.db.Table("products").
 		Where("productid = ?", productid).

services/productService.go

@@ -109,6 +109,21 @@ func (s *productService) CreateProductStock(stocks []models.Productstock) error
 	// column on products cannot express it anyway: the same product can be
 	// stocked at one outlet and empty at another.
 	if len(locRefs) > 0 {
+		// Shelve before deriving availability, and in that order.
+		//
+		// Stock arriving at an outlet that has no productlocations row is the
+		// case this exists for: the ledger was written, the console showed the
+		// balance climbing, and the customer app never saw the product because
+		// its query joins productlocations and filters on the outlet. The whole
+		// request-approve-receive path ended in an invisible product.
+		//
+		// SyncProductLocationStatus cannot do this itself — it is an UPDATE,
+		// and with no row it changes nothing and reports success. Running it
+		// second means the row created here immediately gets its real status
+		// derived from the ledger rather than keeping the seeded one.
+		if err := s.repo.EnsureProductLocation(locRefs); err != nil {
+			return err
+		}
 		if err := s.repo.SyncProductLocationStatus(locRefs); err != nil {
 			return err
 		}
@@ -271,6 +286,29 @@ func (s *productService) ImportCatalogueProduct(reqs []models.ImportCataloguePro
 			if err := s.repo.UpdateProductPricing(productID, req.Retailprice, req.Productcost, req.Taxpercent); err != nil {
 				return err
 			}
+			// Re-importing corrects the CATEGORY too, not just the price.
+			//
+			// This branch used to update pricing alone, which made a product
+			// imported without a category permanently unreachable: the customer
+			// app’s endpoint rejects categoryid 0, UpdateProduct writes only
+			// productlocations.status, and re-importing — the obvious repair —
+			// silently changed nothing. There was no path back.
+			//
+			// UpdateProductCategory ignores a zero, so an import that does not
+			// carry a category still cannot erase one that is already right.
+			if err := s.repo.UpdateProductCategory(productID, req.Categoryid, req.Subcategoryid); err != nil {
+				return err
+			}
+			// Re-importing corrects the CATEGORY too, not just the price.
+			//
+			// This branch used to update pricing alone, which made a product
+			// imported without a category permanently unreachable: the customer
+			// app's endpoint rejects categoryid 0, `UpdateProduct` writes only
+			// productlocations.status, and re-importing — the obvious repair —
+			// silently changed nothing. There was no path back.
+			//
+			// UpdateProductCategory ignores a zero, so an import that does not
+			// carry a category still cannot erase one that is already right.
 		} else {
 			snapshot := models.Products{
 				Tenantid:      req.Tenantid,

services/productVisibility_test.go

@@ -0,0 +1,236 @@
+package services
+
+import (
+	"testing"
+
+	"nearle/models"
+	"nearle/repositories"
+)
+
+/*
+The rule these tests defend, read off the customer app's own query
+(`repositories/productRepository.go` GetProducts and
+`controllers/productController.go` GetProductsBySubcategory):
+
+	SELECT ... FROM products a
+	LEFT JOIN productlocations pl ON pl.productid = a.productid AND pl.tenantid = a.tenantid
+	WHERE a.categoryid = ?        -- and the controller rejects 0 outright
+	  AND pl.locationid = ?       -- so the LEFT JOIN behaves as an inner one
+
+A product is therefore visible to a shopper only when BOTH hold: it has a
+non-zero category, and it has a productlocations row at that outlet. Nothing
+else gates it — not `approved`, not `publishedat`, not `productstatus`, not the
+stock level.
+
+Both conditions were reachable states that no API could produce or repair, and
+a sweep of all 262 tenants found 7 products and 6 outlets already in them —
+outlets showing an empty shop while the console listed their stock.
+*/
+
+// fakeProductRepo records calls in order. The interface is large, so it is
+// embedded rather than implemented: anything these tests do not exercise is nil
+// and panics loudly if the code under test starts depending on it, which is the
+// failure we want rather than a silent zero value.
+type fakeProductRepo struct {
+	repositories.ProductRepository
+
+	calls []string
+
+	existing *models.Products
+
+	ensuredRefs []models.ProductLocationRef
+	syncedRefs  []models.ProductLocationRef
+	categorySet map[int][2]int // productid -> {categoryid, subcategoryid}
+}
+
+func newFakeRepo() *fakeProductRepo {
+	return &fakeProductRepo{categorySet: map[int][2]int{}}
+}
+
+func (f *fakeProductRepo) CreateProductStock(stocks []models.Productstock) error {
+	f.calls = append(f.calls, "CreateProductStock")
+	return nil
+}
+
+func (f *fakeProductRepo) EnsureProductLocation(refs []models.ProductLocationRef) error {
+	f.calls = append(f.calls, "EnsureProductLocation")
+	f.ensuredRefs = append(f.ensuredRefs, refs...)
+	return nil
+}
+
+func (f *fakeProductRepo) SyncProductLocationStatus(refs []models.ProductLocationRef) error {
+	f.calls = append(f.calls, "SyncProductLocationStatus")
+	f.syncedRefs = append(f.syncedRefs, refs...)
+	return nil
+}
+
+func (f *fakeProductRepo) FindTenantProductByCatalogueRef(tenantid int, brand string, catalogueid int64) (*models.Products, error) {
+	f.calls = append(f.calls, "FindTenantProductByCatalogueRef")
+	return f.existing, nil
+}
+
+func (f *fakeProductRepo) UpdateProductPricing(productid int, retailprice, productcost, taxpercent float64) error {
+	f.calls = append(f.calls, "UpdateProductPricing")
+	return nil
+}
+
+func (f *fakeProductRepo) UpdateProductCategory(productid, categoryid, subcategoryid int) error {
+	f.calls = append(f.calls, "UpdateProductCategory")
+	// Mirrors the real implementation's guard so the tests exercise the same
+	// rule the database would.
+	if categoryid <= 0 {
+		return nil
+	}
+	f.categorySet[productid] = [2]int{categoryid, subcategoryid}
+	return nil
+}
+
+func (f *fakeProductRepo) CreateProductLocation(input []models.Productlocations) error {
+	f.calls = append(f.calls, "CreateProductLocation")
+	return nil
+}
+
+// fakeCatalogueService returns one fixed catalogue product.
+type fakeCatalogueService struct {
+	CatalogueService
+	product *models.CatalogueProduct
+}
+
+func (f *fakeCatalogueService) GetProductByID(brand string, id int64) (*models.CatalogueProduct, error) {
+	return f.product, nil
+}
+
+func indexOf(calls []string, want string) int {
+	for i, c := range calls {
+		if c == want {
+			return i
+		}
+	}
+	return -1
+}
+
+// The end-to-end requirement, at the point it was broken: request a product,
+// approve it, receive the stock — and the shopper can buy it.
+//
+// Receiving used to write the stock ledger and then call
+// SyncProductLocationStatus, which is an UPDATE. At an outlet with no
+// productlocations row it updated nothing and returned success, so the console
+// showed the stock climbing and the app never listed the product.
+func TestReceivingStockPutsTheProductOnTheOutletsShelf(t *testing.T) {
+	repo := newFakeRepo()
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	err := svc.CreateProductStock([]models.Productstock{{
+		Tenantid:   1135,
+		Locationid: 1170,
+		Productid:  7086,
+		Quantity:   25,
+		Stocktype:  "in",
+	}})
+	if err != nil {
+		t.Fatalf("CreateProductStock: %v", err)
+	}
+
+	if len(repo.ensuredRefs) != 1 {
+		t.Fatalf("the product was never shelved: EnsureProductLocation saw %d refs, want 1", len(repo.ensuredRefs))
+	}
+	got := repo.ensuredRefs[0]
+	if got.Tenantid != 1135 || got.Locationid != 1170 || got.Productid != 7086 {
+		t.Errorf("shelved the wrong row: %+v", got)
+	}
+}
+
+// Order matters and is not incidental.
+//
+// EnsureProductLocation seeds the new row "outofstock" because that is true at
+// the instant of the insert. SyncProductLocationStatus then derives the real
+// value from the ledger. Run the other way round, a genuine receipt would leave
+// a freshly created row reading "outofstock" until the NEXT delivery.
+func TestTheShelfIsCreatedBeforeAvailabilityIsDerived(t *testing.T) {
+	repo := newFakeRepo()
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	if err := svc.CreateProductStock([]models.Productstock{{
+		Tenantid: 1135, Locationid: 1170, Productid: 7086, Quantity: 25, Stocktype: "in",
+	}}); err != nil {
+		t.Fatalf("CreateProductStock: %v", err)
+	}
+
+	ensure := indexOf(repo.calls, "EnsureProductLocation")
+	sync := indexOf(repo.calls, "SyncProductLocationStatus")
+	if ensure < 0 || sync < 0 {
+		t.Fatalf("expected both calls, got %v", repo.calls)
+	}
+	if ensure > sync {
+		t.Errorf("EnsureProductLocation must run before SyncProductLocationStatus, got %v", repo.calls)
+	}
+}
+
+// An "out" movement must shelve the product too.
+//
+// The location row is what makes the product addressable at all, so a sale
+// recorded at an outlet that somehow has no row should create it and then be
+// correctly derived as outofstock — not be silently dropped.
+func TestAnOutwardMovementAlsoShelves(t *testing.T) {
+	repo := newFakeRepo()
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	if err := svc.CreateProductStock([]models.Productstock{{
+		Tenantid: 1135, Locationid: 1170, Productid: 7086, Quantity: 3, Stocktype: "out",
+	}}); err != nil {
+		t.Fatalf("CreateProductStock: %v", err)
+	}
+	if len(repo.ensuredRefs) != 1 {
+		t.Fatalf("an out movement did not shelve: %v", repo.calls)
+	}
+}
+
+// Re-importing is the repair path for a product stuck at categoryid 0, and
+// until now it silently was not: the existing-product branch corrected pricing
+// and nothing else, so the obvious fix appeared to work and changed nothing.
+func TestReimportingCorrectsAnUncategorisedProduct(t *testing.T) {
+	repo := newFakeRepo()
+	repo.existing = &models.Products{Productid: 7086, Tenantid: 1135, Categoryid: 0}
+	svc := NewProductService(repo, &fakeCatalogueService{
+		product: &models.CatalogueProduct{ID: 42, Brand: "cadbury", ProductName: "Cadbury Dairy Milk 100g"},
+	})
+
+	err := svc.ImportCatalogueProduct([]models.ImportCatalogueProductRequest{{
+		Tenantid: 1135, Locationid: 1170, Brand: "cadbury", Catalogueid: 42,
+		Categoryid: 2, Retailprice: 50,
+	}})
+	if err != nil {
+		t.Fatalf("ImportCatalogueProduct: %v", err)
+	}
+
+	got, ok := repo.categorySet[7086]
+	if !ok {
+		t.Fatalf("re-import did not touch the category; calls were %v", repo.calls)
+	}
+	if got[0] != 2 {
+		t.Errorf("categoryid = %d, want 2", got[0])
+	}
+}
+
+// The correction must never run backwards. An import that omits the category
+// has nothing to say about it, and writing 0 would create exactly the invisible
+// state this whole change exists to remove.
+func TestReimportWithoutACategoryLeavesTheExistingOneAlone(t *testing.T) {
+	repo := newFakeRepo()
+	repo.existing = &models.Products{Productid: 7043, Tenantid: 1135, Categoryid: 2}
+	svc := NewProductService(repo, &fakeCatalogueService{
+		product: &models.CatalogueProduct{ID: 9, Brand: "generic", ProductName: "Apple"},
+	})
+
+	err := svc.ImportCatalogueProduct([]models.ImportCatalogueProductRequest{{
+		Tenantid: 1135, Locationid: 1170, Brand: "generic", Catalogueid: 9,
+		Categoryid: 0, Retailprice: 120,
+	}})
+	if err != nil {
+		t.Fatalf("ImportCatalogueProduct: %v", err)
+	}
+
+	if _, overwritten := repo.categorySet[7043]; overwritten {
+		t.Errorf("a category-less import overwrote an existing category")
+	}
+}