From d55f101834763b3a2849e1aa15dcc3796a2f2f77 Mon Sep 17 00:00:00 2001 From: abhishek Date: Mon, 31 Aug 2026 15:51:36 +0530 Subject: [PATCH] fix on shelf --- controllers/productController.go | 25 ++++ main.go | 23 +++ models/product.go | 97 ++++++++----- repositories/catalogueColumns_test.go | 54 +++++++ repositories/catalogueRepository.go | 52 ++++++- repositories/productRepository.go | 65 ++++++++- routes/productroutes.go | 8 +- services/catalogueRelink.go | 167 +++++++++++++++++++++ services/catalogueRelink_test.go | 201 ++++++++++++++++++++++++++ services/productService.go | 45 +++++- services/productVisibility_test.go | 22 +++ 11 files changed, 704 insertions(+), 55 deletions(-) create mode 100644 services/catalogueRelink.go create mode 100644 services/catalogueRelink_test.go diff --git a/controllers/productController.go b/controllers/productController.go index e1fe584..0ab4728 100644 --- a/controllers/productController.go +++ b/controllers/productController.go @@ -854,3 +854,28 @@ func (ctl *ProductController) UnpublishProduct(c *fiber.Ctx) error { "details": fiber.Map{"productid": input.Productid, "outlets": outlets}, }) } + +// RelinkCatalogue repairs a tenant's pointers back into the global catalogue. +// +// Defaults to a DRY RUN. The pass rewrites the column that decides what a +// browse screen claims a shop already holds, and clearing a link is not +// reversible from the outside — so the whole plan should be readable before any +// of it happens. Pass `apply=true` to write. +func (ctl *ProductController) RelinkCatalogue(c *fiber.Ctx) error { + tenantID, _ := strconv.Atoi(c.Query("tenantid", "0")) + if tenantID <= 0 { + return c.Status(http.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, + "message": "tenantid is required — this repairs one merchant's links, not the platform's", + "status": false, + }) + } + // Anything but an explicit `apply=true` is a dry run, including a typo. + apply := c.Query("apply", "") == "true" + + report, err := ctl.productService.RelinkCatalogue(tenantID, !apply) + if err != nil { + return c.JSON(fiber.Map{"code": http.StatusInternalServerError, "message": err.Error(), "status": false}) + } + return c.JSON(fiber.Map{"code": 200, "message": "Success", "status": true, "details": report}) +} diff --git a/main.go b/main.go index 4ba51c7..7ef8dc6 100644 --- a/main.go +++ b/main.go @@ -102,6 +102,29 @@ func main() { log.Fatal("could not backfill productlocations.publishedat:", err) } + // The catalogue's own stable key for an imported product. + // + // `catalogueid` was never able to be this. The catalogue is rebuilt by + // scrape and renumbered every time — pepsico's live ids run 3, 6, 9 … 27, + // 30 — so a product imported when it was id 26 now points at nothing. + // Measured 2026-08-31: eleven of the nineteen links on the platform were + // dangling, which silently breaks three things (the "already imported" + // ticks, re-importing, and dedupe on the next scrape). + // + // Additive and nullable: every existing row keeps working, and a re-import + // or `/relinkcatalogue` is how one acquires the key. + if err := db.DB.Exec( + `ALTER TABLE products ADD COLUMN IF NOT EXISTS imageid text`).Error; err != nil { + log.Fatal("could not add products.imageid:", err) + } + // Not unique: two tenants legitimately stock the same catalogue product, + // and each keeps its own snapshot row. The lookup is always per tenant. + if err := db.DB.Exec( + `CREATE INDEX IF NOT EXISTS products_tenant_imageid_idx + ON products (tenantid, imageid)`).Error; err != nil { + log.Println("⚠️ could not add products.imageid index:", err) + } + // Receipts for spreadsheets sent to the catalogue ingest service. // // The ingest service holds a drop on its own terms: an unreviewed one is diff --git a/models/product.go b/models/product.go index 582498d..10e279f 100644 --- a/models/product.go +++ b/models/product.go @@ -47,21 +47,35 @@ type Productvariant struct { } type Products struct { - Productid int `json:"productid" gorm:"primaryKey;autoIncrement"` - AppLocationid int `json:"applocationid" gorm:"column:applocationid"` - Productlocationid int `json:"productlocationid" gorm:"->"` - Tenantid int `json:"tenantid,omitempty"` - Categoryid int `json:"categoryid"` - Categoryname string `json:"categoryname" gorm:"->"` - Subcategoryid int `json:"subcategoryid,omitempty"` - Subcategoryname string `json:"Subcategoryname" gorm:"->"` - Catalogueid int `json:"catalogueid,omitempty"` - Addonid int `json:"addonid,omitempty"` - Discountid int `json:"discountid"` - Discountvalue float64 `json:"discountvalue"` - Pricingid int `json:"pricingid,omitempty"` - Productname string `json:"productname,omitempty"` - Productimage string `json:"productimage,omitempty"` + Productid int `json:"productid" gorm:"primaryKey;autoIncrement"` + AppLocationid int `json:"applocationid" gorm:"column:applocationid"` + Productlocationid int `json:"productlocationid" gorm:"->"` + Tenantid int `json:"tenantid,omitempty"` + Categoryid int `json:"categoryid"` + Categoryname string `json:"categoryname" gorm:"->"` + Subcategoryid int `json:"subcategoryid,omitempty"` + Subcategoryname string `json:"Subcategoryname" gorm:"->"` + Catalogueid int `json:"catalogueid,omitempty"` + // The catalogue's own stable key for this product, e.g. `cheetos_chips_2d6bf74f`. + // + // `catalogueid` cannot do this job and never could. The catalogue is + // rebuilt by scrape and every row is renumbered when it is: pepsico's live + // ids run 3, 6, 9 … 27, 30, so a product imported when it was id 26 now + // points at nothing. Eleven of the nineteen links on the platform were + // dangling this way, and none could be repaired — the re-scrape had also + // changed the pack sizes, so the product they named no longer existed. + // + // `image_id` is the key the catalogue itself deduplicates on and it + // survives both. Written on import; `catalogueid` is kept beside it for + // rows imported before this column existed, and as the id the import call + // still addresses. + Imageid string `json:"imageid,omitempty" gorm:"column:imageid"` + Addonid int `json:"addonid,omitempty"` + Discountid int `json:"discountid"` + Discountvalue float64 `json:"discountvalue"` + Pricingid int `json:"pricingid,omitempty"` + Productname string `json:"productname,omitempty"` + Productimage string `json:"productimage,omitempty"` // Every photo the product has, as a JSON array of URLs. // @@ -77,23 +91,23 @@ type Products struct { // Held as a string rather than a []string because GORM's raw scan-into-struct // silently drops slice-kind destination fields — the same reason // `catalogueProductColumns` casts its text[] columns to text. - Productimages string `json:"productimages,omitempty" gorm:"column:productimages;type:jsonb"` + Productimages string `json:"productimages,omitempty" gorm:"column:productimages;type:jsonb"` - Productdesc string `json:"productdesc,omitempty"` - Productsku string `json:"productsku,omitempty"` - Brandid int `json:"brandid,omitempty"` - Productbrand string `json:"productbrand,omitempty"` - Productunit string `json:"productunit,omitempty"` - Unitvalue string `json:"unitvalue,omitempty"` - Toppicks string `json:"toppicks,omitempty"` - Productcost float64 `json:"productcost,omitempty"` - Taxamount float64 `json:"taxamount,omitempty"` - Taxpercent float64 `json:"taxpercent,omitempty"` - Producttax int `json:"producttax" gorm:"default:0"` - Productstock int `json:"productstock" gorm:"default:0"` - Productcombo int `json:"productcombo" gorm:"default:0"` - Variants int `json:"variants" gorm:"default:0"` - Quantity int `json:"quantity"` + Productdesc string `json:"productdesc,omitempty"` + Productsku string `json:"productsku,omitempty"` + Brandid int `json:"brandid,omitempty"` + Productbrand string `json:"productbrand,omitempty"` + Productunit string `json:"productunit,omitempty"` + Unitvalue string `json:"unitvalue,omitempty"` + Toppicks string `json:"toppicks,omitempty"` + Productcost float64 `json:"productcost,omitempty"` + Taxamount float64 `json:"taxamount,omitempty"` + Taxpercent float64 `json:"taxpercent,omitempty"` + Producttax int `json:"producttax" gorm:"default:0"` + Productstock int `json:"productstock" gorm:"default:0"` + Productcombo int `json:"productcombo" gorm:"default:0"` + Variants int `json:"variants" gorm:"default:0"` + Quantity int `json:"quantity"` // Price is the EFFECTIVE selling price at the location a query was scoped // to: productlocations.price when the store has set one, otherwise the // master Retailprice below. Read-only — it is computed by the query, never @@ -101,13 +115,13 @@ type Products struct { // a price the admin sets per store can never reach the customer app: they // returned only Retailprice, which the admin catalogue never writes. // Same meaning as Locationproducts.Price, so both product feeds agree. - Price float64 `json:"price" gorm:"->"` - Retailprice float64 `json:"retailprice,omitempty"` - Diffprice float64 `json:"diffprice,omitempty"` - Diffpercent float64 `json:"diffpercent,omitempty"` - Othercost float64 `json:"othercost,omitempty"` - Approve int `json:"approve"` - Productstatus string `json:"productstatus" ` + Price float64 `json:"price" gorm:"->"` + Retailprice float64 `json:"retailprice,omitempty"` + Diffprice float64 `json:"diffprice,omitempty"` + Diffpercent float64 `json:"diffpercent,omitempty"` + Othercost float64 `json:"othercost,omitempty"` + Approve int `json:"approve"` + Productstatus string `json:"productstatus" ` // Populated only by queries scoped to a specific location (e.g. // GetProductByVariant when locationid is passed): Productstock becomes // the live SUM(in)-SUM(out) balance from productstocks — the same @@ -313,6 +327,13 @@ type TenantCategory struct { type ImportedCatalogueRef struct { Brand string `json:"brand"` Catalogueid int64 `json:"catalogueid"` + // The stable key, when the product carries one. + // + // A browse screen should match on this in preference to the id: the id is + // renumbered by every re-scrape, so a tick placed by catalogueid lands on + // whatever product now holds that number, or on nothing at all. Empty for a + // product imported before the column existed. + Imageid string `json:"imageid"` } // ImportCatalogueProductRequest is the payload for importing a product from diff --git a/repositories/catalogueColumns_test.go b/repositories/catalogueColumns_test.go index c7cc986..dea15c4 100644 --- a/repositories/catalogueColumns_test.go +++ b/repositories/catalogueColumns_test.go @@ -154,3 +154,57 @@ func TestNoRecordedUrlsFallsBackToTheBucket(t *testing.T) { t.Errorf("expected no photos without an image store, got %#v", got) } } + +/* +Brand names arrive in two spellings, and only one of them is a key. + +The catalogue keys brands by table suffix (`24_mantra`); the ingest service's +run manifest reports the display name for the same brand ("24 Mantra"). Anything +acting on a manifest — the step that puts an uploaded sheet on a shop's shelf, +above all — holds the second and has to be able to look up the first. + +Real values, taken from the manifest of the R mart upload on 2026-08-31, which +failed with "Unknown brand: 24 Mantra" and shelved nothing. +*/ +func TestNormaliseBrandKeyFoldsDisplayNamesOntoTableKeys(t *testing.T) { + cases := map[string]string{ + "24 Mantra": "24_mantra", + "Paper Boat": "paper_boat", + "Too Yumm": "too_yumm", + "Clinic All Clear": "clinic_all_clear", + "hindustan unilever": "hindustan_unilever", + "coca-cola": "coca_cola", + + // Single-word brands were never broken; they must stay unbroken. + "Colin": "colin", + "Society": "society", + "itc": "itc", + "Kohinoor": "kohinoor", + + // A key that is already in table form passes through untouched, which is + // what makes the fallback safe to apply to every lookup. + "24_mantra": "24_mantra", + + // Separators collapse rather than doubling up, and the edges are clean — + // " P&G " must not become "__p_g_". + " P&G ": "p_g", + "Dabur Red": "dabur_red", + } + + for input, want := range cases { + if got := normaliseBrandKey(input); got != want { + t.Errorf("normaliseBrandKey(%q) = %q, want %q", input, got, want) + } + } +} + +// A brand of nothing but punctuation yields an empty key rather than a stray +// underscore. Empty misses the map and answers "unknown brand", which is the +// right answer; "_" could in principle collide with a real table suffix. +func TestNormaliseBrandKeyRefusesToInventAKey(t *testing.T) { + for _, input := range []string{"", " ", "---", "&&&"} { + if got := normaliseBrandKey(input); got != "" { + t.Errorf("normaliseBrandKey(%q) = %q, want empty", input, got) + } + } +} diff --git a/repositories/catalogueRepository.go b/repositories/catalogueRepository.go index ecb889c..fd8b69b 100644 --- a/repositories/catalogueRepository.go +++ b/repositories/catalogueRepository.go @@ -396,12 +396,56 @@ func (r *catalogueRepository) discoverBrandTables() (map[string]string, error) { return out, nil } +// tableForBrand resolves a brand to its table, tolerating the display spelling. +// +// The catalogue keys brands by table suffix — `24_mantra`, `paper_boat`, +// `clinic_all_clear` — while the ingest service's run manifest reports the +// DISPLAY name for the same brand: "24 Mantra", "Paper Boat", "Clinic All +// Clear", "hindustan unilever", "coca-cola". They are the same brand written two +// ways, and anything holding a manifest is holding the second. +// +// That mismatch broke shelving outright. Putting an uploaded sheet on a shop's +// shelf reads the catalogue once per brand to resolve `image_id`, and a single +// multi-word brand failed the whole batch with "Unknown brand: 24 Mantra". +// Every brand of more than one word was affected, which in a twenty-product +// sheet is most of them. +// +// The exact key is still tried first, so a brand whose real key genuinely +// contains a separator can never be shadowed by the normalised form. func (r *catalogueRepository) tableForBrand(brand string) (string, error) { - table, ok := r.brandTables()[strings.ToLower(strings.TrimSpace(brand))] - if !ok { - return "", ErrUnknownBrand + tables := r.brandTables() + + if table, ok := tables[strings.ToLower(strings.TrimSpace(brand))]; ok { + return table, nil } - return table, nil + if table, ok := tables[normaliseBrandKey(brand)]; ok { + return table, nil + } + return "", ErrUnknownBrand +} + +// normaliseBrandKey folds a display brand name onto the catalogue's own key. +// +// Anything that is not a letter or a digit becomes an underscore, and runs of +// them collapse — which is exactly how the catalogue builds both its table names +// and the `image_id` prefix: "24 Mantra" becomes `24_mantra`, and the product +// key `24_mantra_24_mantra_organic_moong_dal_500g`. Deliberately not a general +// slugifier; matching that one convention is its whole job. +func normaliseBrandKey(brand string) string { + out := make([]rune, 0, len(brand)) + lastWasSep := true // leading separators are dropped rather than kept + for _, r := range strings.ToLower(strings.TrimSpace(brand)) { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + out = append(out, r) + lastWasSep = false + continue + } + if !lastWasSep { + out = append(out, '_') + lastWasSep = true + } + } + return strings.TrimSuffix(string(out), "_") } func (r *catalogueRepository) GetBrands() ([]models.CatalogueBrand, error) { diff --git a/repositories/productRepository.go b/repositories/productRepository.go index 88cd9da..9730614 100644 --- a/repositories/productRepository.go +++ b/repositories/productRepository.go @@ -46,6 +46,9 @@ type ProductRepository interface { PublishProduct(tenantID, productID int, price, taxPercent float64) (int, error) UnpublishProduct(tenantID, productID int) (int, error) FindTenantProductByCatalogueRef(tenantid int, brand string, catalogueid int64) (*models.Products, error) + FindTenantProductByImageID(tenantid int, imageid string) (*models.Products, error) + SetCatalogueLink(productid int, imageid string, catalogueid int) error + ListCatalogueLinkedProducts(tenantid int) ([]models.Products, error) CreateProductReturningID(product models.Products) (int, error) GetImportedCatalogueRefs(tenantid int, brand string) ([]models.ImportedCatalogueRef, error) GetTenantCategories(tenantid int) ([]models.TenantCategory, error) @@ -1216,6 +1219,60 @@ func (r *productRepository) CreateProductReturningID(product models.Products) (i return product.Productid, nil } +// FindTenantProductByImageID looks up a tenant's snapshot by the catalogue's +// own stable key. +// +// Preferred over FindTenantProductByCatalogueRef wherever an image_id is +// available, because that one keys on a number the catalogue renumbers. After a +// re-scrape the id no longer names the product it was stored for, so the lookup +// misses, the import believes it is seeing the product for the first time, and +// the shop gets a second copy of something it already stocks. +// +// No brand in the key: image_id already carries it (`cheetos_chips_2d6bf74f`, +// `pepsico_kurkure_masala_munch_90g`) and is unique across the whole catalogue, +// which is exactly what the bare catalogueid is not. +func (r *productRepository) FindTenantProductByImageID(tenantid int, imageid string) (*models.Products, error) { + if strings.TrimSpace(imageid) == "" { + return nil, nil + } + var product models.Products + result := r.db.Table("products"). + Where("tenantid = ? AND imageid = ?", tenantid, imageid). + First(&product) + if result.Error != nil { + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return nil, nil + } + return nil, result.Error + } + return &product, nil +} + +// SetCatalogueLink repairs one product's pointer back into the catalogue. +// +// Both halves move together and both can be cleared, which is the point. +// `imageid` is what a product should be linked by from now on; a `catalogueid` +// of 0 marks a link that could not be repaired at all — the row it named is +// gone and the re-scrape left nothing matching behind it. Clearing it is not +// data loss: the pointer already pointed at nothing, and leaving it in place +// makes a browse screen claim the product is imported from a row that does not +// exist, and makes re-importing fail outright. +func (r *productRepository) SetCatalogueLink(productid int, imageid string, catalogueid int) error { + return r.db.Table("products"). + Where("productid = ?", productid). + Updates(map[string]any{"imageid": imageid, "catalogueid": catalogueid}).Error +} + +// ListCatalogueLinkedProducts returns every product of a tenant that claims to +// have come from the catalogue, so each claim can be checked. +func (r *productRepository) ListCatalogueLinkedProducts(tenantid int) ([]models.Products, error) { + products := make([]models.Products, 0) + err := r.db.Table("products"). + Where("tenantid = ? AND catalogueid IS NOT NULL AND catalogueid != 0", tenantid). + Find(&products).Error + return products, err +} + // GetImportedCatalogueRefs returns the (brand, catalogueid) pairs this // tenant has already imported, so a catalogue browse screen can mark items // as already-imported without diffing full product lists client-side. Brand @@ -1225,7 +1282,7 @@ func (r *productRepository) CreateProductReturningID(product models.Products) (i func (r *productRepository) GetImportedCatalogueRefs(tenantid int, brand string) ([]models.ImportedCatalogueRef, error) { refs := make([]models.ImportedCatalogueRef, 0) query := r.db.Table("products"). - Select("productbrand AS brand, catalogueid"). + Select("productbrand AS brand, catalogueid, COALESCE(imageid, '') AS imageid"). Where("tenantid = ? AND catalogueid IS NOT NULL AND catalogueid != 0", tenantid) if brand != "" { query = query.Where("productbrand = ?", brand) @@ -1289,9 +1346,9 @@ func (r *productRepository) UpdateProductCategory(productid, categoryid, subcate // 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 +// 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 diff --git a/routes/productroutes.go b/routes/productroutes.go index 3fb63a3..36d39be 100644 --- a/routes/productroutes.go +++ b/routes/productroutes.go @@ -29,6 +29,12 @@ func RegisterProductRoutes(api fiber.Router, f *facade.Facade) { products.Post("/createproductlocation", f.ProductController.CreateProductLocation) products.Post("/importcatalogueproduct", f.ProductController.ImportCatalogueProduct) products.Get("/getimportedcatalogueproducts", f.ProductController.GetImportedCatalogueProducts) + + // Repairing catalogue links. A dry run unless `apply=true` — see the handler, + // and `services/catalogueRelink.go` for why a dangling link can only be + // adopted or cleared, never repaired by name. + products.Get("/relinkcatalogue", f.ProductController.RelinkCatalogue) + products.Get("/gettenantcategories", f.ProductController.GetTenantCategories) products.Delete("/deleteproductlocation", f.ProductController.DeleteProductLocation) @@ -39,7 +45,7 @@ func RegisterProductRoutes(api fiber.Router, f *facade.Facade) { 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) products.Put("/updatestockrequest", f.StockRequestController.UpdateStockRequest) diff --git a/services/catalogueRelink.go b/services/catalogueRelink.go new file mode 100644 index 0000000..2772c44 --- /dev/null +++ b/services/catalogueRelink.go @@ -0,0 +1,167 @@ +package services + +import ( + "fmt" +) + +// Repairing the pointers a shop's products keep back into the global catalogue. +// +// Every product imported from the catalogue stores the id of the row it came +// from. That id is not stable: the catalogue is rebuilt by scrape and renumbered +// each time, so pepsico's live ids run 3, 6, 9 … 27, 30 and a product imported +// when it was id 26 now points at nothing. Measured 2026-08-31: eleven of the +// nineteen links on the platform were dangling. +// +// Three things break, all of them quietly: +// +// 1. The catalogue browser's "already imported" ticks land on ids that no +// longer exist, so the rows a shop really holds show as not imported — +// and importing them again makes a duplicate. +// 2. Re-importing fails outright: the import reads the catalogue row by id +// first, and answers "catalogue product not found". +// 3. The next scrape creates duplicates, because the dedupe keys on the id +// that just changed. +// +// `image_id` fixes all three going forward. This pass fixes what is already +// stored, and it can only do one of two things to a product — neither of which +// invents anything: +// +// - The id still resolves: adopt that row's `image_id`. The link now survives +// the next scrape. +// - It does not: clear the id. NOT a repair by name — the re-scrape that +// renumbered these also changed their pack sizes (Cheetos Chips 100g became +// 250g, Kurkure Menthol 10g became 50g), so the product the link named no +// longer exists in any form and matching by name would attach a shop's +// product to a DIFFERENT one. Clearing is the honest outcome: the pointer +// already pointed at nothing, and the product goes on selling from its own +// row exactly as before. + +// RelinkOutcome is what happened to one product. +type RelinkOutcome struct { + Productid int `json:"productid"` + Productname string `json:"productname"` + Brand string `json:"brand"` + Catalogueid int `json:"catalogueid"` + // "linked" — the id resolved and the stable key was adopted. + // "cleared" — the id resolved to nothing and was removed. + // "already" — the product already carried the right stable key. + Action string `json:"action"` + Imageid string `json:"imageid,omitempty"` + Reason string `json:"reason,omitempty"` +} + +// RelinkReport is the whole pass over one tenant. +type RelinkReport struct { + Tenantid int `json:"tenantid"` + Checked int `json:"checked"` + Linked int `json:"linked"` + Cleared int `json:"cleared"` + Already int `json:"already"` + DryRun bool `json:"dryrun"` + Outcomes []RelinkOutcome `json:"outcomes"` +} + +// RelinkCatalogue checks every catalogue-linked product of one tenant. +// +// `dryRun` is the default at the caller, deliberately: this rewrites a column +// that decides what a browse screen claims a shop already has, and it should be +// possible to read the whole plan before any of it happens. +func (s *productService) RelinkCatalogue(tenantid int, dryRun bool) (*RelinkReport, error) { + products, err := s.repo.ListCatalogueLinkedProducts(tenantid) + if err != nil { + return nil, err + } + + report := &RelinkReport{Tenantid: tenantid, DryRun: dryRun, Outcomes: []RelinkOutcome{}} + + for _, product := range products { + report.Checked++ + outcome := RelinkOutcome{ + Productid: product.Productid, + Productname: product.Productname, + Brand: product.Productbrand, + Catalogueid: product.Catalogueid, + } + + row, err := s.catalogueService.GetProductByID(product.Productbrand, int64(product.Catalogueid)) + // A lookup that ERRORS is not the same as one that finds nothing, and + // the difference decides whether a link is cleared. A catalogue that is + // down would otherwise read as "every row is gone" and wipe every link + // on the platform in one pass. + if err != nil { + return nil, fmt.Errorf("catalogue lookup failed for %s/%d: %w", + product.Productbrand, product.Catalogueid, err) + } + + if row == nil { + outcome.Action = "cleared" + outcome.Reason = "no catalogue row with this id — the scrape that renumbered it also removed this pack size" + if !dryRun { + if err := s.repo.SetCatalogueLink(product.Productid, "", 0); err != nil { + return nil, err + } + } + report.Cleared++ + report.Outcomes = append(report.Outcomes, outcome) + continue + } + + outcome.Imageid = row.ImageID + if product.Imageid == row.ImageID && row.ImageID != "" { + outcome.Action = "already" + report.Already++ + report.Outcomes = append(report.Outcomes, outcome) + continue + } + + // The id resolves, but to a DIFFERENT product than the one stored. + // Cleared rather than adopted: a renumber can hand an id to an unrelated + // product, and silently repointing a shop's row at it would attach the + // wrong catalogue entry — worse than no link at all. + if !sameProduct(row.ProductName, product.Productname) { + outcome.Action = "cleared" + outcome.Reason = fmt.Sprintf("id %d now names %q, not %q", + product.Catalogueid, row.ProductName, product.Productname) + outcome.Imageid = "" + if !dryRun { + if err := s.repo.SetCatalogueLink(product.Productid, "", 0); err != nil { + return nil, err + } + } + report.Cleared++ + report.Outcomes = append(report.Outcomes, outcome) + continue + } + + outcome.Action = "linked" + if !dryRun { + if err := s.repo.SetCatalogueLink(product.Productid, row.ImageID, int(row.ID)); err != nil { + return nil, err + } + } + report.Linked++ + report.Outcomes = append(report.Outcomes, outcome) + } + + return report, nil +} + +// sameProduct is deliberately strict: a name differing by one character is a +// different image_id and therefore a different product in the catalogue's own +// terms, so anything looser here would adopt the wrong row. +func sameProduct(a, b string) bool { + return normaliseName(a) == normaliseName(b) +} + +func normaliseName(value string) string { + out := make([]rune, 0, len(value)) + for _, r := range value { + switch { + case r >= 'a' && r <= 'z', r >= '0' && r <= '9': + out = append(out, r) + case r >= 'A' && r <= 'Z': + out = append(out, r+('a'-'A')) + } + } + return string(out) +} diff --git a/services/catalogueRelink_test.go b/services/catalogueRelink_test.go new file mode 100644 index 0000000..3977c98 --- /dev/null +++ b/services/catalogueRelink_test.go @@ -0,0 +1,201 @@ +package services + +import ( + "errors" + "testing" + + "nearle/models" + "nearle/repositories" +) + +/* +The rules this pass must not break. + +A catalogue link is the id a shop's product keeps back into the global +catalogue. That id is renumbered by every scrape — pepsico's live ids run +3, 6, 9 … 27, 30 — so eleven of the nineteen links on the platform pointed at +rows that no longer existed (measured 2026-08-31). + +Repairing them by NAME was the obvious move and is wrong: the scrape that +renumbered them also changed their pack sizes (Cheetos Chips 100g became 250g, +Kurkure Menthol 10g became 50g). Tested against the live catalogue, zero of the +eleven had a name match — and a looser match would have attached a shop's 100g +product to a 250g one, which is a different SKU at a different price. + +So a link can only be adopted or cleared, and the two failures that matter are +adopting the wrong row and clearing on a catalogue that is merely unreachable. +*/ + +type fakeRelinkRepo struct { + repositories.ProductRepository + + products []models.Products + writes []struct { + productid int + imageid string + catalogueid int + } +} + +func (f *fakeRelinkRepo) ListCatalogueLinkedProducts(tenantid int) ([]models.Products, error) { + return f.products, nil +} + +func (f *fakeRelinkRepo) SetCatalogueLink(productid int, imageid string, catalogueid int) error { + f.writes = append(f.writes, struct { + productid int + imageid string + catalogueid int + }{productid, imageid, catalogueid}) + return nil +} + +type fakeCatalogue struct { + CatalogueService + + rows map[int64]*models.CatalogueProduct + err error +} + +func (f *fakeCatalogue) GetProductByID(brand string, id int64) (*models.CatalogueProduct, error) { + if f.err != nil { + return nil, f.err + } + return f.rows[id], nil +} + +func newRelinkService(repo *fakeRelinkRepo, cat *fakeCatalogue) *productService { + return &productService{repo: repo, catalogueService: cat} +} + +// The happy path: the id still resolves to the same product, so the stable key +// is adopted and the link survives the next scrape. +func TestRelinkAdoptsTheStableKey(t *testing.T) { + repo := &fakeRelinkRepo{products: []models.Products{ + {Productid: 7075, Tenantid: 1147, Productbrand: "pepsico", Catalogueid: 27, Productname: "Cheetos Chips 250g"}, + }} + cat := &fakeCatalogue{rows: map[int64]*models.CatalogueProduct{ + 27: {ID: 27, Brand: "pepsico", ProductName: "Cheetos Chips 250g", ImageID: "cheetos_chips_2d6bf74f"}, + }} + + report, err := newRelinkService(repo, cat).RelinkCatalogue(1147, false) + if err != nil { + t.Fatalf("RelinkCatalogue: %v", err) + } + if report.Linked != 1 || report.Cleared != 0 { + t.Fatalf("want 1 linked 0 cleared, got %+v", report) + } + if len(repo.writes) != 1 || repo.writes[0].imageid != "cheetos_chips_2d6bf74f" { + t.Errorf("stable key not written: %+v", repo.writes) + } +} + +// The dangling case — eleven real products. The id resolves to nothing, so the +// link is cleared rather than guessed at. +func TestRelinkClearsADanglingLink(t *testing.T) { + repo := &fakeRelinkRepo{products: []models.Products{ + {Productid: 7075, Tenantid: 1147, Productbrand: "pepsico", Catalogueid: 26, Productname: "Cheetos Chips 100g"}, + }} + cat := &fakeCatalogue{rows: map[int64]*models.CatalogueProduct{}} + + report, err := newRelinkService(repo, cat).RelinkCatalogue(1147, false) + if err != nil { + t.Fatalf("RelinkCatalogue: %v", err) + } + if report.Cleared != 1 { + t.Fatalf("want 1 cleared, got %+v", report) + } + if repo.writes[0].imageid != "" || repo.writes[0].catalogueid != 0 { + t.Errorf("a cleared link must keep nothing: %+v", repo.writes[0]) + } +} + +/* +The dangerous one. A renumber can hand an old id to an UNRELATED product, and +adopting it would silently point a shop's row at the wrong catalogue entry — +worse than no link at all, because it then looks repaired. +*/ +func TestRelinkRefusesAnIdThatNowNamesSomethingElse(t *testing.T) { + repo := &fakeRelinkRepo{products: []models.Products{ + {Productid: 7070, Tenantid: 1141, Productbrand: "pepsico", Catalogueid: 6, Productname: "Kurkure Menthol 10g"}, + }} + cat := &fakeCatalogue{rows: map[int64]*models.CatalogueProduct{ + // Id 6 exists, but it is the 50g pack now — a different SKU. + 6: {ID: 6, Brand: "pepsico", ProductName: "Kurkure Menthol 50g", ImageID: "kurkure_menthol_49ef2d35"}, + }} + + report, err := newRelinkService(repo, cat).RelinkCatalogue(1141, false) + if err != nil { + t.Fatalf("RelinkCatalogue: %v", err) + } + if report.Linked != 0 || report.Cleared != 1 { + t.Fatalf("a different product must never be adopted: %+v", report) + } + if repo.writes[0].imageid != "" { + t.Errorf("wrong row adopted: %+v", repo.writes[0]) + } +} + +/* +An unreachable catalogue is not an empty one. + +Without this the pass would read "every row is gone" the moment the catalogue +database was down, and clear every link on the platform in a single run — the +one outcome here that cannot be undone. +*/ +func TestRelinkAbortsWhenTheCatalogueCannotBeRead(t *testing.T) { + repo := &fakeRelinkRepo{products: []models.Products{ + {Productid: 7075, Tenantid: 1147, Productbrand: "pepsico", Catalogueid: 26, Productname: "Cheetos Chips 100g"}, + }} + cat := &fakeCatalogue{err: errors.New("connection refused")} + + if _, err := newRelinkService(repo, cat).RelinkCatalogue(1147, false); err == nil { + t.Fatal("want an error rather than a pass that clears every link") + } + if len(repo.writes) != 0 { + t.Errorf("nothing may be written when the catalogue cannot be read: %+v", repo.writes) + } +} + +// A dry run reports exactly what a real one would do, and writes none of it. +// It is the default at the caller precisely so the plan can be read first. +func TestRelinkDryRunWritesNothing(t *testing.T) { + repo := &fakeRelinkRepo{products: []models.Products{ + {Productid: 7075, Tenantid: 1147, Productbrand: "pepsico", Catalogueid: 26, Productname: "Cheetos Chips 100g"}, + {Productid: 7081, Tenantid: 1147, Productbrand: "cadbury", Catalogueid: 9, Productname: "Cadbury Bournvita 500g"}, + }} + cat := &fakeCatalogue{rows: map[int64]*models.CatalogueProduct{ + 9: {ID: 9, Brand: "cadbury", ProductName: "Cadbury Bournvita 500g", ImageID: "cadbury_bournvita_500g"}, + }} + + report, err := newRelinkService(repo, cat).RelinkCatalogue(1147, true) + if err != nil { + t.Fatalf("RelinkCatalogue: %v", err) + } + if report.Checked != 2 || report.Cleared != 1 || report.Linked != 1 { + t.Fatalf("the plan is wrong: %+v", report) + } + if len(repo.writes) != 0 { + t.Errorf("a dry run must write nothing, wrote %+v", repo.writes) + } +} + +// Re-running the pass is a no-op on what it already fixed, so it is safe to +// leave in an operator's hands. +func TestRelinkIsIdempotent(t *testing.T) { + repo := &fakeRelinkRepo{products: []models.Products{ + {Productid: 7081, Tenantid: 1147, Productbrand: "cadbury", Catalogueid: 9, + Productname: "Cadbury Bournvita 500g", Imageid: "cadbury_bournvita_500g"}, + }} + cat := &fakeCatalogue{rows: map[int64]*models.CatalogueProduct{ + 9: {ID: 9, Brand: "cadbury", ProductName: "Cadbury Bournvita 500g", ImageID: "cadbury_bournvita_500g"}, + }} + + report, err := newRelinkService(repo, cat).RelinkCatalogue(1147, false) + if err != nil { + t.Fatalf("RelinkCatalogue: %v", err) + } + if report.Already != 1 || len(repo.writes) != 0 { + t.Errorf("want a no-op, got %+v / %+v", report, repo.writes) + } +} diff --git a/services/productService.go b/services/productService.go index 33ef81a..fbf40c6 100644 --- a/services/productService.go +++ b/services/productService.go @@ -35,6 +35,7 @@ type ProductService interface { DeleteProductLocation(tenantid, locationid, productid int) error ImportCatalogueProduct(reqs []models.ImportCatalogueProductRequest) error GetImportedCatalogueRefs(tenantid int, brand string) ([]models.ImportedCatalogueRef, error) + RelinkCatalogue(tenantid int, dryRun bool) (*RelinkReport, error) GetTenantCategories(tenantid int) ([]models.TenantCategory, error) PublishProduct(tenantID, productID int, price, taxPercent float64) (int, error) UnpublishProduct(tenantID, productID int) (int, error) @@ -226,7 +227,6 @@ func (s *productService) GetProductsBySubcategory(params models.ProductFilter) ( products = InStockOnly(products, params.LocationID) - var details []models.SubcategoryProductResponse var uncategorized []models.Products @@ -305,10 +305,28 @@ func (s *productService) ImportCatalogueProduct(reqs []models.ImportCataloguePro return fmt.Errorf("catalogue product not found: brand=%s id=%d", req.Brand, req.Catalogueid) } - existing, err := s.repo.FindTenantProductByCatalogueRef(req.Tenantid, req.Brand, req.Catalogueid) + // image_id first, catalogueid second. + // + // The order is the whole fix. `catalogueid` is renumbered by every + // re-scrape, so after one the ref lookup misses, this import believes it + // is seeing the product for the first time, and the shop ends up with a + // second copy of something it already stocks. `image_id` is the key the + // catalogue itself deduplicates on and survives both a renumber and a + // rename. + // + // The fallback is not dead code and will not become so: products + // imported before `imageid` existed carry only the id, and a re-import is + // how they acquire the stable key. + existing, err := s.repo.FindTenantProductByImageID(req.Tenantid, catalogueProduct.ImageID) if err != nil { return err } + if existing == nil { + existing, err = s.repo.FindTenantProductByCatalogueRef(req.Tenantid, req.Brand, req.Catalogueid) + if err != nil { + return err + } + } productID := 0 if existing != nil { @@ -329,6 +347,15 @@ func (s *productService) ImportCatalogueProduct(reqs []models.ImportCataloguePro if err := s.repo.UpdateProductCategory(productID, req.Categoryid, req.Subcategoryid); err != nil { return err } + // A re-import is also how an older product acquires the stable key, + // and how one whose id has since moved gets pointed back at the row + // it actually came from. Both are written together so they can never + // disagree about which catalogue row this is. + if catalogueProduct.ImageID != "" && existing.Imageid != catalogueProduct.ImageID { + if err := s.repo.SetCatalogueLink(productID, catalogueProduct.ImageID, int(catalogueProduct.ID)); err != nil { + return err + } + } // Re-importing corrects the CATEGORY too, not just the price. // // This branch used to update pricing alone, which made a product @@ -349,11 +376,14 @@ func (s *productService) ImportCatalogueProduct(reqs []models.ImportCataloguePro Productsku: catalogueProduct.ProductSKU, Productbrand: catalogueProduct.Brand, Catalogueid: int(catalogueProduct.ID), - Productunit: catalogueProduct.Size, - Productcost: req.Productcost, - Retailprice: req.Retailprice, - Taxpercent: req.Taxpercent, - Approve: 1, + // Stored beside the id, and the one that will still mean + // something after the next scrape. + Imageid: catalogueProduct.ImageID, + Productunit: catalogueProduct.Size, + Productcost: req.Productcost, + Retailprice: req.Retailprice, + Taxpercent: req.Taxpercent, + Approve: 1, } if len(catalogueProduct.Images) > 0 { // The first stays where every reader already looks for it. @@ -415,7 +445,6 @@ func (s *productService) GetTenantCategories(tenantid int) ([]models.TenantCateg return s.repo.GetTenantCategories(tenantid) } - // PublishProduct releases a product to every outlet the tenant runs. // // The price check lives in the repository rather than here, so it holds for any diff --git a/services/productVisibility_test.go b/services/productVisibility_test.go index 24cab46..d1c1dbd 100644 --- a/services/productVisibility_test.go +++ b/services/productVisibility_test.go @@ -37,6 +37,10 @@ type fakeProductRepo struct { calls []string existing *models.Products + // Set only by tests that exercise the stable-key path; nil leaves the + // import falling through to the catalogueid lookup as before. + existingByImage *models.Products + linked [][2]any ensuredRefs []models.ProductLocationRef syncedRefs []models.ProductLocationRef @@ -64,6 +68,24 @@ func (f *fakeProductRepo) SyncProductLocationStatus(refs []models.ProductLocatio return nil } +// The stable-key lookup, which the import now tries FIRST. +// +// Answers nothing unless a test sets `existingByImage`, so the fallback to the +// catalogueid ref still runs and the existing cases keep exercising it. That +// order is the fix for renumbering: `catalogueid` changes on every re-scrape, +// so a miss there makes the import believe it has never seen the product and +// create a duplicate. +func (f *fakeProductRepo) FindTenantProductByImageID(tenantid int, imageid string) (*models.Products, error) { + f.calls = append(f.calls, "FindTenantProductByImageID") + return f.existingByImage, nil +} + +func (f *fakeProductRepo) SetCatalogueLink(productid int, imageid string, catalogueid int) error { + f.calls = append(f.calls, "SetCatalogueLink") + f.linked = append(f.linked, [2]any{productid, imageid}) + return nil +} + func (f *fakeProductRepo) FindTenantProductByCatalogueRef(tenantid int, brand string, catalogueid int64) (*models.Products, error) { f.calls = append(f.calls, "FindTenantProductByCatalogueRef") return f.existing, nil