From a0b5e3a0154038578424bd0ca5eb2f2c9cedb377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20Arkan?= Date: Wed, 6 Aug 2025 19:58:56 +0100 Subject: [PATCH 1/4] Enhance search functionality with new query filters and utility functions - Added multiple test cases for searching issues with various query filters in `issues_test.go`. - Introduced utility functions in `search_utils.go` to check for specific filters and extract repository information from queries. - Created comprehensive tests in `search_utils_test.go` to validate the new filtering logic and ensure accurate query parsing. --- pkg/github/issues_test.go | 94 ++++++++++ pkg/github/search_utils.go | 37 +++- pkg/github/search_utils_test.go | 294 ++++++++++++++++++++++++++++++++ 3 files changed, 423 insertions(+), 2 deletions(-) create mode 100644 pkg/github/search_utils_test.go diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 2bdb89b06..453da27a8 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -410,6 +410,100 @@ func Test_SearchIssues(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing is:issue filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "query with existing repo: filter and conflicting owner/repo params - uses query filter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:issue repo:github/github-mcp-server critical", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server critical", + "owner": "different-owner", + "repo": "different-repo", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "query with both is: and repo: filters already present", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:issue repo:octocat/Hello-World bug", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:issue repo:octocat/Hello-World bug", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with multiple OR operators and existing filters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "search issues fails", mockedClient: mock.NewMockedHTTPClient( diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index a6ff1f782..1f8d4e039 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -6,11 +6,35 @@ import ( "fmt" "io" "net/http" + "regexp" "github.com/google/go-github/v73/github" "github.com/mark3labs/mcp-go/mcp" ) +func hasFilter(query, filterType string) bool { + pattern := fmt.Sprintf(`(^|\s)%s:\S+`, regexp.QuoteMeta(filterType)) + matched, _ := regexp.MatchString(pattern, query) + return matched +} + +func hasSpecificFilter(query, filterType, filterValue string) bool { + pattern := fmt.Sprintf(`(^|\s)%s:%s($|\s)`, regexp.QuoteMeta(filterType), regexp.QuoteMeta(filterValue)) + matched, _ := regexp.MatchString(pattern, query) + return matched +} + +func extractRepoFilter(query string) (owner, repo string, found bool) { + pattern := `(?:^|\s)repo:([^/\s]+)/([^\s]+)` + re := regexp.MustCompile(pattern) + matches := re.FindStringSubmatch(query) + + if len(matches) >= 3 { + return matches[1], matches[2], true + } + return "", "", false +} + func searchHandler( ctx context.Context, getClient GetClientFn, @@ -22,7 +46,10 @@ func searchHandler( if err != nil { return mcp.NewToolResultError(err.Error()), nil } - query = fmt.Sprintf("is:%s %s", searchType, query) + + if !hasSpecificFilter(query, "is", searchType) { + query = fmt.Sprintf("is:%s %s", searchType, query) + } owner, err := OptionalParam[string](request, "owner") if err != nil { @@ -35,7 +62,13 @@ func searchHandler( } if owner != "" && repo != "" { - query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) + _, _, hasRepoFilter := extractRepoFilter(query) + + // TODO: Existing owner and existing repo? + if !hasRepoFilter { + query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) + } + } sort, err := OptionalParam[string](request, "sort") diff --git a/pkg/github/search_utils_test.go b/pkg/github/search_utils_test.go new file mode 100644 index 000000000..f2ede11c6 --- /dev/null +++ b/pkg/github/search_utils_test.go @@ -0,0 +1,294 @@ +package github + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_hasFilter(t *testing.T) { + tests := []struct { + name string + query string + filterType string + expected bool + }{ + { + name: "query has is:issue filter", + query: "is:issue bug report", + filterType: "is", + expected: true, + }, + { + name: "query has repo: filter", + query: "repo:github/github-mcp-server critical bug", + filterType: "repo", + expected: true, + }, + { + name: "query has multiple is: filters", + query: "is:issue is:open bug", + filterType: "is", + expected: true, + }, + { + name: "query has filter at the beginning", + query: "is:issue some text", + filterType: "is", + expected: true, + }, + { + name: "query has filter in the middle", + query: "some text is:issue more text", + filterType: "is", + expected: true, + }, + { + name: "query has filter at the end", + query: "some text is:issue", + filterType: "is", + expected: true, + }, + { + name: "query does not have the filter", + query: "bug report critical", + filterType: "is", + expected: false, + }, + { + name: "query has similar text but not the filter", + query: "this issue is important", + filterType: "is", + expected: false, + }, + { + name: "empty query", + query: "", + filterType: "is", + expected: false, + }, + { + name: "query has label: filter but looking for is:", + query: "label:bug critical", + filterType: "is", + expected: false, + }, + { + name: "query has author: filter", + query: "author:octocat bug", + filterType: "author", + expected: true, + }, + { + name: "query with complex OR expression", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + filterType: "is", + expected: true, + }, + { + name: "query with complex OR expression checking repo", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + filterType: "repo", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasFilter(tt.query, tt.filterType) + assert.Equal(t, tt.expected, result, "hasFilter(%q, %q) = %v, expected %v", tt.query, tt.filterType, result, tt.expected) + }) + } +} + +func Test_extractRepoFilter(t *testing.T) { + tests := []struct { + name string + query string + expectedOwner string + expectedRepo string + expectedFound bool + }{ + { + name: "query with repo: filter at beginning", + query: "repo:github/github-mcp-server is:issue", + expectedOwner: "github", + expectedRepo: "github-mcp-server", + expectedFound: true, + }, + { + name: "query with repo: filter in middle", + query: "is:issue repo:octocat/Hello-World bug", + expectedOwner: "octocat", + expectedRepo: "Hello-World", + expectedFound: true, + }, + { + name: "query with repo: filter at end", + query: "is:issue critical repo:owner/repo-name", + expectedOwner: "owner", + expectedRepo: "repo-name", + expectedFound: true, + }, + { + name: "query with complex repo name", + query: "repo:microsoft/vscode-extension-samples bug", + expectedOwner: "microsoft", + expectedRepo: "vscode-extension-samples", + expectedFound: true, + }, + { + name: "query without repo: filter", + query: "is:issue bug critical", + expectedOwner: "", + expectedRepo: "", + expectedFound: false, + }, + { + name: "query with malformed repo: filter (no slash)", + query: "repo:github bug", + expectedOwner: "", + expectedRepo: "", + expectedFound: false, + }, + { + name: "empty query", + query: "", + expectedOwner: "", + expectedRepo: "", + expectedFound: false, + }, + { + name: "query with multiple repo: filters (should match first)", + query: "repo:github/first repo:octocat/second", + expectedOwner: "github", + expectedRepo: "first", + expectedFound: true, + }, + { + name: "query with repo: in text but not as filter", + query: "this repo: is important", + expectedOwner: "", + expectedRepo: "", + expectedFound: false, + }, + { + name: "query with complex OR expression", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + expectedOwner: "github", + expectedRepo: "github-mcp-server", + expectedFound: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + owner, repo, found := extractRepoFilter(tt.query) + assert.Equal(t, tt.expectedOwner, owner, "extractRepoFilter(%q) owner = %q, expected %q", tt.query, owner, tt.expectedOwner) + assert.Equal(t, tt.expectedRepo, repo, "extractRepoFilter(%q) repo = %q, expected %q", tt.query, repo, tt.expectedRepo) + assert.Equal(t, tt.expectedFound, found, "extractRepoFilter(%q) found = %v, expected %v", tt.query, found, tt.expectedFound) + }) + } +} + +func Test_hasSpecificFilter(t *testing.T) { + tests := []struct { + name string + query string + filterType string + filterValue string + expected bool + }{ + { + name: "query has exact is:issue filter", + query: "is:issue bug report", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has is:open but looking for is:issue", + query: "is:open bug report", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "query has both is:issue and is:open, looking for is:issue", + query: "is:issue is:open bug", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has both is:issue and is:open, looking for is:open", + query: "is:issue is:open bug", + filterType: "is", + filterValue: "open", + expected: true, + }, + { + name: "query has is:issue at the beginning", + query: "is:issue some text", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has is:issue in the middle", + query: "some text is:issue more text", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has is:issue at the end", + query: "some text is:issue", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query does not have is:issue", + query: "bug report critical", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "query has similar text but not the exact filter", + query: "this issue is important", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "empty query", + query: "", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "partial match should not count", + query: "is:issues bug", // "issues" vs "issue" + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "complex query with parentheses", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + filterType: "is", + filterValue: "issue", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasSpecificFilter(tt.query, tt.filterType, tt.filterValue) + assert.Equal(t, tt.expected, result, "hasSpecificFilter(%q, %q, %q) = %v, expected %v", tt.query, tt.filterType, tt.filterValue, result, tt.expected) + }) + } +} From 4deed67dd8202cde934b21cffe4d231f7b8dc3c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20Arkan?= Date: Wed, 6 Aug 2025 20:18:53 +0100 Subject: [PATCH 2/4] Refactor repository filter extraction in search utilities - Renamed `extractRepoFilter` to `hasRepoFilter` to simplify the function's purpose. - Updated test cases in `search_utils_test.go` to reflect the new function name and logic. - Adjusted tests to focus on the presence of the `repo:` filter rather than extracting owner and repo details. --- pkg/github/search_utils.go | 21 ++------ pkg/github/search_utils_test.go | 96 +++++++++++++-------------------- 2 files changed, 40 insertions(+), 77 deletions(-) diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 1f8d4e039..36651a4a1 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -24,15 +24,8 @@ func hasSpecificFilter(query, filterType, filterValue string) bool { return matched } -func extractRepoFilter(query string) (owner, repo string, found bool) { - pattern := `(?:^|\s)repo:([^/\s]+)/([^\s]+)` - re := regexp.MustCompile(pattern) - matches := re.FindStringSubmatch(query) - - if len(matches) >= 3 { - return matches[1], matches[2], true - } - return "", "", false +func hasRepoFilter(query string) bool { + return hasFilter(query, "repo") } func searchHandler( @@ -61,14 +54,8 @@ func searchHandler( return mcp.NewToolResultError(err.Error()), nil } - if owner != "" && repo != "" { - _, _, hasRepoFilter := extractRepoFilter(query) - - // TODO: Existing owner and existing repo? - if !hasRepoFilter { - query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) - } - + if owner != "" && repo != "" && !hasRepoFilter(query) { + query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) } sort, err := OptionalParam[string](request, "sort") diff --git a/pkg/github/search_utils_test.go b/pkg/github/search_utils_test.go index f2ede11c6..b2c819d0e 100644 --- a/pkg/github/search_utils_test.go +++ b/pkg/github/search_utils_test.go @@ -101,92 +101,68 @@ func Test_hasFilter(t *testing.T) { } } -func Test_extractRepoFilter(t *testing.T) { +func Test_hasRepoFilter(t *testing.T) { tests := []struct { - name string - query string - expectedOwner string - expectedRepo string - expectedFound bool + name string + query string + expected bool }{ { - name: "query with repo: filter at beginning", - query: "repo:github/github-mcp-server is:issue", - expectedOwner: "github", - expectedRepo: "github-mcp-server", - expectedFound: true, + name: "query with repo: filter at beginning", + query: "repo:github/github-mcp-server is:issue", + expected: true, }, { - name: "query with repo: filter in middle", - query: "is:issue repo:octocat/Hello-World bug", - expectedOwner: "octocat", - expectedRepo: "Hello-World", - expectedFound: true, + name: "query with repo: filter in middle", + query: "is:issue repo:octocat/Hello-World bug", + expected: true, }, { - name: "query with repo: filter at end", - query: "is:issue critical repo:owner/repo-name", - expectedOwner: "owner", - expectedRepo: "repo-name", - expectedFound: true, + name: "query with repo: filter at end", + query: "is:issue critical repo:owner/repo-name", + expected: true, }, { - name: "query with complex repo name", - query: "repo:microsoft/vscode-extension-samples bug", - expectedOwner: "microsoft", - expectedRepo: "vscode-extension-samples", - expectedFound: true, + name: "query with complex repo name", + query: "repo:microsoft/vscode-extension-samples bug", + expected: true, }, { - name: "query without repo: filter", - query: "is:issue bug critical", - expectedOwner: "", - expectedRepo: "", - expectedFound: false, + name: "query without repo: filter", + query: "is:issue bug critical", + expected: false, }, { - name: "query with malformed repo: filter (no slash)", - query: "repo:github bug", - expectedOwner: "", - expectedRepo: "", - expectedFound: false, + name: "query with malformed repo: filter (no slash)", + query: "repo:github bug", + expected: true, // hasRepoFilter only checks for repo: prefix, not format }, { - name: "empty query", - query: "", - expectedOwner: "", - expectedRepo: "", - expectedFound: false, + name: "empty query", + query: "", + expected: false, }, { - name: "query with multiple repo: filters (should match first)", - query: "repo:github/first repo:octocat/second", - expectedOwner: "github", - expectedRepo: "first", - expectedFound: true, + name: "query with multiple repo: filters", + query: "repo:github/first repo:octocat/second", + expected: true, }, { - name: "query with repo: in text but not as filter", - query: "this repo: is important", - expectedOwner: "", - expectedRepo: "", - expectedFound: false, + name: "query with repo: in text but not as filter", + query: "this repo: is important", + expected: false, }, { - name: "query with complex OR expression", - query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", - expectedOwner: "github", - expectedRepo: "github-mcp-server", - expectedFound: true, + name: "query with complex OR expression", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + expected: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - owner, repo, found := extractRepoFilter(tt.query) - assert.Equal(t, tt.expectedOwner, owner, "extractRepoFilter(%q) owner = %q, expected %q", tt.query, owner, tt.expectedOwner) - assert.Equal(t, tt.expectedRepo, repo, "extractRepoFilter(%q) repo = %q, expected %q", tt.query, repo, tt.expectedRepo) - assert.Equal(t, tt.expectedFound, found, "extractRepoFilter(%q) found = %v, expected %v", tt.query, found, tt.expectedFound) + result := hasRepoFilter(tt.query) + assert.Equal(t, tt.expected, result, "hasRepoFilter(%q) = %v, expected %v", tt.query, result, tt.expected) }) } } From 3c97c6b8b4a75236738aed4be13703be75e1c133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20Arkan?= Date: Mon, 11 Aug 2025 16:36:46 +0100 Subject: [PATCH 3/4] Enhance search functionality with additional query filters and utility functions - Added new test cases for searching pull requests and users with various query filters in `pullrequests_test.go` and `search_test.go`. - Implemented a utility function `hasTypeFilter` in `search_utils.go` to check for the presence of `type:` filters in queries. - Updated the search handler to conditionally prepend the `type:` filter based on the presence of existing filters, improving query handling. --- pkg/github/pullrequests_test.go | 71 +++++++++++++++++++++++++++++ pkg/github/search.go | 5 ++- pkg/github/search_test.go | 80 +++++++++++++++++++++++++++++++++ pkg/github/search_utils.go | 4 ++ pkg/github/search_utils_test.go | 56 +++++++++++++++++++++++ 5 files changed, 215 insertions(+), 1 deletion(-) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index f759885ee..ed6921477 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1030,6 +1030,77 @@ func Test_SearchPullRequests(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing is:pr filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:github/github-mcp-server is:open draft:false", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:pr repo:github/github-mcp-server is:open draft:false", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "query with existing repo: filter and conflicting owner/repo params - uses query filter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:github/github-mcp-server author:octocat", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server author:octocat", + "owner": "different-owner", + "repo": "different-repo", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with existing is:pr filter and OR operators", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "search pull requests fails", mockedClient: mock.NewMockedHTTPClient( diff --git a/pkg/github/search.go b/pkg/github/search.go index 4fe390f86..248f17e17 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -204,7 +204,10 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - searchQuery := "type:" + accountType + " " + query + searchQuery := query + if !hasTypeFilter(query) { + searchQuery = "type:" + accountType + " " + query + } result, resp, err := client.Search.Users(ctx, searchQuery, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 66b57a8d4..cfc87c02b 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -410,6 +410,46 @@ func Test_SearchUsers(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing type:user filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:user location:seattle followers:>100", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:user location:seattle followers:>100", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with existing type:user filter and OR operators", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:user (location:seattle OR location:california) followers:>50", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:user (location:seattle OR location:california) followers:>50", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "search users fails", mockedClient: mock.NewMockedHTTPClient( @@ -537,6 +577,46 @@ func Test_SearchOrgs(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing type:org filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:org location:california followers:>1000", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:org location:california followers:>1000", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with existing type:org filter and OR operators", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:org (location:seattle OR location:california OR location:newyork) repos:>10", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:org (location:seattle OR location:california OR location:newyork) repos:>10", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "org search fails", mockedClient: mock.NewMockedHTTPClient( diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index f714e6648..6cecbf20c 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -28,6 +28,10 @@ func hasRepoFilter(query string) bool { return hasFilter(query, "repo") } +func hasTypeFilter(query string) bool { + return hasFilter(query, "type") +} + func searchHandler( ctx context.Context, getClient GetClientFn, diff --git a/pkg/github/search_utils_test.go b/pkg/github/search_utils_test.go index b2c819d0e..ef264505b 100644 --- a/pkg/github/search_utils_test.go +++ b/pkg/github/search_utils_test.go @@ -268,3 +268,59 @@ func Test_hasSpecificFilter(t *testing.T) { }) } } + +func Test_hasTypeFilter(t *testing.T) { + tests := []struct { + name string + query string + expected bool + }{ + { + name: "query with type:user filter at beginning", + query: "type:user location:seattle", + expected: true, + }, + { + name: "query with type:org filter in middle", + query: "location:california type:org followers:>100", + expected: true, + }, + { + name: "query with type:user filter at end", + query: "location:seattle followers:>50 type:user", + expected: true, + }, + { + name: "query without type: filter", + query: "location:seattle followers:>50", + expected: false, + }, + { + name: "empty query", + query: "", + expected: false, + }, + { + name: "query with type: in text but not as filter", + query: "this type: is important", + expected: false, + }, + { + name: "query with multiple type: filters", + query: "type:user type:org", + expected: true, + }, + { + name: "complex query with OR expression", + query: "type:user (location:seattle OR location:california)", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasTypeFilter(tt.query) + assert.Equal(t, tt.expected, result, "hasTypeFilter(%q) = %v, expected %v", tt.query, result, tt.expected) + }) + } +} From 2b62f5062b5aaa20b922c40d5c769a8a221ecf6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20Arkan?= Date: Tue, 12 Aug 2025 17:21:45 +0100 Subject: [PATCH 4/4] Updated both regex patterns to handle this edge case --- pkg/github/search_utils.go | 7 +++++-- pkg/github/search_utils_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 6cecbf20c..159518c91 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -13,13 +13,16 @@ import ( ) func hasFilter(query, filterType string) bool { - pattern := fmt.Sprintf(`(^|\s)%s:\S+`, regexp.QuoteMeta(filterType)) + // Match filter at start of string, after whitespace, or after non-word characters like '(' + pattern := fmt.Sprintf(`(^|\s|\W)%s:\S+`, regexp.QuoteMeta(filterType)) matched, _ := regexp.MatchString(pattern, query) return matched } func hasSpecificFilter(query, filterType, filterValue string) bool { - pattern := fmt.Sprintf(`(^|\s)%s:%s($|\s)`, regexp.QuoteMeta(filterType), regexp.QuoteMeta(filterValue)) + // Match specific filter:value at start, after whitespace, or after non-word characters + // End with word boundary, whitespace, or non-word characters like ')' + pattern := fmt.Sprintf(`(^|\s|\W)%s:%s($|\s|\W)`, regexp.QuoteMeta(filterType), regexp.QuoteMeta(filterValue)) matched, _ := regexp.MatchString(pattern, query) return matched } diff --git a/pkg/github/search_utils_test.go b/pkg/github/search_utils_test.go index ef264505b..85f953eed 100644 --- a/pkg/github/search_utils_test.go +++ b/pkg/github/search_utils_test.go @@ -91,6 +91,18 @@ func Test_hasFilter(t *testing.T) { filterType: "repo", expected: true, }, + { + name: "filter in parentheses at start", + query: "(label:bug OR owner:bob) is:issue", + filterType: "label", + expected: true, + }, + { + name: "filter after opening parenthesis", + query: "is:issue (label:critical OR repo:test/test)", + filterType: "label", + expected: true, + }, } for _, tt := range tests { @@ -259,6 +271,20 @@ func Test_hasSpecificFilter(t *testing.T) { filterValue: "issue", expected: true, }, + { + name: "filter:value in parentheses at start", + query: "(is:issue OR is:pr) label:bug", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "filter:value after opening parenthesis", + query: "repo:test/repo (is:issue AND label:bug)", + filterType: "is", + filterValue: "issue", + expected: true, + }, } for _, tt := range tests {