diff --git a/controllers/productVariantLinkController.go b/controllers/productVariantLinkController.go new file mode 100644 index 0000000..3b3894c --- /dev/null +++ b/controllers/productVariantLinkController.go @@ -0,0 +1,96 @@ +package controllers + +import ( + "errors" + "strconv" + + "nearle/models" + "nearle/repositories" + + "github.com/gofiber/fiber/v2" +) + +/* +Attaching sizes to a product, and taking them off again. + +The pair that was missing. `createproductvariant` wrote a name into a table +without a parent, so there was no way to say "500ml is a size of Coke" — and +without that the ordering screen had nothing but `products.variants` to work +from, a bare group number that is 0 on every product. +*/ + +// AddProductVariant — POST /products/addproductvariant +// +// Body: {tenantid, productid, variantproductid, variantname, varianttype?, price?} +func (ctl *ProductController) AddProductVariant(c *fiber.Ctx) error { + var body models.Productvariant + if err := c.BodyParser(&body); err != nil { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": fiber.StatusBadRequest, "status": false, + "message": "could not read the request body", + }) + } + + if body.Tenantid <= 0 { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": fiber.StatusBadRequest, "status": false, + "message": "tenantid is required", + }) + } + + saved, err := ctl.productService.AddProductVariant(body) + if err != nil { + // The refusals are all about the CALLER's request, so they answer 400 + // with the reason. Anything else is ours and answers 500. + for _, known := range []error{ + repositories.ErrVariantParentMissing, + repositories.ErrVariantProductMissing, + repositories.ErrVariantSelfReference, + repositories.ErrVariantNotYours, + repositories.ErrVariantDuplicate, + } { + if errors.Is(err, known) { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": fiber.StatusBadRequest, "status": false, + "message": err.Error(), + }) + } + } + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ + "code": fiber.StatusInternalServerError, "status": false, + "message": "could not add the variant", + }) + } + + return c.Status(fiber.StatusCreated).JSON(fiber.Map{ + "code": fiber.StatusCreated, "status": true, + "message": "Success", "details": saved, + }) +} + +// RemoveProductVariant — DELETE /products/removeproductvariant?tenantid=&variantid= +// +// Detaches the size. Neither product is deleted: a variant is a relationship, +// and removing it leaves two ordinary products behind. +func (ctl *ProductController) RemoveProductVariant(c *fiber.Ctx) error { + tenantid, _ := strconv.Atoi(c.Query("tenantid")) + variantid, _ := strconv.Atoi(c.Query("variantid")) + + if tenantid <= 0 || variantid <= 0 { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{ + "code": fiber.StatusBadRequest, "status": false, + "message": "tenantid and variantid are both required", + }) + } + + if err := ctl.productService.RemoveProductVariant(tenantid, variantid); err != nil { + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ + "code": fiber.StatusInternalServerError, "status": false, + "message": "could not remove the variant", + }) + } + + return c.JSON(fiber.Map{ + "code": fiber.StatusOK, "status": true, "message": "Success", + }) +} diff --git a/main.go b/main.go index 7a00545..bfaf000 100644 --- a/main.go +++ b/main.go @@ -133,6 +133,45 @@ func main() { log.Println("productlocations.publishedat added and backfilled (one time)") } + // A key generator for productvariants.variantid. + // + // The column is NOT NULL with no default and no identity, unlike + // products.productid next door which is an identity column. So every insert + // had to supply the id by hand, and GORM does not — it sent nothing and + // Postgres refused the row with a not-null violation. That is why variants + // could never be attached to a product: the write could not land at all. + // + // Guarded on the absence of a default rather than tracked as a migration + // version, matching the checks above: the question is answerable from the + // schema itself. The sequence starts above whatever ids are already there, + // so the two rows on production keep theirs. + var variantKeyed int64 + if err := db.DB.Raw(` + SELECT COUNT(1) FROM information_schema.columns + WHERE table_name = 'productvariants' AND column_name = 'variantid' + AND (column_default IS NOT NULL OR is_identity = 'YES')`). + Scan(&variantKeyed).Error; err != nil { + log.Fatal("could not check productvariants.variantid:", err) + } + if variantKeyed == 0 { + if err := db.DB.Exec(` + CREATE SEQUENCE IF NOT EXISTS productvariants_variantid_seq + START WITH 1 OWNED BY productvariants.variantid`).Error; err != nil { + log.Fatal("could not create productvariants_variantid_seq:", err) + } + if err := db.DB.Exec(` + SELECT setval('productvariants_variantid_seq', + COALESCE((SELECT MAX(variantid) FROM productvariants), 0) + 1, false)`).Error; err != nil { + log.Fatal("could not position productvariants_variantid_seq:", err) + } + if err := db.DB.Exec(` + ALTER TABLE productvariants + ALTER COLUMN variantid SET DEFAULT nextval('productvariants_variantid_seq')`).Error; err != nil { + log.Fatal("could not default productvariants.variantid:", err) + } + log.Println("productvariants.variantid given a key generator (one time)") + } + // The catalogue's own stable key for an imported product. // // `catalogueid` was never able to be this. The catalogue is rebuilt by diff --git a/models/product.go b/models/product.go index 10e279f..49da448 100644 --- a/models/product.go +++ b/models/product.go @@ -36,14 +36,48 @@ type ProductCategory struct { Updated time.Time `json:"updated"` } +// One size of one product — "500ml" hanging under the parent "Coke". +// +// The table always had the two columns that make this a relationship rather +// than a list of words: `productid` names the PARENT, `variantproductid` names +// the product a shopper actually orders when they pick this size. Neither was +// mapped here, so nothing could read or write them: `createproductvariant` +// wrote rows with no parent, and `getproductvariants` returned a flat +// per-tenant list of names. Measured 2026-09-02 across ten tenants: two rows +// existed on the whole platform, named "testing" and "demo", attached to +// nothing. +// +// The consequence is the ordering bug. With no way to attach a variant to a +// product, the app had only `products.variants` — a bare group number, 0 on +// every product — to go on. type Productvariant struct { - Variantid int `json:"variantid" gorm:"Primary_Key"` - Tenantid int `json:"tenantid"` - Variantname string `json:"variantname"` + Variantid int `json:"variantid" gorm:"primaryKey;autoIncrement"` + Tenantid int `json:"tenantid"` + + // The parent. A variant is meaningless without one, so this is what + // AddProductVariant refuses to accept as zero. + Productid int `json:"productid"` + + // The product to actually put in the basket for this size. It is a real + // product row, so it carries its own price, stock and barcode — which is + // why a variant does not need to duplicate any of them. + Variantproductid int `json:"variantproductid"` + + Variantname string `json:"variantname"` + Varianttype string `json:"varianttype,omitempty"` + Price float64 `json:"price,omitempty"` + Categoryid int `json:"categoryid" gorm:"default:0"` Categoryname string `json:"categoryname" gorm:"-"` Subcategoryid int `json:"subcategoryid"` - Status string `json:"status" gorm:"default:active"` + Status string `json:"status" gorm:"default:Active"` + + // Read-only, from the variant's own product row. The app needs a name and a + // price to draw a size picker; without these it would have to fetch each + // variant separately to render one screen. + Variantproductname string `json:"variantproductname" gorm:"->"` + Variantprice float64 `json:"variantprice" gorm:"->"` + Variantstock int `json:"variantstock" gorm:"->"` } type Products struct { @@ -107,7 +141,20 @@ type Products struct { 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"` + + // The sizes hanging under this product, so one call draws the whole screen. + // + // A separate field from `Variants` above, which is the legacy group NUMBER + // and stays exactly as it was — renaming it would break every caller, and it + // is still what an old client reads. This is the list. + // + // Always present, and EMPTY for a product with no sizes. That is the case + // that matters: an empty list is a complete answer meaning "order this one + // directly", which is what lets the app proceed instead of stalling on a + // choice that does not exist. + Variantoptions []Productvariant `json:"variantoptions" gorm:"-"` + + 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 diff --git a/repositories/productRepository.go b/repositories/productRepository.go index d7a514a..2d57bed 100644 --- a/repositories/productRepository.go +++ b/repositories/productRepository.go @@ -56,6 +56,9 @@ type ProductRepository interface { UpdateProductPricing(productid int, retailprice, productcost, taxpercent float64) error UpdateProductCategory(productid, categoryid, subcategoryid int) error UpdateProductVariant(productid, variantid int) error + AddProductVariant(v models.Productvariant) (models.Productvariant, error) + RemoveProductVariant(tenantid, variantid int) error + VariantsForProducts(tenantid, locationid int, productids []int) (map[int][]models.Productvariant, error) } type productRepository struct { @@ -1023,6 +1026,31 @@ func (r *productRepository) GetProductByVariant(tenantid, variantid, locationid, return nil, err } + // The sizes, attached to the products they belong to. + // + // This is what makes one call enough to draw the ordering screen: the app + // asks for the product it was tapped on and gets back the product plus every + // size hanging under it. A product with none comes back with an empty list, + // which is the answer that lets an order proceed. + // + // A failure here does not fail the request. The product and its price are + // the answer; the sizes are an enrichment, and returning the product without + // them beats returning nothing at all. + ids := make([]int, 0, len(data)) + for i := range data { + ids = append(ids, data[i].Productid) + } + if byParent, vErr := r.VariantsForProducts(tenantid, locationid, ids); vErr == nil { + for i := range data { + data[i].Variantoptions = byParent[data[i].Productid] + if data[i].Variantoptions == nil { + // An explicit empty list, never a JSON null: a client that does + // `variantoptions.length` must not have to null-check first. + data[i].Variantoptions = []models.Productvariant{} + } + } + } + return data, nil } diff --git a/repositories/productVariantLink.go b/repositories/productVariantLink.go new file mode 100644 index 0000000..87c8d89 --- /dev/null +++ b/repositories/productVariantLink.go @@ -0,0 +1,153 @@ +package repositories + +import ( + "errors" + + "nearle/models" + + "gorm.io/gorm" +) + +/* +Variants, as a relationship between products. + +The `productvariants` table always had `productid` (the parent) and +`variantproductid` (the product a shopper actually orders for that size). The +model did not map either, so no code could use them — which is why the ordering +screen fell back to `products.variants`, a bare group number that is 0 on every +product, and asked the backend for "everything in group 0". That matched the +tenant's entire ungrouped catalogue. + +This file is the missing half: attach a size to a product, and read the sizes +back with the one call that fetches the product. +*/ + +// ErrVariantParentMissing and friends are returned to the controller so a bad +// request answers 400 with a reason rather than 500 with a stack trace. +var ( + ErrVariantParentMissing = errors.New("a variant needs a parent product: send productid") + ErrVariantProductMissing = errors.New("a variant needs a product to order: send variantproductid") + ErrVariantSelfReference = errors.New("a product cannot be a variant of itself") + ErrVariantNotYours = errors.New("both products must belong to this tenant") + ErrVariantDuplicate = errors.New("that product is already a variant of this one") +) + +// AddProductVariant hangs one product under another as a size. +// +// Every check here is about a shape that would break the ordering screen rather +// than merely store bad data: +// +// - No parent, or no variant product: the row could never be read back by +// either side of the relationship. +// - A product as its own variant: the size picker would offer the thing you +// already tapped, and picking it would loop. +// - A product from another tenant: one shop's basket could be filled from +// another shop's shelf. +// - The same pair twice: the picker would show the size twice, and there is +// no way for a shopper to tell the duplicates apart. +func (r *productRepository) AddProductVariant(v models.Productvariant) (models.Productvariant, error) { + if v.Productid <= 0 { + return v, ErrVariantParentMissing + } + if v.Variantproductid <= 0 { + return v, ErrVariantProductMissing + } + if v.Productid == v.Variantproductid { + return v, ErrVariantSelfReference + } + + // Both ends checked in one query: two rows back means both exist and both + // are this tenant's. Anything less is a refusal, and the caller does not + // need to know which end was wrong to fix the request. + var owned int64 + if err := r.db.Table("products"). + Where("tenantid = ? AND productid IN (?, ?)", v.Tenantid, v.Productid, v.Variantproductid). + Count(&owned).Error; err != nil { + return v, err + } + if owned < 2 { + return v, ErrVariantNotYours + } + + var clash int64 + if err := r.db.Table("productvariants"). + Where("tenantid = ? AND productid = ? AND variantproductid = ?", + v.Tenantid, v.Productid, v.Variantproductid). + Count(&clash).Error; err != nil { + return v, err + } + if clash > 0 { + return v, ErrVariantDuplicate + } + + if v.Status == "" { + v.Status = "Active" + } + if err := r.db.Table("productvariants").Create(&v).Error; err != nil { + return v, err + } + return v, nil +} + +// RemoveProductVariant detaches a size. Scoped by tenant so an id from one +// shop cannot delete another's row. +func (r *productRepository) RemoveProductVariant(tenantid, variantid int) error { + if tenantid <= 0 || variantid <= 0 { + return ErrVariantParentMissing + } + return r.db.Table("productvariants"). + Where("tenantid = ? AND variantid = ?", tenantid, variantid). + Delete(nil).Error +} + +// VariantsForProducts reads the sizes for a set of parents in ONE query. +// +// Batched deliberately: the alternative is a query per product inside the loop +// that builds the response, which is the classic N+1 — a 200-product listing +// would fire 200 extra round trips to add a field that is empty for almost +// every row. +// +// The variant's own product supplies the name, the live price and the stock, so +// the app can draw a size picker — including greying out a size that is out of +// stock — without a second call. Price follows the same rule as everywhere +// else: the store's own price when it has set one, the master price otherwise. +func (r *productRepository) VariantsForProducts(tenantid, locationid int, productids []int) (map[int][]models.Productvariant, error) { + out := map[int][]models.Productvariant{} + if tenantid <= 0 || len(productids) == 0 { + return out, nil + } + + var rows []models.Productvariant + err := r.db. + Table("productvariants v"). + Select(` + v.*, + p.productname AS variantproductname, + COALESCE(NULLIF(( + SELECT pl.price FROM productlocations pl + WHERE pl.productid = v.variantproductid AND pl.tenantid = v.tenantid AND pl.locationid = ? + LIMIT 1 + ), 0), p.retailprice, 0) AS variantprice, + 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) + FROM productstocks ps + WHERE ps.productid = v.variantproductid AND ps.tenantid = v.tenantid AND ps.locationid = ? + ), 0) AS variantstock + `, locationid, locationid). + // INNER, and that is the intended filter: a variant whose product was + // deleted is not a size a shopper can be offered. + Joins("JOIN products p ON p.productid = v.variantproductid AND p.tenantid = v.tenantid"). + Where("v.tenantid = ? AND v.productid IN ?", tenantid, productids). + Where("LOWER(COALESCE(v.status, 'active')) <> 'inactive'"). + Order("v.variantid"). + Scan(&rows).Error + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { + return out, err + } + + for _, row := range rows { + out[row.Productid] = append(out[row.Productid], row) + } + return out, nil +} diff --git a/repositories/productVariantLink_test.go b/repositories/productVariantLink_test.go new file mode 100644 index 0000000..7593db9 --- /dev/null +++ b/repositories/productVariantLink_test.go @@ -0,0 +1,67 @@ +package repositories + +import ( + "errors" + "testing" + + "nearle/models" +) + +/* +Variants are a relationship between two products, and every test here is about +a shape that would break the ordering screen rather than merely store bad data. + +The bug this replaces: `productvariants` had `productid` and `variantproductid` +all along, neither was mapped, so a variant could never be attached to a +product. The app fell back to `products.variants` — a group number that is 0 on +every product — and asked for "everything in group 0", which matched the +tenant's whole ungrouped catalogue. Six unrelated products came back as each +other's sizes and the order could not be placed. +*/ + +func TestAVariantWithoutAParentIsRefused(t *testing.T) { + r := &productRepository{} + _, err := r.AddProductVariant(models.Productvariant{Tenantid: 1, Variantproductid: 7}) + if !errors.Is(err, ErrVariantParentMissing) { + t.Fatalf("expected a missing-parent refusal, got %v", err) + } +} + +func TestAVariantThatOrdersNothingIsRefused(t *testing.T) { + // Without `variantproductid` there is no product to put in the basket, so + // the size would be pickable and unbuyable. + r := &productRepository{} + _, err := r.AddProductVariant(models.Productvariant{Tenantid: 1, Productid: 7}) + if !errors.Is(err, ErrVariantProductMissing) { + t.Fatalf("expected a missing-product refusal, got %v", err) + } +} + +func TestAProductCannotBeItsOwnVariant(t *testing.T) { + // The picker would offer the thing already tapped, and choosing it loops. + r := &productRepository{} + _, err := r.AddProductVariant(models.Productvariant{Tenantid: 1, Productid: 7, Variantproductid: 7}) + if !errors.Is(err, ErrVariantSelfReference) { + t.Fatalf("expected a self-reference refusal, got %v", err) + } +} + +func TestNoProductsMeansNoQueryAndNoVariants(t *testing.T) { + // Guards the N+1: the batch loader must not run a query for an empty page. + // `r.db` is nil here, so reaching the database at all would panic. + r := &productRepository{} + got, err := r.VariantsForProducts(1, 1, nil) + if err != nil { + t.Fatalf("VariantsForProducts: %v", err) + } + if len(got) != 0 { + t.Errorf("expected no variants, got %d", len(got)) + } +} + +func TestAnUnknownTenantAsksTheDatabaseNothing(t *testing.T) { + r := &productRepository{} + if _, err := r.VariantsForProducts(0, 1, []int{7, 8}); err != nil { + t.Fatalf("VariantsForProducts: %v", err) + } +} diff --git a/routes/productroutes.go b/routes/productroutes.go index 36d39be..f370dac 100644 --- a/routes/productroutes.go +++ b/routes/productroutes.go @@ -45,6 +45,9 @@ 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) + // Sizes as a relationship: attach one product under another, and detach it. + products.Post("/addproductvariant", f.ProductController.AddProductVariant) + products.Delete("/removeproductvariant", f.ProductController.RemoveProductVariant) products.Post("/createstockrequest", f.StockRequestController.CreateStockRequest) products.Get("/getstockrequests", f.StockRequestController.GetStockRequests) diff --git a/services/productService.go b/services/productService.go index 604b1f9..a6784b9 100644 --- a/services/productService.go +++ b/services/productService.go @@ -18,6 +18,8 @@ type ProductService interface { GetProductStocks(tenantID, locationID string) ([]models.Productstocks, error) UpdateProductStatus(productIDs []int, status string) error UpdateProductVariant(productid, variantid int) error + AddProductVariant(v models.Productvariant) (models.Productvariant, error) + RemoveProductVariant(tenantid, variantid int) error CreateProductStock(stocks []models.Productstock) error CreateProduct(product models.Products) error UpdateProduct(product models.Products) error @@ -144,6 +146,16 @@ func (s *productService) UpdateProductVariant(productid, variantid int) error { return s.repo.UpdateProductVariant(productid, variantid) } +// AddProductVariant hangs a product under another as one of its sizes. +func (s *productService) AddProductVariant(v models.Productvariant) (models.Productvariant, error) { + return s.repo.AddProductVariant(v) +} + +// RemoveProductVariant detaches a size, leaving both products themselves alone. +func (s *productService) RemoveProductVariant(tenantid, variantid int) error { + return s.repo.RemoveProductVariant(tenantid, variantid) +} + func (s *productService) UpdateProductStatus(productIDs []int, status string) error { return s.repo.UpdateProductStatus(productIDs, status) }