From 508f6787f0f1bd2bd50f232d9026381bb00a9f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Nov 2019 20:52:34 +0100 Subject: [PATCH] Have PullRequestForBranch accept "owner:branch" value for forks When on a `patch-1` branch locally, `gh pr view` would happily open the first open PR it finds with "patch-1" as its head, even those coming from forks. --- api/queries_pr.go | 66 +++++++++++++++++++++-------------- command/pr.go | 6 ++-- command/pr_test.go | 6 ++-- test/fixtures/prStatus.json | 12 ++++--- test/fixtures/prView.json | 68 +++++++++++++------------------------ 5 files changed, 77 insertions(+), 81 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 2be64e46c..83104db29 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -2,6 +2,7 @@ package api import ( "fmt" + "strings" ) type PullRequestsPayload struct { @@ -141,7 +142,6 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st title url headRefName - headRefName headRepositoryOwner { login } @@ -172,7 +172,7 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st } query($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - pullRequests(headRefName: $headRefName, states: OPEN, first: 1) { + pullRequests(headRefName: $headRefName, states: OPEN, first: $per_page) { edges { node { ...prWithReviews @@ -209,12 +209,17 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st viewerQuery := fmt.Sprintf("repo:%s/%s state:open is:pr author:%s", owner, repo, currentUsername) reviewerQuery := fmt.Sprintf("repo:%s/%s state:open review-requested:%s", owner, repo, currentUsername) + branchWithoutOwner := currentBranch + if idx := strings.Index(currentBranch, ":"); idx >= 0 { + branchWithoutOwner = currentBranch[idx+1:] + } + variables := map[string]interface{}{ "viewerQuery": viewerQuery, "reviewerQuery": reviewerQuery, "owner": owner, "repo": repo, - "headRefName": currentBranch, + "headRefName": branchWithoutOwner, } var resp response @@ -235,7 +240,9 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st var currentPR *PullRequest for _, edge := range resp.Repository.PullRequests.Edges { - currentPR = &edge.Node + if edge.Node.HeadLabel() == currentBranch { + currentPR = &edge.Node + } } payload := PullRequestsPayload{ @@ -289,36 +296,42 @@ func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, return &resp.Repository.PullRequest, nil } -func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRequest, error) { +func PullRequestForBranch(client *Client, ghRepo Repo, branch string) (*PullRequest, error) { type response struct { Repository struct { PullRequests struct { - Edges []struct { - Node PullRequest - } + Nodes []PullRequest } } } query := ` - query($owner: String!, $repo: String!, $headRefName: String!) { - repository(owner: $owner, name: $repo) { - pullRequests(headRefName: $headRefName, states: OPEN, first: 1) { - edges { - node { - number - title - url - } - } - } - } - }` + query($owner: String!, $repo: String!, $headRefName: String!) { + repository(owner: $owner, name: $repo) { + pullRequests(headRefName: $headRefName, states: OPEN, first: 30) { + nodes { + number + title + url + headRefName + headRepositoryOwner { + login + } + isCrossRepository + } + } + } + }` + + branchWithoutOwner := branch + if idx := strings.Index(branch, ":"); idx >= 0 { + branchWithoutOwner = branch[idx+1:] + } variables := map[string]interface{}{ "owner": ghRepo.RepoOwner(), "repo": ghRepo.RepoName(), - "headRefName": branch, + "headRefName": branchWithoutOwner, } var resp response @@ -327,12 +340,13 @@ func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRe return nil, err } - prs := []PullRequest{} - for _, edge := range resp.Repository.PullRequests.Edges { - prs = append(prs, edge.Node) + for _, pr := range resp.Repository.PullRequests.Nodes { + if pr.HeadLabel() == branch { + return &pr, nil + } } - return prs, nil + return nil, fmt.Errorf("no open pull requests found for branch %q", branch) } func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{}) (*PullRequest, error) { diff --git a/command/pr.go b/command/pr.go index ab1ad4ad5..64cd6b871 100644 --- a/command/pr.go +++ b/command/pr.go @@ -243,13 +243,11 @@ func prView(cmd *cobra.Command, args []string) error { return err } - prs, err := api.PullRequestsForBranch(apiClient, baseRepo, currentBranch) + pr, err := api.PullRequestForBranch(apiClient, baseRepo, currentBranch) if err != nil { return err - } else if len(prs) < 1 { - return fmt.Errorf("the '%s' branch has no open pull requests", currentBranch) } - openURL = prs[0].URL + openURL = pr.URL } fmt.Printf("Opening %s in your browser.\n", openURL) diff --git a/command/pr_test.go b/command/pr_test.go index 47851a5e8..24cc14735 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -22,7 +22,7 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } func TestPRStatus(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() jsonFile, _ := os.Open("../test/fixtures/prStatus.json") @@ -100,7 +100,7 @@ func TestPRList_filtering(t *testing.T) { } func TestPRView(t *testing.T) { - initBlankContext("OWNER/REPO", "master") + initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() jsonFile, _ := os.Open("../test/fixtures/prView.json") @@ -148,7 +148,7 @@ func TestPRView_NoActiveBranch(t *testing.T) { defer restoreCmd() output, err := test.RunCommand(RootCmd, "pr view") - if err == nil || err.Error() != "the 'master' branch has no open pull requests" { + if err == nil || err.Error() != `no open pull requests found for branch "master"` { t.Errorf("error running command `pr view`: %v", err) } diff --git a/test/fixtures/prStatus.json b/test/fixtures/prStatus.json index 444fa5e01..f6cd8d1cd 100644 --- a/test/fixtures/prStatus.json +++ b/test/fixtures/prStatus.json @@ -7,7 +7,11 @@ "number": 10, "title": "Blueberries are a good fruit", "url": "https://github.com/github/gh-cli/pull/10", - "headRefName": "[blueberries]" + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "OWNER" + }, + "isCrossRepository": false } } ] @@ -20,7 +24,7 @@ "number": 8, "title": "Strawberries are not actually berries", "url": "https://github.com/github/gh-cli/pull/8", - "headRefName": "[strawberries]" + "headRefName": "strawberries" } } ], @@ -33,7 +37,7 @@ "number": 9, "title": "Apples are tasty", "url": "https://github.com/github/gh-cli/pull/9", - "headRefName": "[apples]" + "headRefName": "apples" } }, { @@ -41,7 +45,7 @@ "number": 11, "title": "Figs are my favorite", "url": "https://github.com/github/gh-cli/pull/1", - "headRefName": "[figs]" + "headRefName": "figs" } } ], diff --git a/test/fixtures/prView.json b/test/fixtures/prView.json index 5ac892498..c8ae23ce6 100644 --- a/test/fixtures/prView.json +++ b/test/fixtures/prView.json @@ -1,50 +1,30 @@ -{"data":{ - "repository": { - "pullRequests": { - "edges": [ - { - "node": { +{ + "data": { + "repository": { + "pullRequests": { + "nodes": [ + { + "number": 12, + "title": "Blueberries are from a fork", + "url": "https://github.com/OWNER/REPO/pull/12", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "hubot" + }, + "isCrossRepository": true + }, + { "number": 10, "title": "Blueberries are a good fruit", "url": "https://github.com/OWNER/REPO/pull/10", - "headRefName": "[blueberries]" + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "OWNER" + }, + "isCrossRepository": false } - } - ] + ] + } } - }, - "viewerCreated": { - "edges": [ - { - "node": { - "number": 8, - "title": "Strawberries are not actually berries", - "url": "https://github.com/OWNER/REPO/pull/8", - "headRefName": "[strawberries]" - } - } - ], - "pageInfo": { "hasNextPage": false } - }, - "reviewRequested": { - "edges": [ - { - "node": { - "number": 9, - "title": "Apples are tasty", - "url": "https://github.com/OWNER/REPO/pull/9", - "headRefName": "[apples]" - } - }, - { - "node": { - "number": 11, - "title": "Figs are my favorite", - "url": "https://github.com/OWNER/REPO/pull/1", - "headRefName": "[figs]" - } - } - ], - "pageInfo": { "hasNextPage": false } } -}} \ No newline at end of file +}