fix on shelf

This commit is contained in:
2026-08-31 15:51:36 +05:30
parent 4bce5ac854
commit d55f101834
11 changed files with 704 additions and 55 deletions

167
services/catalogueRelink.go Normal file
View File

@@ -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)
}

View File

@@ -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)
}
}

View File

@@ -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

View File

@@ -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