diff --git a/controllers/productController.go b/controllers/productController.go index 4ce2bd9..ecf68f0 100644 --- a/controllers/productController.go +++ b/controllers/productController.go @@ -406,8 +406,17 @@ func (ctl *ProductController) GetProductByVariant(c *fiber.Ctx) error { tenantID, _ := strconv.Atoi(c.Query("tenantid")) variantid, _ := strconv.Atoi(c.Query("variantid")) locationID, _ := strconv.Atoi(c.Query("locationid")) + // `productid` is the parameter the ordering screen should send. + // + // With it the caller no longer has to know whether the thing it tapped has + // sizes: a product in a variant group comes back with the whole group to + // choose from, a product in none comes back on its own. Previously only + // `variantid` was accepted, so an ungrouped product — `variants = 0`, which + // is most of them — could not be requested at all and the order screen had + // nothing to work with. + productID, _ := strconv.Atoi(c.Query("productid")) - result, err := ctl.productService.GetProductByVariant(tenantID, variantid, locationID) + result, err := ctl.productService.GetProductByVariant(tenantID, variantid, locationID, productID) if err != nil { diff --git a/repositories/productRepository.go b/repositories/productRepository.go index 6a71e35..31f73a0 100644 --- a/repositories/productRepository.go +++ b/repositories/productRepository.go @@ -32,7 +32,7 @@ type ProductRepository interface { GetLocationProductSummary(tenantID, locationID int) ([]models.ProductSummary, error) GetSaleTemplate(tenantID, locationID int) (*models.SaleTemplate, error) FetchFilteredProducts(categoryID, subcategoryID, productID, applocationID, tenantID, locationID int, keyword, productStatus, approve string, pageno, pagesize int) ([]models.Tenantproducts, error) - GetProductByVariant(tenantid, variantid, locationid int) ([]models.Products, error) + GetProductByVariant(tenantid, variantid, locationid, productid int) ([]models.Products, error) GetSubcategories(categoryID int) ([]models.Subcategory, error) GetProducts(params models.ProductFilter) ([]models.Products, error) GetTenantInfo(tenantID, applocationID int) (map[string]interface{}, error) @@ -842,10 +842,37 @@ func (r *productRepository) FetchFilteredProducts( return results, nil } -func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid int) ([]models.Products, error) { +// GetProductByVariant returns what the app can offer for one tapped product. +// +// Two shapes, decided by the product rather than by the caller: a product that +// belongs to a variant group comes back with ALL its siblings, so the shopper +// picks a size; a product in no group comes back alone, ready to order. That is +// what the ordering screen needs, and until now only the first half existed. +// +// The query keyed solely on `p.variants = ?`, so an ungrouped product — which +// means `variants = 0`, and that is EVERY product for most tenants — could not +// be fetched at all. Asking for one returned an empty list and the app had +// nothing to place an order against. `productid` is the way in: given one, this +// reads that product's own group and answers accordingly. +// +// An explicit `variantid` still wins, so existing callers are unaffected. +func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid, productid int) ([]models.Products, error) { var data []models.Products + // Which group to return, resolved from the product when not named outright. + group := variantid + if group <= 0 && productid > 0 { + var found struct{ Variants int } + if err := r.db.Table("products"). + Select("variants"). + Where("productid = ? AND tenantid = ?", productid, tenantid). + Scan(&found).Error; err != nil { + return nil, err + } + group = found.Variants + } + // productstock/quantity are correlated subqueries (not a JOIN+GROUP BY) so // they can coexist with `p.*` without having to enumerate every products // column. quantity is duplicated on purpose: it's placed after `p.*` so it @@ -855,7 +882,7 @@ func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid // productlocations join simply match nothing, so // Productstock/Quantity/Locationstatus come back zero-valued — same // response shape as before this field existed, not an error. - err := r.db. + q := r.db. Table("products p"). Select(` p.*, @@ -864,6 +891,11 @@ func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid COALESCE(pd.discountvalue, 0) AS discountvalue, pd.discountid, pl.status AS locationstatus, + COALESCE(NULLIF(( + SELECT pl2.price FROM productlocations pl2 + WHERE pl2.productid = p.productid AND pl2.tenantid = p.tenantid AND pl2.locationid = ? + LIMIT 1 + ), 0), p.retailprice, 0) AS price, COALESCE(( SELECT SUM(CASE WHEN LOWER(ps.stocktype) = 'in' THEN ps.quantity ELSE 0 END) - SUM(CASE WHEN LOWER(ps.stocktype) = 'out' THEN ps.quantity ELSE 0 END) @@ -876,12 +908,25 @@ func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid FROM productstocks ps WHERE ps.productid = p.productid AND ps.tenantid = p.tenantid AND ps.locationid = ? ), 0) AS quantity - `, locationid, locationid). + `, locationid, locationid, locationid). Joins("LEFT JOIN productcategories c ON p.categoryid = c.categoryid"). Joins("LEFT JOIN productsubcategories d ON p.subcategoryid = d.subcatid"). Joins("LEFT JOIN productdiscounts pd ON pd.productid = p.productid"). - Joins("LEFT JOIN productlocations pl ON pl.productid = p.productid AND pl.tenantid = p.tenantid AND pl.locationid = ?", locationid). - Where("p.tenantid = ? AND p.variants = ?", tenantid, variantid). + Joins("LEFT JOIN productlocations pl ON pl.productid = p.productid AND pl.tenantid = p.tenantid AND pl.locationid = ?", locationid) + + // A group returns the whole family; no group returns the one product. + // Neither given keeps the original behaviour exactly, so nothing that + // called this before sees a different answer. + switch { + case group > 0: + q = q.Where("p.tenantid = ? AND p.variants = ?", tenantid, group) + case productid > 0: + q = q.Where("p.tenantid = ? AND p.productid = ?", tenantid, productid) + default: + q = q.Where("p.tenantid = ? AND p.variants = ?", tenantid, variantid) + } + + err := q. Order("p.productid DESC"). Scan(&data).Error diff --git a/services/productService.go b/services/productService.go index aedae61..8bd2a61 100644 --- a/services/productService.go +++ b/services/productService.go @@ -26,7 +26,7 @@ type ProductService interface { GetLocationProductSummary(tenantID, locationID int) ([]models.ProductSummary, error) GetSaleTemplate(tenantID, locationID int) (*models.SaleTemplate, error) FetchFilteredProducts(categoryID, subcategoryID, productID, applocationID, tenantID, locationID int, keyword, productStatus, approve string, pageno, pagesize int) ([]models.Tenantproducts, error) - GetProductByVariant(tenantid, variantid, locationid int) ([]models.Products, error) + GetProductByVariant(tenantid, variantid, locationid, productid int) ([]models.Products, error) GetProductsBySubcategory(params models.ProductFilter) (map[string]interface{}, error) UpdateProductLocation(input models.Productlocations) error CreateProductLocation(input []models.Productlocations) error @@ -169,19 +169,21 @@ func (s *productService) FetchFilteredProducts(categoryID, subcategoryID, produc return s.repo.FetchFilteredProducts(categoryID, subcategoryID, productID, applocationID, tenantID, locationID, keyword, productStatus, approve, pageno, pagesize) } -func (s *productService) GetProductByVariant(tenantid, variantid, locationid int) ([]models.Products, error) { +func (s *productService) GetProductByVariant(tenantid, variantid, locationid, productid int) ([]models.Products, error) { - var data []models.Products - - result, err := s.repo.GetProductByVariant(tenantid, variantid, locationid) + result, err := s.repo.GetProductByVariant(tenantid, variantid, locationid, productid) if err != nil { return nil, err } - data = result - - return data, nil + // Same rule as the browse list: you can only order what the shop has. + // + // 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 } diff --git a/services/productVisibility_test.go b/services/productVisibility_test.go index f9a0d3d..736fbec 100644 --- a/services/productVisibility_test.go +++ b/services/productVisibility_test.go @@ -353,3 +353,97 @@ func TestTheBrowseEndpointOffersOnlyStockedProducts(t *testing.T) { t.Errorf("offered product %d, want 7086 — the only one Peelamedu has", offered[0].Productid) } } + +/* +The ordering screen's rule: a product with sizes offers the whole family, a +product without offers itself. + +The repository decides that from the product's own `variants` column, so these +tests drive the service and assert on what the app receives. +*/ + +type variantRepo struct { + fakeProductRepo + gotVariantID int + gotProductID int + rows []models.Products +} + +func (v *variantRepo) GetProductByVariant(tenantid, variantid, locationid, productid int) ([]models.Products, error) { + v.gotVariantID = variantid + v.gotProductID = productid + return v.rows, nil +} + +// The case that did not work at all: an ungrouped product. Every product for +// Suriya Store is `variants = 0`, so the old signature had no way to name one +// and the order screen received an empty list. +func TestOrderingAnUngroupedProductAsksByProductId(t *testing.T) { + repo := &variantRepo{rows: []models.Products{ + {Productid: 7086, Productname: "Cadbury Dairy Milk 100g", Productstock: 25}, + }} + svc := NewProductService(repo, &fakeCatalogueService{}) + + got, err := svc.GetProductByVariant(1135, 0, 1170, 7086) + if err != nil { + t.Fatalf("GetProductByVariant: %v", err) + } + if repo.gotProductID != 7086 { + t.Errorf("productid %d never reached the repository", repo.gotProductID) + } + if len(got) != 1 || got[0].Productid != 7086 { + t.Fatalf("want just the tapped product, got %v", names(got)) + } +} + +// A product that does belong to a group still returns the whole group, so the +// shopper can pick a size. +func TestOrderingAGroupedProductOffersEveryVariant(t *testing.T) { + repo := &variantRepo{rows: []models.Products{ + {Productid: 6995, Productname: "Strawberries 250g", Productstock: 4}, + {Productid: 6996, Productname: "Strawberries 500g", Productstock: 9}, + }} + svc := NewProductService(repo, &fakeCatalogueService{}) + + got, err := svc.GetProductByVariant(1087, 44, 1097, 0) + if err != nil { + t.Fatalf("GetProductByVariant: %v", err) + } + if repo.gotVariantID != 44 { + t.Errorf("variantid %d never reached the repository", repo.gotVariantID) + } + if len(got) != 2 { + t.Fatalf("want both sizes offered, got %v", names(got)) + } +} + +// 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) { + repo := &variantRepo{rows: []models.Products{ + {Productid: 6995, Productname: "Strawberries 250g", Productstock: 0}, + {Productid: 6996, Productname: "Strawberries 500g", Productstock: 9}, + }} + svc := NewProductService(repo, &fakeCatalogueService{}) + + got, err := svc.GetProductByVariant(1087, 44, 1097, 0) + 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)) + } +} + +// Oversold is not stock. Strawberries at Ragul Stores really did read -262. +func TestAnOversoldVariantIsNotOfferedForOrder(t *testing.T) { + repo := &variantRepo{rows: []models.Products{ + {Productid: 6995, Productname: "Strawberries", Productstock: -262}, + }} + svc := NewProductService(repo, &fakeCatalogueService{}) + + got, _ := svc.GetProductByVariant(1087, 44, 1097, 0) + if len(got) != 0 { + t.Errorf("an oversold product was offered for order") + } +}