From 40500f936a2b888be6197e6d1ffbee65dedb9c49 Mon Sep 17 00:00:00 2001 From: abhishek Date: Fri, 28 Aug 2026 16:54:55 +0530 Subject: [PATCH] variant and price --- repositories/productRepository.go | 24 +++++++++- services/productService.go | 26 ++++++++--- services/productVisibility_test.go | 72 ++++++++++++++++++++++++------ 3 files changed, 101 insertions(+), 21 deletions(-) diff --git a/repositories/productRepository.go b/repositories/productRepository.go index 31f73a0..f0916a4 100644 --- a/repositories/productRepository.go +++ b/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, diff --git a/services/productService.go b/services/productService.go index 8bd2a61..ffa40cb 100644 --- a/services/productService.go +++ b/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 } diff --git a/services/productVisibility_test.go b/services/productVisibility_test.go index 736fbec..24cab46 100644 --- a/services/productVisibility_test.go +++ b/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)) } }