Avoid re-requesting reviewers in case of CODEOWNERS

Requesting a review via `hub pull-request -r foo` would print a
misleading error message that `foo` couldn't be requested for review in
case `foo` was already requested per CODEOWNERS.

Fixes #1840

commands/pull_request.go

@@ -400,16 +400,21 @@ of text is the title and the rest is the description.`, fullBase, fullHead))
 			teamReviewers := []string{}
 			for _, reviewer := range flagPullRequestReviewers {
 				if strings.Contains(reviewer, "/") {
-					teamReviewers = append(teamReviewers, strings.SplitN(reviewer, "/", 2)[1])
-				} else {
+					teamName := strings.SplitN(reviewer, "/", 2)[1]
+					if !pr.HasRequestedTeam(teamName) {
+						teamReviewers = append(teamReviewers, teamName)
+					}
+				} else if !pr.HasRequestedReviewer(reviewer) {
 					userReviewers = append(userReviewers, reviewer)
 				}
 			}
-			err = client.RequestReview(baseProject, pr.Number, map[string]interface{}{
-				"reviewers":      userReviewers,
-				"team_reviewers": teamReviewers,
-			})
-			utils.Check(err)
+			if len(userReviewers) > 0 || len(teamReviewers) > 0 {
+				err = client.RequestReview(baseProject, pr.Number, map[string]interface{}{
+					"reviewers":      userReviewers,
+					"team_reviewers": teamReviewers,
+				})
+				utils.Check(err)
+			}
 		}
 	}
 

features/pull_request.feature

@@ -865,6 +865,43 @@ Feature: hub pull-request
     When I successfully run `hub pull-request -m hereyougo -r mislav,josh -rgithub/robots -rpcorpet -r github/js`
     Then the output should contain exactly "the://url\n"
 
+  Scenario: Pull request with reviewers from CODEOWNERS
+    Given I am on the "feature" branch with upstream "origin/feature"
+    Given the GitHub API server:
+      """
+      post('/repos/mislav/coral/pulls') {
+        assert :head  => "mislav:feature"
+        status 201
+        json :html_url => "the://url", :number => 1234,
+          :requested_reviewers => [{ :login => "josh" }],
+          :requested_teams => [{ :name => "robots" }]
+      }
+      post('/repos/mislav/coral/pulls/1234/requested_reviewers') {
+        halt 415 unless request.accept?('application/vnd.github.thor-preview+json')
+        assert :reviewers => ["mislav", "pcorpet"]
+        assert :team_reviewers => ["js"]
+        status 201
+        json :html_url => "the://url"
+      }
+      """
+    When I successfully run `hub pull-request -m hereyougo -r mislav,josh -rgithub/robots -rpcorpet -r github/js`
+    Then the output should contain exactly "the://url\n"
+
+  Scenario: Pull request avoids re-requesting reviewers
+    Given I am on the "feature" branch with upstream "origin/feature"
+    Given the GitHub API server:
+      """
+      post('/repos/mislav/coral/pulls') {
+        assert :head  => "mislav:feature"
+        status 201
+        json :html_url => "the://url", :number => 1234,
+          :requested_reviewers => [{ :login => "josh" }, { :login => "mislav" }],
+          :requested_teams => [{ :name => "robots" }]
+      }
+      """
+    When I successfully run `hub pull-request -m hereyougo -r mislav,josh -rgithub/robots`
+    Then the output should contain exactly "the://url\n"
+
   Scenario: Requesting reviewers failed
     Given I am on the "feature" branch with upstream "origin/feature"
     Given the GitHub API server:

github/client.go

@@ -551,6 +551,9 @@ type Issue struct {
 	CreatedAt time.Time    `json:"created_at"`
 	UpdatedAt time.Time    `json:"updated_at"`
 
+	RequestedReviewers []User `json:"requested_reviewers"`
+	RequestedTeams     []Team `json:"requested_teams"`
+
 	ApiUrl  string `json:"url"`
 	HtmlUrl string `json:"html_url"`
 }
@@ -570,6 +573,24 @@ func (pr *PullRequest) IsSameRepo() bool {
 		pr.Head.Repo.Owner.Login == pr.Base.Repo.Owner.Login
 }
 
+func (pr *PullRequest) HasRequestedReviewer(name string) bool {
+	for _, user := range pr.RequestedReviewers {
+		if strings.EqualFold(user.Login, name) {
+			return true
+		}
+	}
+	return false
+}
+
+func (pr *PullRequest) HasRequestedTeam(name string) bool {
+	for _, team := range pr.RequestedTeams {
+		if strings.EqualFold(team.Name, name) {
+			return true
+		}
+	}
+	return false
+}
+
 type IssueLabel struct {
 	Name  string `json:"name"`
 	Color string `json:"color"`
@@ -579,6 +600,10 @@ type User struct {
 	Login string `json:"login"`
 }
 
+type Team struct {
+	Name string `json:"name"`
+}
+
 type Milestone struct {
 	Number int    `json:"number"`
 	Title  string `json:"title"`