diff --git a/services/actions/workflows.go b/services/actions/workflows.go index 1e4d92fa2f..18cf92e64d 100644 --- a/services/actions/workflows.go +++ b/services/actions/workflows.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "strconv" actions_model "forgejo.org/models/actions" "forgejo.org/models/perm" @@ -50,6 +49,28 @@ type Workflow struct { type InputValueGetter func(key string) string +func resolveDispatchInput(key, value string, input act_model.WorkflowDispatchInput) (string, error) { + if len(value) == 0 { + value = input.Default + if len(value) == 0 { + if input.Required { + name := input.Description + if len(name) == 0 { + name = key + } + return "", InputRequiredErr{Name: name} + } + } + } else if input.Type == "boolean" { + // Temporary compatibility shim for people that upgrade to Forgejo 14. Can be removed with Forgejo 15. + if value == "on" { + value = "true" + } + } + + return value, nil +} + func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGetter, repo *repo_model.Repository, doer *user.User) (r *actions_model.ActionRun, j []string, err error) { content, err := actions.GetContentFromEntry(entry.GitEntry) if err != nil { @@ -72,25 +93,12 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette inputsAny := make(map[string]any) if workflowDispatch := wf.WorkflowDispatchConfig(); workflowDispatch != nil { for key, input := range workflowDispatch.Inputs { - val := inputGetter(key) - if len(val) == 0 { - val = input.Default - if len(val) == 0 { - if input.Required { - name := input.Description - if len(name) == 0 { - name = key - } - return nil, nil, InputRequiredErr{Name: name} - } - continue - } - } else if input.Type == "boolean" { - // Since "boolean" inputs are rendered as a checkbox in html, the value inside the form is "on" - val = strconv.FormatBool(val == "on") + value, err := resolveDispatchInput(key, inputGetter(key), input) + if err != nil { + return nil, nil, err } - inputs[key] = val - inputsAny[key] = val + inputs[key] = value + inputsAny[key] = value } } diff --git a/services/actions/workflows_test.go b/services/actions/workflows_test.go index 7f50d7f982..555308018d 100644 --- a/services/actions/workflows_test.go +++ b/services/actions/workflows_test.go @@ -121,3 +121,135 @@ func TestConfigureActionRunConcurrency(t *testing.T) { }) } } + +func TestResolveDispatchInputAcceptsValidInput(t *testing.T) { + for _, tc := range []struct { + name string + key string + value string + input act_model.WorkflowDispatchInput + expected string + }{ + { + name: "on_converted_to_true", + key: "my_boolean", + value: "on", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: false, Type: "boolean", Options: []string{}}, + expected: "true", + }, + // It might make sense to validate booleans in the future and then turn it into an error. + { + name: "ON_stays_ON", + key: "my_boolean", + value: "ON", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: false, Type: "boolean", Options: []string{}}, + expected: "ON", + }, + { + name: "true_stays_true", + key: "my_boolean", + value: "true", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: false, Type: "boolean", Options: []string{}}, + expected: "true", + }, + { + name: "false_stays_false", + key: "my_boolean", + value: "false", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: false, Type: "boolean", Options: []string{}}, + expected: "false", + }, + { + name: "empty_results_in_default_value_true", + key: "my_boolean", + value: "", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: false, Default: "true", Type: "boolean", Options: []string{}}, + expected: "true", + }, + { + name: "empty_results_in_default_value_false", + key: "my_boolean", + value: "", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: false, Default: "false", Type: "boolean", Options: []string{}}, + expected: "false", + }, + { + name: "string_results_in_input", + key: "my_string", + value: "hello", + input: act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "string", Options: []string{}}, + expected: "hello", + }, + { + name: "string_option_results_in_input", + value: "a", + input: act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "string", Options: []string{"a", "b"}}, + expected: "a", + }, + // Test ensures that the old behaviour (ignoring option mismatch) is retained. It might + // make sense to turn it into an error in the future. + { + name: "invalid_string_option_results_in_input", + key: "option", + value: "c", + input: act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "string", Options: []string{"a", "b"}}, + expected: "c", + }, + { + name: "number_results_in_input", + key: "my_number", + value: "123", + input: act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "number", Options: []string{}}, + expected: "123", + }, + } { + t.Run(tc.name, func(t *testing.T) { + actual, _ := resolveDispatchInput(tc.key, tc.value, tc.input) + assert.Equal(t, tc.expected, actual) + }) + } +} + +func TestResolveDispatchInputRejectsInvalidInput(t *testing.T) { + for _, tc := range []struct { + name string + key string + value string + input act_model.WorkflowDispatchInput + expected error + }{ + { + name: "missing_required_boolean", + key: "missing_boolean", + value: "", + input: act_model.WorkflowDispatchInput{Description: "a boolean", Required: true, Type: "boolean", Options: []string{}}, + expected: InputRequiredErr{Name: "a boolean"}, + }, + { + name: "missing_required_boolean_without_description", + key: "missing_boolean", + value: "", + input: act_model.WorkflowDispatchInput{Required: true, Type: "boolean", Options: []string{}}, + expected: InputRequiredErr{Name: "missing_boolean"}, + }, + { + name: "missing_required_string", + key: "missing_string", + value: "", + input: act_model.WorkflowDispatchInput{Description: "a string", Required: true, Type: "string", Options: []string{}}, + expected: InputRequiredErr{Name: "a string"}, + }, + { + name: "missing_required_string_without_description", + key: "missing_string", + value: "", + input: act_model.WorkflowDispatchInput{Required: true, Type: "string", Options: []string{}}, + expected: InputRequiredErr{Name: "missing_string"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := resolveDispatchInput(tc.key, tc.value, tc.input) + assert.Equal(t, tc.expected, err) + }) + } +} diff --git a/templates/repo/actions/dispatch.tmpl b/templates/repo/actions/dispatch.tmpl index 53da1ea660..3f539c256f 100644 --- a/templates/repo/actions/dispatch.tmpl +++ b/templates/repo/actions/dispatch.tmpl @@ -34,8 +34,8 @@
{{/* These two inputs need to stay in exactly this order (checkbox first, hidden second) or boolean fields won't work correctly! */}} - - + +
{{else}}