Move --type into the search Qualifiers
The --type filter was concatenated into ImmutableKeywords in SearchQueryBuild, which bypassed search.Query's quoting and let values that contained quotes (or extra qualifiers) corrupt the final query. Per babakks's suggestion in the review thread, model it as a regular qualifier instead. Add an IssueType field to Qualifiers tagged `qualifier:"type"` so it shares a key with the existing Type field, and rework Qualifiers.Map to honour the tag and concatenate values when multiple fields share the same key. SearchQueryBuild now drops its bespoke type:X formatting and just sets Qualifiers.IssueType, leaving the keyword/qualifier escaping to pkg/search. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
6bbe6e5bac
commit
72859237e2
5 changed files with 56 additions and 27 deletions
|
|
@ -561,7 +561,7 @@ func Test_issueList(t *testing.T) {
|
|||
"owner": "OWNER",
|
||||
"repo": "REPO",
|
||||
"limit": float64(30),
|
||||
"query": "( type:Bug ) repo:OWNER/REPO state:open type:issue",
|
||||
"query": "repo:OWNER/REPO state:open type:Bug type:issue",
|
||||
"type": "ISSUE_ADVANCED",
|
||||
}, params)
|
||||
}))
|
||||
|
|
|
|||
|
|
@ -241,18 +241,6 @@ func SearchQueryBuild(options FilterOptions, advancedIssueSearchSyntax bool) str
|
|||
is = "merged"
|
||||
}
|
||||
|
||||
searchTerms := options.Search
|
||||
if options.IssueType != "" {
|
||||
if searchTerms != "" {
|
||||
searchTerms += " "
|
||||
}
|
||||
if strings.Contains(options.IssueType, " ") {
|
||||
searchTerms += fmt.Sprintf(`type:"%s"`, options.IssueType)
|
||||
} else {
|
||||
searchTerms += "type:" + options.IssueType
|
||||
}
|
||||
}
|
||||
|
||||
query := search.Query{
|
||||
Qualifiers: search.Qualifiers{
|
||||
Assignee: options.Assignee,
|
||||
|
|
@ -260,6 +248,7 @@ func SearchQueryBuild(options FilterOptions, advancedIssueSearchSyntax bool) str
|
|||
Base: options.BaseBranch,
|
||||
Draft: options.Draft,
|
||||
Head: options.HeadBranch,
|
||||
IssueType: options.IssueType,
|
||||
Label: options.Labels,
|
||||
Mentions: options.Mention,
|
||||
Milestone: options.Milestone,
|
||||
|
|
@ -268,7 +257,7 @@ func SearchQueryBuild(options FilterOptions, advancedIssueSearchSyntax bool) str
|
|||
Is: []string{is},
|
||||
Type: options.Entity,
|
||||
},
|
||||
ImmutableKeywords: searchTerms,
|
||||
ImmutableKeywords: options.Search,
|
||||
}
|
||||
|
||||
if !advancedIssueSearchSyntax {
|
||||
|
|
|
|||
|
|
@ -173,6 +173,32 @@ func Test_listURLWithQuery(t *testing.T) {
|
|||
want: "https://example.com/path?q=label%3A%22help+wanted%22+label%3Adocs+milestone%3A%22Codename+%5C%22What+Was+Missing%5C%22%22+state%3Aopen+type%3Apr",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "issue type",
|
||||
args: args{
|
||||
listURL: "https://example.com/path",
|
||||
options: FilterOptions{
|
||||
Entity: "issue",
|
||||
State: "open",
|
||||
IssueType: "Bug",
|
||||
},
|
||||
},
|
||||
want: "https://example.com/path?q=state%3Aopen+type%3ABug+type%3Aissue",
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "issue type with spaces is quoted",
|
||||
args: args{
|
||||
listURL: "https://example.com/path",
|
||||
options: FilterOptions{
|
||||
Entity: "issue",
|
||||
State: "open",
|
||||
IssueType: `Hot "Spicy" Bug`,
|
||||
},
|
||||
},
|
||||
want: "https://example.com/path?q=state%3Aopen+type%3A%22Hot+%5C%22Spicy%5C%22+Bug%22+type%3Aissue",
|
||||
wantErr: false,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -96,6 +96,7 @@ type Qualifiers struct {
|
|||
Topics string
|
||||
Tree string
|
||||
Type string
|
||||
IssueType string `qualifier:"type"`
|
||||
Updated string
|
||||
User []string
|
||||
}
|
||||
|
|
@ -234,39 +235,42 @@ func groupWithOR(qualifier string, vs []string) string {
|
|||
return fmt.Sprintf("(%s)", strings.Join(all, " OR "))
|
||||
}
|
||||
|
||||
// Map turns the qualifiers into a slice-keyed map ready for query
|
||||
// formatting. Multiple struct fields can share the same key when
|
||||
// tagged with `qualifier:"<name>"`; in that case their values are
|
||||
// concatenated under the shared key.
|
||||
func (q Qualifiers) Map() map[string][]string {
|
||||
m := map[string][]string{}
|
||||
v := reflect.ValueOf(q)
|
||||
t := reflect.TypeOf(q)
|
||||
for i := 0; i < v.NumField(); i++ {
|
||||
fieldName := t.Field(i).Name
|
||||
key := camelToKebab(fieldName)
|
||||
typ := v.FieldByName(fieldName).Kind()
|
||||
value := v.FieldByName(fieldName)
|
||||
switch typ {
|
||||
field := t.Field(i)
|
||||
key := field.Tag.Get("qualifier")
|
||||
if key == "" {
|
||||
key = camelToKebab(field.Name)
|
||||
}
|
||||
value := v.Field(i)
|
||||
switch value.Kind() {
|
||||
case reflect.Ptr:
|
||||
if value.IsNil() {
|
||||
continue
|
||||
}
|
||||
v := reflect.Indirect(value)
|
||||
m[key] = []string{fmt.Sprintf("%v", v)}
|
||||
m[key] = append(m[key], fmt.Sprintf("%v", reflect.Indirect(value)))
|
||||
case reflect.Slice:
|
||||
if value.IsNil() {
|
||||
continue
|
||||
}
|
||||
s := []string{}
|
||||
for i := 0; i < value.Len(); i++ {
|
||||
if value.Index(i).IsZero() {
|
||||
for j := 0; j < value.Len(); j++ {
|
||||
if value.Index(j).IsZero() {
|
||||
continue
|
||||
}
|
||||
s = append(s, fmt.Sprintf("%v", value.Index(i)))
|
||||
m[key] = append(m[key], fmt.Sprintf("%v", value.Index(j)))
|
||||
}
|
||||
m[key] = s
|
||||
default:
|
||||
if value.IsZero() {
|
||||
continue
|
||||
}
|
||||
m[key] = []string{fmt.Sprintf("%v", value)}
|
||||
m[key] = append(m[key], fmt.Sprintf("%v", value))
|
||||
}
|
||||
}
|
||||
return m
|
||||
|
|
|
|||
|
|
@ -308,6 +308,16 @@ func TestQualifiersMap(t *testing.T) {
|
|||
"user": {"user"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "concatenates fields that share a qualifier key",
|
||||
qualifiers: Qualifiers{
|
||||
Type: "issue",
|
||||
IssueType: "Bug",
|
||||
},
|
||||
out: map[string][]string{
|
||||
"type": {"issue", "Bug"},
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue