diff --git a/internal/featuredetection/detector_mock.go b/internal/featuredetection/detector_mock.go index 6f760f209..2d41c707f 100644 --- a/internal/featuredetection/detector_mock.go +++ b/internal/featuredetection/detector_mock.go @@ -20,6 +20,10 @@ func (md *DisabledDetectorMock) ProjectsV1() gh.ProjectsV1Support { return gh.ProjectsV1Unsupported } +func (md *DisabledDetectorMock) SearchFeatures() (SearchFeatures, error) { + return advancedIssueSearchNotSupported, nil +} + type EnabledDetectorMock struct{} func (md *EnabledDetectorMock) IssueFeatures() (IssueFeatures, error) { @@ -37,3 +41,34 @@ func (md *EnabledDetectorMock) RepositoryFeatures() (RepositoryFeatures, error) func (md *EnabledDetectorMock) ProjectsV1() gh.ProjectsV1Support { return gh.ProjectsV1Supported } + +func (md *EnabledDetectorMock) SearchFeatures() (SearchFeatures, error) { + return advancedIssueSearchNotSupported, nil +} + +type AdvancedIssueSearchDetectorMock struct { + EnabledDetectorMock + searchFeatures SearchFeatures +} + +func (md *AdvancedIssueSearchDetectorMock) SearchFeatures() (SearchFeatures, error) { + return md.searchFeatures, nil +} + +func AdvancedIssueSearchUnsupported() *AdvancedIssueSearchDetectorMock { + return &AdvancedIssueSearchDetectorMock{ + searchFeatures: advancedIssueSearchNotSupported, + } +} + +func AdvancedIssueSearchSupportedAsOptIn() *AdvancedIssueSearchDetectorMock { + return &AdvancedIssueSearchDetectorMock{ + searchFeatures: advancedIssueSearchSupportedAsOptIn, + } +} + +func AdvancedIssueSearchSupportedAsOnlyBackend() *AdvancedIssueSearchDetectorMock { + return &AdvancedIssueSearchDetectorMock{ + searchFeatures: advancedIssueSearchSupportedAsOnlyBackend, + } +} diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index c61f47aeb..786093d93 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -16,6 +16,7 @@ type Detector interface { PullRequestFeatures() (PullRequestFeatures, error) RepositoryFeatures() (RepositoryFeatures, error) ProjectsV1() gh.ProjectsV1Support + SearchFeatures() (SearchFeatures, error) } type IssueFeatures struct { @@ -55,6 +56,43 @@ var allRepositoryFeatures = RepositoryFeatures{ AutoMerge: true, } +type SearchFeatures struct { + // AdvancedIssueSearch indicates whether the host supports advanced issue + // search via API calls. + AdvancedIssueSearchAPI bool + // AdvancedIssueSearchOptIn indicates whether the host supports advanced + // issue search as an opt-in feature, which has to be explicitly enabled in + // API calls. + AdvancedIssueSearchAPIOptIn bool + + // TODO advancedSearchFuture + // When advanced issue search is supported in Pull Requests tab, or in + // global search we can introduce more fields to reflect the support status. +} + +// advancedIssueSearchNotSupported mimics GHE <3.18 where advanced issue search +// is either not supported or is not meant to be used due to not being stable +// enough (i.e. in preview). +var advancedIssueSearchNotSupported = SearchFeatures{ + AdvancedIssueSearchAPI: false, +} + +// advancedIssueSearchSupportedAsOptIn mimics github.com and GHE >=3.18 before +// the full cleanup of temp types (i.e. ISSUE_ADVANCED search type is still +// present on the schema). +var advancedIssueSearchSupportedAsOptIn = SearchFeatures{ + AdvancedIssueSearchAPI: true, + AdvancedIssueSearchAPIOptIn: true, +} + +// advancedIssueSearchSupportedAsOnlyBackend mimics github.com and GHE >=3.18 +// after the full cleanup of temp types (i.e. ISSUE_ADVANCED search type is +// removed from the schema). +var advancedIssueSearchSupportedAsOnlyBackend = SearchFeatures{ + AdvancedIssueSearchAPI: true, + AdvancedIssueSearchAPIOptIn: false, +} + type detector struct { host string httpClient *http.Client @@ -225,6 +263,101 @@ func (d *detector) ProjectsV1() gh.ProjectsV1Support { return gh.ProjectsV1Unsupported } +const ( + // enterpriseAdvancedIssueSearchSupport is the minimum version of GHES that + // supports advanced issue search and gh should use it. + // + // Note that advanced issue search is also available on GHES 3.17, but it's + // at the preview stage and is not as mature as it is on github.com or later + // GHES version. + enterpriseAdvancedIssueSearchSupport = "3.18.0" +) + +func (d *detector) SearchFeatures() (SearchFeatures, error) { + // TODO advancedIssueSearchCleanup + // Once GHES 3.17 support ends, we don't need this and, probably, the entire search feature detection. + + // Regarding the release of advanced issue search (AIS, for short), there + // are three time spans/periods: + // + // 1. Pre-deprecation: where both legacy search and AIS are available + // - GraphQL: `ISSUE` and `ISSUE_ADVANCED` search types in GraphQL behave differently + // - REST: `advance_search=true` query parameter can be used to switch to AIS + // 2. Deprecation: only AIS available + // - GraphQL: `ISSUE` and `ISSUE_ADVANCED` search types in GraphQL behave the same (AIS) + // - REST: `advance_search` query parameter has no effect (AIS) + // 3. Cleanup: only AIS available + // - GraphQL: `ISSUE` search type in GraphQL is the only available option (AIS) + // - REST: `advance_search` query parameter has no effect (AIS) + // + // Since there's no schema-wise difference between pre-deprecation and + // deprecation periods (i.e. `ISSUE_ADVANCED` is available during both), + // we cannot figure out the exact time period. The consensus is to to use + // the advanced search syntax during both periods. + + var feature SearchFeatures + + if ghauth.IsEnterprise(d.host) { + enterpriseAISSupportVersion, err := version.NewVersion(enterpriseAdvancedIssueSearchSupport) + if err != nil { + return SearchFeatures{}, err + } + + hostVersion, err := resolveEnterpriseVersion(d.httpClient, d.host) + if err != nil { + return SearchFeatures{}, err + } + + if hostVersion.GreaterThanOrEqual(enterpriseAISSupportVersion) { + // As of August 2025, advanced issue search is going to be available + // on GHES 3.18+, including Issues tabs in repositories. + feature.AdvancedIssueSearchAPI = true + + // TODO advancedSearchFuture + // When the advanced search syntax is supported in global search or + // Pull Requests tabs (in repositories), we can add and enable the + // corresponding fields. + } + } else { + // As of August 2025, advanced issue search is available on github.com, + // including Issues tabs in repositories. + feature.AdvancedIssueSearchAPI = true + + // TODO advancedSearchFuture + // When the advanced search syntax is supported in global search or + // Pull Requests tabs (in repositories), we can add and enable the + // corresponding fields. + } + + if !feature.AdvancedIssueSearchAPI { + return feature, nil + } + + var searchTypeFeatureDetection struct { + SearchType struct { + EnumValues []struct { + Name string + } `graphql:"enumValues(includeDeprecated: true)"` + } `graphql:"SearchType: __type(name: \"SearchType\")"` + } + + gql := api.NewClientFromHTTP(d.httpClient) + if err := gql.Query(d.host, "SearchType_enumValues", &searchTypeFeatureDetection, nil); err != nil { + return SearchFeatures{}, err + } + + for _, enumValue := range searchTypeFeatureDetection.SearchType.EnumValues { + if enumValue.Name == "ISSUE_ADVANCED" { + // As long as ISSUE_ADVANCED is present on the schema, we should + // explicitly opt-in when making API calls. + feature.AdvancedIssueSearchAPIOptIn = true + break + } + } + + return feature, nil +} + func resolveEnterpriseVersion(httpClient *http.Client, host string) (*version.Version, error) { var metaResponse struct { InstalledVersion string `json:"installed_version"` diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index e850546a7..18a846268 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -439,3 +439,149 @@ func TestProjectV1Support(t *testing.T) { }) } } + +func TestAdvancedIssueSearchSupport(t *testing.T) { + withIssueAdvanced := `{"data":{"SearchType":{"enumValues":[{"name":"ISSUE"},{"name":"ISSUE_ADVANCED"},{"name":"REPOSITORY"},{"name":"USER"},{"name":"DISCUSSION"}]}}}` + withoutIssueAdvanced := `{"data":{"SearchType":{"enumValues":[{"name":"ISSUE"},{"name":"REPOSITORY"},{"name":"USER"},{"name":"DISCUSSION"}]}}}` + + tests := []struct { + name string + hostname string + httpStubs func(*httpmock.Registry) + wantFeatures SearchFeatures + }{ + { + name: "github.com, before ISSUE_ADVANCED cleanup", + hostname: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOptIn, + }, + { + name: "github.com, after ISSUE_ADVANCED cleanup", + hostname: "github.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withoutIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOnlyBackend, + }, + { + name: "ghec data residency (ghe.com), before ISSUE_ADVANCED cleanup", + hostname: "stampname.ghe.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOptIn, + }, + { + name: "ghec data residency (ghe.com), after ISSUE_ADVANCED cleanup", + hostname: "stampname.ghe.com", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withoutIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOnlyBackend, + }, + { + name: "GHE 3.18, before ISSUE_ADVANCED cleanup", + hostname: "git.my.org", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "api/v3/meta"), + httpmock.StringResponse(`{"installed_version":"3.18.0"}`), + ) + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOptIn, + }, + { + name: "GHE 3.18, after ISSUE_ADVANCED cleanup", + hostname: "git.my.org", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "api/v3/meta"), + httpmock.StringResponse(`{"installed_version":"3.18.0"}`), + ) + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withoutIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOnlyBackend, + }, + { + name: "GHE >3.18, before ISSUE_ADVANCED cleanup", + hostname: "git.my.org", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "api/v3/meta"), + httpmock.StringResponse(`{"installed_version":"3.18.1"}`), + ) + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOptIn, + }, + { + name: "GHE >3.18, after ISSUE_ADVANCED cleanup", + hostname: "git.my.org", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "api/v3/meta"), + httpmock.StringResponse(`{"installed_version":"3.18.1"}`), + ) + reg.Register( + httpmock.GraphQL(`query SearchType_enumValues\b`), + httpmock.StringResponse(withoutIssueAdvanced), + ) + }, + wantFeatures: advancedIssueSearchSupportedAsOnlyBackend, + }, + { + name: "GHE <3.18 (no advanced issue search support)", + hostname: "git.my.org", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "api/v3/meta"), + httpmock.StringResponse(`{"installed_version":"3.17.999"}`), + ) + }, + wantFeatures: advancedIssueSearchNotSupported, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + httpClient := &http.Client{} + httpmock.ReplaceTripper(httpClient, reg) + + detector := NewDetector(httpClient, tt.hostname) + + features, err := detector.SearchFeatures() + require.NoError(t, err) + require.Equal(t, tt.wantFeatures, features) + }) + } +} diff --git a/pkg/cmd/extension/browse/browse_test.go b/pkg/cmd/extension/browse/browse_test.go index 956ea0fc4..8120da4d3 100644 --- a/pkg/cmd/extension/browse/browse_test.go +++ b/pkg/cmd/extension/browse/browse_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/cli/cli/v2/internal/config" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/repo/view" @@ -125,7 +126,7 @@ func Test_getExtensionRepos(t *testing.T) { }), ) - searcher := search.NewSearcher(client, "github.com") + searcher := search.NewSearcher(client, "github.com", &fd.DisabledDetectorMock{}) emMock := &extensions.ExtensionManagerMock{} emMock.ListFunc = func() []extensions.Extension { return []extensions.Extension{ diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 354a91cc3..057f91140 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -12,6 +12,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" @@ -164,7 +165,8 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { query.Qualifiers = qualifiers host, _ := cfg.Authentication().DefaultHost() - searcher := search.NewSearcher(client, host) + detector := featuredetection.NewDetector(client, host) + searcher := search.NewSearcher(client, host, detector) if webMode { url := searcher.URL(query) @@ -507,7 +509,8 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return err } - searcher := search.NewSearcher(api.NewCachedHTTPClient(client, time.Hour*24), host) + detector := featuredetection.NewDetector(client, host) + searcher := search.NewSearcher(api.NewCachedHTTPClient(client, time.Hour*24), host, detector) gc.Stderr = gio.Discard diff --git a/pkg/cmd/issue/list/http.go b/pkg/cmd/issue/list/http.go index fcbfe7240..0fceb3e97 100644 --- a/pkg/cmd/issue/list/http.go +++ b/pkg/cmd/issue/list/http.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" ) @@ -112,7 +113,16 @@ loop: return &res, nil } -func searchIssues(client *api.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) { +func searchIssues(client *api.Client, detector fd.Detector, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) { + // TODO advancedIssueSearchCleanup + // We won't need feature detection when GHES 3.17 support ends, since + // the advanced issue search is the only available search backend for + // issues. + features, err := detector.SearchFeatures() + if err != nil { + return nil, err + } + fragments := fmt.Sprintf("fragment issue on Issue {%s}", api.IssueGraphQL(filters.Fields)) query := fragments + `query IssueSearch($repo: String!, $owner: String!, $type: SearchType!, $limit: Int, $after: String, $query: String!) { @@ -143,18 +153,27 @@ func searchIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Fi } } - filters.Repo = ghrepo.FullName(repo) - filters.Entity = "issue" - q := prShared.SearchQueryBuild(filters) - perPage := min(limit, 100) variables := map[string]interface{}{ "owner": repo.RepoOwner(), "repo": repo.RepoName(), - "type": "ISSUE", "limit": perPage, - "query": q, + } + + filters.Repo = ghrepo.FullName(repo) + filters.Entity = "issue" + + if features.AdvancedIssueSearchAPI { + variables["query"] = prShared.SearchQueryBuild(filters, true) + if features.AdvancedIssueSearchAPIOptIn { + variables["type"] = "ISSUE_ADVANCED" + } else { + variables["type"] = "ISSUE" + } + } else { + variables["query"] = prShared.SearchQueryBuild(filters, false) + variables["type"] = "ISSUE" } ic := api.IssuesAndTotalCount{SearchCapped: limit > 1000} diff --git a/pkg/cmd/issue/list/http_test.go b/pkg/cmd/issue/list/http_test.go index a929746d1..747bd0a4b 100644 --- a/pkg/cmd/issue/list/http_test.go +++ b/pkg/cmd/issue/list/http_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/httpmock" @@ -165,3 +166,51 @@ func TestIssueList_pagination(t *testing.T) { assert.Equal(t, []string{"enhancement"}, getLabels(res.Issues[1])) assert.Equal(t, []string{"user2"}, getAssignees(res.Issues[1])) } + +// TODO advancedIssueSearchCleanup +// Remove this test once GHES 3.17 support ends. +func TestSearchIssuesAndAdvancedSearch(t *testing.T) { + tests := []struct { + name string + detector fd.Detector + wantSearchType string + }{ + { + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), + wantSearchType: "ISSUE", + }, + { + name: "advanced issue search supported as opt-in", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + wantSearchType: "ISSUE_ADVANCED", + }, + { + name: "advanced issue search supported as only backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), + wantSearchType: "ISSUE", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.GraphQL(`query IssueSearch\b`), + httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { + assert.Equal(t, tt.wantSearchType, vars["type"]) + // Since no repeated usage of special search qualifiers is possible + // with our current implementation, we can assert against the same + // query for both search backend (i.e. legacy and advanced issue search). + assert.Equal(t, "repo:OWNER/REPO state:open type:issue", vars["query"]) + })) + + httpClient := &http.Client{Transport: reg} + client := api.NewClientFromHTTP(httpClient) + + searchIssues(client, tt.detector, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{State: "open"}, 30) + }) + } +} diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index 46c0e2cb0..9a6495ec2 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -58,12 +58,18 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "list", Short: "List issues in a repository", - Long: heredoc.Doc(` + // TODO advancedIssueSearchCleanup + // Update the links and remove the mention at GHES 3.17 version. + Long: heredoc.Docf(` List issues in a GitHub repository. By default, this only lists open issues. The search query syntax is documented here: - `), + + On supported GitHub hosts, advanced issue search syntax can be used in the + %[1]s--search%[1]s query. For more information about advanced issue search, see: + + `, "`"), Example: heredoc.Doc(` $ gh issue list --label "bug" --label "help wanted" $ gh issue list --author monalisa @@ -165,8 +171,21 @@ func listRun(opts *ListOptions) error { isTerminal := opts.IO.IsStdoutTTY() if opts.WebMode { + // TODO advancedIssueSearchCleanup + // We won't need feature detection when GHES 3.17 support ends, since + // the advanced issue search is the only available search backend for + // issues, and the GUI (i.e. Issues tab of repos) already supports the + // advanced syntax. + searchFeatures, err := opts.Detector.SearchFeatures() + if err != nil { + return err + } + issueListURL := ghrepo.GenerateRepoURL(baseRepo, "issues") - openURL, err := prShared.ListURLWithQuery(issueListURL, filterOptions) + + // Note that if the advanced issue search API is available, the syntax is + // also supported in the Issues tab. + openURL, err := prShared.ListURLWithQuery(issueListURL, filterOptions, searchFeatures.AdvancedIssueSearchAPI) if err != nil { return err } @@ -181,7 +200,7 @@ func listRun(opts *ListOptions) error { filterOptions.Fields = opts.Exporter.Fields() } - listResult, err := issueList(httpClient, baseRepo, filterOptions, opts.LimitResults) + listResult, err := issueList(httpClient, opts.Detector, baseRepo, filterOptions, opts.LimitResults) if err != nil { return err } @@ -212,7 +231,7 @@ func listRun(opts *ListOptions) error { return nil } -func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) { +func issueList(client *http.Client, detector fd.Detector, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) { apiClient := api.NewClientFromHTTP(client) if filters.Search != "" || len(filters.Labels) > 0 || filters.Milestone != "" { @@ -224,7 +243,7 @@ func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.Filt filters.Milestone = milestone.Title } - return searchIssues(apiClient, repo, filters, limit) + return searchIssues(apiClient, detector, repo, filters, limit) } var err error diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 852f0a46b..83dc313ce 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" @@ -189,50 +190,80 @@ func TestIssueList_disabledIssues(t *testing.T) { } } +// TODO advancedIssueSearchCleanup +// Simplify this test to only a single test case once GHES 3.17 support ends. func TestIssueList_web(t *testing.T) { - ios, _, stdout, stderr := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) - browser := &browser.Stub{} - - reg := &httpmock.Registry{} - defer reg.Verify(t) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - err := listRun(&ListOptions{ - IO: ios, - Browser: browser, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil + tests := []struct { + name string + detector fd.Detector + }{ + { + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil + { + name: "advanced issue search supported as opt-in", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + }, + { + name: "advanced issue search supported as only backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), }, - WebMode: true, - State: "all", - Assignee: "peter", - Author: "john", - Labels: []string{"bug", "docs"}, - Mention: "frank", - Milestone: "v1.1", - LimitResults: 10, - }) - if err != nil { - t.Errorf("error running command `issue list` with `--web` flag: %v", err) } - assert.Equal(t, "", stdout.String()) - assert.Equal(t, "Opening https://github.com/OWNER/REPO/issues in your browser.\n", stderr.String()) - browser.Verify(t, "https://github.com/OWNER/REPO/issues?q=assignee%3Apeter+author%3Ajohn+label%3Abug+label%3Adocs+mentions%3Afrank+milestone%3Av1.1+type%3Aissue") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + browser := &browser.Stub{} + + reg := &httpmock.Registry{} + defer reg.Verify(t) + + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + opts := &ListOptions{ + IO: ios, + Browser: browser, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Detector: tt.detector, + WebMode: true, + State: "all", + Assignee: "peter", + Author: "john", + Labels: []string{"bug", "docs"}, + Mention: "frank", + Milestone: "v1.1", + LimitResults: 10, + } + + err := listRun(opts) + require.NoError(t, err) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "Opening https://github.com/OWNER/REPO/issues in your browser.\n", stderr.String()) + + // Since no repeated usage of special search qualifiers is possible + // with our current implementation, we can assert against the same + // URL for both search backend (i.e. legacy and advanced issue search). + browser.Verify(t, "https://github.com/OWNER/REPO/issues?q=assignee%3Apeter+author%3Ajohn+label%3Abug+label%3Adocs+mentions%3Afrank+milestone%3Av1.1+type%3Aissue") + }) + } } func Test_issueList(t *testing.T) { type args struct { - repo ghrepo.Interface - filters prShared.FilterOptions - limit int + detector fd.Detector + repo ghrepo.Interface + filters prShared.FilterOptions + limit int } tests := []struct { name string @@ -270,8 +301,11 @@ func Test_issueList(t *testing.T) { { name: "milestone by number", args: args{ - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + limit: 30, + repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ Entity: "issue", State: "open", @@ -301,7 +335,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": "milestone:1.x repo:OWNER/REPO state:open type:issue", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -309,8 +343,11 @@ func Test_issueList(t *testing.T) { { name: "milestone by title", args: args{ - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + limit: 30, + repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ Entity: "issue", State: "open", @@ -333,7 +370,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": "milestone:1.x repo:OWNER/REPO state:open type:issue", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -377,8 +414,11 @@ func Test_issueList(t *testing.T) { { name: "@me with search", args: args{ - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + limit: 30, + repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ Entity: "issue", State: "open", @@ -404,7 +444,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": "auth bug assignee:@me author:@me mentions:@me repo:OWNER/REPO state:open type:issue", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -412,8 +452,11 @@ func Test_issueList(t *testing.T) { { name: "with labels", args: args{ - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + limit: 30, + repo: ghrepo.New("OWNER", "REPO"), filters: prShared.FilterOptions{ Entity: "issue", State: "open", @@ -436,7 +479,7 @@ func Test_issueList(t *testing.T) { "repo": "REPO", "limit": float64(30), "query": `label:"one world" label:hello repo:OWNER/REPO state:open type:issue`, - "type": "ISSUE", + "type": "ISSUE_ADVANCED", }, params) })) }, @@ -450,7 +493,7 @@ func Test_issueList(t *testing.T) { tt.httpStubs(httpreg) } client := &http.Client{Transport: httpreg} - _, err := issueList(client, tt.args.repo, tt.args.filters, tt.args.limit) + _, err := issueList(client, tt.args.detector, tt.args.repo, tt.args.filters, tt.args.limit) if tt.wantErr { assert.Error(t, err) } else { @@ -507,6 +550,7 @@ func TestIssueList_withProjectItems(t *testing.T) { client := &http.Client{Transport: reg} issuesAndTotalCount, err := issueList( client, + nil, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "issue", @@ -572,7 +616,7 @@ func TestIssueList_Search_withProjectItems(t *testing.T) { require.Equal(t, map[string]interface{}{ "owner": "OWNER", "repo": "REPO", - "type": "ISSUE", + "type": "ISSUE_ADVANCED", "limit": float64(30), "query": "just used to force the search API branch repo:OWNER/REPO type:issue", }, params) @@ -581,6 +625,9 @@ func TestIssueList_Search_withProjectItems(t *testing.T) { client := &http.Client{Transport: reg} issuesAndTotalCount, err := issueList( client, + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + fd.AdvancedIssueSearchSupportedAsOptIn(), ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "issue", diff --git a/pkg/cmd/pr/list/http.go b/pkg/cmd/pr/list/http.go index 4c69af708..4f82d5006 100644 --- a/pkg/cmd/pr/list/http.go +++ b/pkg/cmd/pr/list/http.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" ) @@ -13,9 +14,9 @@ func shouldUseSearch(filters prShared.FilterOptions) bool { return filters.Draft != nil || filters.Author != "" || filters.Assignee != "" || filters.Search != "" || len(filters.Labels) > 0 } -func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) { +func listPullRequests(httpClient *http.Client, detector fd.Detector, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) { if shouldUseSearch(filters) { - return searchPullRequests(httpClient, repo, filters, limit) + return searchPullRequests(httpClient, detector, repo, filters, limit) } return prShared.NewLister(httpClient).List(prShared.ListOptions{ @@ -28,7 +29,16 @@ func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters pr }) } -func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) { +func searchPullRequests(httpClient *http.Client, detector fd.Detector, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) { + // TODO advancedIssueSearchCleanup + // We won't need feature detection when GHES 3.17 support ends, since + // the advanced issue search is the only available search backend for + // issues. + features, err := detector.SearchFeatures() + if err != nil { + return nil, err + } + type response struct { Search struct { Nodes []api.PullRequest @@ -44,10 +54,11 @@ func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters query := fragment + ` query PullRequestSearch( $q: String!, + $type: SearchType!, $limit: Int!, $endCursor: String, ) { - search(query: $q, type: ISSUE, first: $limit, after: $endCursor) { + search(query: $q, type: $type, first: $limit, after: $endCursor) { issueCount nodes { ...pr @@ -59,12 +70,24 @@ func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters } }` + variables := map[string]interface{}{} + filters.Repo = ghrepo.FullName(repo) filters.Entity = "pr" - q := prShared.SearchQueryBuild(filters) + + if features.AdvancedIssueSearchAPI { + variables["q"] = prShared.SearchQueryBuild(filters, true) + if features.AdvancedIssueSearchAPIOptIn { + variables["type"] = "ISSUE_ADVANCED" + } else { + variables["type"] = "ISSUE" + } + } else { + variables["q"] = prShared.SearchQueryBuild(filters, false) + variables["type"] = "ISSUE" + } pageLimit := min(limit, 100) - variables := map[string]interface{}{"q": q} res := api.PullRequestAndTotalCount{SearchCapped: limit > 1000} var check = make(map[int]struct{}) diff --git a/pkg/cmd/pr/list/http_test.go b/pkg/cmd/pr/list/http_test.go index 1aa16ae1d..5f08498f7 100644 --- a/pkg/cmd/pr/list/http_test.go +++ b/pkg/cmd/pr/list/http_test.go @@ -5,16 +5,19 @@ import ( "reflect" "testing" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/httpmock" + "github.com/stretchr/testify/assert" ) func Test_ListPullRequests(t *testing.T) { type args struct { - repo ghrepo.Interface - filters prShared.FilterOptions - limit int + detector fd.Detector + repo ghrepo.Interface + filters prShared.FilterOptions + limit int } tests := []struct { name string @@ -75,8 +78,11 @@ func Test_ListPullRequests(t *testing.T) { { name: "with labels", args: args{ - repo: ghrepo.New("OWNER", "REPO"), - limit: 30, + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + repo: ghrepo.New("OWNER", "REPO"), + limit: 30, filters: prShared.FilterOptions{ State: "open", Labels: []string{"hello", "one world"}, @@ -88,6 +94,7 @@ func Test_ListPullRequests(t *testing.T) { httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { want := map[string]interface{}{ "q": `label:"one world" label:hello repo:OWNER/REPO state:open type:pr`, + "type": "ISSUE_ADVANCED", "limit": float64(30), } if !reflect.DeepEqual(vars, want) { @@ -99,8 +106,11 @@ func Test_ListPullRequests(t *testing.T) { { name: "with author", args: args{ - repo: ghrepo.New("OWNER", "REPO"), - limit: 30, + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + repo: ghrepo.New("OWNER", "REPO"), + limit: 30, filters: prShared.FilterOptions{ State: "open", Author: "monalisa", @@ -112,6 +122,7 @@ func Test_ListPullRequests(t *testing.T) { httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { want := map[string]interface{}{ "q": "author:monalisa repo:OWNER/REPO state:open type:pr", + "type": "ISSUE_ADVANCED", "limit": float64(30), } if !reflect.DeepEqual(vars, want) { @@ -123,8 +134,11 @@ func Test_ListPullRequests(t *testing.T) { { name: "with search", args: args{ - repo: ghrepo.New("OWNER", "REPO"), - limit: 30, + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + repo: ghrepo.New("OWNER", "REPO"), + limit: 30, filters: prShared.FilterOptions{ State: "open", Search: "one world in:title", @@ -136,6 +150,7 @@ func Test_ListPullRequests(t *testing.T) { httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { want := map[string]interface{}{ "q": "one world in:title repo:OWNER/REPO state:open type:pr", + "type": "ISSUE_ADVANCED", "limit": float64(30), } if !reflect.DeepEqual(vars, want) { @@ -153,7 +168,7 @@ func Test_ListPullRequests(t *testing.T) { } httpClient := &http.Client{Transport: reg} - _, err := listPullRequests(httpClient, tt.args.repo, tt.args.filters, tt.args.limit) + _, err := listPullRequests(httpClient, tt.args.detector, tt.args.repo, tt.args.filters, tt.args.limit) if (err != nil) != tt.wantErr { t.Errorf("ListPullRequests() error = %v, wantErr %v", err, tt.wantErr) return @@ -161,3 +176,51 @@ func Test_ListPullRequests(t *testing.T) { }) } } + +// TODO advancedIssueSearchCleanup +// Remove this test once GHES 3.17 support ends. +func TestSearchPullRequestsAndAdvancedSearch(t *testing.T) { + tests := []struct { + name string + detector fd.Detector + wantSearchType string + }{ + { + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), + wantSearchType: "ISSUE", + }, + { + name: "advanced issue search supported as opt-in", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + wantSearchType: "ISSUE_ADVANCED", + }, + { + name: "advanced issue search supported as only backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), + wantSearchType: "ISSUE", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.GraphQL(`query PullRequestSearch\b`), + httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) { + assert.Equal(t, tt.wantSearchType, vars["type"]) + + // Since no repeated usage of special search qualifiers is possible + // with our current implementation, we can assert against the same + // query for both search backend (i.e. legacy and advanced issue search). + assert.Equal(t, "repo:OWNER/REPO state:open type:pr", vars["q"]) + })) + + httpClient := &http.Client{Transport: reg} + + searchPullRequests(httpClient, tt.detector, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{State: "open"}, 30) + }) + } +} diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index 7188df1a5..a7e40384a 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -10,6 +10,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" @@ -24,6 +25,7 @@ type ListOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser + Detector fd.Detector WebMode bool LimitResults int @@ -54,12 +56,18 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "list", Short: "List pull requests in a repository", - Long: heredoc.Doc(` + // TODO advancedIssueSearchCleanup + // Update the links and remove the mention at GHES 3.17 version. + Long: heredoc.Docf(` List pull requests in a GitHub repository. By default, this only lists open PRs. The search query syntax is documented here: - `), + + On supported GitHub hosts, advanced issue search syntax can be used in the + %[1]s--search%[1]s query. For more information about advanced issue search, see: + + `, "`"), Example: heredoc.Doc(` # List PRs authored by you $ gh pr list --author "@me" @@ -142,6 +150,11 @@ func listRun(opts *ListOptions) error { return err } + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) + } + prState := strings.ToLower(opts.State) if prState == "open" && shared.QueryHasStateClause(opts.Search) { prState = "" @@ -164,7 +177,12 @@ func listRun(opts *ListOptions) error { } if opts.WebMode { prListURL := ghrepo.GenerateRepoURL(baseRepo, "pulls") - openURL, err := shared.ListURLWithQuery(prListURL, filters) + + // TODO advancedSearchFuture + // As of August 2025, the advanced issue search syntax is not supported + // in Pull Requests tab of repositories. When it's supported we can + // change the argument to true. + openURL, err := shared.ListURLWithQuery(prListURL, filters, false) if err != nil { return err } @@ -175,7 +193,7 @@ func listRun(opts *ListOptions) error { return opts.Browser.Browse(openURL) } - listResult, err := listPullRequests(httpClient, baseRepo, filters, opts.LimitResults) + listResult, err := listPullRequests(httpClient, opts.Detector, baseRepo, filters, opts.LimitResults) if err != nil { return err } diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index ecd0326b5..4953b8888 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -11,6 +11,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -23,7 +24,7 @@ import ( "github.com/stretchr/testify/require" ) -func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { +func runCommand(rt http.RoundTripper, detector fd.Detector, isTTY bool, cli string) (*test.CmdOut, error) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(isTTY) ios.SetStdinTTY(isTTY) @@ -47,6 +48,7 @@ func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, err cmd := NewCmdList(factory, func(opts *ListOptions) error { opts.Now = fakeNow + opts.Detector = detector return listRun(opts) }) @@ -78,7 +80,7 @@ func TestPRList(t *testing.T) { http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prList.json")) - output, err := runCommand(http, true, "") + output, err := runCommand(http, nil, true, "") if err != nil { t.Fatal(err) } @@ -101,7 +103,7 @@ func TestPRList_nontty(t *testing.T) { http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prList.json")) - output, err := runCommand(http, false, "") + output, err := runCommand(http, nil, false, "") if err != nil { t.Fatal(err) } @@ -124,7 +126,7 @@ func TestPRList_filtering(t *testing.T) { assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{})) })) - output, err := runCommand(http, true, `-s all`) + output, err := runCommand(http, nil, true, `-s all`) assert.Error(t, err) assert.Equal(t, "", output.String()) @@ -139,7 +141,7 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) { httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prListWithDuplicates.json")) - output, err := runCommand(http, true, "") + output, err := runCommand(http, nil, true, "") if err != nil { t.Fatal(err) } @@ -162,7 +164,7 @@ func TestPRList_filteringClosed(t *testing.T) { assert.Equal(t, []interface{}{"CLOSED", "MERGED"}, params["state"].([]interface{})) })) - _, err := runCommand(http, true, `-s closed`) + _, err := runCommand(http, nil, true, `-s closed`) assert.Error(t, err) } @@ -176,7 +178,7 @@ func TestPRList_filteringHeadBranch(t *testing.T) { assert.Equal(t, interface{}("bug-fix"), params["headBranch"]) })) - _, err := runCommand(http, true, `-H bug-fix`) + _, err := runCommand(http, nil, true, `-H bug-fix`) assert.Error(t, err) } @@ -190,7 +192,9 @@ func TestPRList_filteringAssignee(t *testing.T) { assert.Equal(t, `assignee:hubot base:develop is:merged label:"needs tests" repo:OWNER/REPO type:pr`, params["q"].(string)) })) - _, err := runCommand(http, true, `-s merged -l "needs tests" -a hubot -B develop`) + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + _, err := runCommand(http, fd.AdvancedIssueSearchSupportedAsOptIn(), true, `-s merged -l "needs tests" -a hubot -B develop`) assert.Error(t, err) } @@ -223,7 +227,9 @@ func TestPRList_filteringDraft(t *testing.T) { assert.Equal(t, test.expectedQuery, params["q"].(string)) })) - _, err := runCommand(http, true, test.cli) + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + _, err := runCommand(http, fd.AdvancedIssueSearchSupportedAsOptIn(), true, test.cli) assert.Error(t, err) }) } @@ -268,7 +274,9 @@ func TestPRList_filteringAuthor(t *testing.T) { assert.Equal(t, test.expectedQuery, params["q"].(string)) })) - _, err := runCommand(http, true, test.cli) + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + _, err := runCommand(http, fd.AdvancedIssueSearchSupportedAsOptIn(), true, test.cli) assert.Error(t, err) }) } @@ -277,7 +285,7 @@ func TestPRList_filteringAuthor(t *testing.T) { func TestPRList_withInvalidLimitFlag(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - _, err := runCommand(http, true, `--limit=0`) + _, err := runCommand(http, nil, true, `--limit=0`) assert.EqualError(t, err, "invalid value for --limit: 0") } @@ -312,7 +320,7 @@ func TestPRList_web(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - output, err := runCommand(http, true, "--web "+test.cli) + output, err := runCommand(http, nil, true, "--web "+test.cli) if err != nil { t.Errorf("error running command `pr list` with `--web` flag: %v", err) } @@ -370,6 +378,7 @@ func TestPRList_withProjectItems(t *testing.T) { client := &http.Client{Transport: reg} prsAndTotalCount, err := listPullRequests( client, + nil, ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "pr", @@ -433,12 +442,16 @@ func TestPRList_Search_withProjectItems(t *testing.T) { require.Equal(t, map[string]interface{}{ "limit": float64(30), "q": "just used to force the search API branch repo:OWNER/REPO state:open type:pr", + "type": "ISSUE_ADVANCED", }, params) })) client := &http.Client{Transport: reg} prsAndTotalCount, err := listPullRequests( client, + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + fd.AdvancedIssueSearchSupportedAsOptIn(), ghrepo.New("OWNER", "REPO"), prShared.FilterOptions{ Entity: "pr", diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index c3315aeae..9504ecef4 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -186,20 +186,20 @@ func (opts *FilterOptions) IsDefault() bool { return true } -func ListURLWithQuery(listURL string, options FilterOptions) (string, error) { +func ListURLWithQuery(listURL string, options FilterOptions, advancedIssueSearchSyntax bool) (string, error) { u, err := url.Parse(listURL) if err != nil { return "", err } params := u.Query() - params.Set("q", SearchQueryBuild(options)) + params.Set("q", SearchQueryBuild(options, advancedIssueSearchSyntax)) u.RawQuery = params.Encode() return u.String(), nil } -func SearchQueryBuild(options FilterOptions) string { +func SearchQueryBuild(options FilterOptions, advancedIssueSearchSyntax bool) string { var is, state string switch options.State { case "open", "closed": @@ -207,7 +207,7 @@ func SearchQueryBuild(options FilterOptions) string { case "merged": is = "merged" } - q := search.Query{ + query := search.Query{ Qualifiers: search.Qualifiers{ Assignee: options.Assignee, Author: options.Author, @@ -223,10 +223,18 @@ func SearchQueryBuild(options FilterOptions) string { Type: options.Entity, }, } - if options.Search != "" { - return fmt.Sprintf("%s %s", options.Search, q.String()) + + var q string + if advancedIssueSearchSyntax { + q = query.AdvancedIssueSearchString() + } else { + q = query.StandardSearchString() } - return q.String() + + if options.Search != "" { + return fmt.Sprintf("%s %s", options.Search, q) + } + return q } func QueryHasStateClause(searchQuery string) bool { diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 3c95a9a5d..024965630 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -19,8 +19,9 @@ func Test_listURLWithQuery(t *testing.T) { falseBool := false type args struct { - listURL string - options FilterOptions + listURL string + options FilterOptions + advancedIssueSearchSyntax bool } tests := []struct { @@ -41,6 +42,19 @@ func Test_listURLWithQuery(t *testing.T) { want: "https://example.com/path?a=b&q=state%3Aopen+type%3Aissue", wantErr: false, }, + { + name: "blank, advanced search", + args: args{ + listURL: "https://example.com/path?a=b", + options: FilterOptions{ + Entity: "issue", + State: "open", + }, + advancedIssueSearchSyntax: true, + }, + want: "https://example.com/path?a=b&q=state%3Aopen+type%3Aissue", + wantErr: false, + }, { name: "draft", args: args{ @@ -54,6 +68,20 @@ func Test_listURLWithQuery(t *testing.T) { want: "https://example.com/path?q=draft%3Atrue+state%3Aopen+type%3Apr", wantErr: false, }, + { + name: "draft, advanced search", + args: args{ + listURL: "https://example.com/path", + options: FilterOptions{ + Entity: "pr", + State: "open", + Draft: &trueBool, + }, + advancedIssueSearchSyntax: true, + }, + want: "https://example.com/path?q=draft%3Atrue+state%3Aopen+type%3Apr", + wantErr: false, + }, { name: "non-draft", args: args{ @@ -67,6 +95,20 @@ func Test_listURLWithQuery(t *testing.T) { want: "https://example.com/path?q=draft%3Afalse+state%3Aopen+type%3Apr", wantErr: false, }, + { + name: "non-draft, advanced search", + args: args{ + listURL: "https://example.com/path", + options: FilterOptions{ + Entity: "pr", + State: "open", + Draft: &falseBool, + }, + advancedIssueSearchSyntax: true, + }, + want: "https://example.com/path?q=draft%3Afalse+state%3Aopen+type%3Apr", + wantErr: false, + }, { name: "all", args: args{ @@ -84,6 +126,24 @@ func Test_listURLWithQuery(t *testing.T) { want: "https://example.com/path?q=assignee%3Abo+author%3Aka+base%3Atrunk+head%3Abug-fix+mentions%3Anu+state%3Aopen+type%3Aissue", wantErr: false, }, + { + name: "all, advanced search", + args: args{ + listURL: "https://example.com/path", + options: FilterOptions{ + Entity: "issue", + State: "open", + Assignee: "bo", + Author: "ka", + BaseBranch: "trunk", + HeadBranch: "bug-fix", + Mention: "nu", + }, + advancedIssueSearchSyntax: true, + }, + want: "https://example.com/path?q=assignee%3Abo+author%3Aka+base%3Atrunk+head%3Abug-fix+mentions%3Anu+state%3Aopen+type%3Aissue", + wantErr: false, + }, { name: "spaces in values", args: args{ @@ -98,10 +158,25 @@ 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: "spaces in values, advanced search", + args: args{ + listURL: "https://example.com/path", + options: FilterOptions{ + Entity: "pr", + State: "open", + Labels: []string{"docs", "help wanted"}, + Milestone: `Codename "What Was Missing"`, + }, + advancedIssueSearchSyntax: true, + }, + 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, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ListURLWithQuery(tt.args.listURL, tt.args.options) + got, err := ListURLWithQuery(tt.args.listURL, tt.args.options, tt.args.advancedIssueSearchSyntax) if (err != nil) != tt.wantErr { t.Errorf("listURLWithQuery() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 0508c6d80..d896c9224 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -213,5 +213,5 @@ func searchQuery(owner string, filter FilterOptions) string { }, } - return q.String() + return q.StandardSearchString() } diff --git a/pkg/cmd/search/code/code_test.go b/pkg/cmd/search/code/code_test.go index efb5f4b57..471a9d0cb 100644 --- a/pkg/cmd/search/code/code_test.go +++ b/pkg/cmd/search/code/code_test.go @@ -7,6 +7,7 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" ghmock "github.com/cli/cli/v2/internal/gh/mock" "github.com/cli/cli/v2/pkg/cmdutil" @@ -336,7 +337,7 @@ func TestCodeRun(t *testing.T) { Extension: "go", }, }, - Searcher: search.NewSearcher(nil, "github.com"), + Searcher: search.NewSearcher(nil, "github.com", &fd.DisabledDetectorMock{}), WebMode: true, }, wantBrowse: "https://github.com/search?q=map+path%3Atesting.go&type=code", @@ -354,7 +355,7 @@ func TestCodeRun(t *testing.T) { Extension: ".cpp", }, }, - Searcher: search.NewSearcher(nil, "github.com"), + Searcher: search.NewSearcher(nil, "github.com", &fd.DisabledDetectorMock{}), WebMode: true, }, wantBrowse: "https://github.com/search?q=map+path%3Atesting.cpp&type=code", @@ -381,7 +382,7 @@ func TestCodeRun(t *testing.T) { Extension: "go", }, }, - Searcher: search.NewSearcher(nil, "example.com"), + Searcher: search.NewSearcher(nil, "example.com", &fd.DisabledDetectorMock{}), WebMode: true, }, wantBrowse: "https://example.com/search?q=map+extension%3Ago+filename%3Atesting&type=code", diff --git a/pkg/cmd/search/issues/issues.go b/pkg/cmd/search/issues/issues.go index 1d6ec6428..e616430fb 100644 --- a/pkg/cmd/search/issues/issues.go +++ b/pkg/cmd/search/issues/issues.go @@ -26,6 +26,8 @@ func NewCmdIssues(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *c cmd := &cobra.Command{ Use: "issues []", Short: "Search for issues", + // TODO advancedIssueSearchCleanup + // Update the links and remove the mention at GHES 3.17 version. Long: heredoc.Docf(` Search for issues on GitHub. @@ -35,6 +37,10 @@ func NewCmdIssues(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *c GitHub search syntax is documented at: + On supported GitHub hosts, advanced issue search syntax can be used in the + %[1]s--search%[1]s query. For more information about advanced issue search, see: + + For more information on handling search queries containing a hyphen, run %[1]sgh search --help%[1]s. `, "`"), Example: heredoc.Doc(` diff --git a/pkg/cmd/search/prs/prs.go b/pkg/cmd/search/prs/prs.go index 98ab730b5..498569863 100644 --- a/pkg/cmd/search/prs/prs.go +++ b/pkg/cmd/search/prs/prs.go @@ -28,6 +28,8 @@ func NewCmdPrs(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *cobr cmd := &cobra.Command{ Use: "prs []", Short: "Search for pull requests", + // TODO advancedIssueSearchCleanup + // Update the links and remove the mention at GHES 3.17 version. Long: heredoc.Docf(` Search for pull requests on GitHub. @@ -37,6 +39,10 @@ func NewCmdPrs(f *cmdutil.Factory, runF func(*shared.IssuesOptions) error) *cobr GitHub search syntax is documented at: + On supported GitHub hosts, advanced issue search syntax can be used in the + %[1]s--search%[1]s query. For more information about advanced issue search, see: + + For more information on handling search queries containing a hyphen, run %[1]sgh search --help%[1]s. `, "`"), Example: heredoc.Doc(` diff --git a/pkg/cmd/search/shared/shared.go b/pkg/cmd/search/shared/shared.go index 3282599cf..1989bb9d3 100644 --- a/pkg/cmd/search/shared/shared.go +++ b/pkg/cmd/search/shared/shared.go @@ -7,6 +7,7 @@ import ( "time" "github.com/cli/cli/v2/internal/browser" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmdutil" @@ -42,12 +43,16 @@ func Searcher(f *cmdutil.Factory) (search.Searcher, error) { if err != nil { return nil, err } + host, _ := cfg.Authentication().DefaultHost() client, err := f.HttpClient() if err != nil { return nil, err } - return search.NewSearcher(client, host), nil + + detector := fd.NewDetector(client, host) + + return search.NewSearcher(client, host, detector), nil } func SearchIssues(opts *IssuesOptions) error { diff --git a/pkg/search/query.go b/pkg/search/query.go index c517f90d8..0419e359a 100644 --- a/pkg/search/query.go +++ b/pkg/search/query.go @@ -3,7 +3,7 @@ package search import ( "fmt" "reflect" - "sort" + "slices" "strings" "unicode" ) @@ -86,13 +86,120 @@ type Qualifiers struct { User []string } -func (q Query) String() string { - qualifiers := formatQualifiers(q.Qualifiers) +// String returns the string representation of the query which can be used with +// the legacy search backend, which is used in global search GUI (i.e. +// github.com/search), or Pull Requests tab (in repositories). Note that this is +// a common query format that can be used to search for various entity types +// (e.g., issues, commits, repositories, etc) +// +// With the legacy search backend, the query is made of concatenating keywords +// and qualifiers with whitespaces. Note that at the backend side, most of the +// repeated qualifiers are AND-ed, while a handful of qualifiers (i.e. +// is:private/public, repo:, user:, or in:) are implicitly OR-ed. The legacy +// search backend does not support the advanced syntax which allows for nested +// queries and explicit OR operators. +// +// At the moment, the advanced search syntax is only available for searching +// issues, and it's called advanced issue search. +func (q Query) StandardSearchString() string { + qualifiers := formatQualifiers(q.Qualifiers, nil) keywords := formatKeywords(q.Keywords) all := append(keywords, qualifiers...) return strings.TrimSpace(strings.Join(all, " ")) } +// AdvancedIssueSearchString returns the string representation of the query +// compatible with the advanced issue search syntax. The query can be used in +// Issues tab (of repositories) and the Issues dashboard (i.e. +// github.com/issues). +// +// As the name suggests, this query syntax is only supported for searching +// issues (i.e. issues and PRs). The advanced syntax allows nested queries and +// explicit OR operators. Unlike the legacy search backend, the advanced issue +// search does not OR repeated instances of special qualifiers (i.e. +// is:private/public, repo:, user:, or in:). +// +// To keep the gh experience consistent and backward-compatible, the mentioned +// special qualifiers are explicitly grouped and combined with an OR operator. +// +// The advanced syntax is documented at https://github.blog/changelog/2025-03-06-github-issues-projects-api-support-for-issues-advanced-search-and-more +func (q Query) AdvancedIssueSearchString() string { + qualifiers := formatQualifiers(q.Qualifiers, formatAdvancedIssueSearch) + keywords := formatKeywords(q.Keywords) + all := append(keywords, qualifiers...) + return strings.TrimSpace(strings.Join(all, " ")) +} + +func formatAdvancedIssueSearch(qualifier string, vs []string) (s []string, applicable bool) { + switch qualifier { + case "in": + return formatSpecialQualifiers("in", vs, [][]string{{"title", "body", "comments"}}), true + case "is": + return formatSpecialQualifiers("is", vs, [][]string{{"blocked", "blocking"}, {"closed", "open"}, {"issue", "pr"}, {"locked", "unlocked"}, {"merged", "unmerged"}, {"private", "public"}}), true + case "user", "repo": + return []string{groupWithOR(qualifier, vs)}, true + } + // Let the default formatting take over + return nil, false +} + +func formatSpecialQualifiers(qualifier string, vs []string, specialGroupsToOR [][]string) []string { + specialGroups := make([][]string, len(specialGroupsToOR)) + rest := make([]string, 0, len(vs)) + for _, v := range vs { + var isSpecial bool + for i, subValuesToOR := range specialGroupsToOR { + if slices.Contains(subValuesToOR, v) { + specialGroups[i] = append(specialGroups[i], v) + isSpecial = true + break + } + } + + if isSpecial { + continue + } + + rest = append(rest, v) + } + + all := make([]string, 0, len(specialGroups)+len(rest)) + + for _, group := range specialGroups { + if len(group) == 0 { + continue + } + all = append(all, groupWithOR(qualifier, group)) + } + + if len(rest) > 0 { + for _, v := range rest { + all = append(all, fmt.Sprintf("%s:%s", qualifier, quote(v))) + } + } + + slices.Sort(all) + return all +} + +func groupWithOR(qualifier string, vs []string) string { + if len(vs) == 0 { + return "" + } + + all := make([]string, 0, len(vs)) + for _, v := range vs { + all = append(all, fmt.Sprintf("%s:%s", qualifier, quote(v))) + } + + if len(all) == 1 { + return all[0] + } + + slices.Sort(all) + return fmt.Sprintf("(%s)", strings.Join(all, " OR ")) +} + func (q Qualifiers) Map() map[string][]string { m := map[string][]string{} v := reflect.ValueOf(q) @@ -138,15 +245,51 @@ func quote(s string) string { return s } -func formatQualifiers(qs Qualifiers) []string { - var all []string - for k, vs := range qs.Map() { - for _, v := range vs { - all = append(all, fmt.Sprintf("%s:%s", k, quote(v))) - } +// formatQualifiers renders qualifiers into a plain query. +// +// The formatter is a custom formatting function that can be used to modify the +// output of each qualifier. If the formatter returns (nil, false) the default +// formatting will be applied. +func formatQualifiers(qs Qualifiers, formatter func(qualifier string, vs []string) (s []string, applicable bool)) []string { + type entry struct { + key string + values []string } - sort.Strings(all) - return all + + var all []entry + for k, vs := range qs.Map() { + if len(vs) == 0 { + continue + } + + e := entry{key: k} + + if formatter != nil { + if s, applicable := formatter(k, vs); applicable { + e.values = s + all = append(all, e) + continue + } + } + + for _, v := range vs { + e.values = append(e.values, fmt.Sprintf("%s:%s", k, quote(v))) + } + if len(e.values) > 1 { + slices.Sort(e.values) + } + all = append(all, e) + } + + slices.SortFunc(all, func(a, b entry) int { + return strings.Compare(a.key, b.key) + }) + + result := make([]string, 0, len(all)) + for _, e := range all { + result = append(result, e.values...) + } + return result } func formatKeywords(ks []string) []string { diff --git a/pkg/search/query_test.go b/pkg/search/query_test.go index ddec211cb..7c20e5348 100644 --- a/pkg/search/query_test.go +++ b/pkg/search/query_test.go @@ -8,7 +8,7 @@ import ( var trueBool = true -func TestQueryString(t *testing.T) { +func TestStandardSearchString(t *testing.T) { tests := []struct { name string query Query @@ -73,7 +73,109 @@ func TestQueryString(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.out, tt.query.String()) + assert.Equal(t, tt.out, tt.query.StandardSearchString()) + }) + } +} + +func TestAdvancedIssueSearchString(t *testing.T) { + tests := []struct { + name string + query Query + out string + }{ + { + name: "quotes keywords", + query: Query{ + Keywords: []string{"quote keywords"}, + }, + out: `"quote keywords"`, + }, + { + name: "quotes keywords that are qualifiers", + query: Query{ + Keywords: []string{"quote:keywords", "quote:multiword keywords"}, + }, + out: `quote:keywords quote:"multiword keywords"`, + }, + { + name: "quotes qualifiers", + query: Query{ + Qualifiers: Qualifiers{ + Label: []string{"quote qualifier"}, + }, + }, + out: `label:"quote qualifier"`, + }, + { + name: "unused qualifiers should not appear in query", + query: Query{ + Keywords: []string{"keyword"}, + Qualifiers: Qualifiers{ + Label: []string{"foo", "bar"}, + }, + }, + out: `keyword label:bar label:foo`, + }, + { + name: "special qualifiers when used once", + query: Query{ + Keywords: []string{"keyword"}, + Qualifiers: Qualifiers{ + Repo: []string{"foo/bar"}, + Is: []string{"private"}, + User: []string{"johndoe"}, + In: []string{"title"}, + }, + }, + out: `keyword in:title is:private repo:foo/bar user:johndoe`, + }, + { + name: "special qualifiers are OR-ed when used multiple times", + query: Query{ + Keywords: []string{"keyword"}, + Qualifiers: Qualifiers{ + Repo: []string{"foo/bar", "foo/baz"}, + Is: []string{"private", "public", "issue", "pr", "open", "closed", "locked", "unlocked", "merged", "unmerged", "blocked", "blocking", "foo"}, // "foo" is to ensure only "public" and "private" are grouped + User: []string{"johndoe", "janedoe"}, + In: []string{"title", "body", "comments", "foo"}, // "foo" is to ensure only "title", "body", and "comments" are grouped + }, + }, + out: `keyword (in:body OR in:comments OR in:title) in:foo (is:blocked OR is:blocking) (is:closed OR is:open) (is:issue OR is:pr) (is:locked OR is:unlocked) (is:merged OR is:unmerged) (is:private OR is:public) is:foo (repo:foo/bar OR repo:foo/baz) (user:janedoe OR user:johndoe)`, + }, + { + // Since this is a general purpose package, we can't assume with know all + // use cases of special qualifiers. So, here we ensure unknown values are + // not OR-ed by default. + name: "special qualifiers without special values", + query: Query{ + Keywords: []string{"keyword"}, + Qualifiers: Qualifiers{ + Is: []string{"foo", "bar"}, + In: []string{"foo", "bar"}, + }, + }, + out: `keyword in:bar in:foo is:bar is:foo`, + }, + { + name: "non-special qualifiers used multiple times", + query: Query{ + Keywords: []string{"keyword"}, + Qualifiers: Qualifiers{ + In: []string{"foo", "bar"}, // "in:" is a special qualifier but its values here are not special + Is: []string{"foo", "bar"}, // "is:" is a special qualifier but its values here are not special + Label: []string{"foo", "bar"}, + License: []string{"foo", "bar"}, + No: []string{"foo", "bar"}, + Topic: []string{"foo", "bar"}, + }, + }, + out: `keyword in:bar in:foo is:bar is:foo label:bar label:foo license:bar license:foo no:bar no:foo topic:bar topic:foo`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.out, tt.query.AdvancedIssueSearchString()) }) } } diff --git a/pkg/search/searcher.go b/pkg/search/searcher.go index 7cbd35562..5cc94ac9e 100644 --- a/pkg/search/searcher.go +++ b/pkg/search/searcher.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghinstance" ) @@ -34,8 +35,9 @@ type Searcher interface { } type searcher struct { - client *http.Client - host string + client *http.Client + detector fd.Detector + host string } type httpError struct { @@ -52,10 +54,11 @@ type httpErrorItem struct { Resource string } -func NewSearcher(client *http.Client, host string) Searcher { +func NewSearcher(client *http.Client, host string, detector fd.Detector) Searcher { return &searcher{ - client: client, - host: host, + client: client, + host: host, + detector: detector, } } @@ -205,7 +208,31 @@ func (s searcher) search(query Query, result interface{}) (*http.Response, error qs := url.Values{} qs.Set("page", strconv.Itoa(query.Page)) qs.Set("per_page", strconv.Itoa(query.Limit)) - qs.Set("q", query.String()) + + if query.Kind == KindIssues { + // TODO advancedIssueSearchCleanup + // We won't need feature detection when GHES 3.17 support ends, since + // the advanced issue search is the only available search backend for + // issues. + features, err := s.detector.SearchFeatures() + if err != nil { + return nil, err + } + + if !features.AdvancedIssueSearchAPI { + qs.Set("q", query.StandardSearchString()) + } else { + qs.Set("q", query.AdvancedIssueSearchString()) + + if features.AdvancedIssueSearchAPIOptIn { + // Advanced syntax should be explicitly enabled + qs.Set("advanced_search", "true") + } + } + } else { + qs.Set("q", query.StandardSearchString()) + } + if query.Order != "" { qs.Set(orderKey, query.Order) } @@ -240,11 +267,19 @@ func (s searcher) search(query Query, result interface{}) (*http.Response, error return resp, nil } +// URL returns URL to the global search in web GUI (i.e. github.com/search). func (s searcher) URL(query Query) string { path := fmt.Sprintf("https://%s/search", s.host) qs := url.Values{} qs.Set("type", query.Kind) - qs.Set("q", query.String()) + + // TODO advancedSearchFuture + // Currently, the global search GUI does not support the advanced issue + // search syntax (even for the issues/PRs tab on the sidebar). When the GUI + // is updated, we can use feature detection, and, if available, use the + // advanced search syntax. + qs.Set("q", query.StandardSearchString()) + if query.Order != "" { qs.Set(orderKey, query.Order) } diff --git a/pkg/search/searcher_test.go b/pkg/search/searcher_test.go index fb7bb616a..f5467be37 100644 --- a/pkg/search/searcher_test.go +++ b/pkg/search/searcher_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/MakeNowJust/heredoc" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/assert" ) @@ -265,7 +266,7 @@ func TestSearcherCode(t *testing.T) { if tt.host == "" { tt.host = "github.com" } - searcher := NewSearcher(client, tt.host) + searcher := NewSearcher(client, tt.host, &fd.DisabledDetectorMock{}) result, err := searcher.Code(tt.query) if tt.wantErr { assert.EqualError(t, err, tt.errMsg) @@ -551,7 +552,7 @@ func TestSearcherCommits(t *testing.T) { if tt.host == "" { tt.host = "github.com" } - searcher := NewSearcher(client, tt.host) + searcher := NewSearcher(client, tt.host, &fd.DisabledDetectorMock{}) result, err := searcher.Commits(tt.query) if tt.wantErr { assert.EqualError(t, err, tt.errMsg) @@ -837,7 +838,7 @@ func TestSearcherRepositories(t *testing.T) { if tt.host == "" { tt.host = "github.com" } - searcher := NewSearcher(client, tt.host) + searcher := NewSearcher(client, tt.host, &fd.DisabledDetectorMock{}) result, err := searcher.Repositories(tt.query) if tt.wantErr { assert.EqualError(t, err, tt.errMsg) @@ -1123,7 +1124,7 @@ func TestSearcherIssues(t *testing.T) { if tt.host == "" { tt.host = "github.com" } - searcher := NewSearcher(client, tt.host) + searcher := NewSearcher(client, tt.host, fd.AdvancedIssueSearchUnsupported()) result, err := searcher.Issues(tt.query) if tt.wantErr { assert.EqualError(t, err, tt.errMsg) @@ -1135,6 +1136,88 @@ func TestSearcherIssues(t *testing.T) { } } +func TestSearcherIssuesAdvancedSyntax(t *testing.T) { + query := Query{ + Kind: KindIssues, + Limit: 1, + Keywords: []string{"keyword"}, + Qualifiers: Qualifiers{ + // Ordinary qualifiers + Author: "johndoe", + Label: []string{"foo", "bar"}, + // Special qualifiers (that should be grouped and OR-ed when using advanced issue search) + Repo: []string{"foo/bar", "foo/baz"}, + Is: []string{"private", "public"}, + User: []string{"johndoe", "janedoe"}, + In: []string{"title", "body", "comments"}, + }, + } + + tests := []struct { + name string + query Query + detector fd.Detector + wantValues url.Values + wantErr string + }{ + { + // TODO advancedIssueSearchCleanup + // Remove this test case once GHES 3.17 support ends. + name: "advanced issue search not supported", + detector: fd.AdvancedIssueSearchUnsupported(), + query: query, + wantValues: url.Values{ + "q": []string{"keyword author:johndoe in:body in:comments in:title is:private is:public label:bar label:foo repo:foo/bar repo:foo/baz user:janedoe user:johndoe"}, + "advanced_search": nil, // assert absence + }, + }, + { + // TODO advancedIssueSearchCleanup + // Remove this test case once GHES 3.17 support ends. + name: "advanced issue search supported as an opt-in feature", + detector: fd.AdvancedIssueSearchSupportedAsOptIn(), + query: query, + wantValues: url.Values{ + "q": []string{"keyword author:johndoe (in:body OR in:comments OR in:title) (is:private OR is:public) label:bar label:foo (repo:foo/bar OR repo:foo/baz) (user:janedoe OR user:johndoe)"}, + "advanced_search": []string{"true"}, // opt-in + }, + }, + { + // TODO advancedIssueSearchCleanup + // No need for feature detection once GHES 3.17 support ends. + name: "advanced issue search supported as the only search backend", + detector: fd.AdvancedIssueSearchSupportedAsOnlyBackend(), + query: query, + wantValues: url.Values{ + "q": []string{"keyword author:johndoe (in:body OR in:comments OR in:title) (is:private OR is:public) label:bar label:foo (repo:foo/bar OR repo:foo/baz) (user:janedoe OR user:johndoe)"}, + "advanced_search": nil, // assert absence + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.QueryMatcher("GET", "search/issues", tt.wantValues), + httpmock.JSONResponse(IssuesResult{}), + ) + + client := &http.Client{Transport: reg} + searcher := NewSearcher(client, "github.com", tt.detector) + + _, err := searcher.Issues(tt.query) + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + } + }) + } +} + func TestSearcherURL(t *testing.T) { query := Query{ Keywords: []string{"keyword"}, @@ -1179,7 +1262,7 @@ func TestSearcherURL(t *testing.T) { if tt.host == "" { tt.host = "github.com" } - searcher := NewSearcher(nil, tt.host) + searcher := NewSearcher(nil, tt.host, nil) assert.Equal(t, tt.url, searcher.URL(tt.query)) }) }