From 4eeb0f895716cedf905388e59a485227711eb7e5 Mon Sep 17 00:00:00 2001 From: Rogee Date: Mon, 24 Aug 2026 13:09:13 +0800 Subject: [PATCH] HH-597: repair assignment policy migration parity (#156) * fix(HH-597): repair assignment policy schema migration * fix(HH-597): harden assignment policy migration --------- Co-authored-by: Rogee --- .../assignment_policy_migration_test.go | 79 +++++++++++ .../database/upload_migration_test.go | 2 +- .../repository/assignment_policy_repo.go | 57 ++++---- .../repository/assignment_policy_repo_test.go | 126 ++++++++---------- .../service/assignment_policy_service.go | 4 +- ...6_repair_assignment_policy_schema.down.sql | 8 ++ ...086_repair_assignment_policy_schema.up.sql | 64 +++++++++ 7 files changed, 236 insertions(+), 104 deletions(-) create mode 100644 backend/internal/database/assignment_policy_migration_test.go create mode 100644 backend/migrations/000086_repair_assignment_policy_schema.down.sql create mode 100644 backend/migrations/000086_repair_assignment_policy_schema.up.sql diff --git a/backend/internal/database/assignment_policy_migration_test.go b/backend/internal/database/assignment_policy_migration_test.go new file mode 100644 index 00000000..e39e3221 --- /dev/null +++ b/backend/internal/database/assignment_policy_migration_test.go @@ -0,0 +1,79 @@ +package database + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + "github.com/gochat/gochat/internal/repository" +) + +func TestInboxAssignmentPolicyProductionMigration(t *testing.T) { + db, dbURL := openUploadMigrationPostgres(t) + migrations := productionMigrationsPath(t) + require.NoError(t, MigrateSteps(dbURL, migrations, 85)) + assert.False(t, db.Migrator().HasTable("inbox_assignment_policies")) + + var accountID, inboxID, policyID uint + require.NoError(t, db.Raw("INSERT INTO accounts(name) VALUES (?) RETURNING id", "assignment migration account").Scan(&accountID).Error) + require.NoError(t, db.Raw(`INSERT INTO inboxes(account_id, name, channel_type, channel_id) + VALUES (?, 'assignment migration inbox', 'web_widget', 1) RETURNING id`, accountID).Scan(&inboxID).Error) + require.NoError(t, db.Raw(`INSERT INTO assignment_policies(account_id, inbox_id, policy_type) + VALUES (?, ?, 'round_robin') RETURNING id`, accountID, inboxID).Scan(&policyID).Error) + + require.NoError(t, MigrateSteps(dbURL, migrations, 1)) + assert.True(t, db.Migrator().HasTable("inbox_assignment_policies")) + assert.True(t, db.Migrator().HasIndex("assignment_policies", "idx_account_policy_name")) + assert.True(t, db.Migrator().HasIndex("inbox_assignment_policies", "idx_inbox_assignment_policies_inbox_id")) + assert.True(t, db.Migrator().HasIndex("inbox_assignment_policies", "idx_inbox_assignment_policies_assignment_policy_id")) + + policy, err := repository.NewInboxAssignmentPolicyRepo(db).FindPolicyByInbox(context.Background(), accountID, inboxID) + require.NoError(t, err) + assert.Equal(t, policyID, policy.ID) + assert.Equal(t, "round_robin "+fmt.Sprint(policyID), policy.Name) + + var newInboxID, newPolicyID uint + require.NoError(t, db.Raw(`INSERT INTO inboxes(account_id, name, channel_type, channel_id) + VALUES (?, 'post-up inbox', 'web_widget', 2) RETURNING id`, accountID).Scan(&newInboxID).Error) + require.NoError(t, db.Raw(`INSERT INTO assignment_policies(account_id, name, description) + VALUES (?, 'post-up policy', 'must survive rollback') RETURNING id`, accountID).Scan(&newPolicyID).Error) + require.NoError(t, db.Exec("INSERT INTO inbox_assignment_policies(inbox_id, assignment_policy_id) VALUES (?, ?)", newInboxID, newPolicyID).Error) + + down, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000086_repair_assignment_policy_schema.down.sql")) + require.NoError(t, err) + err = db.Transaction(func(tx *gorm.DB) error { return tx.Exec(string(down)).Error }) + require.ErrorContains(t, err, "migration 000086 is irreversible") + assert.True(t, db.Migrator().HasTable("inbox_assignment_policies")) + assert.True(t, db.Migrator().HasColumn("assignment_policies", "description")) + + var description string + require.NoError(t, db.Raw("SELECT description FROM assignment_policies WHERE id = ?", newPolicyID).Scan(&description).Error) + assert.Equal(t, "must survive rollback", description) + var links int64 + require.NoError(t, db.Table("inbox_assignment_policies").Where("inbox_id = ? AND assignment_policy_id = ?", newInboxID, newPolicyID).Count(&links).Error) + assert.Equal(t, int64(1), links) +} + +func TestInboxAssignmentPolicyMigrationRejectsDuplicateLegacyBindings(t *testing.T) { + db, dbURL := openUploadMigrationPostgres(t) + migrations := productionMigrationsPath(t) + require.NoError(t, MigrateSteps(dbURL, migrations, 85)) + + var accountID, inboxID uint + require.NoError(t, db.Raw("INSERT INTO accounts(name) VALUES (?) RETURNING id", "duplicate migration account").Scan(&accountID).Error) + require.NoError(t, db.Raw(`INSERT INTO inboxes(account_id, name, channel_type, channel_id) + VALUES (?, 'duplicate migration inbox', 'web_widget', 1) RETURNING id`, accountID).Scan(&inboxID).Error) + for _, policyType := range []string{"round_robin", "balanced"} { + require.NoError(t, db.Exec(`INSERT INTO assignment_policies(account_id, inbox_id, policy_type) + VALUES (?, ?, ?)`, accountID, inboxID, policyType).Error) + } + + err := MigrateSteps(dbURL, migrations, 1) + require.ErrorContains(t, err, "multiple legacy assignment policies target one inbox") +} diff --git a/backend/internal/database/upload_migration_test.go b/backend/internal/database/upload_migration_test.go index b3543efa..0b73be39 100644 --- a/backend/internal/database/upload_migration_test.go +++ b/backend/internal/database/upload_migration_test.go @@ -72,7 +72,7 @@ func TestUploadPostgresProductionMigrationsFromEmptySchema(t *testing.T) { require.NoError(t, RunMigrations(dbURL, productionMigrationsPath(t))) version, dirty, err := CurrentVersion(dbURL, productionMigrationsPath(t)) require.NoError(t, err) - assert.Equal(t, uint(85), version) + assert.Equal(t, uint(86), version) assert.False(t, dirty) exerciseUploadProductionSchema(t, db) } diff --git a/backend/internal/repository/assignment_policy_repo.go b/backend/internal/repository/assignment_policy_repo.go index 0ee6e2aa..a5402d5c 100644 --- a/backend/internal/repository/assignment_policy_repo.go +++ b/backend/internal/repository/assignment_policy_repo.go @@ -94,26 +94,13 @@ func NewInboxAssignmentPolicyRepo(db *gorm.DB) *InboxAssignmentPolicyRepo { return &InboxAssignmentPolicyRepo{db: db} } -// Create creates a new inbox assignment policy. -func (r *InboxAssignmentPolicyRepo) Create(ctx context.Context, policy *model.InboxAssignmentPolicy) error { - return r.db.WithContext(ctx).Create(policy).Error -} - -// FindByID retrieves an inbox assignment policy by primary key. -func (r *InboxAssignmentPolicyRepo) FindByID(ctx context.Context, id uint) (*model.InboxAssignmentPolicy, error) { - var policy model.InboxAssignmentPolicy - if err := r.db.WithContext(ctx).First(&policy, id).Error; err != nil { - return nil, err - } - return &policy, nil -} - // FindByInbox retrieves the active assignment policy override for an inbox. func (r *InboxAssignmentPolicyRepo) FindByInbox(ctx context.Context, accountID, inboxID uint) (*model.InboxAssignmentPolicy, error) { var policy model.InboxAssignmentPolicy if err := r.db.WithContext(ctx). Joins("JOIN assignment_policies ON assignment_policies.id = inbox_assignment_policies.assignment_policy_id"). - Where("assignment_policies.account_id = ? AND inbox_assignment_policies.inbox_id = ?", accountID, inboxID). + Joins("JOIN inboxes ON inboxes.id = inbox_assignment_policies.inbox_id"). + Where("assignment_policies.account_id = ? AND inboxes.account_id = ? AND inbox_assignment_policies.inbox_id = ?", accountID, accountID, inboxID). First(&policy).Error; err != nil { return nil, err } @@ -125,7 +112,8 @@ func (r *InboxAssignmentPolicyRepo) FindPolicyByInbox(ctx context.Context, accou var policy model.AssignmentPolicy if err := r.db.WithContext(ctx). Joins("JOIN inbox_assignment_policies ON inbox_assignment_policies.assignment_policy_id = assignment_policies.id"). - Where("assignment_policies.account_id = ? AND inbox_assignment_policies.inbox_id = ?", accountID, inboxID). + Joins("JOIN inboxes ON inboxes.id = inbox_assignment_policies.inbox_id"). + Where("assignment_policies.account_id = ? AND inboxes.account_id = ? AND inbox_assignment_policies.inbox_id = ?", accountID, accountID, inboxID). First(&policy).Error; err != nil { return nil, err } @@ -138,16 +126,24 @@ func (r *InboxAssignmentPolicyRepo) FindInboxesByPolicy(ctx context.Context, acc err := r.db.WithContext(ctx). Joins("JOIN inbox_assignment_policies ON inbox_assignment_policies.inbox_id = inboxes.id"). Joins("JOIN assignment_policies ON assignment_policies.id = inbox_assignment_policies.assignment_policy_id"). - Where("assignment_policies.account_id = ? AND assignment_policies.id = ?", accountID, policyID). + Where("inboxes.account_id = ? AND assignment_policies.account_id = ? AND assignment_policies.id = ?", accountID, accountID, policyID). Order("inboxes.id ASC"). Find(&inboxes).Error return inboxes, err } // ReplaceForInbox attaches one policy to an inbox, replacing an old association. -func (r *InboxAssignmentPolicyRepo) ReplaceForInbox(ctx context.Context, inboxID, policyID uint) (*model.InboxAssignmentPolicy, error) { +func (r *InboxAssignmentPolicyRepo) ReplaceForInbox(ctx context.Context, accountID, inboxID, policyID uint) (*model.InboxAssignmentPolicy, error) { var policy model.InboxAssignmentPolicy err := r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + var inbox model.Inbox + if err := tx.Select("id").Where("account_id = ? AND id = ?", accountID, inboxID).First(&inbox).Error; err != nil { + return err + } + var assignmentPolicy model.AssignmentPolicy + if err := tx.Select("id").Where("account_id = ? AND id = ?", accountID, policyID).First(&assignmentPolicy).Error; err != nil { + return err + } if err := tx.Where("inbox_id = ?", inboxID).Delete(&model.InboxAssignmentPolicy{}).Error; err != nil { return err } @@ -161,18 +157,15 @@ func (r *InboxAssignmentPolicyRepo) ReplaceForInbox(ctx context.Context, inboxID } // DeleteByInbox removes an inbox assignment policy association. -func (r *InboxAssignmentPolicyRepo) DeleteByInbox(ctx context.Context, inboxID uint) error { - return r.db.WithContext(ctx). - Where("inbox_id = ?", inboxID). - Delete(&model.InboxAssignmentPolicy{}).Error -} - -// Update updates an inbox assignment policy. -func (r *InboxAssignmentPolicyRepo) Update(ctx context.Context, policy *model.InboxAssignmentPolicy) error { - return r.db.WithContext(ctx).Save(policy).Error -} - -// Delete soft-deletes an inbox assignment policy. -func (r *InboxAssignmentPolicyRepo) Delete(ctx context.Context, id uint) error { - return r.db.WithContext(ctx).Delete(&model.InboxAssignmentPolicy{}, id).Error +func (r *InboxAssignmentPolicyRepo) DeleteByInbox(ctx context.Context, accountID, inboxID uint) error { + result := r.db.WithContext(ctx). + Where("inbox_id = ? AND EXISTS (SELECT 1 FROM inboxes WHERE inboxes.id = inbox_assignment_policies.inbox_id AND inboxes.account_id = ?)", inboxID, accountID). + Delete(&model.InboxAssignmentPolicy{}) + if result.Error != nil { + return result.Error + } + if result.RowsAffected == 0 { + return gorm.ErrRecordNotFound + } + return nil } diff --git a/backend/internal/repository/assignment_policy_repo_test.go b/backend/internal/repository/assignment_policy_repo_test.go index e3821bc0..8a546edb 100644 --- a/backend/internal/repository/assignment_policy_repo_test.go +++ b/backend/internal/repository/assignment_policy_repo_test.go @@ -16,6 +16,7 @@ func setupAssignmentPolicyTestDB(t *testing.T) *gorm.DB { return setupTestDB(t, &model.AssignmentPolicy{}, &model.InboxAssignmentPolicy{}, + &model.Inbox{}, ) } @@ -141,13 +142,12 @@ func TestAssignmentPolicyRepo_CountAssignedInboxes(t *testing.T) { db := setupAssignmentPolicyTestDB(t) ctx := context.Background() repo := NewAssignmentPolicyRepo(db) - inboxRepo := NewInboxAssignmentPolicyRepo(db) policy := &model.AssignmentPolicy{AccountID: 1, Name: "P1"} require.NoError(t, repo.Create(ctx, policy)) - require.NoError(t, inboxRepo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: 1, AssignmentPolicyID: policy.ID})) - require.NoError(t, inboxRepo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: 2, AssignmentPolicyID: policy.ID})) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: 1, AssignmentPolicyID: policy.ID}).Error) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: 2, AssignmentPolicyID: policy.ID}).Error) count, err := repo.CountAssignedInboxes(ctx, policy.ID) require.NoError(t, err) @@ -158,39 +158,6 @@ func TestAssignmentPolicyRepo_CountAssignedInboxes(t *testing.T) { // InboxAssignmentPolicyRepo // --------------------------------------------------------------------------- -func TestInboxAssignmentPolicyRepo_Create(t *testing.T) { - db := setupAssignmentPolicyTestDB(t) - ctx := context.Background() - repo := NewInboxAssignmentPolicyRepo(db) - - iap := &model.InboxAssignmentPolicy{InboxID: 1, AssignmentPolicyID: 1} - err := repo.Create(ctx, iap) - require.NoError(t, err) - assert.NotZero(t, iap.ID) -} - -func TestInboxAssignmentPolicyRepo_FindByID(t *testing.T) { - db := setupAssignmentPolicyTestDB(t) - ctx := context.Background() - repo := NewInboxAssignmentPolicyRepo(db) - - iap := &model.InboxAssignmentPolicy{InboxID: 1, AssignmentPolicyID: 1} - require.NoError(t, repo.Create(ctx, iap)) - - found, err := repo.FindByID(ctx, iap.ID) - require.NoError(t, err) - assert.Equal(t, iap.ID, found.ID) -} - -func TestInboxAssignmentPolicyRepo_FindByID_NotFound(t *testing.T) { - db := setupAssignmentPolicyTestDB(t) - ctx := context.Background() - repo := NewInboxAssignmentPolicyRepo(db) - - _, err := repo.FindByID(ctx, 99999) - assert.ErrorIs(t, err, gorm.ErrRecordNotFound) -} - func TestInboxAssignmentPolicyRepo_FindByInbox(t *testing.T) { db := setupAssignmentPolicyTestDB(t) ctx := context.Background() @@ -199,9 +166,11 @@ func TestInboxAssignmentPolicyRepo_FindByInbox(t *testing.T) { policy := &model.AssignmentPolicy{AccountID: 1, Name: "P1"} require.NoError(t, policyRepo.Create(ctx, policy)) - require.NoError(t, repo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: 10, AssignmentPolicyID: policy.ID})) + inbox := &model.Inbox{AccountID: 1, Name: "I1", ChannelType: "web_widget"} + require.NoError(t, db.Create(inbox).Error) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inbox.ID, AssignmentPolicyID: policy.ID}).Error) - found, err := repo.FindByInbox(ctx, 1, 10) + found, err := repo.FindByInbox(ctx, 1, inbox.ID) require.NoError(t, err) assert.Equal(t, policy.ID, found.AssignmentPolicyID) } @@ -223,9 +192,11 @@ func TestInboxAssignmentPolicyRepo_FindPolicyByInbox(t *testing.T) { policy := &model.AssignmentPolicy{AccountID: 1, Name: "P1"} require.NoError(t, policyRepo.Create(ctx, policy)) - require.NoError(t, repo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: 10, AssignmentPolicyID: policy.ID})) + inbox := &model.Inbox{AccountID: 1, Name: "I1", ChannelType: "web_widget"} + require.NoError(t, db.Create(inbox).Error) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inbox.ID, AssignmentPolicyID: policy.ID}).Error) - found, err := repo.FindPolicyByInbox(ctx, 1, 10) + found, err := repo.FindPolicyByInbox(ctx, 1, inbox.ID) require.NoError(t, err) assert.Equal(t, policy.ID, found.ID) assert.Equal(t, "P1", found.Name) @@ -253,8 +224,8 @@ func TestInboxAssignmentPolicyRepo_FindInboxesByPolicy(t *testing.T) { inbox2 := &model.Inbox{AccountID: 1, Name: "I2", ChannelType: "web_widget"} require.NoError(t, db.Create(inbox1).Error) require.NoError(t, db.Create(inbox2).Error) - require.NoError(t, repo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: inbox1.ID, AssignmentPolicyID: policy.ID})) - require.NoError(t, repo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: inbox2.ID, AssignmentPolicyID: policy.ID})) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inbox1.ID, AssignmentPolicyID: policy.ID}).Error) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inbox2.ID, AssignmentPolicyID: policy.ID}).Error) inboxes, err := repo.FindInboxesByPolicy(ctx, 1, policy.ID) require.NoError(t, err) @@ -265,14 +236,21 @@ func TestInboxAssignmentPolicyRepo_ReplaceForInbox(t *testing.T) { db := setupAssignmentPolicyTestDB(t) ctx := context.Background() repo := NewInboxAssignmentPolicyRepo(db) + inbox := &model.Inbox{AccountID: 1, Name: "I1", ChannelType: "web_widget"} + policy1 := &model.AssignmentPolicy{AccountID: 1, Name: "P1"} + policy2 := &model.AssignmentPolicy{AccountID: 1, Name: "P2"} + require.NoError(t, db.Create(inbox).Error) + for _, policy := range []*model.AssignmentPolicy{policy1, policy2} { + require.NoError(t, db.Create(policy).Error) + } // Create initial association - require.NoError(t, repo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: 10, AssignmentPolicyID: 1})) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inbox.ID, AssignmentPolicyID: policy1.ID}).Error) // Replace - result, err := repo.ReplaceForInbox(ctx, 10, 2) + result, err := repo.ReplaceForInbox(ctx, 1, inbox.ID, policy2.ID) require.NoError(t, err) - assert.Equal(t, uint(2), result.AssignmentPolicyID) + assert.Equal(t, policy2.ID, result.AssignmentPolicyID) // Verify old one is gone all := []model.InboxAssignmentPolicy{} @@ -284,40 +262,50 @@ func TestInboxAssignmentPolicyRepo_DeleteByInbox(t *testing.T) { db := setupAssignmentPolicyTestDB(t) ctx := context.Background() repo := NewInboxAssignmentPolicyRepo(db) + policy := &model.AssignmentPolicy{AccountID: 1, Name: "P1"} + inbox := &model.Inbox{AccountID: 1, Name: "I1", ChannelType: "web_widget"} + require.NoError(t, db.Create(policy).Error) + require.NoError(t, db.Create(inbox).Error) - require.NoError(t, repo.Create(ctx, &model.InboxAssignmentPolicy{InboxID: 10, AssignmentPolicyID: 1})) + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inbox.ID, AssignmentPolicyID: policy.ID}).Error) - require.NoError(t, repo.DeleteByInbox(ctx, 10)) + require.NoError(t, repo.DeleteByInbox(ctx, 1, inbox.ID)) - _, err := repo.FindByInbox(ctx, 1, 10) + _, err := repo.FindByInbox(ctx, 1, inbox.ID) assert.ErrorIs(t, err, gorm.ErrRecordNotFound) } -func TestInboxAssignmentPolicyRepo_Update(t *testing.T) { +func TestInboxAssignmentPolicyRepoRejectsCrossTenantAccess(t *testing.T) { db := setupAssignmentPolicyTestDB(t) ctx := context.Background() repo := NewInboxAssignmentPolicyRepo(db) + policyA := &model.AssignmentPolicy{AccountID: 1, Name: "A"} + policyB := &model.AssignmentPolicy{AccountID: 2, Name: "B"} + inboxA := &model.Inbox{AccountID: 1, Name: "A", ChannelType: "web_widget"} + inboxB := &model.Inbox{AccountID: 2, Name: "B", ChannelType: "web_widget"} + for _, value := range []any{policyA, policyB, inboxA, inboxB} { + require.NoError(t, db.Create(value).Error) + } + require.NoError(t, db.Create(&model.InboxAssignmentPolicy{InboxID: inboxB.ID, AssignmentPolicyID: policyB.ID}).Error) - iap := &model.InboxAssignmentPolicy{InboxID: 1, AssignmentPolicyID: 1} - require.NoError(t, repo.Create(ctx, iap)) + _, err := repo.ReplaceForInbox(ctx, 1, inboxB.ID, policyA.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + _, err = repo.ReplaceForInbox(ctx, 1, inboxA.ID, policyB.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + require.ErrorIs(t, repo.DeleteByInbox(ctx, 1, inboxB.ID), gorm.ErrRecordNotFound) - iap.AssignmentPolicyID = 2 - require.NoError(t, repo.Update(ctx, iap)) + policy, err := repo.FindPolicyByInbox(ctx, 2, inboxB.ID) + require.NoError(t, err) + assert.Equal(t, policyB.ID, policy.ID) - found, _ := repo.FindByID(ctx, iap.ID) - assert.Equal(t, uint(2), found.AssignmentPolicyID) -} - -func TestInboxAssignmentPolicyRepo_Delete(t *testing.T) { - db := setupAssignmentPolicyTestDB(t) - ctx := context.Background() - repo := NewInboxAssignmentPolicyRepo(db) - - iap := &model.InboxAssignmentPolicy{InboxID: 1, AssignmentPolicyID: 1} - require.NoError(t, repo.Create(ctx, iap)) - - require.NoError(t, repo.Delete(ctx, iap.ID)) - - _, err := repo.FindByID(ctx, iap.ID) - assert.ErrorIs(t, err, gorm.ErrRecordNotFound) + require.NoError(t, db.Model(&model.InboxAssignmentPolicy{}). + Where("inbox_id = ?", inboxB.ID). + Update("assignment_policy_id", policyA.ID).Error) + _, err = repo.FindByInbox(ctx, 1, inboxB.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + _, err = repo.FindPolicyByInbox(ctx, 1, inboxB.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + inboxes, err := repo.FindInboxesByPolicy(ctx, 1, policyA.ID) + require.NoError(t, err) + assert.Empty(t, inboxes) } diff --git a/backend/internal/service/assignment_policy_service.go b/backend/internal/service/assignment_policy_service.go index 8dc1f3b1..df64376c 100644 --- a/backend/internal/service/assignment_policy_service.go +++ b/backend/internal/service/assignment_policy_service.go @@ -237,7 +237,7 @@ func (s *AssignmentPolicyService) CreateInboxPolicy(ctx context.Context, account if _, err := s.policyRepo.FindByAccountAndID(ctx, accountID, req.AssignmentPolicyID); err != nil { return nil, fmt.Errorf("assignment policy not found: %w", err) } - if _, err := s.inboxPolicyRepo.ReplaceForInbox(ctx, req.InboxID, req.AssignmentPolicyID); err != nil { + if _, err := s.inboxPolicyRepo.ReplaceForInbox(ctx, accountID, req.InboxID, req.AssignmentPolicyID); err != nil { return nil, fmt.Errorf("create inbox assignment policy: %w", err) } return s.GetInboxPolicy(ctx, accountID, req.InboxID) @@ -257,7 +257,7 @@ func (s *AssignmentPolicyService) DeleteInboxPolicy(ctx context.Context, id, acc return fmt.Errorf("inbox assignment policy not found: %w", err) } - if err := s.inboxPolicyRepo.DeleteByInbox(ctx, id); err != nil { + if err := s.inboxPolicyRepo.DeleteByInbox(ctx, accountID, id); err != nil { return fmt.Errorf("failed to delete inbox assignment policy: %w", err) } return nil diff --git a/backend/migrations/000086_repair_assignment_policy_schema.down.sql b/backend/migrations/000086_repair_assignment_policy_schema.down.sql new file mode 100644 index 00000000..4a16c793 --- /dev/null +++ b/backend/migrations/000086_repair_assignment_policy_schema.down.sql @@ -0,0 +1,8 @@ +-- v86 backfills legacy bindings and may receive new assignment-policy data. +-- Removing its table, columns, or indexes would destroy that data and could +-- also remove objects which predated this migration. +DO $$ +BEGIN + RAISE EXCEPTION 'migration 000086 is irreversible: restore from backup instead of migrating down'; +END +$$; diff --git a/backend/migrations/000086_repair_assignment_policy_schema.up.sql b/backend/migrations/000086_repair_assignment_policy_schema.up.sql new file mode 100644 index 00000000..e73369dd --- /dev/null +++ b/backend/migrations/000086_repair_assignment_policy_schema.up.sql @@ -0,0 +1,64 @@ +ALTER TABLE assignment_policies + ADD COLUMN IF NOT EXISTS name VARCHAR(255), + ADD COLUMN IF NOT EXISTS description TEXT NOT NULL DEFAULT '', + ADD COLUMN IF NOT EXISTS assignment_order INTEGER NOT NULL DEFAULT 0, + ADD COLUMN IF NOT EXISTS conversation_priority INTEGER NOT NULL DEFAULT 0, + ADD COLUMN IF NOT EXISTS fair_distribution_limit INTEGER NOT NULL DEFAULT 100, + ADD COLUMN IF NOT EXISTS fair_distribution_window INTEGER NOT NULL DEFAULT 3600; + +UPDATE assignment_policies +SET name = COALESCE(NULLIF(policy_type, ''), 'Policy') || ' ' || id +WHERE name IS NULL OR name = ''; + +ALTER TABLE assignment_policies + ALTER COLUMN name SET NOT NULL; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_account_policy_name + ON assignment_policies(account_id, name); + +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM assignment_policies + WHERE inbox_id IS NOT NULL + GROUP BY inbox_id + HAVING COUNT(*) > 1 + ) THEN + RAISE EXCEPTION 'migration 000086: multiple legacy assignment policies target one inbox'; + END IF; + + IF EXISTS ( + SELECT 1 + FROM assignment_policies + JOIN inboxes ON inboxes.id = assignment_policies.inbox_id + WHERE assignment_policies.inbox_id IS NOT NULL + AND assignment_policies.account_id <> inboxes.account_id + ) THEN + RAISE EXCEPTION 'migration 000086: legacy assignment policy and inbox belong to different accounts'; + END IF; +END +$$; + +CREATE TABLE IF NOT EXISTS inbox_assignment_policies ( + id BIGSERIAL PRIMARY KEY, + inbox_id BIGINT NOT NULL REFERENCES inboxes(id) ON DELETE CASCADE, + assignment_policy_id BIGINT NOT NULL REFERENCES assignment_policies(id) ON DELETE CASCADE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +INSERT INTO inbox_assignment_policies(inbox_id, assignment_policy_id, created_at, updated_at) +SELECT assignment_policies.inbox_id, assignment_policies.id, assignment_policies.created_at, assignment_policies.updated_at +FROM assignment_policies +WHERE assignment_policies.inbox_id IS NOT NULL + AND NOT EXISTS ( + SELECT 1 + FROM inbox_assignment_policies + WHERE inbox_assignment_policies.inbox_id = assignment_policies.inbox_id + ); + +CREATE UNIQUE INDEX IF NOT EXISTS idx_inbox_assignment_policies_inbox_id + ON inbox_assignment_policies(inbox_id); +CREATE INDEX IF NOT EXISTS idx_inbox_assignment_policies_assignment_policy_id + ON inbox_assignment_policies(assignment_policy_id);