diff --git a/commands/mr/approvers/mr_approvers.go b/commands/mr/approvers/mr_approvers.go index 70f836413f453e3f076da086da97d10b0ae7c23a..4ab7382cfe61ff681f9ed4df8dc46a05bb7f053e 100644 --- a/commands/mr/approvers/mr_approvers.go +++ b/commands/mr/approvers/mr_approvers.go @@ -4,29 +4,27 @@ import ( "fmt" "github.com/profclems/glab/api" - "github.com/profclems/glab/commands/mr/mrutils" - "github.com/profclems/glab/pkg/tableprinter" - "github.com/profclems/glab/commands/cmdutils" + "github.com/profclems/glab/commands/mr/mrutils" "github.com/spf13/cobra" - "github.com/xanzy/go-gitlab" ) func NewCmdApprovers(f *cmdutils.Factory) *cobra.Command { var mrApproversCmd = &cobra.Command{ Use: "approvers [ | ] [flags]", - Short: `List merge request eligible approvers`, + Short: `List eligible approvers for merge requests in any state`, Long: ``, Aliases: []string{}, - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - c := f.IO.Color() apiClient, err := f.HttpClient() if err != nil { return err } - mr, repo, err := mrutils.MRFromArgs(f, args, "opened") + // Obtain the MR from the positional arguments, but allow users to find approvers for + // merge requests in any valid state + mr, repo, err := mrutils.MRFromArgs(f, args, "any") if err != nil { return err } @@ -37,43 +35,9 @@ func NewCmdApprovers(f *cmdutils.Factory) *cobra.Command { if err != nil { return err } - if mrApprovals.ApprovalRulesOverwritten { - fmt.Fprintln(f.IO.StdOut, c.Yellow("Approval rules overwritten")) - } - for _, rule := range mrApprovals.Rules { - table := tableprinter.NewTablePrinter() - if rule.Approved { - fmt.Fprintln(f.IO.StdOut, c.Green(fmt.Sprintf("Rule %q sufficient approvals (%d/%d required):", rule.Name, len(rule.ApprovedBy), rule.ApprovalsRequired))) - } else { - fmt.Fprintln(f.IO.StdOut, c.Yellow(fmt.Sprintf("Rule %q insufficient approvals (%d/%d required):", rule.Name, len(rule.ApprovedBy), rule.ApprovalsRequired))) - } - eligibleApprovers := rule.EligibleApprovers + mrutils.PrintMRApprovalState(f.IO, mrApprovals) - approvedBy := map[string]*gitlab.BasicUser{} - for _, by := range rule.ApprovedBy { - approvedBy[by.Username] = by - } - - for _, eligibleApprover := range eligibleApprovers { - approved := "-" - source := "" - if _, exists := approvedBy[eligibleApprover.Username]; exists { - approved = "👍" - } - if rule.SourceRule != nil { - source = rule.SourceRule.RuleType - } - table.AddRow(eligibleApprover.Name, eligibleApprover.Username, approved, source) - delete(approvedBy, eligibleApprover.Username) - } - - for _, approver := range approvedBy { - approved := "👍" - table.AddRow(approver.Name, approver.Username, approved, "") - } - fmt.Fprintln(f.IO.StdOut, table) - } return nil }, } diff --git a/commands/mr/mrutils/mrutils.go b/commands/mr/mrutils/mrutils.go index 04ef79db4567a2604d488df2ad151a55dcf7a7a2..c29a773b1e0e568f5fb4a8214f278e12e110fca7 100644 --- a/commands/mr/mrutils/mrutils.go +++ b/commands/mr/mrutils/mrutils.go @@ -117,12 +117,12 @@ func DisplayAllMRs(streams *iostreams.IOStreams, mrs []*gitlab.MergeRequest, pro return table.Render() } -//MRFromArgs is wrapper around MRFromArgsWithOpts without any custom options +// MRFromArgs is wrapper around MRFromArgsWithOpts without any custom options 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 +// MRFromArgsWithOpts gets MR with custom request options passed down to it func MRFromArgsWithOpts( f *cmdutils.Factory, args []string, @@ -216,7 +216,6 @@ func MRsFromArgs(f *cmdutils.Factory, args []string, state string) ([]*gitlab.Me return nil, nil, err } return mrs, baseRepo, nil - } var getMRForBranch = func(apiClient *gitlab.Client, baseRepo glrepo.Interface, arg string, state string) (*gitlab.MergeRequest, error) { @@ -328,3 +327,48 @@ func RebaseMR(ios *iostreams.IOStreams, apiClient *gitlab.Client, repo glrepo.In fmt.Fprintln(ios.StdOut, ios.Color().GreenCheck(), "Rebase successful") return nil } + +// PrintMRApprovalState renders an output to summarize the approval state of a merge request +func PrintMRApprovalState(ios *iostreams.IOStreams, mrApprovals *gitlab.MergeRequestApprovalState) { + const approvedIcon = "👍" + + c := ios.Color() + + if mrApprovals.ApprovalRulesOverwritten { + fmt.Fprintln(ios.StdOut, c.Yellow("Approval rules overwritten")) + } + for _, rule := range mrApprovals.Rules { + table := tableprinter.NewTablePrinter() + if rule.Approved { + fmt.Fprintln(ios.StdOut, c.Green(fmt.Sprintf("Rule %q sufficient approvals (%d/%d required):", rule.Name, len(rule.ApprovedBy), rule.ApprovalsRequired))) + } else { + fmt.Fprintln(ios.StdOut, c.Yellow(fmt.Sprintf("Rule %q insufficient approvals (%d/%d required):", rule.Name, len(rule.ApprovedBy), rule.ApprovalsRequired))) + } + + eligibleApprovers := rule.EligibleApprovers + + approvedBy := map[string]*gitlab.BasicUser{} + for _, by := range rule.ApprovedBy { + approvedBy[by.Username] = by + } + + for _, eligibleApprover := range eligibleApprovers { + approved := "-" + source := "" + if _, exists := approvedBy[eligibleApprover.Username]; exists { + approved = approvedIcon + } + if rule.SourceRule != nil { + source = rule.SourceRule.RuleType + } + table.AddRow(eligibleApprover.Name, eligibleApprover.Username, approved, source) + delete(approvedBy, eligibleApprover.Username) + } + + for _, approver := range approvedBy { + approved := approvedIcon + table.AddRow(approver.Name, approver.Username, approved, "") + } + fmt.Fprintln(ios.StdOut, table) + } +} diff --git a/commands/mr/mrutils/mrutils_test.go b/commands/mr/mrutils/mrutils_test.go index 6c2daa826b2820da8c854d5853b75e63f6ae7c52..cc33c3d433d6eb893c30387062b0781f28139dca 100644 --- a/commands/mr/mrutils/mrutils_test.go +++ b/commands/mr/mrutils/mrutils_test.go @@ -543,3 +543,154 @@ func Test_DisplayAllMRs(t *testing.T) { got := DisplayAllMRs(streams, mrs, "unused") assert.Equal(t, expected, got) } + +func Test_PrintMRApprovalState(t *testing.T) { + scenarios := []struct { + name string + approvalState *gitlab.MergeRequestApprovalState + expected string + }{ + { + name: "approved, min approvers met", + approvalState: &gitlab.MergeRequestApprovalState{ + Rules: []*gitlab.MergeRequestApprovalRule{ + { + ID: 1, + Name: "rule 1", + ApprovalsRequired: 2, + ApprovedBy: []*gitlab.BasicUser{ + { + ID: 1, + Username: "user1", + Name: "User One", + }, + { + ID: 2, + Username: "user2", + Name: "User Two", + }, + }, + Approved: true, + }, + }, + }, + expected: `Rule "rule 1" sufficient approvals (2/2 required): +User One user1 👍 +User Two user2 👍 + +`, + }, + { + name: "not-approved, min approvers not met", + approvalState: &gitlab.MergeRequestApprovalState{ + Rules: []*gitlab.MergeRequestApprovalRule{ + { + ID: 1, + Name: "rule 1", + ApprovalsRequired: 2, + ApprovedBy: []*gitlab.BasicUser{ + { + ID: 1, + Username: "user1", + Name: "User One", + }, + }, + Approved: false, + }, + }, + }, + expected: `Rule "rule 1" insufficient approvals (1/2 required): +User One user1 👍 + +`, + }, + { + name: "approved, eligible approvers", + approvalState: &gitlab.MergeRequestApprovalState{ + Rules: []*gitlab.MergeRequestApprovalRule{ + { + ID: 1, + Name: "rule 1", + ApprovalsRequired: 2, + EligibleApprovers: []*gitlab.BasicUser{ + { + ID: 1, + Username: "user1", + Name: "User One", + }, + { + ID: 2, + Username: "user2", + Name: "User Two", + }, + }, + ApprovedBy: []*gitlab.BasicUser{ + { + ID: 1, + Username: "user1", + Name: "User One", + }, + { + ID: 2, + Username: "user2", + Name: "User Two", + }, + }, + Approved: true, + }, + }, + }, + expected: `Rule "rule 1" sufficient approvals (2/2 required): +User One user1 👍 +User Two user2 👍 + +`, + }, + { + name: "not approved, missing eligible approver", + approvalState: &gitlab.MergeRequestApprovalState{ + Rules: []*gitlab.MergeRequestApprovalRule{ + { + ID: 1, + Name: "rule 1", + ApprovalsRequired: 2, + EligibleApprovers: []*gitlab.BasicUser{ + { + ID: 1, + Username: "user1", + Name: "User One", + }, + { + ID: 2, + Username: "user2", + Name: "User Two", + }, + }, + ApprovedBy: []*gitlab.BasicUser{ + { + ID: 1, + Username: "user1", + Name: "User One", + }, + }, + Approved: false, + }, + }, + }, + expected: `Rule "rule 1" insufficient approvals (1/2 required): +User One user1 👍 +User Two user2 - + +`, + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + streams, _, stdout, _ := iostreams.Test() + + PrintMRApprovalState(streams, scenario.approvalState) + assert.Equal(t, scenario.expected, stdout.String()) + }) + } +} diff --git a/commands/mr/view/mr_view.go b/commands/mr/view/mr_view.go index 2b0ac54fe1878d3195741f8e1a548a1e8f1a1768..0f68b75911c1132da237024f9e682ef8baf1be85 100644 --- a/commands/mr/view/mr_view.go +++ b/commands/mr/view/mr_view.go @@ -51,9 +51,15 @@ func NewCmdView(f *cmdutils.Factory) *cobra.Command { if err != nil { return err } + + mrApprovals, err := api.GetMRApprovalState(apiClient, baseRepo.FullName(), mr.IID) + if err != nil { + return err + } + cfg, _ := f.Config() - if opts.OpenInBrowser { //open in browser if --web flag is specified + if opts.OpenInBrowser { // open in browser if --web flag is specified if f.IO.IsOutputTTY() { fmt.Fprintf(f.IO.StdErr, "Opening %s in your browser.\n", utils.DisplayURL(mr.WebURL)) } @@ -85,7 +91,7 @@ func NewCmdView(f *cmdutils.Factory) *cobra.Command { defer f.IO.StopPager() if f.IO.IsOutputTTY() { - return printTTYMRPreview(opts, mr, notes) + return printTTYMRPreview(opts, mr, mrApprovals, notes) } return printRawMRPreview(opts, mr) }, @@ -136,7 +142,7 @@ func mrState(c *iostreams.ColorPalette, mr *gitlab.MergeRequest) (mrState string return mrState } -func printTTYMRPreview(opts *ViewOpts, mr *gitlab.MergeRequest, notes []*gitlab.Note) error { +func printTTYMRPreview(opts *ViewOpts, mr *gitlab.MergeRequest, mrApprovals *gitlab.MergeRequestApprovalState, notes []*gitlab.Note) error { c := opts.IO.Color() out := opts.IO.StdOut mrTimeAgo := utils.TimeToPrettyTimeAgo(*mr.CreatedAt) @@ -192,6 +198,10 @@ func printTTYMRPreview(opts *ViewOpts, mr *gitlab.MergeRequest, notes []*gitlab. fmt.Fprintf(out, "%s Requires pipeline to succeed before merging\n", c.WarnIcon()) } } + if mrApprovals != nil { + fmt.Fprintln(out, c.Bold("Approvals Status:")) + mrutils.PrintMRApprovalState(opts.IO, mrApprovals) + } fmt.Fprintf(out, "%s This merge request has %s changes\n", c.GreenCheck(), c.Yellow(mr.ChangesCount)) if mr.State == "merged" && mr.MergedBy != nil { fmt.Fprintf(out, "%s The changes were merged into %s by %s %s\n", c.GreenCheck(), mr.TargetBranch, mr.MergedBy.Name, utils.TimeToPrettyTimeAgo(*mr.MergedAt))