Fix bd-437: Use addDependencyUnchecked during collision resolution
When remapping dependencies during collision resolution, skip semantic validation (like parent-child direction checks) since we're just updating IDs on existing dependencies that were already validated. Fixes parent-child validation error during import --resolve-collisions.
This commit is contained in:
@@ -474,7 +474,9 @@ func updateDependencyReferences(ctx context.Context, s *SQLiteStorage, idMapping
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Add new dependency with updated IDs
|
// Add new dependency with updated IDs
|
||||||
if err := s.AddDependency(ctx, update.newDep, "import-remap"); err != nil {
|
// Use addDependencyUnchecked to skip semantic validation (like parent-child direction)
|
||||||
|
// since we're just remapping existing dependencies that were already validated
|
||||||
|
if err := s.addDependencyUnchecked(ctx, update.newDep, "import-remap"); err != nil {
|
||||||
return fmt.Errorf("failed to add updated dependency %s -> %s: %w",
|
return fmt.Errorf("failed to add updated dependency %s -> %s: %w",
|
||||||
update.newDep.IssueID, update.newDep.DependsOnID, err)
|
update.newDep.IssueID, update.newDep.DependsOnID, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -154,6 +154,115 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency
|
|||||||
return tx.Commit()
|
return tx.Commit()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// addDependencyUnchecked adds a dependency with minimal validation, used during
|
||||||
|
// import/remap operations where we're preserving existing dependencies with new IDs.
|
||||||
|
// Skips semantic validation (parent-child direction) but keeps essential checks:
|
||||||
|
// - Issue existence validation
|
||||||
|
// - Self-dependency prevention
|
||||||
|
// - Cycle detection
|
||||||
|
func (s *SQLiteStorage) addDependencyUnchecked(ctx context.Context, dep *types.Dependency, actor string) error {
|
||||||
|
// Validate dependency type
|
||||||
|
if !dep.Type.IsValid() {
|
||||||
|
return fmt.Errorf("invalid dependency type: %s", dep.Type)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate that both issues exist
|
||||||
|
issueExists, err := s.GetIssue(ctx, dep.IssueID)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check issue %s: %w", dep.IssueID, err)
|
||||||
|
}
|
||||||
|
if issueExists == nil {
|
||||||
|
return fmt.Errorf("issue %s not found", dep.IssueID)
|
||||||
|
}
|
||||||
|
|
||||||
|
dependsOnExists, err := s.GetIssue(ctx, dep.DependsOnID)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check dependency %s: %w", dep.DependsOnID, err)
|
||||||
|
}
|
||||||
|
if dependsOnExists == nil {
|
||||||
|
return fmt.Errorf("dependency target %s not found", dep.DependsOnID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Prevent self-dependency
|
||||||
|
if dep.IssueID == dep.DependsOnID {
|
||||||
|
return fmt.Errorf("issue cannot depend on itself")
|
||||||
|
}
|
||||||
|
|
||||||
|
// NOTE: We skip parent-child direction validation here because during import/remap,
|
||||||
|
// we're just updating IDs on existing dependencies that were already validated.
|
||||||
|
|
||||||
|
dep.CreatedAt = time.Now()
|
||||||
|
dep.CreatedBy = actor
|
||||||
|
|
||||||
|
tx, err := s.db.BeginTx(ctx, nil)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to begin transaction: %w", err)
|
||||||
|
}
|
||||||
|
defer tx.Rollback()
|
||||||
|
|
||||||
|
// Cycle detection (same as AddDependency)
|
||||||
|
var cycleExists bool
|
||||||
|
err = tx.QueryRowContext(ctx, `
|
||||||
|
WITH RECURSIVE paths AS (
|
||||||
|
SELECT
|
||||||
|
issue_id,
|
||||||
|
depends_on_id,
|
||||||
|
1 as depth
|
||||||
|
FROM dependencies
|
||||||
|
WHERE issue_id = ?
|
||||||
|
|
||||||
|
UNION ALL
|
||||||
|
|
||||||
|
SELECT
|
||||||
|
d.issue_id,
|
||||||
|
d.depends_on_id,
|
||||||
|
p.depth + 1
|
||||||
|
FROM dependencies d
|
||||||
|
JOIN paths p ON d.issue_id = p.depends_on_id
|
||||||
|
WHERE p.depth < ?
|
||||||
|
)
|
||||||
|
SELECT EXISTS(
|
||||||
|
SELECT 1 FROM paths
|
||||||
|
WHERE depends_on_id = ?
|
||||||
|
)
|
||||||
|
`, dep.DependsOnID, maxDependencyDepth, dep.IssueID).Scan(&cycleExists)
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check for cycles: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if cycleExists {
|
||||||
|
return fmt.Errorf("cannot add dependency: would create a cycle (%s → %s → ... → %s)",
|
||||||
|
dep.IssueID, dep.DependsOnID, dep.IssueID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Insert dependency
|
||||||
|
_, err = tx.ExecContext(ctx, `
|
||||||
|
INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by)
|
||||||
|
VALUES (?, ?, ?, ?, ?)
|
||||||
|
`, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to add dependency: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Record event
|
||||||
|
_, err = tx.ExecContext(ctx, `
|
||||||
|
INSERT INTO events (issue_id, event_type, actor, comment)
|
||||||
|
VALUES (?, ?, ?, ?)
|
||||||
|
`, dep.IssueID, types.EventDependencyAdded, actor,
|
||||||
|
fmt.Sprintf("Added dependency: %s %s %s", dep.IssueID, dep.Type, dep.DependsOnID))
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to record event: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mark both issues as dirty
|
||||||
|
if err := markIssuesDirtyTx(ctx, tx, []string{dep.IssueID, dep.DependsOnID}); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
return tx.Commit()
|
||||||
|
}
|
||||||
|
|
||||||
// RemoveDependency removes a dependency
|
// RemoveDependency removes a dependency
|
||||||
func (s *SQLiteStorage) RemoveDependency(ctx context.Context, issueID, dependsOnID string, actor string) error {
|
func (s *SQLiteStorage) RemoveDependency(ctx context.Context, issueID, dependsOnID string, actor string) error {
|
||||||
tx, err := s.db.BeginTx(ctx, nil)
|
tx, err := s.db.BeginTx(ctx, nil)
|
||||||
|
|||||||
Reference in New Issue
Block a user