From 154952f4191ab815a1da90736e221a53c731b905 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Sat, 15 Jul 2023 10:08:02 -0400 Subject: [PATCH 1/2] lab: Add show mr approval rules The WebUI provides a mechanism to view the status of MR approval rules on a specific MR. Add this functionality to lab. Usage: 'lab mr approval-rule 1234' One thing to note is that, unlike the WebUI, lab currently does not show the 'Commented By' data. This is because getting the data is expensive, and at this time provides little functionality. This can be added if users request it. Signed-off-by: Prarit Bhargava --- cmd/mr_approval_rules.go | 80 +++++++++++++++++++++++++++++++++++++++ internal/gitlab/gitlab.go | 10 +++++ 2 files changed, 90 insertions(+) create mode 100644 cmd/mr_approval_rules.go diff --git a/cmd/mr_approval_rules.go b/cmd/mr_approval_rules.go new file mode 100644 index 00000000..62a34b72 --- /dev/null +++ b/cmd/mr_approval_rules.go @@ -0,0 +1,80 @@ +package cmd + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc/v2" + "github.com/spf13/cobra" + lab "github.com/zaquestion/lab/internal/gitlab" + + gitlab "github.com/xanzy/go-gitlab" +) + +func mrApprovalRuleShow(approvalState *gitlab.MergeRequestApprovalState) { + for _, rule := range approvalState.Rules { + fmt.Println("Rule:", rule.Name) + if rule.Approved { + fmt.Println(" Approved: Y") + } else { + fmt.Println(" Approved:") + } + + if rule.RuleType == "regular" { + users := "" + for u, user := range rule.EligibleApprovers { + users += fmt.Sprintf("%s", user.Username) + if u != (len(rule.EligibleApprovers) - 1) { + users +="," + } + } + fmt.Println(" Approvers:", users) + } else if rule.RuleType == "any_approver" { + fmt.Println(" Approvers: All eligible users") + } + + if rule.ApprovalsRequired > 0 { + fmt.Printf(" Approvals: %d of %d\n", len(rule.ApprovedBy), rule.ApprovalsRequired) + } else { + fmt.Println(" Approvals: Optional") + } + + if len(rule.ApprovedBy) > 0 { + users := "" + for u, user := range rule.ApprovedBy { + users += fmt.Sprintf("%s", user.Username) + if u != (len(rule.ApprovedBy) - 1) { + users +="," + } + } + fmt.Println(" Approved By:", users) + } else { + fmt.Println(" Approved By:") + } + } +} + +var mrApprovalRuleCmd = &cobra.Command{ + Use: "approval-rule [remote] []", + Aliases: []string{}, + Example: heredoc.Doc(` + lab mr approval-rule 1234`), + PersistentPreRun: labPersistentPreRun, + Run: func(cmd *cobra.Command, args []string) { + rn, id, err := parseArgsWithGitBranchMR(args) + if err != nil { + log.Fatal(err) + } + + approvalState, err := lab.GetMRApprovalState(rn, int(id)) + if err != nil { + log.Fatal(err) + } + + // default, no options just show the rules + mrApprovalRuleShow(approvalState) + }, +} + +func init() { + mrCmd.AddCommand(mrApprovalRuleCmd) +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index 65cfd884..6b5e5930 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -1600,6 +1600,16 @@ func GetMRApprovalsConfiguration(projID interface{}, id int) (*gitlab.MergeReque return configuration, err } +// GetMRApprovalsState returns the current MR approval rule +func GetMRApprovalState(projID interface{}, id int) (*gitlab.MergeRequestApprovalState, error) { + state, _, err := lab.MergeRequestApprovals.GetApprovalState(projID, id) + if err != nil { + return nil, err + } + + return state, err +} + // ResolveMRDiscussion resolves a discussion (blocking thread) based on its ID func ResolveMRDiscussion(projID interface{}, mrID int, discussionID string, noteID int) (string, error) { opts := &gitlab.ResolveMergeRequestDiscussionOptions{ From 04dfcfe80475a898315c86bb24b97ed1de911036 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Sat, 15 Jul 2023 13:47:03 -0400 Subject: [PATCH 2/2] lab: Add create and delete mr approval rules Functionality exists to create and delete merge request approval rules, along with the viewing of merge request approval rules. Add the ability to create and delete merge request approval rules. Signed-off-by: Prarit Bhargava --- cmd/mr_approval_rules.go | 154 +++++++++++++++++++++++++++++--------- cmd/mr_list_test.go | 2 +- cmd/mr_note_test.go | 2 +- internal/gitlab/gitlab.go | 44 +++++++++++ 4 files changed, 164 insertions(+), 38 deletions(-) diff --git a/cmd/mr_approval_rules.go b/cmd/mr_approval_rules.go index 62a34b72..4b3b3f1f 100644 --- a/cmd/mr_approval_rules.go +++ b/cmd/mr_approval_rules.go @@ -2,6 +2,8 @@ package cmd import ( "fmt" + "os" + "strconv" "github.com/MakeNowJust/heredoc/v2" "github.com/spf13/cobra" @@ -10,46 +12,44 @@ import ( gitlab "github.com/xanzy/go-gitlab" ) -func mrApprovalRuleShow(approvalState *gitlab.MergeRequestApprovalState) { - for _, rule := range approvalState.Rules { - fmt.Println("Rule:", rule.Name) - if rule.Approved { - fmt.Println(" Approved: Y") - } else { - fmt.Println(" Approved:") - } +func mrApprovalRuleShow(rule *gitlab.MergeRequestApprovalRule) { + fmt.Println("Rule:", rule.Name) + if rule.Approved { + fmt.Println(" Approved: Y") + } else { + fmt.Println(" Approved:") + } - if rule.RuleType == "regular" { - users := "" - for u, user := range rule.EligibleApprovers { - users += fmt.Sprintf("%s", user.Username) - if u != (len(rule.EligibleApprovers) - 1) { - users +="," - } + if rule.RuleType == "regular" { + users := "" + for u, user := range rule.EligibleApprovers { + users += fmt.Sprintf("%s", user.Username) + if u != (len(rule.EligibleApprovers) - 1) { + users += "," } - fmt.Println(" Approvers:", users) - } else if rule.RuleType == "any_approver" { - fmt.Println(" Approvers: All eligible users") } + fmt.Println(" Approvers:", users) + } else if rule.RuleType == "any_approver" { + fmt.Println(" Approvers: All eligible users") + } - if rule.ApprovalsRequired > 0 { - fmt.Printf(" Approvals: %d of %d\n", len(rule.ApprovedBy), rule.ApprovalsRequired) - } else { - fmt.Println(" Approvals: Optional") - } + if rule.ApprovalsRequired > 0 { + fmt.Printf(" Approvals: %d of %d\n", len(rule.ApprovedBy), rule.ApprovalsRequired) + } else { + fmt.Println(" Approvals: Optional") + } - if len(rule.ApprovedBy) > 0 { - users := "" - for u, user := range rule.ApprovedBy { - users += fmt.Sprintf("%s", user.Username) - if u != (len(rule.ApprovedBy) - 1) { - users +="," - } + if len(rule.ApprovedBy) > 0 { + users := "" + for u, user := range rule.ApprovedBy { + users += fmt.Sprintf("%s", user.Username) + if u != (len(rule.ApprovedBy) - 1) { + users += "," } - fmt.Println(" Approved By:", users) - } else { - fmt.Println(" Approved By:") } + fmt.Println(" Approved By:", users) + } else { + fmt.Println(" Approved By:") } } @@ -57,7 +57,11 @@ var mrApprovalRuleCmd = &cobra.Command{ Use: "approval-rule [remote] []", Aliases: []string{}, Example: heredoc.Doc(` - lab mr approval-rule 1234`), + lab mr approval-rule 1234 + lab mr approval-rule 1234 --name "Fancy rule name" + lab mr approval-rule --create --name "Fancy rule name" --user "prarit" --user "zaquestion" + lab mr approval-rule --create --name "Fancy rule name" --user "prarit" --user "zaquestion" --approvals-required 1 + lab mr approval-rule --delete "Fancy rule name"`), PersistentPreRun: labPersistentPreRun, Run: func(cmd *cobra.Command, args []string) { rn, id, err := parseArgsWithGitBranchMR(args) @@ -65,16 +69,94 @@ var mrApprovalRuleCmd = &cobra.Command{ log.Fatal(err) } - approvalState, err := lab.GetMRApprovalState(rn, int(id)) + create, err := cmd.Flags().GetBool("create") if err != nil { log.Fatal(err) } + if create { + _approvalsRequired, err := cmd.Flags().GetString("approvals-required") + if err != nil { + log.Fatal(err) + } + approvalsRequired, err := strconv.Atoi(_approvalsRequired) + if err != nil { + log.Fatal(err) + } + + name, err := cmd.Flags().GetString("name") + if err != nil { + log.Fatal(err) + } + if name == "" { + fmt.Println("The --name option must be used with the --create option") + os.Exit(1) + } + + users, err := cmd.Flags().GetStringSlice("user") + if err != nil { + log.Fatal(err) + } + userIDs := getUserIDs(users) + + groups, err := cmd.Flags().GetStringSlice("group") + if err != nil { + log.Fatal(err) + } + groupIDs := getUserIDs(groups) + + rule, err := lab.CreateMRApprovalRule(rn, approvalsRequired, int(id), name, 0, groupIDs, userIDs) + if err != nil { + log.Fatal(err) + } + + mrApprovalRuleShow(rule) + return + } + + deleteRule, err := cmd.Flags().GetString("delete") + if err != nil { + log.Fatal(err) + } + if deleteRule != "" { + msg, err := lab.DeleteMRApprovalRule(rn, deleteRule, int(id)) + if err != nil { + log.Fatal(err) + } + fmt.Println(msg) + return + } + // default, no options just show the rules - mrApprovalRuleShow(approvalState) + approvalState, err := lab.GetMRApprovalState(rn, int(id)) + if err != nil { + log.Fatal(err) + } + + name, err := cmd.Flags().GetString("name") + if err != nil { + log.Fatal(err) + } + + for _, rule := range approvalState.Rules { + if name != "" { + if rule.Name != name { + continue + } + } + mrApprovalRuleShow(rule) + } }, } func init() { + mrApprovalRuleCmd.Flags().BoolP("create", "c", false, "create a new rule. See 'create:' sub-options in help") + mrApprovalRuleCmd.Flags().StringP("delete", "d", "", "delete the named rule") + mrApprovalRuleCmd.Flags().String("approvals-required", "0", "create: number of approvals required") + mrApprovalRuleCmd.Flags().StringP("name", "n", "", "create: rule name (can also be used to display a rule)") + mrApprovalRuleCmd.Flags().String("project-rule-id", "", "create: project rule id for new rule") + mrApprovalRuleCmd.Flags().StringSliceP("user", "u", []string{}, "create: approvers for new rule; can be used multiple times for different users") + mrApprovalRuleCmd.Flags().StringSliceP("group", "g", []string{}, "create: groups for new rule; can be used multiple times for different groups") + mrCmd.AddCommand(mrApprovalRuleCmd) } diff --git a/cmd/mr_list_test.go b/cmd/mr_list_test.go index 99a40e1e..68d8aa2d 100644 --- a/cmd/mr_list_test.go +++ b/cmd/mr_list_test.go @@ -154,7 +154,7 @@ func Test_mrFilterByTargetBranch(t *testing.T) { } var ( - latestCreatedTestMR = "!968 README: dummy commit for CI tests" + latestCreatedTestMR = "!1447 Test for mr approval rules" latestUpdatedTestMR = "!329 MR for assign and review commands" ) diff --git a/cmd/mr_note_test.go b/cmd/mr_note_test.go index af7f12fc..8d7f02f7 100644 --- a/cmd/mr_note_test.go +++ b/cmd/mr_note_test.go @@ -12,7 +12,7 @@ import ( ) // #3952 is not special, it's just a place to dump discussions as mr #1 filled up, long term should update the tests clean up what they create -const mrCommentSlashDiscussionDumpsterID = "3953" +const mrCommentSlashDiscussionDumpsterID = "3954" func Test_mrCreateNote(t *testing.T) { tests := []struct { diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index 6b5e5930..7e10347f 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -1610,6 +1610,50 @@ func GetMRApprovalState(projID interface{}, id int) (*gitlab.MergeRequestApprova return state, err } +// CreateMRApprovalRule creates a new approval rule for a merge request. Calling this without +// users or groups set defaults to creating an 'All Eligible Members' rule. +func CreateMRApprovalRule(projID interface{}, approvalsRequired int, mrID int, name string, projectRuleID int, groups []int, users []int) (*gitlab.MergeRequestApprovalRule, error) { + + opts := &gitlab.CreateMergeRequestApprovalRuleOptions{ + Name: &name, + ApprovalsRequired: &approvalsRequired, + ApprovalProjectRuleID: &projectRuleID, + UserIDs: &users, + GroupIDs: &groups, + } + + rule, _, err := lab.MergeRequestApprovals.CreateApprovalRule(projID, mrID, opts) + if err != nil { + return nil, err + } + + return rule, nil +} + +// DeleteMRApprovalRule deletes an approval rule for a given MR +func DeleteMRApprovalRule(projID interface{}, name string, mrID int) (string, error) { + approvalState, err := GetMRApprovalState(projID, mrID) + if err != nil { + return "", err + } + + nameID := 0 + for r, rule := range approvalState.Rules { + if rule.Name == name { + nameID = rule.ID + break + } + if r == (len(approvalState.Rules) + 1) { + return "", fmt.Errorf("Could not find '%s' rule for MR !%d", name, mrID) + } + } + _, err = lab.MergeRequestApprovals.DeleteApprovalRule(projID, mrID, nameID) + if err != nil { + return "", fmt.Errorf("Rule %s not deleted for MR !%d", name, mrID) + } + return fmt.Sprintf("Rule %s deleted for MR !%d", name, mrID), nil +} + // ResolveMRDiscussion resolves a discussion (blocking thread) based on its ID func ResolveMRDiscussion(projID interface{}, mrID int, discussionID string, noteID int) (string, error) { opts := &gitlab.ResolveMergeRequestDiscussionOptions{