diff --git a/pkg/workflow/update_discussion.go b/pkg/workflow/update_discussion.go index 85b3ff3e627..801d313d0fc 100644 --- a/pkg/workflow/update_discussion.go +++ b/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) diff --git a/pkg/workflow/update_entity_helpers.go b/pkg/workflow/update_entity_helpers.go index a703d66ab23..d4abd190272 100644 --- a/pkg/workflow/update_entity_helpers.go +++ b/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 + } +} diff --git a/pkg/workflow/update_entity_helpers_test.go b/pkg/workflow/update_entity_helpers_test.go new file mode 100644 index 00000000000..f9c9dcb9709 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/workflow/update_issue.go b/pkg/workflow/update_issue.go index fd07acc0235..e08248c3bc2 100644 --- a/pkg/workflow/update_issue.go +++ b/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 } diff --git a/pkg/workflow/update_pull_request.go b/pkg/workflow/update_pull_request.go index 54c622cca64..1cab16247e5 100644 --- a/pkg/workflow/update_pull_request.go +++ b/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 }