variant and price

repositories/productRepository.go

@@ -748,6 +748,21 @@ func (r *productRepository) FetchFilteredProducts(
 	//     dropped from the balance.
 	// locationID 0 means "not scoped to an outlet": stock is then the tenant's
 	// total across outlets, which is what an unscoped listing should show.
+	//
+	// `price` is computed here for the same reason it is computed in GetProducts
+	// and GetProductByVariant, and its absence was the same bug all three times.
+	// `a.*` carries `products.price`, which nothing ever writes — the selling
+	// price lives in `productlocations.price`, set per outlet when the admin
+	// prices a product, with `products.retailprice` as the tenant-wide fallback.
+	// Without this the endpoint returned price 0 for every row while
+	// retailprice held the real figure, so an app reading `price` showed nothing
+	// at all. Measured on tenant 1135: price 0 / retailprice 50 on this
+	// endpoint, against price 50 on the other three.
+	//
+	// A correlated subquery rather than a read off the joined `b`, because that
+	// derived table takes MAX(locationid) to collapse duplicates and would hand
+	// back an arbitrary outlet's price whenever the caller did not scope to one.
+	// Ordering by locationid at least makes the unscoped answer deterministic.
 	query := r.db.
 		Table("products a").
 		Select(`
@@ -756,9 +771,16 @@ func (r *productRepository) FetchFilteredProducts(
 		b.locationid,
 		c.categoryname,
 		d.subcatname AS subcategoryname,
+		COALESCE(NULLIF((
+			SELECT pl2.price FROM productlocations pl2
+			WHERE pl2.productid = a.productid AND pl2.tenantid = a.tenantid
+			  AND (? = 0 OR pl2.locationid = ?)
+			ORDER BY pl2.locationid
+			LIMIT 1
+		), 0), a.retailprice, 0) AS price,
 		COALESCE(ps.quantity, 0) AS productstock,
 		COALESCE(ps.quantity, 0) AS quantity
-	`).
+	`, locationID, locationID).
 		Joins(`
 		LEFT JOIN (
 			SELECT productid, tenantid,

services/productService.go

@@ -177,13 +177,27 @@ func (s *productService) GetProductByVariant(tenantid, variantid, locationid, pr
 		return nil, err
 	}
 
-	// Same rule as the browse list: you can only order what the shop has.
+	// NOT filtered by stock, and that is the opposite of the browse list on
+	// purpose.
 	//
-	// Applied here too so the two screens cannot disagree. Without it a variant
-	// group would offer every size on the order sheet while the list that led
-	// there showed only the stocked ones, and a shopper could pick a size the
-	// branch has none of. Unscoped calls are left alone — see InStockOnly.
-	return InStockOnly(result, locationid), nil
+	// This endpoint backs the product screen — the one a shopper reaches by
+	// tapping something. It must always return what they tapped, plus every
+	// member of its variant group, whatever the shelf holds. Filtered, it
+	// produced two failures a shopper would meet immediately: an out-of-stock
+	// product answered with ZERO rows, leaving the screen with nothing to draw;
+	// and tapping "Apple" in a family where only Pineapple was stocked returned
+	// Pineapple, so the page showed something the shopper had not asked for.
+	//
+	// The rule the business wants — only order what we have — is still kept,
+	// in the two places that actually enforce it. The browse list offers only
+	// stocked products, and CreateOrder independently refuses a line it cannot
+	// fill (assertStockAvailable). So showing an empty size here cannot sell
+	// one; it only lets the app grey it out, the way every storefront does.
+	//
+	// `productstock` on each row is what the app disables on. A single ungrouped
+	// product comes back as a one-member group, so the app has one code path:
+	// count the rows, and skip the picker when there is only one.
+	return result, nil
 
 }
 

services/productVisibility_test.go

@@ -417,33 +417,77 @@ func TestOrderingAGroupedProductOffersEveryVariant(t *testing.T) {
 	}
 }
 
-// The order screen must not offer a size the branch has none of — the list that
-// led there does not show it, and the two must agree.
-func TestAVariantWithNoStockIsNotOfferedForOrder(t *testing.T) {
+// The product screen shows what was tapped, in stock or not.
+//
+// Filtered, an out-of-stock product answered with zero rows and the screen had
+// nothing to draw. Nothing is sold by showing it: the browse list never offers
+// it, and CreateOrder refuses the line independently.
+func TestTheProductScreenStillShowsAnEmptyProduct(t *testing.T) {
 	repo := &variantRepo{rows: []models.Products{
-		{Productid: 6995, Productname: "Strawberries 250g", Productstock: 0},
-		{Productid: 6996, Productname: "Strawberries 500g", Productstock: 9},
+		{Productid: 7085, Productname: "Cadbury 5 Star 18g", Productstock: 0},
 	}}
 	svc := NewProductService(repo, &fakeCatalogueService{})
 
-	got, err := svc.GetProductByVariant(1087, 44, 1097, 0)
+	got, err := svc.GetProductByVariant(1135, 0, 1170, 7085)
 	if err != nil {
 		t.Fatalf("GetProductByVariant: %v", err)
 	}
-	if len(got) != 1 || got[0].Productid != 6996 {
-		t.Fatalf("want only the stocked size, got %v", names(got))
+	if len(got) != 1 {
+		t.Fatalf("the tapped product vanished from its own screen: %v", names(got))
+	}
+	if got[0].Productstock != 0 {
+		t.Errorf("stock must survive so the app can grey the option out, got %d", got[0].Productstock)
 	}
 }
 
-// Oversold is not stock. Strawberries at Ragul Stores really did read -262.
-func TestAnOversoldVariantIsNotOfferedForOrder(t *testing.T) {
+// Every member of the family comes back, so the shopper sees the full range and
+// the one they tapped is always among them. Tapping "Apple" used to return
+// "Pineapple" — the only stocked member — which is not the product they asked
+// for.
+func TestTheWholeFamilyIsShownIncludingEmptyOnes(t *testing.T) {
 	repo := &variantRepo{rows: []models.Products{
-		{Productid: 6995, Productname: "Strawberries", Productstock: -262},
+		{Productid: 7014, Productname: "Apple", Productstock: 0},
+		{Productid: 6994, Productname: "Pineapple", Productstock: 50},
+		{Productid: 6989, Productname: "Jammu Apple", Productstock: 0},
 	}}
 	svc := NewProductService(repo, &fakeCatalogueService{})
 
-	got, _ := svc.GetProductByVariant(1087, 44, 1097, 0)
-	if len(got) != 0 {
-		t.Errorf("an oversold product was offered for order")
+	got, err := svc.GetProductByVariant(1087, 36, 1097, 7014)
+	if err != nil {
+		t.Fatalf("GetProductByVariant: %v", err)
+	}
+	if len(got) != 3 {
+		t.Fatalf("want all 3 family members, got %v", names(got))
+	}
+	tapped := false
+	for _, p := range got {
+		if p.Productid == 7014 {
+			tapped = true
+		}
+	}
+	if !tapped {
+		t.Error("the tapped product is missing from its own variant list")
+	}
+}
+
+// The browse list is the screen that must NOT offer an empty shelf, and it
+// still does not. The two endpoints deliberately differ.
+func TestBrowseStillHidesWhatTheShopDoesNotHave(t *testing.T) {
+	repo := newFakeRepo()
+	svc := NewProductService(repo, &fakeCatalogueService{})
+
+	result, err := svc.GetProductsBySubcategory(models.ProductFilter{
+		CategoryID: 2, TenantID: 1135, LocationID: 1170,
+	})
+	if err != nil {
+		t.Fatalf("GetProductsBySubcategory: %v", err)
+	}
+	groups, _ := result["details"].([]models.SubcategoryProductResponse)
+	var offered []models.Products
+	for _, g := range groups {
+		offered = append(offered, g.Products...)
+	}
+	if len(offered) != 1 || offered[0].Productid != 7086 {
+		t.Fatalf("browse should still offer only the stocked product, got %v", names(offered))
 	}
 }