Merge pull request #96 from github/pr-current-branch

Improve detecting PR for the current branch
This commit is contained in:
Mislav Marohnić 2019-12-02 20:46:20 +01:00 committed by GitHub
commit ad120874b9
7 changed files with 259 additions and 113 deletions

View file

@ -2,6 +2,7 @@ package api
import (
"fmt"
"strings"
)
type PullRequestsPayload struct {
@ -116,7 +117,7 @@ type Repo interface {
RepoOwner() string
}
func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername string) (*PullRequestsPayload, error) {
func PullRequests(client *Client, ghRepo Repo, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) {
type edges struct {
Edges []struct {
Node PullRequest
@ -130,18 +131,18 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st
type response struct {
Repository struct {
PullRequests edges
PullRequest *PullRequest
}
ViewerCreated edges
ReviewRequested edges
}
query := `
fragments := `
fragment pr on PullRequest {
number
title
url
headRefName
headRefName
headRepositoryOwner {
login
}
@ -170,16 +171,32 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st
...pr
reviewDecision
}
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) {
edges {
node {
...prWithReviews
}
}
}
}
`
queryPrefix := `
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: $per_page) {
edges {
node {
...prWithReviews
}
}
}
}
`
if currentPRNumber > 0 {
queryPrefix = `
query($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $number) {
...prWithReviews
}
}
`
}
query := fragments + queryPrefix + `
viewerCreated: search(query: $viewerQuery, type: ISSUE, first: $per_page) {
edges {
node {
@ -201,7 +218,7 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st
}
}
}
`
`
owner := ghRepo.RepoOwner()
repo := ghRepo.RepoName()
@ -209,12 +226,18 @@ 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 := currentPRHeadRef
if idx := strings.Index(currentPRHeadRef, ":"); idx >= 0 {
branchWithoutOwner = currentPRHeadRef[idx+1:]
}
variables := map[string]interface{}{
"viewerQuery": viewerQuery,
"reviewerQuery": reviewerQuery,
"owner": owner,
"repo": repo,
"headRefName": currentBranch,
"headRefName": branchWithoutOwner,
"number": currentPRNumber,
}
var resp response
@ -233,9 +256,13 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st
reviewRequested = append(reviewRequested, edge.Node)
}
var currentPR *PullRequest
for _, edge := range resp.Repository.PullRequests.Edges {
currentPR = &edge.Node
var currentPR = resp.Repository.PullRequest
if currentPR == nil {
for _, edge := range resp.Repository.PullRequests.Edges {
if edge.Node.HeadLabel() == currentPRHeadRef {
currentPR = &edge.Node
}
}
}
payload := PullRequestsPayload{
@ -289,36 +316,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 +360,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) {

View file

@ -5,9 +5,12 @@ import (
"io"
"os"
"os/exec"
"regexp"
"strconv"
"strings"
"github.com/github/gh-cli/api"
"github.com/github/gh-cli/context"
"github.com/github/gh-cli/git"
"github.com/github/gh-cli/utils"
"github.com/spf13/cobra"
@ -65,7 +68,7 @@ func prStatus(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
currentBranch, err := ctx.Branch()
currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx)
if err != nil {
return err
}
@ -74,7 +77,7 @@ func prStatus(cmd *cobra.Command, args []string) error {
return err
}
prPayload, err := api.PullRequests(apiClient, baseRepo, currentBranch, currentUser)
prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser)
if err != nil {
return err
}
@ -85,7 +88,7 @@ func prStatus(cmd *cobra.Command, args []string) error {
if prPayload.CurrentPR != nil {
printPrs(out, *prPayload.CurrentPR)
} else {
message := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentBranch+"]"))
message := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))
printMessage(out, message)
}
fmt.Fprintln(out)
@ -217,28 +220,76 @@ func prView(cmd *cobra.Command, args []string) error {
return fmt.Errorf("invalid pull request number: '%s'", args[0])
}
} else {
apiClient, err := apiClientForContext(ctx)
if err != nil {
return err
}
currentBranch, err := ctx.Branch()
prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx)
if err != nil {
return err
}
prs, err := api.PullRequestsForBranch(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)
if prNumber > 0 {
openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%d", baseRepo.RepoOwner(), baseRepo.RepoName(), prNumber)
} else {
apiClient, err := apiClientForContext(ctx)
if err != nil {
return err
}
pr, err := api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner)
if err != nil {
return err
}
openURL = pr.URL
}
openURL = prs[0].URL
}
fmt.Printf("Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}
func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef string, err error) {
baseRepo, err := ctx.BaseRepo()
if err != nil {
return
}
prHeadRef, err = ctx.Branch()
if err != nil {
return
}
branchConfig := git.ReadBranchConfig(prHeadRef)
// the branch is configured to merge a special PR head ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
prNumber, _ = strconv.Atoi(m[1])
return
}
var branchOwner string
if branchConfig.RemoteURL != nil {
// the branch merges from a remote specified by URL
if r, err := context.RepoFromURL(branchConfig.RemoteURL); err == nil {
branchOwner = r.RepoOwner()
}
} else if branchConfig.RemoteName != "" {
// the branch merges from a remote specified by name
rem, _ := ctx.Remotes()
if r, err := rem.FindByName(branchConfig.RemoteName); err == nil {
branchOwner = r.RepoOwner()
}
}
if branchOwner != "" {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
if !strings.EqualFold(branchOwner, baseRepo.RepoOwner()) {
prHeadRef = fmt.Sprintf("%s:%s", branchOwner, prHeadRef)
}
}
return
}
func prCheckout(cmd *cobra.Command, args []string) error {
prNumber, err := strconv.Atoi(args[0])
if err != nil {

View file

@ -8,6 +8,7 @@ import (
"os/exec"
"reflect"
"regexp"
"strings"
"testing"
"github.com/github/gh-cli/test"
@ -22,7 +23,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")
@ -99,8 +100,8 @@ func TestPRList_filtering(t *testing.T) {
eq(t, reqBody.Variables.Labels, []string{"one", "two"})
}
func TestPRView(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
func TestPRView_currentBranch(t *testing.T) {
initBlankContext("OWNER/REPO", "blueberries")
http := initFakeHTTP()
jsonFile, _ := os.Open("../test/fixtures/prView.json")
@ -109,8 +110,13 @@ func TestPRView(t *testing.T) {
var seenCmd *exec.Cmd
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
seenCmd = cmd
return &outputStub{}
switch strings.Join(cmd.Args, " ") {
case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`:
return &outputStub{}
default:
seenCmd = cmd
return &outputStub{}
}
})
defer restoreCmd()
@ -132,8 +138,8 @@ func TestPRView(t *testing.T) {
}
}
func TestPRView_NoActiveBranch(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
func TestPRView_noResultsForBranch(t *testing.T) {
initBlankContext("OWNER/REPO", "blueberries")
http := initFakeHTTP()
jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json")
@ -142,21 +148,43 @@ func TestPRView_NoActiveBranch(t *testing.T) {
var seenCmd *exec.Cmd
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
seenCmd = cmd
return &outputStub{}
switch strings.Join(cmd.Args, " ") {
case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`:
return &outputStub{}
default:
seenCmd = cmd
return &outputStub{}
}
})
defer restoreCmd()
_, 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 "blueberries"` {
t.Errorf("error running command `pr view`: %v", err)
}
if seenCmd != nil {
t.Fatalf("unexpected command: %v", seenCmd.Args)
}
}
func TestPRView_numberArg(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequest": {
"url": "https://github.com/OWNER/REPO/pull/23"
} } } }
`))
var seenCmd *exec.Cmd
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
seenCmd = cmd
return &outputStub{}
})
defer restoreCmd()
// Now run again but provide a PR number
output, err := test.RunCommand(RootCmd, "pr view 23")
if err != nil {
t.Errorf("error running command `pr view`: %v", err)

View file

@ -72,6 +72,15 @@ func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url
return
}
// RepoFromURL maps a URL to a GitHubRepository
func RepoFromURL(u *url.URL) (GitHubRepository, error) {
owner, repo, err := repoFromURL(u)
if err != nil {
return nil, err
}
return ghRepo{owner, repo}, nil
}
func repoFromURL(u *url.URL) (string, string, error) {
if !strings.EqualFold(u.Hostname(), defaultHostname) {
return "", "", fmt.Errorf("unsupported hostname: %s", u.Hostname())

View file

@ -4,8 +4,10 @@ import (
"bytes"
"fmt"
"io/ioutil"
"net/url"
"os/exec"
"path/filepath"
"regexp"
"strings"
"github.com/github/gh-cli/utils"
@ -111,6 +113,44 @@ func Push(remote string, ref string) error {
return cmd.Run()
}
type BranchConfig struct {
RemoteName string
RemoteURL *url.URL
MergeRef string
}
// ReadBranchConfig parses the `branch.BRANCH.(remote|merge)` part of git config
func ReadBranchConfig(branch string) (cfg BranchConfig) {
prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
configCmd := GitCommand("config", "--get-regexp", fmt.Sprintf("^%s(remote|merge)$", prefix))
output, err := utils.PrepareCmd(configCmd).Output()
if err != nil {
return
}
for _, line := range outputLines(output) {
parts := strings.SplitN(line, " ", 2)
if len(parts) < 2 {
continue
}
keys := strings.Split(parts[0], ".")
switch keys[len(keys)-1] {
case "remote":
if strings.Contains(parts[1], ":") {
u, err := ParseURL(parts[1])
if err != nil {
continue
}
cfg.RemoteURL = u
} else {
cfg.RemoteName = parts[1]
}
case "merge":
cfg.MergeRef = parts[1]
}
}
return
}
func outputLines(output []byte) []string {
lines := strings.TrimSuffix(string(output), "\n")
return strings.Split(lines, "\n")

View file

@ -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"
}
}
],

View file

@ -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 }
}
}}
}