From d3b11dd32e45f7abe87326c03ff2f9a587ed1147 Mon Sep 17 00:00:00 2001 From: Ryan Bonham Date: Wed, 5 Feb 2020 20:54:51 -0500 Subject: [PATCH 1/3] Add GetApprovalState For MergeRequestApprovalsService --- merge_request_approvals.go | 43 ++++++++++ merge_request_approvals_test.go | 148 ++++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+) diff --git a/merge_request_approvals.go b/merge_request_approvals.go index ff4ef8868..594d6dc0e 100644 --- a/merge_request_approvals.go +++ b/merge_request_approvals.go @@ -76,6 +76,24 @@ type MergeRequestApprovalRule struct { ContainsHiddenGroups bool `json:"contains_hidden_groups"` } +// MergeRequestApprovalState represents a GitLab merge request approval state. +// +// GitLab API docs: +// https://docs.gitlab.com/ee/api/merge_request_approvals.html#get-the-approval-state-of-merge-requests +type MergeRequestApprovalState struct { + ID int `json:"id"` + Name string `json:"name"` + RuleType string `json:"rule_type"` + EligibleApprovers []*BasicUser `json:"eligible_approvers"` + ApprovalsRequired int `json:"approvals_required"` + SourceRule *ProjectApprovalRule `json:"source_rule"` + Users []*BasicUser `json:"users"` + Groups []*Group `json:"groups"` + ContainsHiddenGroups bool `json:"contains_hidden_groups"` + ApprovedBy []*BasicUser `json:"approved_by"` + Approved bool `json:"approved"` +} + // String is a stringify for MergeRequestApprovalRule func (s MergeRequestApprovalRule) String() string { return Stringify(s) @@ -236,6 +254,31 @@ func (s *MergeRequestApprovalsService) GetApprovalRules(pid interface{}, mergeRe return par, resp, err } +// GetApprovalState requests information about a merge request’s approval state +// +// GitLab API docs: +// https://docs.gitlab.com/ee/api/merge_request_approvals.html#get-the-approval-state-of-merge-requests +func (s *MergeRequestApprovalsService) GetApprovalState(pid interface{}, mergeRequest int, options ...OptionFunc) ([]*MergeRequestApprovalState, *Response, error) { + project, err := parseID(pid) + if err != nil { + return nil, nil, err + } + u := fmt.Sprintf("projects/%s/merge_requests/%d/approval_state", pathEscape(project), mergeRequest) + + req, err := s.client.NewRequest("GET", u, nil, options) + if err != nil { + return nil, nil, err + } + + var par []*MergeRequestApprovalState + resp, err := s.client.Do(req, &par) + if err != nil { + return nil, resp, err + } + + return par, resp, err +} + // CreateMergeRequestApprovalRuleOptions represents the available CreateApprovalRule() // options. // diff --git a/merge_request_approvals_test.go b/merge_request_approvals_test.go index d5f0ce0cf..22c18c217 100644 --- a/merge_request_approvals_test.go +++ b/merge_request_approvals_test.go @@ -7,6 +7,154 @@ import ( "testing" ) +func TestGetApprovalState(t *testing.T) { + mux, server, client := setup() + defer teardown(server) + + mux.HandleFunc("/api/v4/projects/1/merge_requests/1/approval_state", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `[ + { + "id": 1, + "name": "security", + "rule_type": "regular", + "eligible_approvers": [ + { + "id": 5, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + }, + { + "id": 50, + "name": "Group Member 1", + "username": "group_member_1", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/group_member_1" + } + ], + "approvals_required": 3, + "source_rule": null, + "users": [ + { + "id": 5, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "groups": [ + { + "id": 5, + "name": "group1", + "path": "group1", + "description": "", + "visibility": "public", + "lfs_enabled": false, + "avatar_url": null, + "web_url": "http://localhost/groups/group1", + "request_access_enabled": false, + "full_name": "group1", + "full_path": "group1", + "parent_id": null, + "ldap_cn": null, + "ldap_access": null + } + ], + "contains_hidden_groups": false, + "approved_by": [ + { + "id": 5, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "approved": false + } + ]`) + }) + + approvals, _, err := client.MergeRequestApprovals.GetApprovalState(1, 1) + if err != nil { + t.Errorf("MergeRequestApprovals.GetApprovalState returned error: %v", err) + } + + want := []*MergeRequestApprovalState{ + &MergeRequestApprovalState{ + ID: 1, + Name: "security", + RuleType: "regular", + EligibleApprovers: []*BasicUser{ + &BasicUser{ + ID: 5, + Name: "John Doe", + Username: "jdoe", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/jdoe", + }, + &BasicUser{ + ID: 50, + Name: "Group Member 1", + Username: "group_member_1", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/group_member_1", + }, + }, + ApprovalsRequired: 3, + Users: []*BasicUser{ + &BasicUser{ + ID: 5, + Name: "John Doe", + Username: "jdoe", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/jdoe", + }, + }, + Groups: []*Group{ + &Group{ + ID: 5, + Name: "group1", + Path: "group1", + Description: "", + Visibility: PublicVisibility, + LFSEnabled: false, + AvatarURL: "", + WebURL: "http://localhost/groups/group1", + RequestAccessEnabled: false, + FullName: "group1", + FullPath: "group1", + }, + }, + ApprovedBy: []*BasicUser{ + &BasicUser{ + ID: 5, + Name: "John Doe", + Username: "jdoe", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/jdoe", + }, + }, + Approved: false, + }, + } + + if !reflect.DeepEqual(want, approvals) { + t.Errorf("MergeRequestApprovals.GetApprovalState returned %+v, want %+v", approvals, want) + } +} + func TestGetApprovalRules(t *testing.T) { mux, server, client := setup() defer teardown(server) From 0d8d75266bf063a6da47440f401a6c2d8e67617f Mon Sep 17 00:00:00 2001 From: Ryan Bonham Date: Wed, 5 Feb 2020 21:25:41 -0500 Subject: [PATCH 2/3] Fix Approval State --- merge_request_approvals.go | 10 ++- merge_request_approvals_test.go | 120 +++++++++++++++++--------------- 2 files changed, 71 insertions(+), 59 deletions(-) diff --git a/merge_request_approvals.go b/merge_request_approvals.go index 594d6dc0e..481bef138 100644 --- a/merge_request_approvals.go +++ b/merge_request_approvals.go @@ -81,6 +81,12 @@ type MergeRequestApprovalRule struct { // GitLab API docs: // https://docs.gitlab.com/ee/api/merge_request_approvals.html#get-the-approval-state-of-merge-requests type MergeRequestApprovalState struct { + ApprovalRulesOverwritten bool `json:"approval_rules_overwritten"` + Rules []*MergeRequestApprovalStateRule `json:"rules"` +} + +// MergeRequestApprovalStateRule represent rules as part of the MergeRequestApprovalState +type MergeRequestApprovalStateRule struct { ID int `json:"id"` Name string `json:"name"` RuleType string `json:"rule_type"` @@ -258,7 +264,7 @@ func (s *MergeRequestApprovalsService) GetApprovalRules(pid interface{}, mergeRe // // GitLab API docs: // https://docs.gitlab.com/ee/api/merge_request_approvals.html#get-the-approval-state-of-merge-requests -func (s *MergeRequestApprovalsService) GetApprovalState(pid interface{}, mergeRequest int, options ...OptionFunc) ([]*MergeRequestApprovalState, *Response, error) { +func (s *MergeRequestApprovalsService) GetApprovalState(pid interface{}, mergeRequest int, options ...OptionFunc) (*MergeRequestApprovalState, *Response, error) { project, err := parseID(pid) if err != nil { return nil, nil, err @@ -270,7 +276,7 @@ func (s *MergeRequestApprovalsService) GetApprovalState(pid interface{}, mergeRe return nil, nil, err } - var par []*MergeRequestApprovalState + var par *MergeRequestApprovalState resp, err := s.client.Do(req, &par) if err != nil { return nil, resp, err diff --git a/merge_request_approvals_test.go b/merge_request_approvals_test.go index 22c18c217..130c12c00 100644 --- a/merge_request_approvals_test.go +++ b/merge_request_approvals_test.go @@ -13,7 +13,9 @@ func TestGetApprovalState(t *testing.T) { mux.HandleFunc("/api/v4/projects/1/merge_requests/1/approval_state", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `[ + fmt.Fprint(w, `{ + "approval_rules_overwritten": true, + "rules": [ { "id": 1, "name": "security", @@ -79,7 +81,8 @@ func TestGetApprovalState(t *testing.T) { ], "approved": false } - ]`) + ] + }`) }) approvals, _, err := client.MergeRequestApprovals.GetApprovalState(1, 1) @@ -87,66 +90,69 @@ func TestGetApprovalState(t *testing.T) { t.Errorf("MergeRequestApprovals.GetApprovalState returned error: %v", err) } - want := []*MergeRequestApprovalState{ - &MergeRequestApprovalState{ - ID: 1, - Name: "security", - RuleType: "regular", - EligibleApprovers: []*BasicUser{ - &BasicUser{ - ID: 5, - Name: "John Doe", - Username: "jdoe", - State: "active", - AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", - WebURL: "http://localhost/jdoe", - }, - &BasicUser{ - ID: 50, - Name: "Group Member 1", - Username: "group_member_1", - State: "active", - AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", - WebURL: "http://localhost/group_member_1", + want := &MergeRequestApprovalState{ + ApprovalRulesOverwritten: true, + Rules: []*MergeRequestApprovalStateRule{ + &MergeRequestApprovalStateRule{ + ID: 1, + Name: "security", + RuleType: "regular", + EligibleApprovers: []*BasicUser{ + &BasicUser{ + ID: 5, + Name: "John Doe", + Username: "jdoe", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/jdoe", + }, + &BasicUser{ + ID: 50, + Name: "Group Member 1", + Username: "group_member_1", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/group_member_1", + }, }, - }, - ApprovalsRequired: 3, - Users: []*BasicUser{ - &BasicUser{ - ID: 5, - Name: "John Doe", - Username: "jdoe", - State: "active", - AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", - WebURL: "http://localhost/jdoe", + ApprovalsRequired: 3, + Users: []*BasicUser{ + &BasicUser{ + ID: 5, + Name: "John Doe", + Username: "jdoe", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/jdoe", + }, }, - }, - Groups: []*Group{ - &Group{ - ID: 5, - Name: "group1", - Path: "group1", - Description: "", - Visibility: PublicVisibility, - LFSEnabled: false, - AvatarURL: "", - WebURL: "http://localhost/groups/group1", - RequestAccessEnabled: false, - FullName: "group1", - FullPath: "group1", + Groups: []*Group{ + &Group{ + ID: 5, + Name: "group1", + Path: "group1", + Description: "", + Visibility: PublicVisibility, + LFSEnabled: false, + AvatarURL: "", + WebURL: "http://localhost/groups/group1", + RequestAccessEnabled: false, + FullName: "group1", + FullPath: "group1", + }, }, - }, - ApprovedBy: []*BasicUser{ - &BasicUser{ - ID: 5, - Name: "John Doe", - Username: "jdoe", - State: "active", - AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", - WebURL: "http://localhost/jdoe", + ApprovedBy: []*BasicUser{ + &BasicUser{ + ID: 5, + Name: "John Doe", + Username: "jdoe", + State: "active", + AvatarURL: "https://www.gravatar.com/avatar/0?s=80&d=identicon", + WebURL: "http://localhost/jdoe", + }, }, + Approved: false, }, - Approved: false, }, } From adf616ca5df241f8169b43777fe019de683677ea Mon Sep 17 00:00:00 2001 From: Ryan Bonham Date: Thu, 6 Feb 2020 08:47:15 -0500 Subject: [PATCH 3/3] MR Comments addressed. --- merge_request_approvals.go | 27 +++++++-------------------- merge_request_approvals_test.go | 4 ++-- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/merge_request_approvals.go b/merge_request_approvals.go index 481bef138..60fd52932 100644 --- a/merge_request_approvals.go +++ b/merge_request_approvals.go @@ -74,6 +74,8 @@ type MergeRequestApprovalRule struct { Users []*BasicUser `json:"users"` Groups []*Group `json:"groups"` ContainsHiddenGroups bool `json:"contains_hidden_groups"` + ApprovedBy []*BasicUser `json:"approved_by"` + Approved bool `json:"approved"` } // MergeRequestApprovalState represents a GitLab merge request approval state. @@ -81,23 +83,8 @@ type MergeRequestApprovalRule struct { // GitLab API docs: // https://docs.gitlab.com/ee/api/merge_request_approvals.html#get-the-approval-state-of-merge-requests type MergeRequestApprovalState struct { - ApprovalRulesOverwritten bool `json:"approval_rules_overwritten"` - Rules []*MergeRequestApprovalStateRule `json:"rules"` -} - -// MergeRequestApprovalStateRule represent rules as part of the MergeRequestApprovalState -type MergeRequestApprovalStateRule struct { - ID int `json:"id"` - Name string `json:"name"` - RuleType string `json:"rule_type"` - EligibleApprovers []*BasicUser `json:"eligible_approvers"` - ApprovalsRequired int `json:"approvals_required"` - SourceRule *ProjectApprovalRule `json:"source_rule"` - Users []*BasicUser `json:"users"` - Groups []*Group `json:"groups"` - ContainsHiddenGroups bool `json:"contains_hidden_groups"` - ApprovedBy []*BasicUser `json:"approved_by"` - Approved bool `json:"approved"` + ApprovalRulesOverwritten bool `json:"approval_rules_overwritten"` + Rules []*MergeRequestApprovalRule `json:"rules"` } // String is a stringify for MergeRequestApprovalRule @@ -276,13 +263,13 @@ func (s *MergeRequestApprovalsService) GetApprovalState(pid interface{}, mergeRe return nil, nil, err } - var par *MergeRequestApprovalState - resp, err := s.client.Do(req, &par) + var pas *MergeRequestApprovalState + resp, err := s.client.Do(req, &pas) if err != nil { return nil, resp, err } - return par, resp, err + return pas, resp, err } // CreateMergeRequestApprovalRuleOptions represents the available CreateApprovalRule() diff --git a/merge_request_approvals_test.go b/merge_request_approvals_test.go index 130c12c00..2f7a415dd 100644 --- a/merge_request_approvals_test.go +++ b/merge_request_approvals_test.go @@ -92,8 +92,8 @@ func TestGetApprovalState(t *testing.T) { want := &MergeRequestApprovalState{ ApprovalRulesOverwritten: true, - Rules: []*MergeRequestApprovalStateRule{ - &MergeRequestApprovalStateRule{ + Rules: []*MergeRequestApprovalRule{ + &MergeRequestApprovalRule{ ID: 1, Name: "security", RuleType: "regular",