Merge pull request #2108 from AliabbasMerchant/fix1309

fix: Project Argument working alongwith --web flag
2021-01-22 23:20:12 +01:00 · 2021-01-22 23:20:12 +01:00 · 20e167a7ae
commit 20e167a7ae
parent 9f7fd6e917 1c54db4919
6 changed files with 243 additions and 13 deletions
--- a/api/queries_repo.go
+++ b/api/queries_repo.go
@ -457,6 +457,33 @@ func (m *RepoMetadataResult) ProjectsToIDs(names []string) ([]string, error) {
 	return ids, nil
 }

+func ProjectsToPaths(projects []RepoProject, names []string) ([]string, error) {
+	var paths []string
+	for _, projectName := range names {
+		found := false
+		for _, p := range projects {
+			if strings.EqualFold(projectName, p.Name) {
+				// format of ResourcePath: /OWNER/REPO/projects/PROJECT_NUMBER or /orgs/ORG/projects/PROJECT_NUMBER
+				// required format of path: OWNER/REPO/PROJECT_NUMBER or ORG/PROJECT_NUMBER
+				var path string
+				pathParts := strings.Split(p.ResourcePath, "/")
+				if pathParts[1] == "orgs" {
+					path = fmt.Sprintf("%s/%s", pathParts[2], pathParts[4])
+				} else {
+					path = fmt.Sprintf("%s/%s/%s", pathParts[1], pathParts[2], pathParts[4])
+				}
+				paths = append(paths, path)
+				found = true
+				break
+			}
+		}
+		if !found {
+			return nil, fmt.Errorf("'%s' not found", projectName)
+		}
+	}
+	return paths, nil
+}
+
 func (m *RepoMetadataResult) MilestoneToID(title string) (string, error) {
 	for _, m := range m.Milestones {
 		if strings.EqualFold(title, m.Title) {
@ -540,20 +567,12 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
 	if input.Projects {
 		count++
 		go func() {
-			projects, err := RepoProjects(client, repo)
+			projects, err := RepoAndOrgProjects(client, repo)
 			if err != nil {
-				errc <- fmt.Errorf("error fetching projects: %w", err)
+				errc <- err
 				return
 			}
 			result.Projects = projects
-
-			orgProjects, err := OrganizationProjects(client, repo)
-			// TODO: better detection of non-org repos
-			if err != nil && !strings.HasPrefix(err.Error(), "Could not resolve to an Organization") {
-				errc <- fmt.Errorf("error fetching organization projects: %w", err)
-				return
-			}
-			result.Projects = append(result.Projects, orgProjects...)
 			errc <- nil
 		}()
 	}
@ -682,8 +701,9 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes
 }

 type RepoProject struct {
-	ID   string
-	Name string
+	ID           string
+	Name         string
+	ResourcePath string
 }

 // RepoProjects fetches all open projects for a repository
@ -726,6 +746,23 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error)
 	return projects, nil
 }

+// RepoAndOrgProjects fetches all open projects for a repository and its org
+func RepoAndOrgProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) {
+	projects, err := RepoProjects(client, repo)
+	if err != nil {
+		return projects, fmt.Errorf("error fetching projects: %w", err)
+	}
+
+	orgProjects, err := OrganizationProjects(client, repo)
+	// TODO: better detection of non-org repos
+	if err != nil && !strings.HasPrefix(err.Error(), "Could not resolve to an Organization") {
+		return projects, fmt.Errorf("error fetching organization projects: %w", err)
+	}
+	projects = append(projects, orgProjects...)
+
+	return projects, nil
+}
+
 type RepoAssignee struct {
 	ID    string
 	Login string
@ -913,3 +950,12 @@ func MilestoneByNumber(client *Client, repo ghrepo.Interface, number int32) (*Re

 	return query.Repository.Milestone, nil
 }
+
+func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string) ([]string, error) {
+	var paths []string
+	projects, err := RepoAndOrgProjects(client, repo)
+	if err != nil {
+		return paths, err
+	}
+	return ProjectsToPaths(projects, projectNames)
+}
--- a/api/queries_repo_test.go
+++ b/api/queries_repo_test.go
@ -141,6 +141,63 @@ func Test_RepoMetadata(t *testing.T) {
 	}
 }

+func Test_ProjectsToPaths(t *testing.T) {
+	expectedProjectPaths := []string{"OWNER/REPO/PROJECT_NUMBER", "ORG/PROJECT_NUMBER"}
+	projects := []RepoProject{
+		{"id1", "My Project", "/OWNER/REPO/projects/PROJECT_NUMBER"},
+		{"id2", "Org Project", "/orgs/ORG/projects/PROJECT_NUMBER"},
+		{"id3", "Project", "/orgs/ORG/projects/PROJECT_NUMBER_2"},
+	}
+	projectNames := []string{"My Project", "Org Project"}
+
+	projectPaths, err := ProjectsToPaths(projects, projectNames)
+	if err != nil {
+		t.Errorf("error resolving projects: %v", err)
+	}
+	if !sliceEqual(projectPaths, expectedProjectPaths) {
+		t.Errorf("expected projects %v, got %v", expectedProjectPaths, projectPaths)
+	}
+}
+
+func Test_ProjectNamesToPaths(t *testing.T) {
+	http := &httpmock.Registry{}
+	client := NewClient(ReplaceTripper(http))
+
+	repo, _ := ghrepo.FromFullName("OWNER/REPO")
+
+	http.Register(
+		httpmock.GraphQL(`query RepositoryProjectList\b`),
+		httpmock.StringResponse(`
+		{ "data": { "repository": { "projects": {
+			"nodes": [
+				{ "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" },
+				{ "name": "Roadmap", "id": "ROADMAPID", "resourcePath": "/OWNER/REPO/projects/2" }
+			],
+			"pageInfo": { "hasNextPage": false }
+		} } } }
+		`))
+	http.Register(
+		httpmock.GraphQL(`query OrganizationProjectList\b`),
+		httpmock.StringResponse(`
+			{ "data": { "organization": { "projects": {
+				"nodes": [
+					{ "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1"  }
+				],
+				"pageInfo": { "hasNextPage": false }
+			} } } }
+			`))
+
+	projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	expectedProjectPaths := []string{"ORG/1", "OWNER/REPO/2"}
+	if !sliceEqual(projectPaths, expectedProjectPaths) {
+		t.Errorf("expected projects paths %v, got %v", expectedProjectPaths, projectPaths)
+	}
+}
+
 func Test_RepoResolveMetadataIDs(t *testing.T) {
 	http := &httpmock.Registry{}
 	client := NewClient(ReplaceTripper(http))
--- a/pkg/cmd/issue/create/create.go
+++ b/pkg/cmd/issue/create/create.go
@ -142,7 +142,14 @@ func createRun(opts *CreateOptions) (err error) {

 	if opts.WebMode {
 		openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new")
-		if opts.Title != "" || opts.Body != "" || len(opts.Assignees) > 0 {
+		if opts.Title != "" || opts.Body != "" || tb.HasMetadata() {
+			if len(opts.Projects) > 0 {
+				var err error
+				tb.Projects, err = api.ProjectNamesToPaths(apiClient, baseRepo, tb.Projects)
+				if err != nil {
+					return fmt.Errorf("could not add to project: %w", err)
+				}
+			}
 			openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb)
 			if err != nil {
 				return
@ -243,6 +250,13 @@ func createRun(opts *CreateOptions) (err error) {
 	}

 	if action == prShared.PreviewAction {
+		if len(tb.Projects) > 0 {
+			var err error
+			tb.Projects, err = api.ProjectNamesToPaths(apiClient, repo, tb.Projects)
+			if err != nil {
+				return fmt.Errorf("could not add to project: %w", err)
+			}
+		}
 		openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new")
 		openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb)
 		if err != nil {
--- a/pkg/cmd/issue/create/create_test.go
+++ b/pkg/cmd/issue/create/create_test.go
@ -554,3 +554,50 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) {

 	assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String())
 }
+
+func TestIssueCreate_webProject(t *testing.T) {
+	http := &httpmock.Registry{}
+	defer http.Verify(t)
+
+	http.Register(
+		httpmock.GraphQL(`query RepositoryProjectList\b`),
+		httpmock.StringResponse(`
+			{ "data": { "repository": { "projects": {
+				"nodes": [
+					{ "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" }
+				],
+				"pageInfo": { "hasNextPage": false }
+			} } } }
+			`))
+	http.Register(
+		httpmock.GraphQL(`query OrganizationProjectList\b`),
+		httpmock.StringResponse(`
+			{ "data": { "organization": { "projects": {
+				"nodes": [
+					{ "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1"  }
+				],
+				"pageInfo": { "hasNextPage": false }
+			} } } }
+			`))
+
+	var seenCmd *exec.Cmd
+	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
+	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
+		seenCmd = cmd
+		return &test.OutputStub{}
+	})
+	defer restoreCmd()
+
+	output, err := runCommand(http, true, `-w -t Title -p Cleanup`)
+	if err != nil {
+		t.Errorf("error running command `issue create`: %v", err)
+	}
+
+	if seenCmd == nil {
+		t.Fatal("expected a command to run")
+	}
+	url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
+	assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url)
+	assert.Equal(t, "", output.String())
+	assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
+}
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@ -612,6 +612,14 @@ func submitPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataS
 }

 func previewPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataState) error {
+	if len(state.Projects) > 0 {
+		var err error
+		state.Projects, err = api.ProjectNamesToPaths(ctx.Client, ctx.BaseRepo, state.Projects)
+		if err != nil {
+			return fmt.Errorf("could not add to project: %w", err)
+		}
+	}
+
 	openURL, err := generateCompareURL(ctx, state)
 	if err != nil {
 		return err
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@ -764,6 +764,64 @@ func TestPRCreate_web(t *testing.T) {
 	assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", browserCall[len(browserCall)-1])
 }

+func TestPRCreate_webProject(t *testing.T) {
+	http := initFakeHTTP()
+	defer http.Verify(t)
+
+	http.StubRepoInfoResponse("OWNER", "REPO", "master")
+	http.StubRepoResponse("OWNER", "REPO")
+	http.Register(
+		httpmock.GraphQL(`query UserCurrent\b`),
+		httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
+	http.Register(
+		httpmock.GraphQL(`query RepositoryProjectList\b`),
+		httpmock.StringResponse(`
+			{ "data": { "repository": { "projects": {
+				"nodes": [
+					{ "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" }
+				],
+				"pageInfo": { "hasNextPage": false }
+			} } } }
+			`))
+	http.Register(
+		httpmock.GraphQL(`query OrganizationProjectList\b`),
+		httpmock.StringResponse(`
+			{ "data": { "organization": { "projects": {
+				"nodes": [
+					{ "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1"  }
+				],
+				"pageInfo": { "hasNextPage": false }
+			} } } }
+			`))
+
+	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
+	cs, cmdTeardown := test.InitCmdStubber()
+	defer cmdTeardown()
+
+	cs.Stub("")                                         // git config --get-regexp (determineTrackingBranch)
+	cs.Stub("")                                         // git show-ref --verify   (determineTrackingBranch)
+	cs.Stub("")                                         // git status
+	cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log
+	cs.Stub("")                                         // git push
+	cs.Stub("")                                         // browser
+
+	ask, cleanupAsk := prompt.InitAskStubber()
+	defer cleanupAsk()
+	ask.StubOne(0)
+
+	output, err := runCommand(http, nil, "feature", true, `--web -p Triage`)
+	require.NoError(t, err)
+
+	assert.Equal(t, "", output.String())
+	assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr())
+
+	assert.Equal(t, 6, len(cs.Calls))
+	assert.Equal(t, "git push --set-upstream origin HEAD:feature", strings.Join(cs.Calls[4].Args, " "))
+	browserCall := cs.Calls[5].Args
+	url := strings.ReplaceAll(browserCall[len(browserCall)-1], "^", "")
+	assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1&projects=ORG%2F1", url)
+}
+
 func Test_determineTrackingBranch_empty(t *testing.T) {
 	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
 	cs, cmdTeardown := test.InitCmdStubber()