fix: refine dbtcloud_job type transition rules to avoid unnecessary replacement (closes #666)

Only CI<->non-CI and Adaptive<->non-Adaptive job type transitions now trigger
resource replacement. Scheduled<->Other and Merge<->Other/Scheduled transitions
are handled in-place. Adds requiresJobTypeReplacement() helper and two new
acceptance test functions.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Author: trouze

pkg/framework/objects/job/resource.go

@@ -36,44 +36,40 @@ type jobResource struct {
 	client *dbt_cloud.Client
 }
 
-// validateJobTypeChange validates if a job type transition is allowed.
-// This mirrors the server-side validation in _validate_job_type_change.
-func validateJobTypeChange(prevJobType, newJobType string) error {
-	// If no change, always allowed
-	if prevJobType == newJobType {
-		return nil
+// requiresJobTypeReplacement reports whether changing from oldType to newType
+// requires the resource to be destroyed and recreated.
+// Only transitions involving CI or Adaptive require replacement; all other
+// transitions (scheduled <-> other <-> merge) can be performed in-place.
+func requiresJobTypeReplacement(oldType, newType string) bool {
+	if oldType == newType {
+		return false
+	}
+	if oldType == JobTypeCI || newType == JobTypeCI {
+		return true
 	}
+	if oldType == JobTypeAdaptive || newType == JobTypeAdaptive {
+		return true
+	}
+	return false
+}
 
-	// If previous type is empty (not set), any new type is allowed
-	if prevJobType == "" {
+// validateJobTypeChange returns an error if the in-place job_type transition is
+// not permitted by the API. CI and Adaptive transitions are handled upstream by
+// requiresJobTypeReplacement (forced replace); this function guards the remaining
+// invalid combinations as a safety net.
+func validateJobTypeChange(prevJobType, newJobType string) error {
+	if prevJobType == newJobType || prevJobType == "" {
 		return nil
 	}
-
-	// CI jobs can only stay CI
 	if prevJobType == JobTypeCI && newJobType != JobTypeCI {
 		return fmt.Errorf("the job type field for this job can only be set to 'ci'")
 	}
-
-	// Adaptive jobs can only stay adaptive
 	if prevJobType == JobTypeAdaptive && newJobType != JobTypeAdaptive {
 		return fmt.Errorf("the job type field for this job can only be set to 'adaptive'")
 	}
-
-	// Scheduled jobs can only change to scheduled or other
-	if prevJobType == JobTypeScheduled && (newJobType == JobTypeCI || newJobType == JobTypeAdaptive) {
-		return fmt.Errorf("the job type field for this job can only be set to 'scheduled' or 'other'")
-	}
-
-	// Other jobs can only change to scheduled or other
-	if prevJobType == JobTypeOther && (newJobType == JobTypeCI || newJobType == JobTypeAdaptive) {
-		return fmt.Errorf("the job type field for this job can only be set to 'scheduled' or 'other'")
+	if newJobType == JobTypeCI || newJobType == JobTypeAdaptive {
+		return fmt.Errorf("cannot change job_type to '%s' from '%s'", newJobType, prevJobType)
 	}
-
-	// Merge jobs - treating similar to CI (cannot change away from merge)
-	if prevJobType == JobTypeMerge && newJobType != JobTypeMerge {
-		return fmt.Errorf("the job type field for this job can only be set to 'merge'")
-	}
-
 	return nil
 }
 
@@ -108,50 +104,40 @@ func (j *jobResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanReq
 		return
 	}
 
-	// Skip checks if necessary fields are null
 	if plan.Triggers == nil || state.Triggers == nil {
 		return
 	}
 
-	// if we change the job type (CI, merge or "empty"), we need to recreate the job as dbt Cloud doesn't allow updating them
-	// the job type is determined by the triggers
-	if plan.Triggers != nil && state.Triggers != nil {
-		oldCI := state.Triggers.GithubWebhook.ValueBool() || state.Triggers.GitProviderWebhook.ValueBool()
-		oldOnMerge := state.Triggers.OnMerge.ValueBool()
-
-		oldType := ""
-		if oldCI {
-			oldType = "ci"
-		} else if oldOnMerge {
-			oldType = "merge"
-		}
-
-		newCI := plan.Triggers.GithubWebhook.ValueBool() || plan.Triggers.GitProviderWebhook.ValueBool()
-		newOnMerge := plan.Triggers.OnMerge.ValueBool()
+	oldCI := state.Triggers.GithubWebhook.ValueBool() || state.Triggers.GitProviderWebhook.ValueBool()
+	oldOnMerge := state.Triggers.OnMerge.ValueBool()
+	oldType := JobTypeOther
+	if oldCI {
+		oldType = JobTypeCI
+	} else if oldOnMerge {
+		oldType = JobTypeMerge
+	}
 
-		newType := ""
-		if newCI {
-			newType = "ci"
-		} else if newOnMerge {
-			newType = "merge"
-		}
+	newCI := plan.Triggers.GithubWebhook.ValueBool() || plan.Triggers.GitProviderWebhook.ValueBool()
+	newOnMerge := plan.Triggers.OnMerge.ValueBool()
+	newType := JobTypeOther
+	if newCI {
+		newType = JobTypeCI
+	} else if newOnMerge {
+		newType = JobTypeMerge
+	}
 
-		if oldType != newType {
-			resp.RequiresReplace = append(resp.RequiresReplace, path.Root("triggers"))
-		}
+	if requiresJobTypeReplacement(oldType, newType) {
+		resp.RequiresReplace = append(resp.RequiresReplace, path.Root("triggers"))
 	}
 
-	// Validate job_type field changes if the field is being explicitly set
-	// Note: If plan.JobType is set but state.JobType is null (first time setting it),
-	// the validation will happen in Update against the actual server value
-	// Skip validation if either value is empty (empty means "not explicitly set")
 	if !plan.JobType.IsNull() && !state.JobType.IsNull() {
 		prevJobType := state.JobType.ValueString()
 		newJobType := plan.JobType.ValueString()
 
-		// Only validate if both values are non-empty (explicitly set)
-		if prevJobType != "" && newJobType != "" {
-			if err := validateJobTypeChange(prevJobType, newJobType); err != nil {
+		if prevJobType != "" && newJobType != "" && prevJobType != newJobType {
+			if requiresJobTypeReplacement(prevJobType, newJobType) {
+				resp.RequiresReplace = append(resp.RequiresReplace, path.Root("job_type"))
+			} else if err := validateJobTypeChange(prevJobType, newJobType); err != nil {
 				resp.Diagnostics.AddError(
 					"Invalid job_type change",
 					fmt.Sprintf("Cannot change job_type from '%s' to '%s': %s", prevJobType, newJobType, err.Error()),

pkg/framework/objects/job/resource_acceptance_job_type_test.go

@@ -120,64 +120,83 @@ func TestValidateJobTypeChange(t *testing.T) {
 			errorSubstr: "can only be set to 'adaptive'",
 		},
 
-		// Merge job type transitions - only merge allowed
+		// Merge job type transitions - merge, scheduled, and other are all allowed in-place;
+		// transitions to CI or Adaptive are blocked (those would require replacement at the
+		// ModifyPlan level but validateJobTypeChange still guards them).
 		{
 			name:        "merge to ci - not allowed",
 			prevType:    JobTypeMerge,
 			newType:     JobTypeCI,
 			expectError: true,
-			errorSubstr: "can only be set to 'merge'",
+			errorSubstr: "cannot change job_type to 'ci'",
 		},
 		{
-			name:        "merge to scheduled - not allowed",
+			name:        "merge to scheduled - allowed in-place",
 			prevType:    JobTypeMerge,
 			newType:     JobTypeScheduled,
-			expectError: true,
-			errorSubstr: "can only be set to 'merge'",
+			expectError: false,
+		},
+		{
+			name:        "merge to other - allowed in-place",
+			prevType:    JobTypeMerge,
+			newType:     JobTypeOther,
+			expectError: false,
 		},
 
-		// Scheduled job type transitions - scheduled or other allowed
+		// Scheduled job type transitions - scheduled, other, and merge are all allowed in-place
 		{
 			name:        "scheduled to other - allowed",
 			prevType:    JobTypeScheduled,
 			newType:     JobTypeOther,
 			expectError: false,
 		},
+		{
+			name:        "scheduled to merge - allowed in-place",
+			prevType:    JobTypeScheduled,
+			newType:     JobTypeMerge,
+			expectError: false,
+		},
 		{
 			name:        "scheduled to ci - not allowed",
 			prevType:    JobTypeScheduled,
 			newType:     JobTypeCI,
 			expectError: true,
-			errorSubstr: "can only be set to 'scheduled' or 'other'",
+			errorSubstr: "cannot change job_type to 'ci'",
 		},
 		{
 			name:        "scheduled to adaptive - not allowed",
 			prevType:    JobTypeScheduled,
 			newType:     JobTypeAdaptive,
 			expectError: true,
-			errorSubstr: "can only be set to 'scheduled' or 'other'",
+			errorSubstr: "cannot change job_type to 'adaptive'",
 		},
 
-		// Other job type transitions - scheduled or other allowed
+		// Other job type transitions - scheduled, other, and merge are all allowed in-place
 		{
 			name:        "other to scheduled - allowed",
 			prevType:    JobTypeOther,
 			newType:     JobTypeScheduled,
 			expectError: false,
 		},
+		{
+			name:        "other to merge - allowed in-place",
+			prevType:    JobTypeOther,
+			newType:     JobTypeMerge,
+			expectError: false,
+		},
 		{
 			name:        "other to ci - not allowed",
 			prevType:    JobTypeOther,
 			newType:     JobTypeCI,
 			expectError: true,
-			errorSubstr: "can only be set to 'scheduled' or 'other'",
+			errorSubstr: "cannot change job_type to 'ci'",
 		},
 		{
 			name:        "other to adaptive - not allowed",
 			prevType:    JobTypeOther,
 			newType:     JobTypeAdaptive,
 			expectError: true,
-			errorSubstr: "can only be set to 'scheduled' or 'other'",
+			errorSubstr: "cannot change job_type to 'adaptive'",
 		},
 	}
 
@@ -228,20 +247,20 @@ func TestValidateJobTypeChange_AllTransitions(t *testing.T) {
 		JobTypeMerge: {
 			JobTypeCI:        false,
 			JobTypeMerge:     true,
-			JobTypeScheduled: false,
-			JobTypeOther:     false,
+			JobTypeScheduled: true, // in-place transition allowed
+			JobTypeOther:     true, // in-place transition allowed
 			JobTypeAdaptive:  false,
 		},
 		JobTypeScheduled: {
 			JobTypeCI:        false,
-			JobTypeMerge:     true, // merge is allowed from scheduled (not in original server code but reasonable)
+			JobTypeMerge:     true, // in-place transition allowed
 			JobTypeScheduled: true,
 			JobTypeOther:     true,
 			JobTypeAdaptive:  false,
 		},
 		JobTypeOther: {
 			JobTypeCI:        false,
-			JobTypeMerge:     true, // merge is allowed from other (not in original server code but reasonable)
+			JobTypeMerge:     true, // in-place transition allowed
 			JobTypeScheduled: true,
 			JobTypeOther:     true,
 			JobTypeAdaptive:  false,
@@ -305,3 +324,57 @@ func TestJobTypeConstants(t *testing.T) {
 		}
 	}
 }
+
+func TestRequiresJobTypeReplacement(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		oldType  string
+		newType  string
+		expected bool
+	}{
+		// Same type — never requires replacement
+		{name: "ci to ci", oldType: JobTypeCI, newType: JobTypeCI, expected: false},
+		{name: "scheduled to scheduled", oldType: JobTypeScheduled, newType: JobTypeScheduled, expected: false},
+		{name: "other to other", oldType: JobTypeOther, newType: JobTypeOther, expected: false},
+		{name: "merge to merge", oldType: JobTypeMerge, newType: JobTypeMerge, expected: false},
+		{name: "adaptive to adaptive", oldType: JobTypeAdaptive, newType: JobTypeAdaptive, expected: false},
+
+		// CI transitions — always require replacement
+		{name: "ci to scheduled", oldType: JobTypeCI, newType: JobTypeScheduled, expected: true},
+		{name: "ci to other", oldType: JobTypeCI, newType: JobTypeOther, expected: true},
+		{name: "ci to merge", oldType: JobTypeCI, newType: JobTypeMerge, expected: true},
+		{name: "ci to adaptive", oldType: JobTypeCI, newType: JobTypeAdaptive, expected: true},
+		{name: "scheduled to ci", oldType: JobTypeScheduled, newType: JobTypeCI, expected: true},
+		{name: "other to ci", oldType: JobTypeOther, newType: JobTypeCI, expected: true},
+		{name: "merge to ci", oldType: JobTypeMerge, newType: JobTypeCI, expected: true},
+
+		// Adaptive transitions — always require replacement
+		{name: "adaptive to scheduled", oldType: JobTypeAdaptive, newType: JobTypeScheduled, expected: true},
+		{name: "adaptive to other", oldType: JobTypeAdaptive, newType: JobTypeOther, expected: true},
+		{name: "adaptive to merge", oldType: JobTypeAdaptive, newType: JobTypeMerge, expected: true},
+		{name: "scheduled to adaptive", oldType: JobTypeScheduled, newType: JobTypeAdaptive, expected: true},
+		{name: "other to adaptive", oldType: JobTypeOther, newType: JobTypeAdaptive, expected: true},
+		{name: "merge to adaptive", oldType: JobTypeMerge, newType: JobTypeAdaptive, expected: true},
+
+		// In-place transitions (scheduled / other / merge)
+		{name: "scheduled to other", oldType: JobTypeScheduled, newType: JobTypeOther, expected: false},
+		{name: "scheduled to merge", oldType: JobTypeScheduled, newType: JobTypeMerge, expected: false},
+		{name: "other to scheduled", oldType: JobTypeOther, newType: JobTypeScheduled, expected: false},
+		{name: "other to merge", oldType: JobTypeOther, newType: JobTypeMerge, expected: false},
+		{name: "merge to scheduled", oldType: JobTypeMerge, newType: JobTypeScheduled, expected: false},
+		{name: "merge to other", oldType: JobTypeMerge, newType: JobTypeOther, expected: false},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got := requiresJobTypeReplacement(tc.oldType, tc.newType)
+			if got != tc.expected {
+				t.Errorf("requiresJobTypeReplacement(%q, %q) = %v, want %v", tc.oldType, tc.newType, got, tc.expected)
+			}
+		})
+	}
+}