diff --git a/bible/architecture.md b/bible/architecture.md index 2e1304e..93cac64 100644 --- a/bible/architecture.md +++ b/bible/architecture.md @@ -115,7 +115,7 @@ HTTP Request ### internal/warehouse/ -`snapshot.go` — warehouse pricelist creation: weighted median, bundle expansion, allocation. +`snapshot.go` — warehouse pricelist creation: weighted average, bundle expansion, allocation. ### internal/lotmatch/ diff --git a/bible/data-rules.md b/bible/data-rules.md index fa60aca..c78424d 100644 --- a/bible/data-rules.md +++ b/bible/data-rules.md @@ -11,7 +11,7 @@ Categories are **always** taken from `lot.lot_category` (table `lot`). ### When creating pricelists -1. **Estimate**: load `lot.lot_category` for all components. +1. **Estimate**: include only components with `current_price > 0` and `is_hidden = 0`, then load `lot.lot_category`. 2. **Warehouse**: load `lot.lot_category` for all positions. 3. Persist into `lot_category` column of `qt_pricelist_items`. @@ -28,7 +28,7 @@ type PricelistItem struct { - Category is **not** a virtual field — it is persisted to DB when the pricelist is created. - JOIN with `lot` is only needed for `lot_description`; category is already in `qt_pricelist_items`. -- Default value when category is absent in source: `PART_`. +- Default value when category is absent in source or LOT row is missing: `PART_`. --- diff --git a/bible/history.md b/bible/history.md index b84b0a5..48a3100 100644 --- a/bible/history.md +++ b/bible/history.md @@ -5,6 +5,51 @@ --- +## 2026-02-20: Pricelist Formation Hardening (Estimate/Warehouse/Meta) + +### Decision + +Hardened pricelist formation rules to remove ambiguity and prevent silent data loss in UI/API. + +### What changed + +- `estimate` snapshot now explicitly excludes hidden components (`qt_lot_metadata.is_hidden = 1`). +- Category snapshot in `qt_pricelist_items.lot_category` now always has a deterministic fallback: + `PART_` is assigned even when a LOT row is missing in `lot`. +- `PricelistItem` JSON contract was normalized to a single category field + (`LotCategory -> json:"category"`), removing duplicate JSON-tag ambiguity. +- Meta-price source expansion now always includes the base LOT, then merges `meta_prices` + sources with deduplication (exact + wildcard overlaps). + +### Rationale + +- Prevent hidden/ignored components from leaking into estimate pricelists. +- Keep frontend category rendering stable for all items. +- Avoid non-deterministic JSON serialization when duplicate tags are present. +- Ensure meta-article pricing includes self-history and does not overcount duplicate sources. + +### Constraints + +- `estimate` pricelist invariant: `current_price > 0 AND is_hidden = 0`. +- `category` in API must map from persisted `qt_pricelist_items.lot_category`. +- Missing category source must default to `PART_`. +- Meta source list must contain each LOT at most once. + +### Files + +- Model: `internal/models/pricelist.go` +- Estimate/Warehouse snapshot service: `internal/services/pricelist/service.go` +- Pricing handler/meta expansion: `internal/handlers/pricing.go` +- Pricing service/meta expansion: `internal/services/pricing/service.go` +- Tests: + - `internal/services/pricelist/service_estimate_test.go` + - `internal/services/pricelist/service_warehouse_test.go` + - `internal/handlers/pricing_meta_expand_test.go` + - `internal/services/pricing/service_meta_test.go` + - `internal/services/stock_import_test.go` + +--- + ## 2026-02-20: Seen Registry Deduplication by Partnumber ### Decision diff --git a/bible/patterns.md b/bible/patterns.md index 9d9cdaf..4e8e166 100644 --- a/bible/patterns.md +++ b/bible/patterns.md @@ -169,9 +169,10 @@ type PricelistItem struct { LotName string `gorm:"size:255"` Price float64 `gorm:"type:decimal(12,2)"` + // Stored category snapshot + LotCategory *string `gorm:"column:lot_category;size:50" json:"category,omitempty"` // Virtual: populated via JOIN LotDescription string `gorm:"-:migration" json:"lot_description,omitempty"` - Category string `gorm:"-:migration" json:"category,omitempty"` // Virtual: populated programmatically AvailableQty *float64 `gorm:"-" json:"available_qty,omitempty"` Partnumbers []string `gorm:"-" json:"partnumbers,omitempty"` diff --git a/bible/pricelist.md b/bible/pricelist.md index bf533d4..9b2a88f 100644 --- a/bible/pricelist.md +++ b/bible/pricelist.md @@ -16,6 +16,10 @@ PriceForge supports three pricelist types (`source` field): **Source**: snapshot of current component prices from `qt_lot_metadata`. +**Filter**: +- Include only rows with `current_price > 0`. +- Exclude hidden rows (`is_hidden = 1`). + **Stores**: - `price_period_days` — price calculation window - `price_coefficient` — markup coefficient diff --git a/internal/handlers/pricing.go b/internal/handlers/pricing.go index a0026cd..26eb84d 100644 --- a/internal/handlers/pricing.go +++ b/internal/handlers/pricing.go @@ -326,12 +326,18 @@ func (h *PricingHandler) getMetaUsageMap(lotNames []string) map[string][]string return result } -// expandMetaPrices expands meta_prices string to list of actual lot names -func (h *PricingHandler) expandMetaPrices(metaPrices, excludeLot string) []string { +// expandMetaPrices expands meta_prices string to list of actual lot names. +// baseLot is always included as a source, then meta sources are added without duplicates. +func (h *PricingHandler) expandMetaPrices(metaPrices, baseLot string) []string { sources := strings.Split(metaPrices, ",") var result []string seen := make(map[string]bool) + if baseLot != "" { + result = append(result, baseLot) + seen[baseLot] = true + } + for _, source := range sources { source = strings.TrimSpace(source) if source == "" { @@ -343,7 +349,7 @@ func (h *PricingHandler) expandMetaPrices(metaPrices, excludeLot string) []strin prefix := strings.TrimSuffix(source, "*") var matchingLots []string h.db.Model(&models.LotMetadata{}). - Where("lot_name LIKE ? AND lot_name != ?", prefix+"%", excludeLot). + Where("lot_name LIKE ?", prefix+"%"). Pluck("lot_name", &matchingLots) for _, lot := range matchingLots { if !seen[lot] { @@ -351,7 +357,7 @@ func (h *PricingHandler) expandMetaPrices(metaPrices, excludeLot string) []strin seen[lot] = true } } - } else if source != excludeLot && !seen[source] { + } else if !seen[source] { result = append(result, source) seen[source] = true } @@ -1018,15 +1024,21 @@ func sortFloat64s(data []float64) { sort.Float64s(data) } -// expandMetaPricesWithCache expands meta_prices using pre-loaded lot names (no DB queries) -func expandMetaPricesWithCache(metaPrices, excludeLot string, allLotNames []string) []string { +// expandMetaPricesWithCache expands meta_prices using pre-loaded lot names (no DB queries). +// baseLot is always included as a source, then meta sources are added without duplicates. +func expandMetaPricesWithCache(metaPrices, baseLot string, allLotNames []string) []string { sources := strings.Split(metaPrices, ",") var result []string seen := make(map[string]bool) + if baseLot != "" { + result = append(result, baseLot) + seen[baseLot] = true + } + for _, source := range sources { source = strings.TrimSpace(source) - if source == "" || source == excludeLot { + if source == "" { continue } @@ -1034,7 +1046,7 @@ func expandMetaPricesWithCache(metaPrices, excludeLot string, allLotNames []stri // Wildcard pattern - find matching lots from cache prefix := strings.TrimSuffix(source, "*") for _, lot := range allLotNames { - if strings.HasPrefix(lot, prefix) && lot != excludeLot && !seen[lot] { + if strings.HasPrefix(lot, prefix) && !seen[lot] { result = append(result, lot) seen[lot] = true } diff --git a/internal/handlers/pricing_meta_expand_test.go b/internal/handlers/pricing_meta_expand_test.go new file mode 100644 index 0000000..4639bbb --- /dev/null +++ b/internal/handlers/pricing_meta_expand_test.go @@ -0,0 +1,33 @@ +package handlers + +import "testing" + +func TestExpandMetaPricesWithCache_IncludesBaseLotAndDeduplicates(t *testing.T) { + allLots := []string{ + "SSD_NVMe_03.2T", + "SSD_NVMe_03.2T_OEM", + "SSD_NVMe_03.2T_ES", + } + + got := expandMetaPricesWithCache( + "SSD_NVMe_03.2T,SSD_NVMe_03.2T*,SSD_NVMe_03.2T_OEM", + "SSD_NVMe_03.2T", + allLots, + ) + + want := []string{ + "SSD_NVMe_03.2T", + "SSD_NVMe_03.2T_OEM", + "SSD_NVMe_03.2T_ES", + } + + if len(got) != len(want) { + t.Fatalf("unexpected result length: got=%d want=%d; values=%v", len(got), len(want), got) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("unexpected value at %d: got=%q want=%q; full=%v", i, got[i], want[i], got) + } + } +} + diff --git a/internal/models/pricelist.go b/internal/models/pricelist.go index 60cc70a..6af44c2 100644 --- a/internal/models/pricelist.go +++ b/internal/models/pricelist.go @@ -71,10 +71,6 @@ type PricelistItem struct { LotDescription string `gorm:"-" json:"lot_description,omitempty"` AvailableQty *float64 `gorm:"-" json:"available_qty,omitempty"` Partnumbers []string `gorm:"-" json:"partnumbers,omitempty"` - - // Historical snapshot of category at time pricelist was created - // (not fetched dynamically from lot table) - Category string `gorm:"column:lot_category;size:50" json:"category,omitempty"` } func (PricelistItem) TableName() string { diff --git a/internal/services/pricelist/service.go b/internal/services/pricelist/service.go index c468073..563513e 100644 --- a/internal/services/pricelist/service.go +++ b/internal/services/pricelist/service.go @@ -210,6 +210,11 @@ func (s *Service) CreateForSourceWithProgress(createdBy, source string, sourceIt Where("lot_name IN ?", missingCategoryLots). Update("lot_category", defaultCategory).Error } + for _, lotName := range lotNames { + if _, ok := categoryMap[lotName]; !ok { + categoryMap[lotName] = &defaultCategory + } + } items = make([]models.PricelistItem, 0, len(sourceItems)) for _, srcItem := range sourceItems { @@ -217,17 +222,12 @@ func (s *Service) CreateForSourceWithProgress(createdBy, source string, sourceIt if lotName == "" || srcItem.Price <= 0 { continue } - var category string - if catPtr, ok := categoryMap[lotName]; ok && catPtr != nil { - category = *catPtr - } items = append(items, models.PricelistItem{ PricelistID: pricelist.ID, LotName: lotName, LotCategory: categoryMap[lotName], Price: srcItem.Price, PriceMethod: strings.TrimSpace(srcItem.PriceMethod), - Category: category, }) } } else { @@ -240,7 +240,7 @@ func (s *Service) CreateForSourceWithProgress(createdBy, source string, sourceIt if err := s.db.Table("qt_lot_metadata as m"). Select("m.*, COALESCE(l.lot_category, '') as lot_category"). Joins("LEFT JOIN lot as l ON l.lot_name = m.lot_name"). - Where("m.current_price IS NOT NULL AND m.current_price > 0"). + Where("m.current_price IS NOT NULL AND m.current_price > 0 AND m.is_hidden = 0"). Scan(&metadata).Error; err != nil { return nil, fmt.Errorf("getting lot metadata with categories: %w", err) } @@ -273,6 +273,11 @@ func (s *Service) CreateForSourceWithProgress(createdBy, source string, sourceIt Where("lot_name IN ?", missingCategoryLots). Update("lot_category", defaultCategory).Error } + for _, lotName := range lotNames { + if _, ok := categoryMap[lotName]; !ok { + categoryMap[lotName] = &defaultCategory + } + } // Create pricelist items with all price settings items = make([]models.PricelistItem, 0, len(metadata)) @@ -290,7 +295,6 @@ func (s *Service) CreateForSourceWithProgress(createdBy, source string, sourceIt PriceCoefficient: m.PriceCoefficient, ManualPrice: m.ManualPrice, MetaPrices: m.MetaPrices, - Category: m.LotCategory, }) } } diff --git a/internal/services/pricelist/service_estimate_test.go b/internal/services/pricelist/service_estimate_test.go new file mode 100644 index 0000000..a9022d0 --- /dev/null +++ b/internal/services/pricelist/service_estimate_test.go @@ -0,0 +1,134 @@ +package pricelist + +import ( + "testing" + + "git.mchus.pro/mchus/priceforge/internal/models" + "git.mchus.pro/mchus/priceforge/internal/repository" + "github.com/glebarez/sqlite" + "gorm.io/gorm" +) + +func TestCreateEstimatePricelist_ExcludesHiddenLots(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + + if err := db.AutoMigrate( + &models.Pricelist{}, + &models.PricelistItem{}, + &models.Lot{}, + ); err != nil { + t.Fatalf("automigrate: %v", err) + } + + if err := db.Exec(` + CREATE TABLE qt_lot_metadata ( + lot_name TEXT PRIMARY KEY, + current_price REAL, + price_method TEXT, + price_period_days INTEGER, + price_coefficient REAL, + manual_price REAL, + meta_prices TEXT, + is_hidden INTEGER + ) + `).Error; err != nil { + t.Fatalf("create qt_lot_metadata: %v", err) + } + + catStorage := "STORAGE" + if err := db.Create(&models.Lot{LotName: "SSD_VISIBLE", LotDescription: "Visible", LotCategory: &catStorage}).Error; err != nil { + t.Fatalf("seed visible lot: %v", err) + } + if err := db.Create(&models.Lot{LotName: "SSD_HIDDEN", LotDescription: "Hidden", LotCategory: &catStorage}).Error; err != nil { + t.Fatalf("seed hidden lot: %v", err) + } + + if err := db.Exec(` + INSERT INTO qt_lot_metadata (lot_name, current_price, price_method, price_period_days, price_coefficient, manual_price, meta_prices, is_hidden) + VALUES + ('SSD_VISIBLE', 100.0, 'median', 90, 0, NULL, '', 0), + ('SSD_HIDDEN', 200.0, 'median', 90, 0, NULL, '', 1) + `).Error; err != nil { + t.Fatalf("seed metadata: %v", err) + } + + repo := repository.NewPricelistRepository(db) + svc := NewService(db, repo, nil, nil) + + pl, err := svc.CreateForSourceWithProgress("tester", string(models.PricelistSourceEstimate), nil, nil) + if err != nil { + t.Fatalf("create estimate pricelist: %v", err) + } + + var items []models.PricelistItem + if err := db.Where("pricelist_id = ?", pl.ID).Order("lot_name").Find(&items).Error; err != nil { + t.Fatalf("load pricelist items: %v", err) + } + if len(items) != 1 { + t.Fatalf("expected 1 item (visible only), got %d", len(items)) + } + if items[0].LotName != "SSD_VISIBLE" { + t.Fatalf("unexpected lot in estimate pricelist: %s", items[0].LotName) + } +} + +func TestCreateEstimatePricelist_DefaultCategoryWhenLotMissing(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + + if err := db.AutoMigrate( + &models.Pricelist{}, + &models.PricelistItem{}, + &models.Lot{}, + &models.Category{}, + ); err != nil { + t.Fatalf("automigrate: %v", err) + } + + if err := db.Exec(` + CREATE TABLE qt_lot_metadata ( + lot_name TEXT PRIMARY KEY, + current_price REAL, + price_method TEXT, + price_period_days INTEGER, + price_coefficient REAL, + manual_price REAL, + meta_prices TEXT, + is_hidden INTEGER + ) + `).Error; err != nil { + t.Fatalf("create qt_lot_metadata: %v", err) + } + + if err := db.Exec(` + INSERT INTO qt_lot_metadata (lot_name, current_price, price_method, price_period_days, price_coefficient, manual_price, meta_prices, is_hidden) + VALUES ('LOT_WITHOUT_ROW', 150.0, 'median', 90, 0, NULL, '', 0) + `).Error; err != nil { + t.Fatalf("seed metadata: %v", err) + } + + repo := repository.NewPricelistRepository(db) + svc := NewService(db, repo, nil, nil) + + pl, err := svc.CreateForSourceWithProgress("tester", string(models.PricelistSourceEstimate), nil, nil) + if err != nil { + t.Fatalf("create estimate pricelist: %v", err) + } + + var item models.PricelistItem + if err := db.Where("pricelist_id = ? AND lot_name = ?", pl.ID, "LOT_WITHOUT_ROW").First(&item).Error; err != nil { + t.Fatalf("load pricelist item: %v", err) + } + if item.LotCategory == nil || *item.LotCategory != models.DefaultLotCategoryCode { + got := "" + if item.LotCategory != nil { + got = *item.LotCategory + } + t.Fatalf("expected default category %q, got %q", models.DefaultLotCategoryCode, got) + } +} diff --git a/internal/services/pricelist/service_warehouse_test.go b/internal/services/pricelist/service_warehouse_test.go index 98046a7..8f5df2e 100644 --- a/internal/services/pricelist/service_warehouse_test.go +++ b/internal/services/pricelist/service_warehouse_test.go @@ -70,3 +70,42 @@ func TestCreateWarehousePricelistFromStockLog(t *testing.T) { t.Fatalf("expected weighted average price 180, got %f", items[0].Price) } } + +func TestCreateWarehousePricelist_DefaultCategoryWhenLotMissing(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + + if err := db.AutoMigrate( + &models.Pricelist{}, + &models.PricelistItem{}, + &models.Lot{}, + &models.Category{}, + ); err != nil { + t.Fatalf("automigrate: %v", err) + } + + repo := repository.NewPricelistRepository(db) + svc := NewService(db, repo, nil, nil) + + items := []CreateItemInput{ + {LotName: "LOT_NOT_IN_LOT_TABLE", Price: 42, PriceMethod: "weighted_avg"}, + } + pl, err := svc.CreateForSourceWithProgress("tester", string(models.PricelistSourceWarehouse), items, nil) + if err != nil { + t.Fatalf("create warehouse pricelist: %v", err) + } + + var item models.PricelistItem + if err := db.Where("pricelist_id = ? AND lot_name = ?", pl.ID, "LOT_NOT_IN_LOT_TABLE").First(&item).Error; err != nil { + t.Fatalf("load pricelist item: %v", err) + } + if item.LotCategory == nil || *item.LotCategory != models.DefaultLotCategoryCode { + got := "" + if item.LotCategory != nil { + got = *item.LotCategory + } + t.Fatalf("expected default category %q, got %q", models.DefaultLotCategoryCode, got) + } +} diff --git a/internal/services/pricing/service.go b/internal/services/pricing/service.go index 66560f7..c0e9fb6 100644 --- a/internal/services/pricing/service.go +++ b/internal/services/pricing/service.go @@ -545,21 +545,26 @@ func (s *Service) RecalculateAllPricesWithProgress(onProgress func(RecalculatePr return updated, errors } -func expandMetaPricesWithCache(metaPrices, excludeLot string, allLotNames []string) []string { +func expandMetaPricesWithCache(metaPrices, baseLot string, allLotNames []string) []string { sources := strings.Split(metaPrices, ",") var result []string seen := make(map[string]bool) + if baseLot != "" { + result = append(result, baseLot) + seen[baseLot] = true + } + for _, source := range sources { source = strings.TrimSpace(source) - if source == "" || source == excludeLot { + if source == "" { continue } if strings.HasSuffix(source, "*") { prefix := strings.TrimSuffix(source, "*") for _, lot := range allLotNames { - if strings.HasPrefix(lot, prefix) && lot != excludeLot && !seen[lot] { + if strings.HasPrefix(lot, prefix) && !seen[lot] { result = append(result, lot) seen[lot] = true } diff --git a/internal/services/pricing/service_meta_test.go b/internal/services/pricing/service_meta_test.go new file mode 100644 index 0000000..72d0b8b --- /dev/null +++ b/internal/services/pricing/service_meta_test.go @@ -0,0 +1,33 @@ +package pricing + +import "testing" + +func TestExpandMetaPricesWithCache_IncludesBaseLotAndDeduplicates(t *testing.T) { + allLots := []string{ + "SSD_NVMe_03.2T", + "SSD_NVMe_03.2T_OEM", + "SSD_NVMe_03.2T_ES", + } + + got := expandMetaPricesWithCache( + "SSD_NVMe_03.2T,SSD_NVMe_03.2T*,SSD_NVMe_03.2T_OEM", + "SSD_NVMe_03.2T", + allLots, + ) + + want := []string{ + "SSD_NVMe_03.2T", + "SSD_NVMe_03.2T_OEM", + "SSD_NVMe_03.2T_ES", + } + + if len(got) != len(want) { + t.Fatalf("unexpected result length: got=%d want=%d; values=%v", len(got), len(want), got) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("unexpected value at %d: got=%q want=%q; full=%v", i, got[i], want[i], got) + } + } +} + diff --git a/internal/services/stock_import_test.go b/internal/services/stock_import_test.go index fd0cc8d..a40a5c7 100644 --- a/internal/services/stock_import_test.go +++ b/internal/services/stock_import_test.go @@ -274,7 +274,7 @@ func TestWeightedMedianFallbackToMedianWhenNoWeights(t *testing.T) { } } -func TestBuildWarehousePricelistItems_UsesPrefixResolver(t *testing.T) { +func TestBuildWarehousePricelistItems_UsesPrefixResolverAndWeightedAverage(t *testing.T) { db := openTestDB(t) if err := db.AutoMigrate(&models.StockLog{}, &models.Lot{}, &models.LotPartnumber{}); err != nil { t.Fatalf("automigrate: %v", err) @@ -306,8 +306,8 @@ func TestBuildWarehousePricelistItems_UsesPrefixResolver(t *testing.T) { if items[0].LotName != "CPU_A" { t.Fatalf("expected lot CPU_A, got %s", items[0].LotName) } - if items[0].Price != 100 { - t.Fatalf("expected weighted median 100, got %v", items[0].Price) + if items[0].Price != 105 { + t.Fatalf("expected weighted average 105, got %v", items[0].Price) } }