From 1f6d66b17f471ab60d185494f5cb13c0ed27f789 Mon Sep 17 00:00:00 2001 From: Amulyam24 Date: Tue, 21 Apr 2026 19:52:23 +0530 Subject: [PATCH] plugins: move transfer-issue to issue management Signed-off-by: Amulyam24 --- cmd/hook/plugin-imports/plugin-imports.go | 1 - pkg/hook/plugin-imports/plugin-imports.go | 1 - .../issue-management/issue_management.go | 40 ++++++ .../issue-management/issue_management_test.go | 123 ++++++++++++++---- .../transfer-issue.go | 63 +-------- .../transfer-issue_test.go | 83 +++++------- 6 files changed, 178 insertions(+), 133 deletions(-) rename pkg/plugins/{transfer-issue => issue-management}/transfer-issue.go (62%) rename pkg/plugins/{transfer-issue => issue-management}/transfer-issue_test.go (77%) diff --git a/cmd/hook/plugin-imports/plugin-imports.go b/cmd/hook/plugin-imports/plugin-imports.go index ecdf5fe31f..32e9285fa7 100644 --- a/cmd/hook/plugin-imports/plugin-imports.go +++ b/cmd/hook/plugin-imports/plugin-imports.go @@ -63,7 +63,6 @@ import ( _ "sigs.k8s.io/prow/pkg/plugins/slackevents" _ "sigs.k8s.io/prow/pkg/plugins/stage" _ "sigs.k8s.io/prow/pkg/plugins/testfreeze" - _ "sigs.k8s.io/prow/pkg/plugins/transfer-issue" _ "sigs.k8s.io/prow/pkg/plugins/trick-or-treat" _ "sigs.k8s.io/prow/pkg/plugins/trigger" _ "sigs.k8s.io/prow/pkg/plugins/updateconfig" diff --git a/pkg/hook/plugin-imports/plugin-imports.go b/pkg/hook/plugin-imports/plugin-imports.go index ecdf5fe31f..32e9285fa7 100644 --- a/pkg/hook/plugin-imports/plugin-imports.go +++ b/pkg/hook/plugin-imports/plugin-imports.go @@ -63,7 +63,6 @@ import ( _ "sigs.k8s.io/prow/pkg/plugins/slackevents" _ "sigs.k8s.io/prow/pkg/plugins/stage" _ "sigs.k8s.io/prow/pkg/plugins/testfreeze" - _ "sigs.k8s.io/prow/pkg/plugins/transfer-issue" _ "sigs.k8s.io/prow/pkg/plugins/trick-or-treat" _ "sigs.k8s.io/prow/pkg/plugins/trigger" _ "sigs.k8s.io/prow/pkg/plugins/updateconfig" diff --git a/pkg/plugins/issue-management/issue_management.go b/pkg/plugins/issue-management/issue_management.go index 6518c3d500..390c308057 100644 --- a/pkg/plugins/issue-management/issue_management.go +++ b/pkg/plugins/issue-management/issue_management.go @@ -39,6 +39,7 @@ var ( unlinkIssueRegex = regexp.MustCompile(`(?mi)^/unlink-issue\s+(.+)$`) pinRegex = regexp.MustCompile(`(?mi)^/pin-issue\s*$`) unpinRegex = regexp.MustCompile(`(?mi)^/unpin-issue\s*$`) + transferRegex = regexp.MustCompile(`(?mi)^/transfer(?:-issue)?(?: +(.*))?$`) ) type githubClient interface { @@ -83,6 +84,13 @@ func helpProvider(_ *plugins.Configuration, _ []config.OrgRepo) (*pluginhelp.Plu WhoCanUse: "Approvers from the top-level OWNERS file", Examples: []string{"/unpin-issue"}, }) + pluginHelp.AddCommand(pluginhelp.Command{ + Usage: "/transfer[-issue] ", + Description: "Transfers an issue to a different repo in the same org.", + Featured: true, + WhoCanUse: "Org members.", + Examples: []string{"/transfer-issue kubectl", "/transfer test-infra"}, + }) return pluginHelp, nil } @@ -113,9 +121,41 @@ func handleIssues(gc githubClient, oc ownersClient, log *logrus.Entry, e github. return handlePinOrUnpinIssue(gc, oc, log, e, false) } + if destRepo, err := parseTransferCommand(gc, e); err != nil { + return err + } else if destRepo != "" { + return handleTransferIssue(gc, log, e, destRepo) + } + return nil } +func parseTransferCommand(gc githubClient, e github.GenericCommentEvent) (string, error) { + matches := transferRegex.FindAllStringSubmatch(e.Body, -1) + if len(matches) == 0 { + return "", nil + } + + if e.IsPR { + return "", gc.CreateComment( + e.Repo.Owner.Login, e.Repo.Name, e.Number, + plugins.FormatResponseRaw(e.Body, e.HTMLURL, e.User.Login, "The `/transfer-issue` command is only supported on issues, not pull requests."), + ) + } + + if e.Action != github.GenericCommentActionCreated { + return "", nil + } + + if len(matches) != 1 || len(matches[0]) != 2 || len(matches[0][1]) == 0 { + return "", gc.CreateComment( + e.Repo.Owner.Login, e.Repo.Name, e.Number, + plugins.FormatResponseRaw(e.Body, e.HTMLURL, e.User.Login, "`/transfer-issue` must only be used once and with a single destination repo."), + ) + } + return strings.TrimSpace(matches[0][1]), nil +} + func parseCommentForLinkCommands(commentBody string) ([]string, []string) { extractIssues := func(re *regexp.Regexp) []string { var issues []string diff --git a/pkg/plugins/issue-management/issue_management_test.go b/pkg/plugins/issue-management/issue_management_test.go index c44b007405..6c904483bf 100644 --- a/pkg/plugins/issue-management/issue_management_test.go +++ b/pkg/plugins/issue-management/issue_management_test.go @@ -29,13 +29,13 @@ func TestHandleIssues(t *testing.T) { log := logrus.WithField("test", "handleIssues") tests := []struct { - name string - commentBody string - fc func(*fakegithub.FakeClient) - froc func() *fakeRepoownersClient - event github.GenericCommentEvent - expectError bool - errorContains string + name string + commentBody string + fc func(*fakegithub.FakeClient) + froc func() *fakeRepoownersClient + event github.GenericCommentEvent + expectComment bool + commentContains string }{ { name: "Routes to link-issue handler when /link-issue command is present", @@ -63,7 +63,6 @@ func TestHandleIssues(t *testing.T) { Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, User: github.User{Login: "user"}, }, - expectError: false, }, { name: "Routes to unlink-issue handler when /unlink-issue command is present", @@ -91,7 +90,6 @@ func TestHandleIssues(t *testing.T) { Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, User: github.User{Login: "user"}, }, - expectError: false, }, { name: "Routes to pin-issue handler when /pin-issue command is present", @@ -113,7 +111,6 @@ func TestHandleIssues(t *testing.T) { Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, User: github.User{Login: "user"}, }, - expectError: false, }, { name: "Routes to unpin-issue handler when /unpin-issue command is present", @@ -135,7 +132,6 @@ func TestHandleIssues(t *testing.T) { Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, User: github.User{Login: "user"}, }, - expectError: false, }, { name: "Returns nil when no matching command is found", @@ -154,7 +150,6 @@ func TestHandleIssues(t *testing.T) { Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, User: github.User{Login: "user"}, }, - expectError: false, }, { name: "Handles case insensitive commands", @@ -176,7 +171,90 @@ func TestHandleIssues(t *testing.T) { Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, User: github.User{Login: "user"}, }, - expectError: false, + }, + { + name: "Routes to transfer-issue handler when /transfer-issue command is present", + commentBody: "/transfer-issue test-repo", + fc: func(fc *fakegithub.FakeClient) { + fc.IssueComments = map[int][]github.IssueComment{} + fc.OrgMembers = map[string][]string{ + "org": {"user"}, + } + }, + froc: func() *fakeRepoownersClient { + return newFakeRepoownersClient([]string{"approver1"}) + }, + event: github.GenericCommentEvent{ + IsPR: false, + Action: github.GenericCommentActionCreated, + Body: "/transfer-issue test-repo", + Number: 1, + Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, + User: github.User{Login: "user"}, + NodeID: "issueNodeID", + }, + }, + { + name: "Routes to transfer handler when /transfer command is present", + commentBody: "/transfer another-repo", + fc: func(fc *fakegithub.FakeClient) { + fc.IssueComments = map[int][]github.IssueComment{} + fc.OrgMembers = map[string][]string{ + "org": {"user"}, + } + }, + froc: func() *fakeRepoownersClient { + return newFakeRepoownersClient([]string{"approver1"}) + }, + event: github.GenericCommentEvent{ + IsPR: false, + Action: github.GenericCommentActionCreated, + Body: "/transfer another-repo", + Number: 1, + Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, + User: github.User{Login: "user"}, + NodeID: "issueNodeID", + }, + }, + { + name: "Returns with comment when transfer command has multiple destinations", + commentBody: "/transfer-issue repo1\n/transfer repo2", + fc: func(fc *fakegithub.FakeClient) { + fc.IssueComments = map[int][]github.IssueComment{} + }, + froc: func() *fakeRepoownersClient { + return newFakeRepoownersClient([]string{"approver1"}) + }, + event: github.GenericCommentEvent{ + IsPR: false, + Action: github.GenericCommentActionCreated, + Body: "/transfer-issue repo1\n/transfer repo2", + Number: 1, + Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, + User: github.User{Login: "user"}, + }, + expectComment: true, + commentContains: "must only be used once", + }, + { + name: "Returns with comment when transfer command has no destination", + commentBody: "/transfer", + fc: func(fc *fakegithub.FakeClient) { + fc.IssueComments = map[int][]github.IssueComment{} + }, + froc: func() *fakeRepoownersClient { + return newFakeRepoownersClient([]string{"approver1"}) + }, + event: github.GenericCommentEvent{ + IsPR: false, + Action: github.GenericCommentActionCreated, + Body: "/transfer", + Number: 1, + Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, + User: github.User{Login: "user"}, + }, + expectComment: true, + commentContains: "single destination repo", }, } @@ -192,15 +270,16 @@ func TestHandleIssues(t *testing.T) { err := handleIssues(gc, oc, log, tc.event) - if tc.expectError { - if err == nil { - t.Errorf("Expected error but got none") - } else if tc.errorContains != "" && !strings.Contains(err.Error(), tc.errorContains) { - t.Errorf("Expected error to contain %q but got: %v", tc.errorContains, err) - } - } else { - if err != nil { - t.Errorf("Expected no error but got: %v", err) + if err != nil { + t.Errorf("Expected no error but got: %v", err) + } + + if tc.expectComment { + comments := fc.IssueCommentsAdded + if len(comments) == 0 { + t.Errorf("Expected comment but none were created") + } else if !strings.Contains(comments[0], tc.commentContains) { + t.Errorf("Expected comment to contain %q but got: %q", tc.commentContains, comments[0]) } } }) diff --git a/pkg/plugins/transfer-issue/transfer-issue.go b/pkg/plugins/issue-management/transfer-issue.go similarity index 62% rename from pkg/plugins/transfer-issue/transfer-issue.go rename to pkg/plugins/issue-management/transfer-issue.go index c6f7b6d3a8..23889e0836 100644 --- a/pkg/plugins/transfer-issue/transfer-issue.go +++ b/pkg/plugins/issue-management/transfer-issue.go @@ -16,80 +16,25 @@ limitations under the License. // Package transferissue implements the `/transfer-issue` command which allows members of the org // to transfer issues between repos -package transferissue +package issuemanagement import ( "context" "fmt" - "regexp" - "strings" githubql "github.com/shurcooL/githubv4" "github.com/sirupsen/logrus" - "sigs.k8s.io/prow/pkg/config" "sigs.k8s.io/prow/pkg/github" - "sigs.k8s.io/prow/pkg/pluginhelp" "sigs.k8s.io/prow/pkg/plugins" ) -const pluginName = "transfer-issue" - -var ( - transferRe = regexp.MustCompile(`(?mi)^/transfer(?:-issue)?(?: +(.*))?$`) -) - -type githubClient interface { - GetRepo(org, name string) (github.FullRepo, error) - CreateComment(org, repo string, number int, comment string) error - IsMember(org, user string) (bool, error) - MutateWithGitHubAppsSupport(context.Context, any, githubql.Input, map[string]any, string) error -} - -func init() { - plugins.RegisterGenericCommentHandler(pluginName, handleGenericComment, helpProvider) -} - -func helpProvider(_ *plugins.Configuration, _ []config.OrgRepo) (*pluginhelp.PluginHelp, error) { - pluginHelp := &pluginhelp.PluginHelp{ - Description: "The transfer-issue plugin transfers a GitHub issue from one repo to another in the same organization.", - } - pluginHelp.AddCommand(pluginhelp.Command{ - Usage: "/transfer[-issue] ", - Description: "Transfers an issue to a different repo in the same org.", - Featured: true, - WhoCanUse: "Org members.", - Examples: []string{"/transfer-issue kubectl", "/transfer test-infra"}, - }) - return pluginHelp, nil -} - -func handleGenericComment(pc plugins.Agent, e github.GenericCommentEvent) error { - return handleTransfer(pc.GitHubClient, pc.Logger, e) -} - -func handleTransfer(gc githubClient, log *logrus.Entry, e github.GenericCommentEvent) error { +func handleTransferIssue(gc githubClient, log *logrus.Entry, e github.GenericCommentEvent, dstRepoName string) error { org := e.Repo.Owner.Login srcRepoName := e.Repo.Name srcRepoPair := org + "/" + srcRepoName - user := e.User.Login - - if e.IsPR || e.Action != github.GenericCommentActionCreated { - return nil - } - matches := transferRe.FindAllStringSubmatch(e.Body, -1) - if len(matches) == 0 { - return nil - } - if len(matches) != 1 || len(matches[0]) != 2 || len(matches[0][1]) == 0 { - return gc.CreateComment( - org, srcRepoName, e.Number, - plugins.FormatResponseRaw(e.Body, e.HTMLURL, user, "/transfer-issue must only be used once and with a single destination repo."), - ) - } - - dstRepoName := strings.TrimSpace(matches[0][1]) dstRepoPair := org + "/" + dstRepoName + user := e.User.Login dstRepo, err := gc.GetRepo(org, dstRepoName) if err != nil { @@ -97,7 +42,7 @@ func handleTransfer(gc githubClient, log *logrus.Entry, e github.GenericCommentE // TODO: Might want to add another GetRepo type call that checks if a repo exists vs a bad request return gc.CreateComment( org, srcRepoName, e.Number, - plugins.FormatResponseRaw(e.Body, e.HTMLURL, user, fmt.Sprintf("Something went wrong or the destination repo %s does not exist.", dstRepoPair)), + plugins.FormatResponseRaw(e.Body, e.HTMLURL, user, fmt.Sprintf("Something went wrong or the destination repo **%s** does not exist.", dstRepoPair)), ) } diff --git a/pkg/plugins/transfer-issue/transfer-issue_test.go b/pkg/plugins/issue-management/transfer-issue_test.go similarity index 77% rename from pkg/plugins/transfer-issue/transfer-issue_test.go rename to pkg/plugins/issue-management/transfer-issue_test.go index f382cc4121..c5b51bdaf0 100644 --- a/pkg/plugins/transfer-issue/transfer-issue_test.go +++ b/pkg/plugins/issue-management/transfer-issue_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package transferissue +package issuemanagement import ( "bytes" @@ -36,51 +36,24 @@ import ( const issuerNum = 1 -func Test_handleTransfer(t *testing.T) { +func Test_handleTransferIssue(t *testing.T) { ts := []struct { - name string - event github.GenericCommentEvent - expectError bool - errorMessage string - comment string - fcFunc func(client *fakegithub.FakeClient) - tcFunc func(client *testClient) + name string + event github.GenericCommentEvent + comment string + fcFunc func(client *fakegithub.FakeClient) + tcFunc func(client *testClient) }{ { - name: "is a pr", - event: github.GenericCommentEvent{IsPR: true}, + name: "Skips transfer when event is on a pull request", + event: github.GenericCommentEvent{IsPR: true, Body: "/transfer-issue test-repo"}, }, { - name: "is not comment added", - event: github.GenericCommentEvent{Action: github.GenericCommentActionDeleted}, + name: "Skips transfer when comment action is not created", + event: github.GenericCommentEvent{Action: github.GenericCommentActionDeleted, Body: "/transfer-issue test-repo"}, }, { - name: "multiple matches", - event: github.GenericCommentEvent{ - Action: github.GenericCommentActionCreated, - Body: `/transfer-issue kubectl -/transfer test-infra`, - HTMLURL: fmt.Sprintf("https://github.com/kubernetes/fake/issues/%d", issuerNum), - Number: issuerNum, - Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, - User: github.User{Login: "user"}, - }, - comment: "single destination", - }, - { - name: "no destination", - event: github.GenericCommentEvent{ - Action: github.GenericCommentActionCreated, - Body: "/transfer", - HTMLURL: fmt.Sprintf("https://github.com/kubernetes/fake/issues/%d", issuerNum), - Number: issuerNum, - Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}, - User: github.User{Login: "user"}, - }, - comment: "single destination", - }, - { - name: "dest repo does not exist", + name: "Returns error comment when destination repo does not exist", event: github.GenericCommentEvent{ Action: github.GenericCommentActionCreated, Body: "/transfer-issue fake", @@ -95,7 +68,7 @@ func Test_handleTransfer(t *testing.T) { }, }, { - name: "not collaborator", + name: "Returns error comment when user is not an org member", event: github.GenericCommentEvent{ Action: github.GenericCommentActionCreated, Body: "/transfer-issue test-infra", @@ -110,7 +83,7 @@ func Test_handleTransfer(t *testing.T) { }, }, { - name: "trims whitespace from user input", + name: "Successfully transfers issue and trims whitespace from destination repo", event: github.GenericCommentEvent{ Action: github.GenericCommentActionCreated, Body: "/transfer-issue test-infra\r", @@ -127,7 +100,7 @@ func Test_handleTransfer(t *testing.T) { }, }, { - name: "happy path", + name: "Successfully transfers issue to destination repo", event: github.GenericCommentEvent{ Action: github.GenericCommentActionCreated, Body: `This belongs elsewhere @@ -158,17 +131,15 @@ Thanks!`, tc.fcFunc(fc) } log := logrus.WithField("plugin", pluginName) - err := handleTransfer(c, log, tc.event) + destRepo, err := parseTransferCommand(c, tc.event) if err != nil { - if !tc.expectError { + t.Fatalf("unexpected error from parseTransferCommand: %v", err) + } + if destRepo != "" { + err = handleTransferIssue(c, log, tc.event, destRepo) + if err != nil { t.Fatalf("unexpected error: %v", err) } - if m := err.Error(); !strings.Contains(m, tc.errorMessage) { - t.Fatalf("expected error to contain: %s got: %v", tc.errorMessage, m) - } - } - if err == nil && tc.expectError { - t.Fatalf("expected error but did not produce") } if len(tc.comment) != 0 { if cm, ok := fc.IssueComments[tc.event.Number]; ok { @@ -219,6 +190,18 @@ func (t *testClient) IsMember(org, user string) (bool, error) { return t.fc.IsMember(org, user) } +func (t *testClient) GetIssue(org, repo string, number int) (*github.Issue, error) { + return t.fc.GetIssue(org, repo, number) +} + +func (t *testClient) GetPullRequest(org, repo string, number int) (*github.PullRequest, error) { + return t.fc.GetPullRequest(org, repo, number) +} + +func (t *testClient) UpdatePullRequest(org, repo string, number int, title, body *string, open *bool, branch *string, canModify *bool) error { + return t.fc.UpdatePullRequest(org, repo, number, title, body, open, branch, canModify) +} + func (t *testClient) MutateWithGitHubAppsSupport(ctx context.Context, m any, input githubv4.Input, vars map[string]any, org string) error { mr := `{"data": { "transferIssue": { "issue": { "url": "https://kubernetes.io/fake" } } } }`