Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Draft: allow searching MRs by a state when it makes sense #623

Merged
merged 18 commits into from
Mar 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion commands/mr/approve/mr_approve.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewCmdApprove(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "opened")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/approvers/mr_approvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func NewCmdApprovers(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "opened")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/checkout/mr_checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/close/mr_close.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewCmdClose(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "opened")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/delete/mr_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func NewCmdDelete(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func diffRun(opts *DiffOptions) error {
if err != nil {
return err
}
mr, baseRepo, err := mrutils.MRFromArgs(opts.factory, opts.Args)
mr, baseRepo, err := mrutils.MRFromArgs(opts.factory, opts.Args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/issues/mr_issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func NewCmdIssues(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/merge/mr_merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewCmdMerge(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "opened")
if err != nil {
return err
}
Expand Down
35 changes: 24 additions & 11 deletions commands/mr/mrutils/mrutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,17 @@ func DisplayAllMRs(c *iostreams.ColorPalette, mrs []*gitlab.MergeRequest, projec
}

//MRFromArgs is wrapper around MRFromArgsWithOpts without any custom options
func MRFromArgs(f *cmdutils.Factory, args []string) (*gitlab.MergeRequest, glrepo.Interface, error) {
return MRFromArgsWithOpts(f, args, &gitlab.GetMergeRequestsOptions{})
func MRFromArgs(f *cmdutils.Factory, args []string, state string) (*gitlab.MergeRequest, glrepo.Interface, error) {
return MRFromArgsWithOpts(f, args, &gitlab.GetMergeRequestsOptions{}, state)
}

//MRFromArgsWithOpts gets MR with custom request options passed down to it
func MRFromArgsWithOpts(f *cmdutils.Factory, args []string, opts *gitlab.GetMergeRequestsOptions) (*gitlab.MergeRequest, glrepo.Interface, error) {
func MRFromArgsWithOpts(
f *cmdutils.Factory,
args []string,
opts *gitlab.GetMergeRequestsOptions,
state string,
) (*gitlab.MergeRequest, glrepo.Interface, error) {
var mrID int
var mr *gitlab.MergeRequest

Expand Down Expand Up @@ -146,7 +151,7 @@ func MRFromArgsWithOpts(f *cmdutils.Factory, args []string, opts *gitlab.GetMerg
}

if mrID == 0 {
mr, err = GetOpenMRForBranch(apiClient, baseRepo, branch)
mr, err = getMRForBranch(apiClient, baseRepo, branch, state)
if err != nil {
return nil, nil, err
}
Expand All @@ -160,15 +165,15 @@ func MRFromArgsWithOpts(f *cmdutils.Factory, args []string, opts *gitlab.GetMerg
return mr, baseRepo, nil
}

func MRsFromArgs(f *cmdutils.Factory, args []string) ([]*gitlab.MergeRequest, glrepo.Interface, error) {
func MRsFromArgs(f *cmdutils.Factory, args []string, state string) ([]*gitlab.MergeRequest, glrepo.Interface, error) {
if len(args) <= 1 {
var arrIDs []string
if len(args) == 1 {
arrIDs = strings.Split(args[0], ",")
}
if len(arrIDs) <= 1 {
// If there are no args then try to auto-detect from the branch name
mr, baseRepo, err := MRFromArgs(f, args)
mr, baseRepo, err := MRFromArgs(f, args, state)
if err != nil {
return nil, nil, err
}
Expand All @@ -189,7 +194,7 @@ func MRsFromArgs(f *cmdutils.Factory, args []string) ([]*gitlab.MergeRequest, gl
errGroup.Go(func() error {
// fetching multiple MRs does not return many major params in the payload
// so we fetch again using the single mr endpoint
mr, _, err := MRFromArgs(f, []string{arg})
mr, _, err := MRFromArgs(f, []string{arg}, state)
if err != nil {
return err
}
Expand All @@ -204,7 +209,7 @@ func MRsFromArgs(f *cmdutils.Factory, args []string) ([]*gitlab.MergeRequest, gl

}

var GetOpenMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string) (*gitlab.MergeRequest, error) {
var getMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string, state string) (*gitlab.MergeRequest, error) {
currentBranch := arg // Assume the user is using only 'branch', not 'OWNER:branch'
var owner string

Expand All @@ -217,10 +222,18 @@ var GetOpenMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interfac
currentBranch = t[1]
}

mrs, err := api.ListMRs(apiClient, baseRepo.FullName(), &gitlab.ListProjectMergeRequestsOptions{
opts := gitlab.ListProjectMergeRequestsOptions{
SourceBranch: gitlab.String(currentBranch),
State: gitlab.String("opened"),
})
}

// Set the state value if it is not empty, if it is empty then it will look at all possible
// values, 'any' is also a descriptive keyword used in the source code that is equivalent to
// passing nothing on
if state != "" && state != "any" {
opts.State = gitlab.String(state)
}

mrs, err := api.ListMRs(apiClient, baseRepo.FullName(), &opts)
if err != nil {
return nil, fmt.Errorf("failed to get open merge request for %q: %w", currentBranch, err)
}
Expand Down
38 changes: 19 additions & 19 deletions commands/mr/mrutils/mrutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,15 @@ func Test_MRCheckErrors(t *testing.T) {
})
}

func Test_GetOpenMRForBranchFails(t *testing.T) {
func Test_getMRForBranchFails(t *testing.T) {
baseRepo := glrepo.NewWithHost("foo", "bar", "gitlab.com")

t.Run("API-call-failed", func(t *testing.T) {
api.ListMRs = func(_ *gitlab.Client, _ interface{}, _ *gitlab.ListProjectMergeRequestsOptions) ([]*gitlab.MergeRequest, error) {
return nil, errors.New("API call failed")
}

got, err := GetOpenMRForBranch(&gitlab.Client{}, baseRepo, "foo")
got, err := getMRForBranch(&gitlab.Client{}, baseRepo, "foo", "opened")
assert.Nil(t, got)
assert.EqualError(t, err, `failed to get open merge request for "foo": API call failed`)
})
Expand All @@ -207,7 +207,7 @@ func Test_GetOpenMRForBranchFails(t *testing.T) {
return []*gitlab.MergeRequest{}, nil
}

got, err := GetOpenMRForBranch(&gitlab.Client{}, baseRepo, "foo")
got, err := getMRForBranch(&gitlab.Client{}, baseRepo, "foo", "opened")
assert.Nil(t, got)
assert.EqualError(t, err, `no open merge request available for "foo"`)
})
Expand All @@ -230,13 +230,13 @@ func Test_GetOpenMRForBranchFails(t *testing.T) {
}, nil
}

got, err := GetOpenMRForBranch(&gitlab.Client{}, baseRepo, "zemzale:foo")
got, err := getMRForBranch(&gitlab.Client{}, baseRepo, "zemzale:foo", "opened")
assert.Nil(t, got)
assert.EqualError(t, err, `no open merge request available for "foo" owned by @zemzale`)
})
}

func Test_GetOpenMRForBranch(t *testing.T) {
func Test_getMRForBranch(t *testing.T) {
baseRepo := glrepo.NewWithHost("foo", "bar", "gitlab.com")

testCases := []struct {
Expand Down Expand Up @@ -293,7 +293,7 @@ func Test_GetOpenMRForBranch(t *testing.T) {
return tC.mrs, nil
}

got, err := GetOpenMRForBranch(&gitlab.Client{}, baseRepo, tC.input)
got, err := getMRForBranch(&gitlab.Client{}, baseRepo, tC.input, "opened")
assert.NoError(t, err)

assert.Equal(t, tC.expect.IID, got.IID)
Expand All @@ -302,7 +302,7 @@ func Test_GetOpenMRForBranch(t *testing.T) {
}
}

func Test_GetOpenMRForBranchPrompt(t *testing.T) {
func Test_getMRForBranchPrompt(t *testing.T) {
baseRepo := glrepo.NewWithHost("foo", "bar", "gitlab.com")

api.ListMRs = func(_ *gitlab.Client, _ interface{}, _ *gitlab.ListProjectMergeRequestsOptions) ([]*gitlab.MergeRequest, error) {
Expand Down Expand Up @@ -333,7 +333,7 @@ func Test_GetOpenMRForBranchPrompt(t *testing.T) {
},
})

got, err := GetOpenMRForBranch(&gitlab.Client{}, baseRepo, "foo")
got, err := getMRForBranch(&gitlab.Client{}, baseRepo, "foo", "opened")
assert.NoError(t, err)

assert.Equal(t, 1, got.IID)
Expand All @@ -351,7 +351,7 @@ func Test_GetOpenMRForBranchPrompt(t *testing.T) {
},
})

got, err := GetOpenMRForBranch(&gitlab.Client{}, baseRepo, "foo")
got, err := getMRForBranch(&gitlab.Client{}, baseRepo, "foo", "opened")
assert.Nil(t, got)
assert.EqualError(t, err, "a merge request must be picked: prompt failed")
})
Expand Down Expand Up @@ -382,7 +382,7 @@ func Test_MRFromArgsWithOpts(t *testing.T) {
t.Skipf("failed to get base repo: %s", err)
}

gotMR, gotRepo, err := MRFromArgs(&f, []string{"2"})
gotMR, gotRepo, err := MRFromArgs(&f, []string{"2"}, "")
assert.NoError(t, err)

assert.Equal(t, expectedRepo.FullName(), gotRepo.FullName())
Expand All @@ -394,7 +394,7 @@ func Test_MRFromArgsWithOpts(t *testing.T) {
t.Run("via-name", func(t *testing.T) {
f := *f

GetOpenMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string) (*gitlab.MergeRequest, error) {
getMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string, state string) (*gitlab.MergeRequest, error) {
return &gitlab.MergeRequest{
IID: 2,
Title: "test mr",
Expand All @@ -415,7 +415,7 @@ func Test_MRFromArgsWithOpts(t *testing.T) {
t.Skipf("failed to get base repo: %s", err)
}

gotMR, gotRepo, err := MRFromArgs(&f, []string{"foo"})
gotMR, gotRepo, err := MRFromArgs(&f, []string{"foo"}, "")
assert.NoError(t, err)

assert.Equal(t, expectedRepo.FullName(), gotRepo.FullName())
Expand All @@ -432,7 +432,7 @@ func Test_MRFromArgsWithOpts(t *testing.T) {

f.HttpClient = func() (*gitlab.Client, error) { return nil, errors.New("failed to create HttpClient") }

gotMR, gotRepo, err := MRFromArgs(&f, []string{})
gotMR, gotRepo, err := MRFromArgs(&f, []string{}, "")
assert.Nil(t, gotMR)
assert.Nil(t, gotRepo)
assert.EqualError(t, err, "failed to create HttpClient")
Expand All @@ -442,7 +442,7 @@ func Test_MRFromArgsWithOpts(t *testing.T) {

f.BaseRepo = func() (glrepo.Interface, error) { return nil, errors.New("failed to create glrepo.Interface") }

gotMR, gotRepo, err := MRFromArgs(&f, []string{})
gotMR, gotRepo, err := MRFromArgs(&f, []string{}, "")
assert.Nil(t, gotMR)
assert.Nil(t, gotRepo)
assert.EqualError(t, err, "failed to create glrepo.Interface")
Expand All @@ -452,27 +452,27 @@ func Test_MRFromArgsWithOpts(t *testing.T) {

f.Branch = func() (string, error) { return "", errors.New("failed to get Branch") }

gotMR, gotRepo, err := MRFromArgs(&f, []string{})
gotMR, gotRepo, err := MRFromArgs(&f, []string{}, "")
assert.Nil(t, gotMR)
assert.Nil(t, gotRepo)
assert.EqualError(t, err, "failed to get Branch")
})
t.Run("Invalid-MR-ID", func(t *testing.T) {
f := *f

gotMR, gotRepo, err := MRFromArgs(&f, []string{"0"})
gotMR, gotRepo, err := MRFromArgs(&f, []string{"0"}, "")
assert.Nil(t, gotMR)
assert.Nil(t, gotRepo)
assert.EqualError(t, err, "invalid merge request ID provided")
})
t.Run("invalid-name", func(t *testing.T) {
f := *f

GetOpenMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string) (*gitlab.MergeRequest, error) {
getMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string, state string) (*gitlab.MergeRequest, error) {
return nil, fmt.Errorf("no merge requests from branch %q", arg)
}

gotMR, gotRepo, err := MRFromArgs(&f, []string{"foo"})
gotMR, gotRepo, err := MRFromArgs(&f, []string{"foo"}, "")
assert.Nil(t, gotMR)
assert.Nil(t, gotRepo)
assert.EqualError(t, err, `no merge requests from branch "foo"`)
Expand All @@ -485,7 +485,7 @@ func Test_MRFromArgsWithOpts(t *testing.T) {
return nil, errors.New("API call failed")
}

gotMR, gotRepo, err := MRFromArgs(&f, []string{"2"})
gotMR, gotRepo, err := MRFromArgs(&f, []string{"2"}, "")
assert.Nil(t, gotMR)
assert.Nil(t, gotRepo)
assert.EqualError(t, err, "failed to get merge request 2: API call failed")
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/note/mr_note_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func NewCmdNote(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/rebase/mr_rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewCmdRebase(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "opened")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/reopen/mr_reopen.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewCmdReopen(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "closed")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/revoke/mr_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewCmdRevoke(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "opened")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/subscribe/mr_subscribe.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewCmdSubscribe(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/todo/mr_todo.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewCmdTodo(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/unsubscribe/mr_unsubscribe.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewCmdUnsubscribe(f *cmdutils.Factory) *cobra.Command {
return err
}

mrs, repo, err := mrutils.MRsFromArgs(f, args)
mrs, repo, err := mrutils.MRsFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion commands/mr/update/mr_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func NewCmdUpdate(f *cmdutils.Factory) *cobra.Command {
return err
}

mr, repo, err := mrutils.MRFromArgs(f, args)
mr, repo, err := mrutils.MRFromArgs(f, args, "any")
if err != nil {
return err
}
Expand Down
Loading