From b223176b378e7cb023b73971dec7579fdf4a23b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Tue, 3 Dec 2019 21:41:22 +0100
Subject: [PATCH] Accept issue URL in `issue view <issue>`

Also validates that the issue passed either by number or by URL exists.
---
 api/queries_issue.go  | 32 +++++++++++++++++++
 command/issue.go      | 33 +++++++++++++++----
 command/issue_test.go | 74 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 126 insertions(+), 13 deletions(-)

diff --git a/api/queries_issue.go b/api/queries_issue.go
index 401920e1b..eb2b3b170 100644
--- a/api/queries_issue.go
+++ b/api/queries_issue.go
@@ -202,3 +202,35 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig
 
 	return resp.Repository.Issues.Nodes, nil
 }
+
+func IssueByNumber(client *Client, ghRepo Repo, number int) (*Issue, error) {
+	type response struct {
+		Repository struct {
+			Issue Issue
+		}
+	}
+
+	query := `
+	query($owner: String!, $repo: String!, $issue_number: Int!) {
+		repository(owner: $owner, name: $repo) {
+			issue(number: $issue_number) {
+				number
+				url
+			}
+		}
+	}`
+
+	variables := map[string]interface{}{
+		"owner":        ghRepo.RepoOwner(),
+		"repo":         ghRepo.RepoName(),
+		"issue_number": number,
+	}
+
+	var resp response
+	err := client.GraphQL(query, variables, &resp)
+	if err != nil {
+		return nil, err
+	}
+
+	return &resp.Repository.Issue, nil
+}
diff --git a/command/issue.go b/command/issue.go
index d8811f5c8..0dd35ca4f 100644
--- a/command/issue.go
+++ b/command/issue.go
@@ -4,10 +4,12 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 
 	"github.com/github/gh-cli/api"
+	"github.com/github/gh-cli/context"
 	"github.com/github/gh-cli/utils"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -183,19 +185,36 @@ func issueView(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-
-	var openURL string
-	if number, err := strconv.Atoi(args[0]); err == nil {
-		// TODO: move URL generation into GitHubRepository
-		openURL = fmt.Sprintf("https://github.com/%s/%s/issues/%d", baseRepo.RepoOwner(), baseRepo.RepoName(), number)
-	} else {
-		return fmt.Errorf("invalid issue number: '%s'", args[0])
+	apiClient, err := apiClientForContext(ctx)
+	if err != nil {
+		return err
 	}
 
+	issue, err := issueFromArg(apiClient, baseRepo, args[0])
+	if err != nil {
+		return err
+	}
+	openURL := issue.URL
+
 	cmd.Printf("Opening %s in your browser.\n", openURL)
 	return utils.OpenInBrowser(openURL)
 }
 
+var issueURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/issues/(\d+)`)
+
+func issueFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.Issue, error) {
+	if issueNumber, err := strconv.Atoi(arg); err == nil {
+		return api.IssueByNumber(apiClient, baseRepo, issueNumber)
+	}
+
+	if m := issueURLRE.FindStringSubmatch(arg); m != nil {
+		issueNumber, _ := strconv.Atoi(m[3])
+		return api.IssueByNumber(apiClient, baseRepo, issueNumber)
+	}
+
+	return nil, fmt.Errorf("invalid issue format: %q", arg)
+}
+
 func issueCreate(cmd *cobra.Command, args []string) error {
 	ctx := contextForCommand(cmd)
 
diff --git a/command/issue_test.go b/command/issue_test.go
index aaf5e2517..b5f8f0272 100644
--- a/command/issue_test.go
+++ b/command/issue_test.go
@@ -96,9 +96,12 @@ func TestIssueView(t *testing.T) {
 	initBlankContext("OWNER/REPO", "master")
 	http := initFakeHTTP()
 
-	jsonFile, _ := os.Open("../test/fixtures/issueView.json")
-	defer jsonFile.Close()
-	http.StubResponse(200, jsonFile)
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "data": { "repository": { "issue": {
+		"number": 123,
+		"url": "https://github.com/OWNER/REPO/issues/123"
+	} } } }
+	`))
 
 	var seenCmd *exec.Cmd
 	restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
@@ -107,7 +110,7 @@ func TestIssueView(t *testing.T) {
 	})
 	defer restoreCmd()
 
-	output, err := RunCommand(issueViewCmd, "issue view 8")
+	output, err := RunCommand(issueViewCmd, "issue view 123")
 	if err != nil {
 		t.Errorf("error running command `issue view`: %v", err)
 	}
@@ -120,9 +123,68 @@ func TestIssueView(t *testing.T) {
 		t.Fatal("expected a command to run")
 	}
 	url := seenCmd.Args[len(seenCmd.Args)-1]
-	if url != "https://github.com/OWNER/REPO/issues/8" {
-		t.Errorf("got: %q", url)
+	eq(t, url, "https://github.com/OWNER/REPO/issues/123")
+}
+
+func TestIssueView_notFound(t *testing.T) {
+	initBlankContext("OWNER/REPO", "master")
+	http := initFakeHTTP()
+
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "errors": [
+		{ "message": "Could not resolve to an Issue with the number of 9999." }
+	] }
+	`))
+
+	var seenCmd *exec.Cmd
+	restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
+		seenCmd = cmd
+		return &outputStub{}
+	})
+	defer restoreCmd()
+
+	_, err := RunCommand(issueViewCmd, "issue view 9999")
+	if err == nil || err.Error() != "graphql error: 'Could not resolve to an Issue with the number of 9999.'" {
+		t.Errorf("error running command `issue view`: %v", err)
 	}
+
+	if seenCmd != nil {
+		t.Fatal("did not expect any command to run")
+	}
+}
+
+func TestIssueView_urlArg(t *testing.T) {
+	initBlankContext("OWNER/REPO", "master")
+	http := initFakeHTTP()
+
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "data": { "repository": { "issue": {
+		"number": 123,
+		"url": "https://github.com/OWNER/REPO/issues/123"
+	} } } }
+	`))
+
+	var seenCmd *exec.Cmd
+	restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
+		seenCmd = cmd
+		return &outputStub{}
+	})
+	defer restoreCmd()
+
+	output, err := RunCommand(issueViewCmd, "issue view https://github.com/OWNER/REPO/issues/123")
+	if err != nil {
+		t.Errorf("error running command `issue view`: %v", err)
+	}
+
+	if output == "" {
+		t.Errorf("command output expected got an empty string")
+	}
+
+	if seenCmd == nil {
+		t.Fatal("expected a command to run")
+	}
+	url := seenCmd.Args[len(seenCmd.Args)-1]
+	eq(t, url, "https://github.com/OWNER/REPO/issues/123")
 }
 
 func TestIssueCreate(t *testing.T) {