Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions pkg/workflow/config_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,40 @@ func parseAllowedLabelsFromConfig(configMap map[string]any) []string {
// to consolidate all time parsing logic in a single location. These functions are used
// for parsing expiration configurations in safe output jobs.

// preprocessExpiresField handles the common expires field preprocessing pattern.
// This function:
// 1. Parses the expires value through parseExpiresFromConfig (handles integers, strings, and boolean false)
// 2. Handles explicit disablement when expires=false (returns -1)
// 3. Normalizes the value to hours and updates configData["expires"] in place
// 4. Logs the parsed value with the provided logger
//
// Returns true if expires was explicitly disabled with false, false otherwise.
// This helper consolidates duplicate preprocessing logic used in parseIssuesConfig and parseDiscussionsConfig.
func preprocessExpiresField(configData map[string]any, log *logger.Logger) bool {
expiresDisabled := false
if configData != nil {
if expires, exists := configData["expires"]; exists {
// Always parse the expires value through parseExpiresFromConfig
// This handles: integers (days), strings (time specs like "48h"), and boolean false
expiresInt := parseExpiresFromConfig(configData)
if expiresInt == -1 {
// Explicitly disabled with false
expiresDisabled = true
configData["expires"] = 0
} else if expiresInt > 0 {
configData["expires"] = expiresInt
} else {
// Invalid or missing - set to 0
configData["expires"] = 0
}
if log != nil {
log.Printf("Parsed expires value %v to %d hours (disabled=%t)", expires, expiresInt, expiresDisabled)
}
}
}
return expiresDisabled
}

// ParseIntFromConfig is a generic helper that extracts and validates an integer value from a map.
// Supports int, int64, float64, and uint64 types.
// Returns the integer value, or 0 if not present or invalid.
Expand Down
118 changes: 118 additions & 0 deletions pkg/workflow/config_parsing_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,124 @@ func TestParseBoolFromConfig(t *testing.T) {
}
}

func TestPreprocessExpiresField(t *testing.T) {
tests := []struct {
name string
input map[string]any
expectedDisabled bool
expectedValue int
}{
{
name: "valid integer days - converted to hours",
input: map[string]any{
"expires": 7,
},
expectedDisabled: false,
expectedValue: 168, // 7 days * 24 hours
},
{
name: "valid string format - 48h",
input: map[string]any{
"expires": "48h",
},
expectedDisabled: false,
expectedValue: 48,
},
{
name: "valid string format - 7d",
input: map[string]any{
"expires": "7d",
},
expectedDisabled: false,
expectedValue: 168,
},
{
name: "explicitly disabled with false",
input: map[string]any{
"expires": false,
},
expectedDisabled: true,
expectedValue: 0,
},
{
name: "invalid - true boolean",
input: map[string]any{
"expires": true,
},
expectedDisabled: false,
expectedValue: 0,
},
{
name: "invalid - 1 hour (below minimum)",
input: map[string]any{
"expires": "1h",
},
expectedDisabled: false,
expectedValue: 0,
},
{
name: "valid - 2 hours (at minimum)",
input: map[string]any{
"expires": "2h",
},
expectedDisabled: false,
expectedValue: 2,
},
{
name: "no expires field",
input: map[string]any{},
expectedDisabled: false,
expectedValue: 0, // configData["expires"] not set when field missing
},
{
name: "invalid string format",
input: map[string]any{
"expires": "invalid",
},
expectedDisabled: false,
expectedValue: 0,
},
{
name: "nil configData",
input: nil,
expectedDisabled: false,
expectedValue: 0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Make a copy of input to check modification
var configData map[string]any
if tt.input != nil {
configData = make(map[string]any)
for k, v := range tt.input {
configData[k] = v
}
}

disabled := preprocessExpiresField(configData, nil)

if disabled != tt.expectedDisabled {
t.Errorf("expected disabled=%v, got %v", tt.expectedDisabled, disabled)
}

// Check that configData["expires"] was updated (if configData is not nil)
if configData != nil && tt.input != nil {
if _, exists := tt.input["expires"]; exists {
expiresValue, ok := configData["expires"].(int)
if !ok && configData["expires"] != nil {
t.Errorf("expected expires to be int, got %T", configData["expires"])
}
if expiresValue != tt.expectedValue {
t.Errorf("expected configData[\"expires\"]=%d, got %d", tt.expectedValue, expiresValue)
}
}
}
})
}
}

func TestUnmarshalConfig(t *testing.T) {
tests := []struct {
name string
Expand Down
20 changes: 1 addition & 19 deletions pkg/workflow/create_discussion.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,7 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu
configData, _ := outputMap["create-discussion"].(map[string]any)

// Pre-process the expires field (convert to hours before unmarshaling)
expiresDisabled := false
if configData != nil {
if expires, exists := configData["expires"]; exists {
// Always parse the expires value through parseExpiresFromConfig
// This handles: integers (days), strings (time specs like "48h"), and boolean false
expiresInt := parseExpiresFromConfig(configData)
if expiresInt == -1 {
// Explicitly disabled with false
expiresDisabled = true
configData["expires"] = 0
} else if expiresInt > 0 {
configData["expires"] = expiresInt
} else {
// Invalid or missing - set to 0
configData["expires"] = 0
}
discussionLog.Printf("Parsed expires value %v to %d hours (disabled=%t)", expires, expiresInt, expiresDisabled)
}
}
expiresDisabled := preprocessExpiresField(configData, discussionLog)

// Unmarshal into typed config struct
var config CreateDiscussionsConfig
Expand Down
20 changes: 1 addition & 19 deletions pkg/workflow/create_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,7 @@ func (c *Compiler) parseIssuesConfig(outputMap map[string]any) *CreateIssuesConf
configData, _ := outputMap["create-issue"].(map[string]any)

// Pre-process the expires field (convert to hours before unmarshaling)
expiresDisabled := false
if configData != nil {
if expires, exists := configData["expires"]; exists {
// Always parse the expires value through parseExpiresFromConfig
// This handles: integers (days), strings (time specs like "48h"), and boolean false
expiresInt := parseExpiresFromConfig(configData)
if expiresInt == -1 {
// Explicitly disabled with false
expiresDisabled = true
configData["expires"] = 0
} else if expiresInt > 0 {
configData["expires"] = expiresInt
} else {
// Invalid or missing - set to 0
configData["expires"] = 0
}
createIssueLog.Printf("Parsed expires value %v to %d hours (disabled=%t)", expires, expiresInt, expiresDisabled)
}
}
expiresDisabled := preprocessExpiresField(configData, createIssueLog)

// Unmarshal into typed config struct
var config CreateIssuesConfig
Expand Down
Loading