From 76554d26e5abf75edb79f058397a274d4d6acb21 Mon Sep 17 00:00:00 2001 From: abhishek Date: Sat, 29 Aug 2026 16:45:47 +0530 Subject: [PATCH] bugs on variant id --- controllers/catalogueController.go | 45 ++++++++++++++++++++++++ controllers/productController.go | 43 +++++++++++++++++++++++ repositories/catalogueRepository.go | 42 +++++++++++++++++++++++ repositories/productRepository.go | 53 ++++++++++++++++++++++++++++- routes/catalogueroutes.go | 2 ++ routes/productroutes.go | 1 + services/catalogueService.go | 8 +++++ services/productService.go | 11 ++++++ 8 files changed, 204 insertions(+), 1 deletion(-) diff --git a/controllers/catalogueController.go b/controllers/catalogueController.go index d550109..e850006 100644 --- a/controllers/catalogueController.go +++ b/controllers/catalogueController.go @@ -146,3 +146,48 @@ func (ctl *CatalogueController) GetProductBySKU(c *fiber.Ctx) error { "details": product, }) } + +// GetProductByImageID resolves a catalogue row by the id the ingest pipeline +// treats as canonical, so a caller holding an ingest manifest can find the +// catalogueid it needs to import the product into a shop. +func (ctl *CatalogueController) GetProductByImageID(c *fiber.Ctx) error { + brand := c.Query("brand") + imageID := c.Query("image_id") + if brand == "" || imageID == "" { + return c.JSON(fiber.Map{ + "code": 400, + "message": "brand and image_id are required", + "status": false, + }) + } + + product, err := ctl.catalogueService.GetProductByImageID(brand, imageID) + if err != nil { + if errors.Is(err, repositories.ErrUnknownBrand) { + return c.JSON(fiber.Map{ + "code": 400, + "message": "Unknown brand: " + brand, + "status": false, + }) + } + return c.JSON(fiber.Map{ + "code": 500, + "message": "Failed to fetch catalogue product", + "status": false, + }) + } + if product == nil { + return c.JSON(fiber.Map{ + "code": 404, + "message": "Product not found", + "status": false, + }) + } + + return c.JSON(fiber.Map{ + "code": http.StatusOK, + "message": "Success", + "status": true, + "details": product, + }) +} diff --git a/controllers/productController.go b/controllers/productController.go index ecf68f0..e1fe584 100644 --- a/controllers/productController.go +++ b/controllers/productController.go @@ -416,6 +416,22 @@ func (ctl *ProductController) GetProductByVariant(c *fiber.Ctx) error { // nothing to work with. productID, _ := strconv.Atoi(c.Query("productid")) + // One of the two has to name something, and `variantid=0` names nothing. + // + // 0 is not a variant group — it is the value every UNGROUPED product + // carries, so a query for it used to match the tenant's whole ungrouped + // catalogue and return six unrelated products as each other's variants. The + // ordering screen then had a list it could not choose from, and no order + // could be placed. Refusing here turns that into one sentence naming the + // parameter to send instead. + if variantid <= 0 && productID <= 0 { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, + "status": false, + "message": "send productid (preferred) or a variantid above 0 — variantid 0 is not a variant group, it is what every ungrouped product carries", + }) + } + result, err := ctl.productService.GetProductByVariant(tenantID, variantid, locationID, productID) if err != nil { @@ -535,6 +551,33 @@ func (ctl *ProductController) CreateProductLocation(c *fiber.Ctx) error { }) } +// UpdateProductVariant puts a product into a variant group, or takes it out. +// +// variantid 0 means ungrouped and is allowed — it is a real thing to want. The +// read path is what refuses to treat 0 as a group to search for. +func (ctl *ProductController) UpdateProductVariant(c *fiber.Ctx) error { + var body struct { + Productid int `json:"productid"` + Variantid int `json:"variantid"` + } + if err := c.BodyParser(&body); err != nil { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, "status": false, "message": "Invalid request body", + }) + } + if body.Productid <= 0 { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, "status": false, "message": "productid is required", + }) + } + if err := ctl.productService.UpdateProductVariant(body.Productid, body.Variantid); err != nil { + return c.Status(fiber.StatusConflict).JSON(fiber.Map{ + "code": http.StatusConflict, "status": false, "message": err.Error(), + }) + } + return c.JSON(fiber.Map{"code": http.StatusOK, "status": true, "message": "Success"}) +} + func (ctl *ProductController) CreateProductVariant(c *fiber.Ctx) error { var input models.Productvariant diff --git a/repositories/catalogueRepository.go b/repositories/catalogueRepository.go index 99764e0..ecb889c 100644 --- a/repositories/catalogueRepository.go +++ b/repositories/catalogueRepository.go @@ -229,6 +229,7 @@ type CatalogueRepository interface { GetProducts(brand, category, keyword string, pageno, pagesize int) ([]models.CatalogueProduct, int64, error) GetProductBySKU(brand, sku string) (*models.CatalogueProduct, error) GetProductByID(brand string, id int64) (*models.CatalogueProduct, error) + GetProductByImageID(brand, imageID string) (*models.CatalogueProduct, error) } type catalogueRepository struct { @@ -615,6 +616,47 @@ func (r *catalogueRepository) GetProductBySKU(brand, sku string) (*models.Catalo return &product, nil } +// GetProductByImageID resolves a catalogue row by the id the ingest pipeline +// calls canonical. +// +// The pipeline reports what it wrote as a manifest of `image_id` values, and +// its own note is blunt about why: image_id is "the primary key every other +// product is deduplicated on", and a product name differing by one character +// is a different product. Importing into a shop needs `catalogueid` — the row +// id — so without this the console would have to match the manifest on NAME, +// which silently creates duplicates instead of updating. +// +// Alongside GetProductBySKU rather than replacing it: a sheet may leave the sku +// column blank, in which case the pipeline mints one and the sku is not a key +// the sender recognises. image_id is derived from brand, name and size and is +// stable across re-uploads. +func (r *catalogueRepository) GetProductByImageID(brand, imageID string) (*models.CatalogueProduct, error) { + if r.db == nil { + return nil, ErrCatalogueDBUnavailable + } + + table, err := r.tableForBrand(brand) + if err != nil { + return nil, err + } + + var row catalogueProductRow + query := fmt.Sprintf( + `SELECT %s FROM %s WHERE image_id = ? LIMIT 1`, + columnsForTable(table), table, + ) + result := r.db.Raw(query, strings.TrimSpace(imageID)).Scan(&row) + if result.Error != nil { + return nil, result.Error + } + if result.RowsAffected == 0 { + return nil, nil + } + + product := row.toModel(strings.ToLower(brand)) + return &product, nil +} + func (r *catalogueRepository) GetProductByID(brand string, id int64) (*models.CatalogueProduct, error) { if r.db == nil { return nil, ErrCatalogueDBUnavailable diff --git a/repositories/productRepository.go b/repositories/productRepository.go index f0916a4..88cd9da 100644 --- a/repositories/productRepository.go +++ b/repositories/productRepository.go @@ -51,6 +51,7 @@ type ProductRepository interface { GetTenantCategories(tenantid int) ([]models.TenantCategory, error) UpdateProductPricing(productid int, retailprice, productcost, taxpercent float64) error UpdateProductCategory(productid, categoryid, subcategoryid int) error + UpdateProductVariant(productid, variantid int) error } type productRepository struct { @@ -945,7 +946,26 @@ func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid, case productid > 0: q = q.Where("p.tenantid = ? AND p.productid = ?", tenantid, productid) default: - q = q.Where("p.tenantid = ? AND p.variants = ?", tenantid, variantid) + // Neither a group nor a product was named, so there is nothing to + // return — and returning nothing is the point. + // + // This branch used to run `WHERE p.variants = 0`, which is not "no + // match": EVERY ungrouped product carries variants 0, so asking for + // variant 0 handed back the tenant's entire ungrouped catalogue as if + // those products were variants of one another. Measured on tenant 1135: + // six unrelated products — a chocolate bar, a chewing gum and an apple — + // returned as each other's variants. + // + // That is what stops an order being placed. The app sends the tapped + // product's `variants` value, which is 0 for almost every product, and + // receives six things it must choose between. There is no correct choice + // to make, so the screen cannot proceed. + // + // An empty result is the honest answer to a question that named nothing. + // The controller rejects this case outright with a message naming + // `productid`; this is the second line of defence, so no future caller + // can reach the match-everything behaviour by another route. + return data, nil } err := q. @@ -1261,6 +1281,37 @@ func (r *productRepository) UpdateProductCategory(productid, categoryid, subcate return r.db.Table("products").Where("productid = ?", productid).Updates(updates).Error } +// UpdateProductVariant puts an existing product into a variant group, or takes +// it out of one. +// +// It exists because nothing else could. `products.variants` is the grouping the +// ordering screen reads — it is what makes three pack sizes one choice rather +// than three unrelated products — and until now it could only ever be set at +// CREATE time, by a caller that already knew the group id: +// +// products/create writes whatever the body carries, variants included +// importcatalogueproduct never sets it, so every imported product is 0 +// UpdateProduct writes productlocations.status only, despite the name +// +// Since importing from the catalogue is how products actually arrive, every +// product this console creates is ungrouped and there was no call that could +// change that. Groups could be created (createproductvariant) and never used. +// +// Zero is allowed here, unlike UpdateProductCategory: ungrouping a product is a +// real thing to want, and 0 is what ungrouped means. The guard that matters for +// this column lives in the read path, which refuses to treat 0 as a group. +func (r *productRepository) UpdateProductVariant(productid, variantid int) error { + if productid <= 0 { + return fmt.Errorf("productid is required") + } + if variantid < 0 { + variantid = 0 + } + return r.db.Table("products"). + Where("productid = ?", productid). + Update("variants", variantid).Error +} + func (r *productRepository) UpdateProductPricing(productid int, retailprice, productcost, taxpercent float64) error { return r.db.Table("products"). Where("productid = ?", productid). diff --git a/routes/catalogueroutes.go b/routes/catalogueroutes.go index c35c4b5..995ff29 100644 --- a/routes/catalogueroutes.go +++ b/routes/catalogueroutes.go @@ -14,6 +14,7 @@ func RegisterCatalogueRoutes(api fiber.Router, f *facade.Facade) { catalogue.Get("/getcategories", f.CatalogueController.GetCategories) catalogue.Get("/getproducts", f.CatalogueController.GetProducts) catalogue.Get("/getproduct", f.CatalogueController.GetProductBySKU) + catalogue.Get("/getproductbyimageid", f.CatalogueController.GetProductByImageID) catalogue = api.Group("/v1/mob/catalogue") @@ -21,4 +22,5 @@ func RegisterCatalogueRoutes(api fiber.Router, f *facade.Facade) { catalogue.Get("/getcategories", f.CatalogueController.GetCategories) catalogue.Get("/getproducts", f.CatalogueController.GetProducts) catalogue.Get("/getproduct", f.CatalogueController.GetProductBySKU) + catalogue.Get("/getproductbyimageid", f.CatalogueController.GetProductByImageID) } diff --git a/routes/productroutes.go b/routes/productroutes.go index c2a2902..3fb63a3 100644 --- a/routes/productroutes.go +++ b/routes/productroutes.go @@ -38,6 +38,7 @@ func RegisterProductRoutes(api fiber.Router, f *facade.Facade) { products.Post("/publishproduct", f.ProductController.PublishProduct) products.Post("/unpublishproduct", f.ProductController.UnpublishProduct) products.Post("/createproductvariant", f.ProductController.CreateProductVariant) + products.Put("/updateproductvariant", f.ProductController.UpdateProductVariant) products.Post("/createstockrequest", f.StockRequestController.CreateStockRequest) products.Get("/getstockrequests", f.StockRequestController.GetStockRequests) diff --git a/services/catalogueService.go b/services/catalogueService.go index 30365b0..618af17 100644 --- a/services/catalogueService.go +++ b/services/catalogueService.go @@ -11,6 +11,7 @@ type CatalogueService interface { GetProducts(brand, category, keyword string, pageno, pagesize int) ([]models.CatalogueProduct, int64, error) GetProductBySKU(brand, sku string) (*models.CatalogueProduct, error) GetProductByID(brand string, id int64) (*models.CatalogueProduct, error) + GetProductByImageID(brand, imageID string) (*models.CatalogueProduct, error) } type catalogueService struct { @@ -37,6 +38,13 @@ func (s *catalogueService) GetProductBySKU(brand, sku string) (*models.Catalogue return s.repo.GetProductBySKU(brand, sku) } +// GetProductByImageID is the join the ingest manifest needs: the pipeline +// reports what it wrote as image_id values, and importing into a shop needs +// the row id. +func (s *catalogueService) GetProductByImageID(brand, imageID string) (*models.CatalogueProduct, error) { + return s.repo.GetProductByImageID(brand, imageID) +} + func (s *catalogueService) GetProductByID(brand string, id int64) (*models.CatalogueProduct, error) { return s.repo.GetProductByID(brand, id) } diff --git a/services/productService.go b/services/productService.go index ffa40cb..33ef81a 100644 --- a/services/productService.go +++ b/services/productService.go @@ -17,6 +17,7 @@ type ProductService interface { GetCatalougeProducts(tenantID, locationID, subcategoryID, pageno, pagesize int, keyword string) ([]models.Products, error) GetProductStocks(tenantID, locationID string) ([]models.Productstocks, error) UpdateProductStatus(productIDs []int, status string) error + UpdateProductVariant(productid, variantid int) error CreateProductStock(stocks []models.Productstock) error CreateProduct(product models.Products) error UpdateProduct(product models.Products) error @@ -132,6 +133,16 @@ func (s *productService) CreateProductStock(stocks []models.Productstock) error return nil } +// UpdateProductVariant groups an existing product, or ungroups it with 0. +// +// The ordering screen reads products.variants to decide whether a product is +// one choice or several, and nothing could write that column after creation — +// so a catalogue import, which is how products actually arrive, produced +// permanently ungrouped products. +func (s *productService) UpdateProductVariant(productid, variantid int) error { + return s.repo.UpdateProductVariant(productid, variantid) +} + func (s *productService) UpdateProductStatus(productIDs []int, status string) error { return s.repo.UpdateProductStatus(productIDs, status) }