e2e changes

services/productService.go

@@ -4,9 +4,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"strings"
+	"time"
+
 	"nearle/models"
 	"nearle/repositories"
-	"time"
 )
 
 type ProductService interface {
@@ -22,7 +24,7 @@ type ProductService interface {
 	RemoveProductVariant(tenantid, variantid int) error
 	VariantChildIDs(tenantid int) (map[int]bool, error)
 	CreateProductStock(stocks []models.Productstock) error
-	CreateProduct(product models.Products) error
+	CreateProduct(product models.Products) (models.Products, error)
 	UpdateProduct(product models.Products) error
 	DeleteProduct(productID int) error
 	GetStockStatement(tenantID, locationID, subcategoryID, pageno, pagesize int, keyword string) ([]models.Productstockstatement, error)
@@ -169,8 +171,27 @@ func (s *productService) UpdateProductStatus(productIDs []int, status string) er
 	return s.repo.UpdateProductStatus(productIDs, status)
 }
 
-func (s *productService) CreateProduct(product models.Products) error {
-	return s.repo.CreateProduct(product)
+// CreateProduct stores one product and hands it back with its id filled in.
+//
+// It used to return only an error, and the id was lost on the way out: the
+// repository took the struct by value, GORM wrote the generated productid onto
+// that copy, and the copy was discarded — so the endpoint answered
+// `productid: 0` for a row that certainly had one.
+//
+// The caller needs it. A product is not sellable until it has been priced at an
+// outlet and stocked there, and both of those calls are keyed on productid, so
+// every importer had to create, re-read the tenant's whole catalogue, and match
+// its own rows back by SKU to carry on — which is also why creating two
+// products with the same SKU quietly attached the second one's stock to the
+// first.
+func (s *productService) CreateProduct(product models.Products) (models.Products, error) {
+	id, err := s.repo.CreateProductReturningID(product)
+	if err != nil {
+		return models.Products{}, err
+	}
+
+	product.Productid = id
+	return product, nil
 }
 
 func (s *productService) UpdateProduct(product models.Products) error {
@@ -308,6 +329,54 @@ func (s *productService) DeleteProductLocation(tenantid, locationid, productid i
 	return s.repo.DeleteProductLocation(tenantid, locationid, productid)
 }
 
+// catalogueFactsOf collects the catalogue fields the product table has no
+// column for, so an import keeps them instead of leaving them behind.
+//
+// Only what the catalogue actually stated: an empty field is omitted rather
+// than written as `""` or `[]`, so a reader can tell "the catalogue did not say"
+// from "the catalogue said none". The drawer prints a row per fact and an empty
+// string would print an empty row.
+//
+// The keys are the catalogue's own wire names. They are what the console
+// already reads off a live catalogue row, so the same rendering works against
+// either source without a translation layer in between.
+func catalogueFactsOf(p *models.CatalogueProduct) map[string]any {
+	facts := map[string]any{}
+	if p == nil {
+		return facts
+	}
+
+	put := func(key, value string) {
+		if v := strings.TrimSpace(value); v != "" {
+			facts[key] = v
+		}
+	}
+	putList := func(key string, values []string) {
+		kept := make([]string, 0, len(values))
+		for _, v := range values {
+			if t := strings.TrimSpace(v); t != "" {
+				kept = append(kept, t)
+			}
+		}
+		if len(kept) > 0 {
+			facts[key] = kept
+		}
+	}
+
+	put("title", p.Title)
+	put("category", p.Category)
+	put("variant_key", p.VariantKey)
+	put("sku_source", p.SKUSource)
+	put("price_range", p.PriceRange)
+	put("fssai_license", p.FSSAILicense)
+	put("search_query", p.SearchQuery)
+	putList("providers", p.Providers)
+	putList("highlights", p.Highlights)
+	putList("nutrients", p.Nutrients)
+
+	return facts
+}
+
 // ImportCatalogueProduct bridges a global catalogue product (CatalogueDB) into
 // a tenant's own store catalogue: it snapshots the catalogue product into the
 // tenant's `products` table on first import (keyed on brand+catalogueid so
@@ -417,6 +486,27 @@ func (s *productService) ImportCatalogueProduct(reqs []models.ImportCataloguePro
 				Taxpercent:  req.Taxpercent,
 				Approve:     1,
 			}
+			// Everything the snapshot has no column for, kept as the catalogue
+			// stated it.
+			//
+			// Ten of the catalogue's eighteen fields used to stop here. Two of
+			// them SHOULD — `category` is remapped to the platform's own
+			// categoryid, and `price_range` is replaced by the price the shop
+			// sets — but they are kept anyway, because what other retailers
+			// charge is the most useful thing on the drawer when somebody is
+			// deciding what to charge, and the catalogue's own category is how
+			// a mis-filed product gets noticed.
+			//
+			// Encoding failure is swallowed, like the images below: the product
+			// is worth creating without its facts, and refusing an import over
+			// a nutrition line would be the wrong trade.
+			if encoded, err := json.Marshal(catalogueFactsOf(catalogueProduct)); err == nil {
+				snapshot.Cataloguefacts = string(encoded)
+			} else {
+				log.Printf("import: could not encode catalogue facts for %s/%d: %v",
+					req.Brand, req.Catalogueid, err)
+			}
+
 			if len(catalogueProduct.Images) > 0 {
 				// The first stays where every reader already looks for it.
 				snapshot.Productimage = catalogueProduct.Images[0]

services/productVisibility_test.go

@@ -1,6 +1,7 @@
 package services
 
 import (
+	"errors"
 	"testing"
 
 	"nearle/models"
@@ -47,6 +48,10 @@ type fakeProductRepo struct {
 	publishedRefs []models.ProductLocationRef
 	created       []models.Products
 	categorySet   map[int][2]int // productid -> {categoryid, subcategoryid}
+
+	// Set to make the insert fail, for the tests that check a failed create
+	// does not hand back a half-made product.
+	createErr error
 }
 
 func newFakeRepo() *fakeProductRepo {
@@ -119,6 +124,9 @@ func (f *fakeProductRepo) UpdateProductCategory(productid, categoryid, subcatego
 // re-import branch, so this only became reachable when publishing did.
 func (f *fakeProductRepo) CreateProductReturningID(product models.Products) (int, error) {
 	f.calls = append(f.calls, "CreateProductReturningID")
+	if f.createErr != nil {
+		return 0, f.createErr
+	}
 	f.created = append(f.created, product)
 	return 9001, nil
 }
@@ -767,3 +775,72 @@ func TestPricingFilterDoesNotDisturbTheCallersSlice(t *testing.T) {
 		t.Error("the caller's slice was modified")
 	}
 }
+
+/* ── Creating a product hands back its id ───────────────────────────────────
+ *
+ * `POST /products/create` answered `productid: 0` for every product it created:
+ * the repository took the struct by value, GORM wrote the generated id onto
+ * that copy, and the copy went out of scope. The endpoint is the only way to
+ * create a product, and a product cannot be priced or stocked without its id,
+ * so every caller had to re-read the tenant's whole catalogue and find its own
+ * row again by SKU — a column nothing enforces, in an importer that creates
+ * duplicates by design.
+ */
+
+func TestCreateProductReturnsTheIdTheDatabaseAssigned(t *testing.T) {
+	repo := &fakeProductRepo{}
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	created, err := svc.CreateProduct(models.Products{
+		Tenantid:    9001,
+		Productname: "Test Rice 5kg",
+		Productsku:  "TM-RICE-5K",
+	})
+	if err != nil {
+		t.Fatalf("CreateProduct: %v", err)
+	}
+
+	// 9001 is what the fake's CreateProductReturningID returns. The point is
+	// that it reaches the caller at all — it used to be dropped.
+	if created.Productid != 9001 {
+		t.Errorf("productid = %d, want 9001 — the id was lost on the way out", created.Productid)
+	}
+
+	// The rest of the product survives the round trip, because the response is
+	// what the console shows and what it prices and stocks against.
+	if created.Productsku != "TM-RICE-5K" || created.Productname != "Test Rice 5kg" {
+		t.Errorf("the product came back altered: %+v", created)
+	}
+}
+
+func TestCreateProductGoesThroughTheOneCreatePath(t *testing.T) {
+	// There were two repository methods doing this same INSERT, one of which
+	// discarded the id. Only one remains, and this is what pins that: a second
+	// path would have to be added here to be used at all.
+	repo := &fakeProductRepo{}
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	if _, err := svc.CreateProduct(models.Products{Tenantid: 9001}); err != nil {
+		t.Fatalf("CreateProduct: %v", err)
+	}
+
+	if len(repo.calls) != 1 || repo.calls[0] != "CreateProductReturningID" {
+		t.Errorf("want exactly one call to CreateProductReturningID, got %v", repo.calls)
+	}
+}
+
+func TestAFailedCreateReturnsNoProduct(t *testing.T) {
+	// The caller prices and stocks against what comes back, so a half-made
+	// product with a zero id would be worse than an error — it would send a
+	// price and a stock movement to product 0.
+	repo := &fakeProductRepo{createErr: errors.New("duplicate key")}
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	created, err := svc.CreateProduct(models.Products{Tenantid: 9001, Productsku: "DUP"})
+	if err == nil {
+		t.Fatal("a failed insert was reported as a success")
+	}
+	if created.Productid != 0 || created.Productsku != "" {
+		t.Errorf("a product was returned for a failed create: %+v", created)
+	}
+}

services/stockrequestService.go

@@ -1,6 +1,8 @@
 package services
 
 import (
+	"errors"
+
 	"nearle/models"
 	"nearle/repositories"
 	"time"
@@ -22,6 +24,21 @@ func NewStockRequestService(repo repositories.StockRequestRepository, productSer
 }
 
 func (s *stockRequestService) CreateStockRequest(req *models.StockRequest) error {
+	// A request for nothing is not a request.
+	//
+	// Nothing downstream rejected it, so a branch could raise a request for
+	// zero units and it sat in the admin's queue looking exactly like a real
+	// one — and approving it moved no stock, which reads as the ledger being
+	// broken rather than the request being empty. A negative would move stock
+	// the wrong way on receipt, since UpdateStockRequest writes Qty straight
+	// into the ledger as an 'in'.
+	//
+	// Returned as an ordinary error: the controller already reports per-item
+	// reasons, so one bad row in a batch is named and the rest still land.
+	if req.Qty <= 0 {
+		return errors.New("quantity must be more than zero")
+	}
+
 	return s.repo.CreateStockRequest(req)
 }