mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-02-20 03:00:08 -05:00
fix: workflow dispatch shouldn't include empty fields in inputs (#10123)
Fix behaviour change from #10089. Empty inputs used to hit a `continue` statement and skip, and are now fired to a workflow. It isn't likely this is a functional bug, but it does change the behaviour unexpectedly. Detected by end-to-end test failure (https://code.forgejo.org/forgejo/end-to-end/actions/runs/4360/jobs/2/attempt/1): ``` { - number2: "" - tags: "" } ``` Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10123 Reviewed-by: Michael Kriese <michael.kriese@gmx.de> Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
parent
ae6e18c518
commit
2ea5a8d22b
2 changed files with 33 additions and 8 deletions
|
|
@ -49,6 +49,8 @@ type Workflow struct {
|
|||
|
||||
type InputValueGetter func(key string) string
|
||||
|
||||
var ErrSkipDispatchInput = errors.New("skip dispatching of input")
|
||||
|
||||
func resolveDispatchInput(key, value string, input act_model.WorkflowDispatchInput) (string, error) {
|
||||
if len(value) == 0 {
|
||||
value = input.Default
|
||||
|
|
@ -60,6 +62,7 @@ func resolveDispatchInput(key, value string, input act_model.WorkflowDispatchInp
|
|||
}
|
||||
return "", InputRequiredErr{Name: name}
|
||||
}
|
||||
return "", ErrSkipDispatchInput
|
||||
}
|
||||
} else if input.Type == "boolean" {
|
||||
// Temporary compatibility shim for people that upgrade to Forgejo 14. Can be removed with Forgejo 15.
|
||||
|
|
@ -94,7 +97,9 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette
|
|||
if workflowDispatch := wf.WorkflowDispatchConfig(); workflowDispatch != nil {
|
||||
for key, input := range workflowDispatch.Inputs {
|
||||
value, err := resolveDispatchInput(key, inputGetter(key), input)
|
||||
if err != nil {
|
||||
if err == ErrSkipDispatchInput {
|
||||
continue
|
||||
} else if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
inputs[key] = value
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
package actions
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
actions_model "forgejo.org/models/actions"
|
||||
|
|
@ -124,11 +125,12 @@ 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 string
|
||||
key string
|
||||
value string
|
||||
input act_model.WorkflowDispatchInput
|
||||
expected string
|
||||
expectedError func(err error) bool
|
||||
}{
|
||||
{
|
||||
name: "on_converted_to_true",
|
||||
|
|
@ -202,10 +204,28 @@ func TestResolveDispatchInputAcceptsValidInput(t *testing.T) {
|
|||
input: act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "number", Options: []string{}},
|
||||
expected: "123",
|
||||
},
|
||||
{
|
||||
name: "empty_value_skipped",
|
||||
key: "my_number",
|
||||
value: "",
|
||||
input: act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "number", Options: []string{}},
|
||||
expectedError: func(err error) bool { return errors.Is(err, ErrSkipDispatchInput) },
|
||||
},
|
||||
{
|
||||
name: "required_missing",
|
||||
key: "my_number",
|
||||
value: "",
|
||||
input: act_model.WorkflowDispatchInput{Required: true, Type: "number", Options: []string{}},
|
||||
expectedError: IsInputRequiredErr,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
actual, _ := resolveDispatchInput(tc.key, tc.value, tc.input)
|
||||
assert.Equal(t, tc.expected, actual)
|
||||
actual, err := resolveDispatchInput(tc.key, tc.value, tc.input)
|
||||
if tc.expectedError != nil {
|
||||
assert.True(t, tc.expectedError(err))
|
||||
} else {
|
||||
assert.Equal(t, tc.expected, actual)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue