Deduplicate configuration revisions and update revisions UI

2026-02-19 14:09:00 +03:00
parent 81203fc7a7
commit 530aa0ae48
10 changed files with 839 additions and 188 deletions
--- a/internal/localdb/local_migrations_test.go
+++ b/internal/localdb/local_migrations_test.go
@@ -4,6 +4,8 @@ import (
 	"path/filepath"
 	"testing"
 	"time"
+
+	"github.com/google/uuid"
 )

 func TestRunLocalMigrationsBackfillsExistingConfigurations(t *testing.T) {
@@ -125,3 +127,129 @@ func TestRunLocalMigrationsFixesPricelistVersionUniqueIndex(t *testing.T) {
 		t.Fatalf("expected 2 pricelists, got %d", count)
 	}
 }
+
+func TestRunLocalMigrationsDeduplicatesConfigurationVersionsBySpecAndPrice(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "versions_dedup.db")
+
+	local, err := New(dbPath)
+	if err != nil {
+		t.Fatalf("open localdb: %v", err)
+	}
+	t.Cleanup(func() { _ = local.Close() })
+
+	cfg := &LocalConfiguration{
+		UUID:             "dedup-cfg",
+		Name:             "Dedup",
+		Items:            LocalConfigItems{{LotName: "CPU_A", Quantity: 1, UnitPrice: 100}},
+		ServerCount:      1,
+		SyncStatus:       "pending",
+		OriginalUsername: "tester",
+		IsActive:         true,
+	}
+	if err := local.SaveConfiguration(cfg); err != nil {
+		t.Fatalf("save seed config: %v", err)
+	}
+
+	baseV1Data, err := BuildConfigurationSnapshot(cfg)
+	if err != nil {
+		t.Fatalf("build v1 snapshot: %v", err)
+	}
+	baseV1 := LocalConfigurationVersion{
+		ID:                uuid.NewString(),
+		ConfigurationUUID: cfg.UUID,
+		VersionNo:         1,
+		Data:              baseV1Data,
+		AppVersion:        "test",
+		CreatedAt:         time.Now(),
+	}
+	if err := local.DB().Create(&baseV1).Error; err != nil {
+		t.Fatalf("insert base v1: %v", err)
+	}
+	if err := local.DB().Model(&LocalConfiguration{}).
+		Where("uuid = ?", cfg.UUID).
+		Update("current_version_id", baseV1.ID).Error; err != nil {
+		t.Fatalf("set current_version_id to v1: %v", err)
+	}
+
+	v2 := LocalConfigurationVersion{
+		ID:                uuid.NewString(),
+		ConfigurationUUID: cfg.UUID,
+		VersionNo:         2,
+		Data:              baseV1.Data,
+		AppVersion:        "test",
+		CreatedAt:         time.Now().Add(1 * time.Second),
+	}
+	if err := local.DB().Create(&v2).Error; err != nil {
+		t.Fatalf("insert duplicate v2: %v", err)
+	}
+
+	modified := *cfg
+	modified.Items = LocalConfigItems{{LotName: "CPU_A", Quantity: 2, UnitPrice: 100}}
+	total := modified.Items.Total()
+	modified.TotalPrice = &total
+	modified.UpdatedAt = time.Now()
+	v3Data, err := BuildConfigurationSnapshot(&modified)
+	if err != nil {
+		t.Fatalf("build v3 snapshot: %v", err)
+	}
+
+	v3 := LocalConfigurationVersion{
+		ID:                uuid.NewString(),
+		ConfigurationUUID: cfg.UUID,
+		VersionNo:         3,
+		Data:              v3Data,
+		AppVersion:        "test",
+		CreatedAt:         time.Now().Add(2 * time.Second),
+	}
+	if err := local.DB().Create(&v3).Error; err != nil {
+		t.Fatalf("insert v3: %v", err)
+	}
+
+	v4 := LocalConfigurationVersion{
+		ID:                uuid.NewString(),
+		ConfigurationUUID: cfg.UUID,
+		VersionNo:         4,
+		Data:              v3Data,
+		AppVersion:        "test",
+		CreatedAt:         time.Now().Add(3 * time.Second),
+	}
+	if err := local.DB().Create(&v4).Error; err != nil {
+		t.Fatalf("insert duplicate v4: %v", err)
+	}
+
+	if err := local.DB().Model(&LocalConfiguration{}).
+		Where("uuid = ?", cfg.UUID).
+		Update("current_version_id", v4.ID).Error; err != nil {
+		t.Fatalf("point current_version_id to duplicate v4: %v", err)
+	}
+
+	if err := local.DB().Where("id = ?", "2026_02_19_configuration_versions_dedup_spec_price").
+		Delete(&LocalSchemaMigration{}).Error; err != nil {
+		t.Fatalf("delete dedup migration record: %v", err)
+	}
+
+	if err := runLocalMigrations(local.DB()); err != nil {
+		t.Fatalf("rerun local migrations: %v", err)
+	}
+
+	var versions []LocalConfigurationVersion
+	if err := local.DB().Where("configuration_uuid = ?", cfg.UUID).
+		Order("version_no ASC").
+		Find(&versions).Error; err != nil {
+		t.Fatalf("load versions after dedup: %v", err)
+	}
+	if len(versions) != 2 {
+		t.Fatalf("expected 2 versions after dedup, got %d", len(versions))
+	}
+	if versions[0].VersionNo != 1 || versions[1].VersionNo != 3 {
+		t.Fatalf("expected kept version numbers [1,3], got [%d,%d]", versions[0].VersionNo, versions[1].VersionNo)
+	}
+
+	var after LocalConfiguration
+	if err := local.DB().Where("uuid = ?", cfg.UUID).First(&after).Error; err != nil {
+		t.Fatalf("load config after dedup: %v", err)
+	}
+	if after.CurrentVersionID == nil || *after.CurrentVersionID != v3.ID {
+		t.Fatalf("expected current_version_id to point to kept latest version v3")
+	}
+}
--- a/internal/localdb/migrations.go
+++ b/internal/localdb/migrations.go
@@ -103,6 +103,11 @@ var localMigrations = []localMigration{
 		name: "Allow NULL project names in local_projects",
 		run:  allowLocalProjectNameNull,
 	},
+	{
+		id:   "2026_02_19_configuration_versions_dedup_spec_price",
+		name: "Deduplicate configuration revisions by spec+price",
+		run:  deduplicateConfigurationVersionsBySpecAndPrice,
+	},
 }

 func runLocalMigrations(db *gorm.DB) error {
@@ -428,6 +433,92 @@ func chooseNonZeroTime(candidate time.Time, fallback time.Time) time.Time {
 	return candidate
 }

+func deduplicateConfigurationVersionsBySpecAndPrice(tx *gorm.DB) error {
+	var configs []LocalConfiguration
+	if err := tx.Select("uuid", "current_version_id").Find(&configs).Error; err != nil {
+		return fmt.Errorf("load configurations for revision deduplication: %w", err)
+	}
+
+	var removedTotal int
+	for i := range configs {
+		cfg := configs[i]
+
+		var versions []LocalConfigurationVersion
+		if err := tx.Where("configuration_uuid = ?", cfg.UUID).
+			Order("version_no ASC, created_at ASC").
+			Find(&versions).Error; err != nil {
+			return fmt.Errorf("load versions for %s: %w", cfg.UUID, err)
+		}
+		if len(versions) < 2 {
+			continue
+		}
+
+		deleteIDs := make([]string, 0)
+		deleteSet := make(map[string]struct{})
+		kept := make([]LocalConfigurationVersion, 0, len(versions))
+		var prevKey string
+		hasPrev := false
+
+		for _, version := range versions {
+			snapshotCfg, err := DecodeConfigurationSnapshot(version.Data)
+			if err != nil {
+				// Keep malformed snapshots untouched and reset chain to avoid accidental removals.
+				kept = append(kept, version)
+				hasPrev = false
+				continue
+			}
+
+			key, err := BuildConfigurationSpecPriceFingerprint(snapshotCfg)
+			if err != nil {
+				kept = append(kept, version)
+				hasPrev = false
+				continue
+			}
+
+			if !hasPrev || key != prevKey {
+				kept = append(kept, version)
+				prevKey = key
+				hasPrev = true
+				continue
+			}
+
+			deleteIDs = append(deleteIDs, version.ID)
+			deleteSet[version.ID] = struct{}{}
+		}
+
+		if len(deleteIDs) == 0 {
+			continue
+		}
+
+		if err := tx.Where("id IN ?", deleteIDs).Delete(&LocalConfigurationVersion{}).Error; err != nil {
+			return fmt.Errorf("delete duplicate versions for %s: %w", cfg.UUID, err)
+		}
+		removedTotal += len(deleteIDs)
+
+		latestKeptID := kept[len(kept)-1].ID
+		if cfg.CurrentVersionID == nil || *cfg.CurrentVersionID == "" {
+			if err := tx.Model(&LocalConfiguration{}).
+				Where("uuid = ?", cfg.UUID).
+				Update("current_version_id", latestKeptID).Error; err != nil {
+				return fmt.Errorf("set missing current_version_id for %s: %w", cfg.UUID, err)
+			}
+			continue
+		}
+
+		if _, deleted := deleteSet[*cfg.CurrentVersionID]; deleted {
+			if err := tx.Model(&LocalConfiguration{}).
+				Where("uuid = ?", cfg.UUID).
+				Update("current_version_id", latestKeptID).Error; err != nil {
+				return fmt.Errorf("repair current_version_id for %s: %w", cfg.UUID, err)
+			}
+		}
+	}
+
+	if removedTotal > 0 {
+		slog.Info("deduplicated configuration revisions", "removed_versions", removedTotal)
+	}
+	return nil
+}

 func fixLocalPricelistIndexes(tx *gorm.DB) error {
 	type indexRow struct {
--- a/internal/localdb/snapshots.go
+++ b/internal/localdb/snapshots.go
@@ -3,6 +3,7 @@ package localdb
 import (
 	"encoding/json"
 	"fmt"
+	"sort"
 	"time"
 )

@@ -94,3 +95,51 @@ func DecodeConfigurationSnapshot(data string) (*LocalConfiguration, error) {
 		OriginalUsername: snapshot.OriginalUsername,
 	}, nil
 }
+
+type configurationSpecPriceFingerprint struct {
+	Items       []configurationSpecPriceFingerprintItem `json:"items"`
+	ServerCount int                                     `json:"server_count"`
+	TotalPrice  *float64                                `json:"total_price,omitempty"`
+	CustomPrice *float64                                `json:"custom_price,omitempty"`
+}
+
+type configurationSpecPriceFingerprintItem struct {
+	LotName   string  `json:"lot_name"`
+	Quantity  int     `json:"quantity"`
+	UnitPrice float64 `json:"unit_price"`
+}
+
+// BuildConfigurationSpecPriceFingerprint returns a stable JSON key based on
+// spec + price fields only, used for revision deduplication.
+func BuildConfigurationSpecPriceFingerprint(localCfg *LocalConfiguration) (string, error) {
+	items := make([]configurationSpecPriceFingerprintItem, 0, len(localCfg.Items))
+	for _, item := range localCfg.Items {
+		items = append(items, configurationSpecPriceFingerprintItem{
+			LotName:   item.LotName,
+			Quantity:  item.Quantity,
+			UnitPrice: item.UnitPrice,
+		})
+	}
+	sort.Slice(items, func(i, j int) bool {
+		if items[i].LotName != items[j].LotName {
+			return items[i].LotName < items[j].LotName
+		}
+		if items[i].Quantity != items[j].Quantity {
+			return items[i].Quantity < items[j].Quantity
+		}
+		return items[i].UnitPrice < items[j].UnitPrice
+	})
+
+	payload := configurationSpecPriceFingerprint{
+		Items:       items,
+		ServerCount: localCfg.ServerCount,
+		TotalPrice:  localCfg.TotalPrice,
+		CustomPrice: localCfg.CustomPrice,
+	}
+
+	raw, err := json.Marshal(payload)
+	if err != nil {
+		return "", fmt.Errorf("marshal spec+price fingerprint: %w", err)
+	}
+	return string(raw), nil
+}
--- a/internal/services/local_configuration.go
+++ b/internal/services/local_configuration.go
@@ -995,6 +995,22 @@ func (s *LocalConfigurationService) saveWithVersionAndPending(localCfg *localdb.
 			return fmt.Errorf("lock configuration row: %w", err)
 		}

+		if operation == "update" {
+			currentVersion, err := s.loadCurrentVersionTx(tx, &locked)
+			if err != nil {
+				return fmt.Errorf("load current version before save: %w", err)
+			}
+
+			sameRevisionContent, err := s.hasSameRevisionContent(localCfg, currentVersion)
+			if err != nil {
+				return fmt.Errorf("compare revision content: %w", err)
+			}
+			if sameRevisionContent {
+				cfg = localdb.LocalToConfiguration(&locked)
+				return nil
+			}
+		}
+
 		if err := tx.Save(localCfg).Error; err != nil {
 			return fmt.Errorf("save local configuration: %w", err)
 		}
@@ -1029,6 +1045,41 @@ func (s *LocalConfigurationService) saveWithVersionAndPending(localCfg *localdb.
 	return cfg, nil
 }

+func (s *LocalConfigurationService) loadCurrentVersionTx(tx *gorm.DB, localCfg *localdb.LocalConfiguration) (*localdb.LocalConfigurationVersion, error) {
+	var version localdb.LocalConfigurationVersion
+	if localCfg.CurrentVersionID != nil && *localCfg.CurrentVersionID != "" {
+		if err := tx.Where("id = ?", *localCfg.CurrentVersionID).First(&version).Error; err == nil {
+			return &version, nil
+		} else if !errors.Is(err, gorm.ErrRecordNotFound) {
+			return nil, err
+		}
+	}
+
+	if err := tx.Where("configuration_uuid = ?", localCfg.UUID).
+		Order("version_no DESC").
+		First(&version).Error; err != nil {
+		return nil, err
+	}
+	return &version, nil
+}
+
+func (s *LocalConfigurationService) hasSameRevisionContent(localCfg *localdb.LocalConfiguration, currentVersion *localdb.LocalConfigurationVersion) (bool, error) {
+	currentSnapshotCfg, err := s.decodeConfigurationSnapshot(currentVersion.Data)
+	if err != nil {
+		return false, fmt.Errorf("decode current version snapshot: %w", err)
+	}
+
+	currentFingerprint, err := localdb.BuildConfigurationSpecPriceFingerprint(currentSnapshotCfg)
+	if err != nil {
+		return false, fmt.Errorf("build current snapshot fingerprint: %w", err)
+	}
+	nextFingerprint, err := localdb.BuildConfigurationSpecPriceFingerprint(localCfg)
+	if err != nil {
+		return false, fmt.Errorf("build next snapshot fingerprint: %w", err)
+	}
+	return currentFingerprint == nextFingerprint, nil
+}
+
 func (s *LocalConfigurationService) appendVersionTx(
 	tx *gorm.DB,
 	localCfg *localdb.LocalConfiguration,
--- a/internal/services/local_configuration_versioning_test.go
+++ b/internal/services/local_configuration_versioning_test.go
@@ -27,8 +27,12 @@ func TestSaveCreatesNewVersionAndUpdatesCurrentPointer(t *testing.T) {
 		t.Fatalf("create config: %v", err)
 	}

-	if _, err := service.RenameNoAuth(created.UUID, "v2"); err != nil {
-		t.Fatalf("rename config: %v", err)
+	if _, err := service.UpdateNoAuth(created.UUID, &CreateConfigRequest{
+		Name:        "v1",
+		Items:       models.ConfigItems{{LotName: "CPU_A", Quantity: 2, UnitPrice: 1000}},
+		ServerCount: 1,
+	}); err != nil {
+		t.Fatalf("update config: %v", err)
 	}

 	versions := loadVersions(t, local, created.UUID)
@@ -60,8 +64,12 @@ func TestRollbackCreatesNewVersionWithTargetData(t *testing.T) {
 		t.Fatalf("create config: %v", err)
 	}

-	if _, err := service.RenameNoAuth(created.UUID, "changed"); err != nil {
-		t.Fatalf("rename config: %v", err)
+	if _, err := service.UpdateNoAuth(created.UUID, &CreateConfigRequest{
+		Name:        "base",
+		Items:       models.ConfigItems{{LotName: "RAM_A", Quantity: 3, UnitPrice: 100}},
+		ServerCount: 1,
+	}); err != nil {
+		t.Fatalf("update config: %v", err)
 	}
 	if _, err := service.RollbackToVersionWithNote(created.UUID, 1, "tester", "test rollback"); err != nil {
 		t.Fatalf("rollback to v1: %v", err)
@@ -79,6 +87,56 @@ func TestRollbackCreatesNewVersionWithTargetData(t *testing.T) {
 	}
 }

+func TestUpdateNoAuthSkipsRevisionWhenSpecAndPriceUnchanged(t *testing.T) {
+	service, local := newLocalConfigServiceForTest(t)
+
+	created, err := service.Create("tester", &CreateConfigRequest{
+		Name:        "dedupe",
+		Items:       models.ConfigItems{{LotName: "CPU_A", Quantity: 1, UnitPrice: 1000}},
+		ServerCount: 1,
+	})
+	if err != nil {
+		t.Fatalf("create config: %v", err)
+	}
+
+	_, err = service.UpdateNoAuth(created.UUID, &CreateConfigRequest{
+		Name:        "dedupe",
+		Items:       models.ConfigItems{{LotName: "CPU_A", Quantity: 2, UnitPrice: 1000}},
+		ServerCount: 1,
+	})
+	if err != nil {
+		t.Fatalf("first update config: %v", err)
+	}
+
+	_, err = service.UpdateNoAuth(created.UUID, &CreateConfigRequest{
+		Name:        "dedupe",
+		Items:       models.ConfigItems{{LotName: "CPU_A", Quantity: 2, UnitPrice: 1000}},
+		ServerCount: 1,
+	})
+	if err != nil {
+		t.Fatalf("second update config: %v", err)
+	}
+
+	versions := loadVersions(t, local, created.UUID)
+	if len(versions) != 2 {
+		t.Fatalf("expected 2 versions (create + first update), got %d", len(versions))
+	}
+	if versions[1].VersionNo != 2 {
+		t.Fatalf("expected latest version_no=2, got %d", versions[1].VersionNo)
+	}
+
+	var pendingCount int64
+	if err := local.DB().
+		Table("pending_changes").
+		Where("entity_type = ? AND entity_uuid = ?", "configuration", created.UUID).
+		Count(&pendingCount).Error; err != nil {
+		t.Fatalf("count pending changes: %v", err)
+	}
+	if pendingCount != 2 {
+		t.Fatalf("expected 2 pending changes (create + first update), got %d", pendingCount)
+	}
+}
+
 func TestAppendOnlyInvariantOldRowsUnchanged(t *testing.T) {
 	service, local := newLocalConfigServiceForTest(t)

@@ -97,8 +155,12 @@ func TestAppendOnlyInvariantOldRowsUnchanged(t *testing.T) {
 	}
 	v1Before := versionsBefore[0]

-	if _, err := service.RenameNoAuth(created.UUID, "after-rename"); err != nil {
-		t.Fatalf("rename config: %v", err)
+	if _, err := service.UpdateNoAuth(created.UUID, &CreateConfigRequest{
+		Name:        "initial",
+		Items:       models.ConfigItems{{LotName: "SSD_A", Quantity: 2, UnitPrice: 300}},
+		ServerCount: 1,
+	}); err != nil {
+		t.Fatalf("update config: %v", err)
 	}
 	if _, err := service.RollbackToVersion(created.UUID, 1, "tester"); err != nil {
 		t.Fatalf("rollback: %v", err)
@@ -144,7 +206,7 @@ func TestConcurrentSaveNoDuplicateVersionNumbers(t *testing.T) {
 		go func() {
 			defer wg.Done()
 			<-start
-			if err := renameWithRetry(service, created.UUID, fmt.Sprintf("name-%d", i)); err != nil {
+			if err := updateWithRetry(service, created.UUID, i+2); err != nil {
 				errCh <- err
 			}
 		}()
@@ -264,10 +326,14 @@ func loadVersions(t *testing.T, local *localdb.LocalDB, configurationUUID string
 	return versions
 }

-func renameWithRetry(service *LocalConfigurationService, uuid string, name string) error {
+func updateWithRetry(service *LocalConfigurationService, uuid string, quantity int) error {
 	var lastErr error
 	for i := 0; i < 6; i++ {
-		_, err := service.RenameNoAuth(uuid, name)
+		_, err := service.UpdateNoAuth(uuid, &CreateConfigRequest{
+			Name:        "base",
+			Items:       models.ConfigItems{{LotName: "NIC_A", Quantity: quantity, UnitPrice: 150}},
+			ServerCount: 1,
+		})
 		if err == nil {
 			return nil
 		}
@@ -278,7 +344,7 @@ func renameWithRetry(service *LocalConfigurationService, uuid string, name strin
 		}
 		return err
 	}
-	return fmt.Errorf("rename retries exhausted: %w", lastErr)
+	return fmt.Errorf("update retries exhausted: %w", lastErr)
 }

 func TestRollbackVersionSnapshotJSONMatchesV1(t *testing.T) {
@@ -292,8 +358,12 @@ func TestRollbackVersionSnapshotJSONMatchesV1(t *testing.T) {
 	if err != nil {
 		t.Fatalf("create config: %v", err)
 	}
-	if _, err := service.RenameNoAuth(created.UUID, "second"); err != nil {
-		t.Fatalf("rename: %v", err)
+	if _, err := service.UpdateNoAuth(created.UUID, &CreateConfigRequest{
+		Name:        "initial",
+		Items:       models.ConfigItems{{LotName: "GPU_A", Quantity: 2, UnitPrice: 2000}},
+		ServerCount: 1,
+	}); err != nil {
+		t.Fatalf("update: %v", err)
 	}
 	if _, err := service.RollbackToVersion(created.UUID, 1, "tester"); err != nil {
 		t.Fatalf("rollback: %v", err)