diff --git a/internal/service/relational/workflows/constants.go b/internal/service/relational/workflows/constants.go index 8138c26d..4c6a1492 100644 --- a/internal/service/relational/workflows/constants.go +++ b/internal/service/relational/workflows/constants.go @@ -144,6 +144,23 @@ const ( StepStatusSkipped StepExecutionStatus = "skipped" ) +// openStepStatuses is the unexported backing slice for OpenStepStatuses. +var openStepStatuses = []string{ + string(StepStatusPending), + string(StepStatusBlocked), + string(StepStatusInProgress), + string(StepStatusOverdue), +} + +// OpenStepStatuses returns a copy of the step execution statuses that represent incomplete work. +// Use it for queries that target non-terminal steps (e.g. cascade deletes, assignment lookups). +// A copy is returned to prevent callers from mutating the shared backing slice. +func OpenStepStatuses() []string { + s := make([]string, len(openStepStatuses)) + copy(s, openStepStatuses) + return s +} + // IsValid checks if the step execution status is valid func (s StepExecutionStatus) IsValid() bool { switch s { diff --git a/internal/service/relational/workflows/step_execution_service.go b/internal/service/relational/workflows/step_execution_service.go index 61b4c1a6..deacb0e5 100644 --- a/internal/service/relational/workflows/step_execution_service.go +++ b/internal/service/relational/workflows/step_execution_service.go @@ -368,7 +368,7 @@ func (s *StepExecutionService) GetCompletedSteps(executionID *uuid.UUID) ([]Step func (s *StepExecutionService) GetAssignedSteps(assignedToType, assignedToID string) ([]StepExecution, error) { var stepExecutions []StepExecution err := s.db.Where("assigned_to_type = ? AND assigned_to_id = ? AND status IN ?", - assignedToType, assignedToID, []string{"pending", "in_progress", "blocked", "overdue"}). + assignedToType, assignedToID, OpenStepStatuses()). Preload("WorkflowExecution"). Preload("WorkflowExecution.WorkflowInstance"). Preload("WorkflowStepDefinition"). diff --git a/internal/service/relational/workflows/workflow_instance_service.go b/internal/service/relational/workflows/workflow_instance_service.go index dde639a1..dfc09324 100644 --- a/internal/service/relational/workflows/workflow_instance_service.go +++ b/internal/service/relational/workflows/workflow_instance_service.go @@ -133,9 +133,46 @@ func (s *WorkflowInstanceService) Update(id *uuid.UUID, updates *WorkflowInstanc return s.base.UpdateEntity(&existing, updates, id, "workflow instance") } -// Delete soft deletes a workflow instance +// Delete soft deletes a workflow instance and cascades to its open step executions and their +// evidence. Closed step executions (completed, failed, skipped) and their evidence are preserved +// to retain the audit trail. All workflow executions belonging to the instance are also removed. func (s *WorkflowInstanceService) Delete(id *uuid.UUID) error { - return s.base.DeleteEntity(&WorkflowInstance{}, id, "workflow instance") + return s.db.Transaction(func(tx *gorm.DB) error { + execSubquery := tx.Model(&WorkflowExecution{}). + Select("id"). + Where("workflow_instance_id = ?", id) + + // Collect open step execution IDs so we can delete their evidence. + var openStepIDs []uuid.UUID + if err := tx.Model(&StepExecution{}). + Select("id"). + Where("workflow_execution_id IN (?) AND status IN ?", execSubquery, OpenStepStatuses()). + Pluck("id", &openStepIDs).Error; err != nil { + return err + } + + if len(openStepIDs) > 0 { + if err := tx. + Where("step_execution_id IN ?", openStepIDs). + Delete(&StepEvidence{}).Error; err != nil { + return err + } + } + + if err := tx. + Where("workflow_execution_id IN (?) AND status IN ?", execSubquery, OpenStepStatuses()). + Delete(&StepExecution{}).Error; err != nil { + return err + } + + if err := tx. + Where("workflow_instance_id = ?", id). + Delete(&WorkflowExecution{}).Error; err != nil { + return err + } + + return NewBaseService(tx).DeleteEntity(&WorkflowInstance{}, id, "workflow instance") + }) } // Activate activates a workflow instance diff --git a/internal/service/relational/workflows/workflow_instance_service_test.go b/internal/service/relational/workflows/workflow_instance_service_test.go index 2167f0bd..094d4a24 100644 --- a/internal/service/relational/workflows/workflow_instance_service_test.go +++ b/internal/service/relational/workflows/workflow_instance_service_test.go @@ -293,6 +293,160 @@ func TestWorkflowInstanceService_Delete(t *testing.T) { assert.Contains(t, err.Error(), "not found") } +// TestWorkflowInstanceService_Delete_SoftDeletesOpenStepExecutions tests that deleting a workflow +// instance also soft-deletes open step executions (BCH-1158). +// Observed: pending/blocked/in_progress/overdue step executions remain alive after instance deletion. +// Expected: open step executions are soft-deleted; completed/failed/skipped are left untouched. +func TestWorkflowInstanceService_Delete_SoftDeletesOpenStepExecutions(t *testing.T) { + db := setupTestDB(t) + service := NewWorkflowInstanceService(db) + + workflowDef := createTestWorkflowDefinition() + require.NoError(t, db.Create(workflowDef).Error) + + instance := createTestWorkflowInstance(workflowDef.ID) + require.NoError(t, db.Create(instance).Error) + + execution := createTestWorkflowExecution(instance.ID) + require.NoError(t, db.Create(execution).Error) + + openStatuses := []string{ + string(StepStatusPending), + string(StepStatusBlocked), + string(StepStatusInProgress), + string(StepStatusOverdue), + } + closedStatuses := []string{ + string(StepStatusCompleted), + string(StepStatusFailed), + string(StepStatusSkipped), + } + + openStepIDs := make([]*uuid.UUID, 0, len(openStatuses)) + for _, status := range openStatuses { + sd := createTestWorkflowStepDefinition(workflowDef.ID) + require.NoError(t, db.Create(sd).Error) + step := createTestStepExecution(execution.ID, sd.ID) + step.Status = status + require.NoError(t, db.Create(step).Error) + openStepIDs = append(openStepIDs, step.ID) + } + + closedStepIDs := make([]*uuid.UUID, 0, len(closedStatuses)) + for _, status := range closedStatuses { + sd := createTestWorkflowStepDefinition(workflowDef.ID) + require.NoError(t, db.Create(sd).Error) + step := createTestStepExecution(execution.ID, sd.ID) + step.Status = status + require.NoError(t, db.Create(step).Error) + closedStepIDs = append(closedStepIDs, step.ID) + } + + require.NoError(t, service.Delete(instance.ID)) + + // Open step executions must be soft-deleted. + for _, id := range openStepIDs { + var step StepExecution + require.NoError(t, db.Unscoped().First(&step, id).Error) + assert.NotNil(t, step.DeletedAt.Time, "open step %s should be soft-deleted", id) + assert.True(t, step.DeletedAt.Valid, "open step %s should have a valid deleted_at", id) + } + + // Closed step executions must be preserved. + for _, id := range closedStepIDs { + var step StepExecution + require.NoError(t, db.First(&step, id).Error, "closed step %s should still exist", id) + assert.False(t, step.DeletedAt.Valid, "closed step %s should not be soft-deleted", id) + } +} + +// TestWorkflowInstanceService_Delete_SoftDeletesWorkflowExecutions tests that deleting a workflow +// instance also soft-deletes all workflow executions belonging to it (BCH-1158). +func TestWorkflowInstanceService_Delete_SoftDeletesWorkflowExecutions(t *testing.T) { + db := setupTestDB(t) + service := NewWorkflowInstanceService(db) + + workflowDef := createTestWorkflowDefinition() + require.NoError(t, db.Create(workflowDef).Error) + + instance := createTestWorkflowInstance(workflowDef.ID) + require.NoError(t, db.Create(instance).Error) + + exec1 := createTestWorkflowExecution(instance.ID) + require.NoError(t, db.Create(exec1).Error) + exec2 := createTestWorkflowExecution(instance.ID) + require.NoError(t, db.Create(exec2).Error) + + require.NoError(t, service.Delete(instance.ID)) + + for _, execID := range []*uuid.UUID{exec1.ID, exec2.ID} { + var we WorkflowExecution + require.NoError(t, db.Unscoped().First(&we, execID).Error) + assert.True(t, we.DeletedAt.Valid, "workflow execution %s should be soft-deleted", execID) + } +} + +// TestWorkflowInstanceService_Delete_SoftDeletesStepEvidenceForOpenSteps tests that deleting a +// workflow instance also soft-deletes evidence submitted for open (non-closed) steps (BCH-1158). +// Closed steps and their evidence must be preserved for the audit trail. +func TestWorkflowInstanceService_Delete_SoftDeletesStepEvidenceForOpenSteps(t *testing.T) { + db := setupTestDB(t) + service := NewWorkflowInstanceService(db) + + workflowDef := createTestWorkflowDefinition() + require.NoError(t, db.Create(workflowDef).Error) + + instance := createTestWorkflowInstance(workflowDef.ID) + require.NoError(t, db.Create(instance).Error) + + execution := createTestWorkflowExecution(instance.ID) + require.NoError(t, db.Create(execution).Error) + + // Open step with evidence that should be deleted. + openStepDef := createTestWorkflowStepDefinition(workflowDef.ID) + require.NoError(t, db.Create(openStepDef).Error) + openStep := createTestStepExecution(execution.ID, openStepDef.ID) + openStep.Status = string(StepStatusInProgress) + require.NoError(t, db.Create(openStep).Error) + + openEvidenceID := uuid.New() + openEvidence := &StepEvidence{ + UUIDModel: relational.UUIDModel{ID: &openEvidenceID}, + StepExecutionID: openStep.ID, + Name: "Open step evidence", + EvidenceType: "document", + } + require.NoError(t, db.Create(openEvidence).Error) + + // Closed step with evidence that must be preserved. + closedStepDef := createTestWorkflowStepDefinition(workflowDef.ID) + require.NoError(t, db.Create(closedStepDef).Error) + closedStep := createTestStepExecution(execution.ID, closedStepDef.ID) + closedStep.Status = string(StepStatusCompleted) + require.NoError(t, db.Create(closedStep).Error) + + closedEvidenceID := uuid.New() + closedEvidence := &StepEvidence{ + UUIDModel: relational.UUIDModel{ID: &closedEvidenceID}, + StepExecutionID: closedStep.ID, + Name: "Closed step evidence", + EvidenceType: "document", + } + require.NoError(t, db.Create(closedEvidence).Error) + + require.NoError(t, service.Delete(instance.ID)) + + // Evidence for the open step must be soft-deleted. + var deletedEvidence StepEvidence + require.NoError(t, db.Unscoped().First(&deletedEvidence, &openEvidenceID).Error) + assert.True(t, deletedEvidence.DeletedAt.Valid, "evidence for open step should be soft-deleted") + + // Evidence for the closed step must be preserved. + var preservedEvidence StepEvidence + require.NoError(t, db.First(&preservedEvidence, &closedEvidenceID).Error, "evidence for closed step should still exist") + assert.False(t, preservedEvidence.DeletedAt.Valid, "evidence for closed step should not be soft-deleted") +} + // TestWorkflowInstanceService_Activate tests the Activate method func TestWorkflowInstanceService_Activate(t *testing.T) { db := setupTestDB(t)