Refactor: Extract generic field parser for update entity configs

pkg/workflow/update_discussion.go

@@ -28,24 +28,13 @@ func (c *Compiler) parseUpdateDiscussionsConfig(outputMap map[string]any) *Updat
 		UpdateEntityConfig: *baseConfig,
 	}
 
-	// Parse discussion-specific fields from config map
-	if configMap != nil {
-		// Parse title - presence of the key (even if nil/empty) indicates field can be updated
-		if _, exists := configMap["title"]; exists {
-			updateDiscussionsConfig.Title = new(bool)
-		}
-
-		// Parse body - presence of the key (even if nil/empty) indicates field can be updated
-		if _, exists := configMap["body"]; exists {
-			updateDiscussionsConfig.Body = new(bool)
-		}
+	// Parse discussion-specific fields using key existence mode
+	updateDiscussionsConfig.Title = parseUpdateEntityBoolField(configMap, "title", FieldParsingKeyExistence)
+	updateDiscussionsConfig.Body = parseUpdateEntityBoolField(configMap, "body", FieldParsingKeyExistence)
+	updateDiscussionsConfig.Labels = parseUpdateEntityBoolField(configMap, "labels", FieldParsingKeyExistence)
 
-		// Parse labels - presence of the key (even if nil/empty) indicates field can be updated
-		if _, exists := configMap["labels"]; exists {
-			updateDiscussionsConfig.Labels = new(bool)
-		}
-
-		// Parse allowed-labels using shared helper
+	// Parse allowed-labels using shared helper
+	if configMap != nil {
 		updateDiscussionsConfig.AllowedLabels = parseAllowedLabelsFromConfig(configMap)
 		if len(updateDiscussionsConfig.AllowedLabels) > 0 {
 			updateDiscussionLog.Printf("Allowed labels configured: %v", updateDiscussionsConfig.AllowedLabels)

pkg/workflow/update_entity_helpers.go

@@ -120,3 +120,57 @@ func (c *Compiler) parseUpdateEntityBase(
 
 	return baseConfig, configMap
 }
+
+// FieldParsingMode determines how boolean fields are parsed from the config
+type FieldParsingMode int
+
+const (
+	// FieldParsingKeyExistence mode: Field presence (even if nil) indicates it can be updated
+	// Used by update-issue and update-discussion
+	FieldParsingKeyExistence FieldParsingMode = iota
+	// FieldParsingBoolValue mode: Field's boolean value determines if it can be updated
+	// Used by update-pull-request (defaults to true if nil)
+	FieldParsingBoolValue
+)
+
+// parseUpdateEntityBoolField is a generic helper that parses boolean fields from config maps
+// based on the specified parsing mode.
+//
+// Parameters:
+//   - configMap: The entity-specific configuration map
+//   - fieldName: The name of the field to parse (e.g., "title", "body", "status")
+//   - mode: The parsing mode (FieldParsingKeyExistence or FieldParsingBoolValue)
+//
+// Returns:
+//   - *bool: A pointer to bool if the field should be enabled, nil if disabled
+//
+// Behavior by mode:
+//   - FieldParsingKeyExistence: Returns new(bool) if key exists, nil otherwise
+//   - FieldParsingBoolValue: Returns &boolValue if key exists and is bool, nil otherwise
+func parseUpdateEntityBoolField(configMap map[string]any, fieldName string, mode FieldParsingMode) *bool {
+	if configMap == nil {
+		return nil
+	}
+
+	val, exists := configMap[fieldName]
+	if !exists {
+		return nil
+	}
+
+	switch mode {
+	case FieldParsingKeyExistence:
+		// Key presence (even if nil) indicates field can be updated
+		return new(bool) // Allocate a new bool pointer (defaults to false)
+
+	case FieldParsingBoolValue:
+		// Parse actual boolean value from config
+		if boolVal, ok := val.(bool); ok {
+			return &boolVal
+		}
+		// If present but not a bool (e.g., null), return nil (no explicit setting)
+		return nil
+
+	default:
+		return nil
+	}
+}

pkg/workflow/update_entity_helpers_test.go

@@ -0,0 +1,139 @@
+package workflow
+
+import (
+	"testing"
+)
+
+func TestParseUpdateEntityBoolField(t *testing.T) {
+	tests := []struct {
+		name      string
+		configMap map[string]any
+		fieldName string
+		mode      FieldParsingMode
+		wantNil   bool
+		wantValue bool // Only checked if wantNil is false
+	}{
+		// FieldParsingKeyExistence mode tests
+		{
+			name:      "key existence mode: key present with nil value",
+			configMap: map[string]any{"title": nil},
+			fieldName: "title",
+			mode:      FieldParsingKeyExistence,
+			wantNil:   false, // Should return non-nil pointer
+			wantValue: false, // Default bool value
+		},
+		{
+			name:      "key existence mode: key present with empty value",
+			configMap: map[string]any{"title": ""},
+			fieldName: "title",
+			mode:      FieldParsingKeyExistence,
+			wantNil:   false,
+			wantValue: false,
+		},
+		{
+			name:      "key existence mode: key not present",
+			configMap: map[string]any{"other": true},
+			fieldName: "title",
+			mode:      FieldParsingKeyExistence,
+			wantNil:   true,
+		},
+		{
+			name:      "key existence mode: nil config map",
+			configMap: nil,
+			fieldName: "title",
+			mode:      FieldParsingKeyExistence,
+			wantNil:   true,
+		},
+		{
+			name:      "key existence mode: empty config map",
+			configMap: map[string]any{},
+			fieldName: "title",
+			mode:      FieldParsingKeyExistence,
+			wantNil:   true,
+		},
+
+		// FieldParsingBoolValue mode tests
+		{
+			name:      "bool value mode: true value",
+			configMap: map[string]any{"title": true},
+			fieldName: "title",
+			mode:      FieldParsingBoolValue,
+			wantNil:   false,
+			wantValue: true,
+		},
+		{
+			name:      "bool value mode: false value",
+			configMap: map[string]any{"title": false},
+			fieldName: "title",
+			mode:      FieldParsingBoolValue,
+			wantNil:   false,
+			wantValue: false,
+		},
+		{
+			name:      "bool value mode: nil value (not a bool)",
+			configMap: map[string]any{"title": nil},
+			fieldName: "title",
+			mode:      FieldParsingBoolValue,
+			wantNil:   true, // Non-bool values return nil
+		},
+		{
+			name:      "bool value mode: string value (not a bool)",
+			configMap: map[string]any{"title": "true"},
+			fieldName: "title",
+			mode:      FieldParsingBoolValue,
+			wantNil:   true,
+		},
+		{
+			name:      "bool value mode: key not present",
+			configMap: map[string]any{"other": true},
+			fieldName: "title",
+			mode:      FieldParsingBoolValue,
+			wantNil:   true,
+		},
+		{
+			name:      "bool value mode: nil config map",
+			configMap: nil,
+			fieldName: "title",
+			mode:      FieldParsingBoolValue,
+			wantNil:   true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := parseUpdateEntityBoolField(tt.configMap, tt.fieldName, tt.mode)
+
+			if tt.wantNil {
+				if result != nil {
+					t.Errorf("Expected nil result, got %v", result)
+				}
+			} else {
+				if result == nil {
+					t.Errorf("Expected non-nil result, got nil")
+				} else if *result != tt.wantValue {
+					t.Errorf("Expected value %v, got %v", tt.wantValue, *result)
+				}
+			}
+		})
+	}
+}
+
+func TestParseUpdateEntityBoolFieldFieldNames(t *testing.T) {
+	// Test that different field names work correctly
+	configMap := map[string]any{
+		"title":  nil,
+		"body":   nil,
+		"status": nil,
+		"labels": nil,
+	}
+
+	fieldNames := []string{"title", "body", "status", "labels"}
+	for _, fieldName := range fieldNames {
+		t.Run(fieldName, func(t *testing.T) {
+			result := parseUpdateEntityBoolField(configMap, fieldName, FieldParsingKeyExistence)
+			if result == nil {
+				t.Errorf("Expected non-nil result for field %s", fieldName)
+			}
+		})
+	}
+}

pkg/workflow/update_issue.go

@@ -27,25 +27,10 @@ func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssu
 		UpdateEntityConfig: *baseConfig,
 	}
 
-	// Parse issue-specific fields from config map
-	if configMap != nil {
-		// Parse status - presence of the key (even if nil/empty) indicates field can be updated
-		if _, exists := configMap["status"]; exists {
-			// If the key exists, it means we can update the status
-			// We don't care about the value - just that the key is present
-			updateIssuesConfig.Status = new(bool) // Allocate a new bool pointer (defaults to false)
-		}
-
-		// Parse title - presence of the key (even if nil/empty) indicates field can be updated
-		if _, exists := configMap["title"]; exists {
-			updateIssuesConfig.Title = new(bool)
-		}
-
-		// Parse body - presence of the key (even if nil/empty) indicates field can be updated
-		if _, exists := configMap["body"]; exists {
-			updateIssuesConfig.Body = new(bool)
-		}
-	}
+	// Parse issue-specific fields using key existence mode
+	updateIssuesConfig.Status = parseUpdateEntityBoolField(configMap, "status", FieldParsingKeyExistence)
+	updateIssuesConfig.Title = parseUpdateEntityBoolField(configMap, "title", FieldParsingKeyExistence)
+	updateIssuesConfig.Body = parseUpdateEntityBoolField(configMap, "body", FieldParsingKeyExistence)
 
 	return updateIssuesConfig
 }

pkg/workflow/update_pull_request.go

@@ -28,24 +28,9 @@ func (c *Compiler) parseUpdatePullRequestsConfig(outputMap map[string]any) *Upda
 		UpdateEntityConfig: *baseConfig,
 	}
 
-	// Parse PR-specific fields from config map
-	if configMap != nil {
-		// Parse title - boolean to enable/disable (defaults to true if nil or not set)
-		if titleVal, exists := configMap["title"]; exists {
-			if titleBool, ok := titleVal.(bool); ok {
-				updatePullRequestsConfig.Title = &titleBool
-			}
-			// If present but not a bool (e.g., null), leave as nil (defaults to enabled)
-		}
-
-		// Parse body - boolean to enable/disable (defaults to true if nil or not set)
-		if bodyVal, exists := configMap["body"]; exists {
-			if bodyBool, ok := bodyVal.(bool); ok {
-				updatePullRequestsConfig.Body = &bodyBool
-			}
-			// If present but not a bool (e.g., null), leave as nil (defaults to enabled)
-		}
-	}
+	// Parse PR-specific fields using bool value mode (defaults to true if nil)
+	updatePullRequestsConfig.Title = parseUpdateEntityBoolField(configMap, "title", FieldParsingBoolValue)
+	updatePullRequestsConfig.Body = parseUpdateEntityBoolField(configMap, "body", FieldParsingBoolValue)
 
 	return updatePullRequestsConfig
 }