Support comma-delimited string as labels in issue template (#21831)

The [labels in issue YAML
templates](https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-issue-forms#top-level-syntax)
can be a string array or a comma-delimited string, so a single string
should be valid labels.

The old codes committed in #20987 ignore this, that's why the warning is
displayed:

<img width="618" alt="image"
src="https://user-images.githubusercontent.com/9418365/202112642-93dc72d0-71c3-40a2-9720-30fc2d48c97c.png">

Fixes #17877.
This commit is contained in:
Jason Song 2022-11-19 23:22:15 +08:00 committed by GitHub
parent c8f3eb6acb
commit d3f850cc0e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 369 additions and 140 deletions

View file

@ -165,7 +165,7 @@ func validateOptions(field *api.IssueFormField, idx int) error {
return position.Errorf("should be a string") return position.Errorf("should be a string")
} }
case api.IssueFormFieldTypeCheckboxes: case api.IssueFormFieldTypeCheckboxes:
opt, ok := option.(map[interface{}]interface{}) opt, ok := option.(map[string]interface{})
if !ok { if !ok {
return position.Errorf("should be a dictionary") return position.Errorf("should be a dictionary")
} }
@ -351,7 +351,7 @@ func (o *valuedOption) Label() string {
return label return label
} }
case api.IssueFormFieldTypeCheckboxes: case api.IssueFormFieldTypeCheckboxes:
if vs, ok := o.data.(map[interface{}]interface{}); ok { if vs, ok := o.data.(map[string]interface{}); ok {
if v, ok := vs["label"].(string); ok { if v, ok := vs["label"].(string); ok {
return v return v
} }

View file

@ -6,17 +6,20 @@ package template
import ( import (
"net/url" "net/url"
"reflect"
"testing" "testing"
"code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/json"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"github.com/stretchr/testify/require"
) )
func TestValidate(t *testing.T) { func TestValidate(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
filename string
content string content string
want *api.IssueTemplate
wantErr string wantErr string
}{ }{
{ {
@ -316,21 +319,9 @@ body:
`, `,
wantErr: "body[0](checkboxes), option[0]: 'required' should be a bool", wantErr: "body[0](checkboxes), option[0]: 'required' should be a bool",
}, },
} {
for _, tt := range tests { name: "valid",
t.Run(tt.name, func(t *testing.T) { content: `
tmpl, err := unmarshal("test.yaml", []byte(tt.content))
if err != nil {
t.Fatal(err)
}
if err := Validate(tmpl); (err == nil) != (tt.wantErr == "") || err != nil && err.Error() != tt.wantErr {
t.Errorf("Validate() error = %v, wantErr %q", err, tt.wantErr)
}
})
}
t.Run("valid", func(t *testing.T) {
content := `
name: Name name: Name
title: Title title: Title
about: About about: About
@ -386,8 +377,8 @@ body:
required: false required: false
- label: Option 3 of checkboxes - label: Option 3 of checkboxes
required: true required: true
` `,
want := &api.IssueTemplate{ want: &api.IssueTemplate{
Name: "Name", Name: "Name",
Title: "Title", Title: "Title",
About: "About", About: "About",
@ -454,28 +445,159 @@ body:
"label": "Label of checkboxes", "label": "Label of checkboxes",
"description": "Description of checkboxes", "description": "Description of checkboxes",
"options": []interface{}{ "options": []interface{}{
map[interface{}]interface{}{"label": "Option 1 of checkboxes", "required": true}, map[string]interface{}{"label": "Option 1 of checkboxes", "required": true},
map[interface{}]interface{}{"label": "Option 2 of checkboxes", "required": false}, map[string]interface{}{"label": "Option 2 of checkboxes", "required": false},
map[interface{}]interface{}{"label": "Option 3 of checkboxes", "required": true}, map[string]interface{}{"label": "Option 3 of checkboxes", "required": true},
}, },
}, },
}, },
}, },
FileName: "test.yaml", FileName: "test.yaml",
},
wantErr: "",
},
{
name: "single label",
content: `
name: Name
title: Title
about: About
labels: label1
ref: Ref
body:
- type: markdown
id: id1
attributes:
value: Value of the markdown
`,
want: &api.IssueTemplate{
Name: "Name",
Title: "Title",
About: "About",
Labels: []string{"label1"},
Ref: "Ref",
Fields: []*api.IssueFormField{
{
Type: "markdown",
ID: "id1",
Attributes: map[string]interface{}{
"value": "Value of the markdown",
},
},
},
FileName: "test.yaml",
},
wantErr: "",
},
{
name: "comma-delimited labels",
content: `
name: Name
title: Title
about: About
labels: label1,label2,,label3 ,,
ref: Ref
body:
- type: markdown
id: id1
attributes:
value: Value of the markdown
`,
want: &api.IssueTemplate{
Name: "Name",
Title: "Title",
About: "About",
Labels: []string{"label1", "label2", "label3"},
Ref: "Ref",
Fields: []*api.IssueFormField{
{
Type: "markdown",
ID: "id1",
Attributes: map[string]interface{}{
"value": "Value of the markdown",
},
},
},
FileName: "test.yaml",
},
wantErr: "",
},
{
name: "empty string as labels",
content: `
name: Name
title: Title
about: About
labels: ''
ref: Ref
body:
- type: markdown
id: id1
attributes:
value: Value of the markdown
`,
want: &api.IssueTemplate{
Name: "Name",
Title: "Title",
About: "About",
Labels: nil,
Ref: "Ref",
Fields: []*api.IssueFormField{
{
Type: "markdown",
ID: "id1",
Attributes: map[string]interface{}{
"value": "Value of the markdown",
},
},
},
FileName: "test.yaml",
},
wantErr: "",
},
{
name: "comma delimited labels in markdown",
filename: "test.md",
content: `---
name: Name
title: Title
about: About
labels: label1,label2,,label3 ,,
ref: Ref
---
Content
`,
want: &api.IssueTemplate{
Name: "Name",
Title: "Title",
About: "About",
Labels: []string{"label1", "label2", "label3"},
Ref: "Ref",
Fields: nil,
Content: "Content\n",
FileName: "test.md",
},
wantErr: "",
},
} }
got, err := unmarshal("test.yaml", []byte(content)) for _, tt := range tests {
if err != nil { t.Run(tt.name, func(t *testing.T) {
t.Fatal(err) filename := "test.yaml"
if tt.filename != "" {
filename = tt.filename
} }
if err := Validate(got); err != nil { tmpl, err := unmarshal(filename, []byte(tt.content))
t.Errorf("Validate() error = %v", err) require.NoError(t, err)
} if tt.wantErr != "" {
if !reflect.DeepEqual(want, got) { require.EqualError(t, Validate(tmpl), tt.wantErr)
jsonWant, _ := json.Marshal(want) } else {
jsonGot, _ := json.Marshal(got) require.NoError(t, Validate(tmpl))
t.Errorf("want:\n%s\ngot:\n%s", jsonWant, jsonGot) want, _ := json.Marshal(tt.want)
got, _ := json.Marshal(tmpl)
require.JSONEq(t, string(want), string(got))
} }
}) })
}
} }
func TestRenderToMarkdown(t *testing.T) { func TestRenderToMarkdown(t *testing.T) {

View file

@ -16,7 +16,7 @@ import (
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"gopkg.in/yaml.v2" "gopkg.in/yaml.v3"
) )
// CouldBe indicates a file with the filename could be a template, // CouldBe indicates a file with the filename could be a template,

View file

@ -9,82 +9,86 @@ import (
"strings" "strings"
"testing" "testing"
"code.gitea.io/gitea/modules/structs"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
func validateMetadata(it structs.IssueTemplate) bool { /*
/* IssueTemplate is a legacy to keep the unit tests working.
A legacy to keep the unit tests working. Copied from structs.IssueTemplate, the original type has been changed a lot to support yaml template.
Copied from the method "func (it IssueTemplate) Valid() bool", the original method has been removed. */
Because it becomes quite complicated to validate an issue template which is support yaml form now. type IssueTemplate struct {
The new way to validate an issue template is to call the Validate in modules/issue/template, Name string `json:"name" yaml:"name"`
*/ Title string `json:"title" yaml:"title"`
About string `json:"about" yaml:"about"`
Labels []string `json:"labels" yaml:"labels"`
Ref string `json:"ref" yaml:"ref"`
}
func (it *IssueTemplate) Valid() bool {
return strings.TrimSpace(it.Name) != "" && strings.TrimSpace(it.About) != "" return strings.TrimSpace(it.Name) != "" && strings.TrimSpace(it.About) != ""
} }
func TestExtractMetadata(t *testing.T) { func TestExtractMetadata(t *testing.T) {
t.Run("ValidFrontAndBody", func(t *testing.T) { t.Run("ValidFrontAndBody", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
body, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s\n%s", sepTest, frontTest, sepTest, bodyTest), &meta) body, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s\n%s", sepTest, frontTest, sepTest, bodyTest), &meta)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, bodyTest, body) assert.Equal(t, bodyTest, body)
assert.Equal(t, metaTest, meta) assert.Equal(t, metaTest, meta)
assert.True(t, validateMetadata(meta)) assert.True(t, meta.Valid())
}) })
t.Run("NoFirstSeparator", func(t *testing.T) { t.Run("NoFirstSeparator", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
_, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", frontTest, sepTest, bodyTest), &meta) _, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", frontTest, sepTest, bodyTest), &meta)
assert.Error(t, err) assert.Error(t, err)
}) })
t.Run("NoLastSeparator", func(t *testing.T) { t.Run("NoLastSeparator", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
_, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, bodyTest), &meta) _, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, bodyTest), &meta)
assert.Error(t, err) assert.Error(t, err)
}) })
t.Run("NoBody", func(t *testing.T) { t.Run("NoBody", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
body, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, sepTest), &meta) body, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, sepTest), &meta)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, "", body) assert.Equal(t, "", body)
assert.Equal(t, metaTest, meta) assert.Equal(t, metaTest, meta)
assert.True(t, validateMetadata(meta)) assert.True(t, meta.Valid())
}) })
} }
func TestExtractMetadataBytes(t *testing.T) { func TestExtractMetadataBytes(t *testing.T) {
t.Run("ValidFrontAndBody", func(t *testing.T) { t.Run("ValidFrontAndBody", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
body, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s\n%s", sepTest, frontTest, sepTest, bodyTest)), &meta) body, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s\n%s", sepTest, frontTest, sepTest, bodyTest)), &meta)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, bodyTest, string(body)) assert.Equal(t, bodyTest, string(body))
assert.Equal(t, metaTest, meta) assert.Equal(t, metaTest, meta)
assert.True(t, validateMetadata(meta)) assert.True(t, meta.Valid())
}) })
t.Run("NoFirstSeparator", func(t *testing.T) { t.Run("NoFirstSeparator", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
_, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", frontTest, sepTest, bodyTest)), &meta) _, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", frontTest, sepTest, bodyTest)), &meta)
assert.Error(t, err) assert.Error(t, err)
}) })
t.Run("NoLastSeparator", func(t *testing.T) { t.Run("NoLastSeparator", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
_, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, bodyTest)), &meta) _, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, bodyTest)), &meta)
assert.Error(t, err) assert.Error(t, err)
}) })
t.Run("NoBody", func(t *testing.T) { t.Run("NoBody", func(t *testing.T) {
var meta structs.IssueTemplate var meta IssueTemplate
body, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, sepTest)), &meta) body, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, sepTest)), &meta)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, "", string(body)) assert.Equal(t, "", string(body))
assert.Equal(t, metaTest, meta) assert.Equal(t, metaTest, meta)
assert.True(t, validateMetadata(meta)) assert.True(t, meta.Valid())
}) })
} }
@ -97,7 +101,7 @@ labels:
- bug - bug
- "test label"` - "test label"`
bodyTest = "This is the body" bodyTest = "This is the body"
metaTest = structs.IssueTemplate{ metaTest = IssueTemplate{
Name: "Test", Name: "Test",
About: "A Test", About: "A Test",
Title: "Test Title", Title: "Test Title",

View file

@ -5,8 +5,12 @@
package structs package structs
import ( import (
"fmt"
"path" "path"
"strings"
"time" "time"
"gopkg.in/yaml.v3"
) )
// StateType issue state type // StateType issue state type
@ -146,13 +150,46 @@ type IssueTemplate struct {
Name string `json:"name" yaml:"name"` Name string `json:"name" yaml:"name"`
Title string `json:"title" yaml:"title"` Title string `json:"title" yaml:"title"`
About string `json:"about" yaml:"about"` // Using "description" in a template file is compatible About string `json:"about" yaml:"about"` // Using "description" in a template file is compatible
Labels []string `json:"labels" yaml:"labels"` Labels IssueTemplateLabels `json:"labels" yaml:"labels"`
Ref string `json:"ref" yaml:"ref"` Ref string `json:"ref" yaml:"ref"`
Content string `json:"content" yaml:"-"` Content string `json:"content" yaml:"-"`
Fields []*IssueFormField `json:"body" yaml:"body"` Fields []*IssueFormField `json:"body" yaml:"body"`
FileName string `json:"file_name" yaml:"-"` FileName string `json:"file_name" yaml:"-"`
} }
type IssueTemplateLabels []string
func (l *IssueTemplateLabels) UnmarshalYAML(value *yaml.Node) error {
var labels []string
if value.IsZero() {
*l = labels
return nil
}
switch value.Kind {
case yaml.ScalarNode:
str := ""
err := value.Decode(&str)
if err != nil {
return err
}
for _, v := range strings.Split(str, ",") {
if v = strings.TrimSpace(v); v == "" {
continue
}
labels = append(labels, v)
}
*l = labels
return nil
case yaml.SequenceNode:
if err := value.Decode(&labels); err != nil {
return err
}
*l = labels
return nil
}
return fmt.Errorf("line %d: cannot unmarshal %s into IssueTemplateLabels", value.Line, value.ShortTag())
}
// IssueTemplateType defines issue template type // IssueTemplateType defines issue template type
type IssueTemplateType string type IssueTemplateType string

View file

@ -8,6 +8,7 @@ import (
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
) )
func TestIssueTemplate_Type(t *testing.T) { func TestIssueTemplate_Type(t *testing.T) {
@ -41,3 +42,65 @@ func TestIssueTemplate_Type(t *testing.T) {
}) })
} }
} }
func TestIssueTemplateLabels_UnmarshalYAML(t *testing.T) {
tests := []struct {
name string
content string
tmpl *IssueTemplate
want *IssueTemplate
wantErr string
}{
{
name: "array",
content: `labels: ["a", "b", "c"]`,
tmpl: &IssueTemplate{
Labels: []string{"should_be_overwrote"},
},
want: &IssueTemplate{
Labels: []string{"a", "b", "c"},
},
},
{
name: "string",
content: `labels: "a,b,c"`,
tmpl: &IssueTemplate{
Labels: []string{"should_be_overwrote"},
},
want: &IssueTemplate{
Labels: []string{"a", "b", "c"},
},
},
{
name: "empty",
content: `labels:`,
tmpl: &IssueTemplate{
Labels: []string{"should_be_overwrote"},
},
want: &IssueTemplate{
Labels: nil,
},
},
{
name: "error",
content: `
labels:
a: aa
b: bb
`,
tmpl: &IssueTemplate{},
wantErr: "line 3: cannot unmarshal !!map into IssueTemplateLabels",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := yaml.Unmarshal([]byte(tt.content), tt.tmpl)
if tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.want, tt.tmpl)
}
})
}
}

View file

@ -16818,11 +16818,7 @@
"x-go-name": "FileName" "x-go-name": "FileName"
}, },
"labels": { "labels": {
"type": "array", "$ref": "#/definitions/IssueTemplateLabels"
"items": {
"type": "string"
},
"x-go-name": "Labels"
}, },
"name": { "name": {
"type": "string", "type": "string",
@ -16839,6 +16835,13 @@
}, },
"x-go-package": "code.gitea.io/gitea/modules/structs" "x-go-package": "code.gitea.io/gitea/modules/structs"
}, },
"IssueTemplateLabels": {
"type": "array",
"items": {
"type": "string"
},
"x-go-package": "code.gitea.io/gitea/modules/structs"
},
"Label": { "Label": {
"description": "Label a label to an issue or a pr", "description": "Label a label to an issue or a pr",
"type": "object", "type": "object",