Merge pull request #5764 from cli/fix-repo-list

Allow repo list to work with GHES earlier than 3.3

internal/featuredetection/feature_detection.go

@@ -36,12 +36,16 @@ type RepositoryFeatures struct {
 	IssueTemplateMutation    bool
 	IssueTemplateQuery       bool
 	PullRequestTemplateQuery bool
+	VisibilityField          bool
+	AutoMerge                bool
 }
 
 var allRepositoryFeatures = RepositoryFeatures{
 	IssueTemplateMutation:    true,
 	IssueTemplateQuery:       true,
 	PullRequestTemplateQuery: true,
+	VisibilityField:          true,
+	AutoMerge:                true,
 }
 
 type detector struct {
@@ -102,6 +106,12 @@ func (d *detector) RepositoryFeatures() (RepositoryFeatures, error) {
 		if field.Name == "pullRequestTemplates" {
 			features.PullRequestTemplateQuery = true
 		}
+		if field.Name == "visibility" {
+			features.VisibilityField = true
+		}
+		if field.Name == "autoMergeAllowed" {
+			features.AutoMerge = true
+		}
 	}
 
 	return features, nil

internal/featuredetection/feature_detection_test.go

@@ -72,6 +72,8 @@ func TestRepositoryFeatures(t *testing.T) {
 				IssueTemplateMutation:    true,
 				IssueTemplateQuery:       true,
 				PullRequestTemplateQuery: true,
+				VisibilityField:          true,
+				AutoMerge:                true,
 			},
 			wantErr: false,
 		},
@@ -105,6 +107,40 @@ func TestRepositoryFeatures(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name:     "GHE has visibility field",
+			hostname: "git.my.org",
+			queryResponse: map[string]string{
+				`query Repository_fields\b`: heredoc.Doc(`
+					{ "data": { "Repository": { "fields": [
+						{"name": "visibility"}
+					] } } }
+				`),
+			},
+			wantFeatures: RepositoryFeatures{
+				IssueTemplateMutation: true,
+				IssueTemplateQuery:    true,
+				VisibilityField:       true,
+			},
+			wantErr: false,
+		},
+		{
+			name:     "GHE has automerge field",
+			hostname: "git.my.org",
+			queryResponse: map[string]string{
+				`query Repository_fields\b`: heredoc.Doc(`
+					{ "data": { "Repository": { "fields": [
+						{"name": "autoMergeAllowed"}
+					] } } }
+				`),
+			},
+			wantFeatures: RepositoryFeatures{
+				IssueTemplateMutation: true,
+				IssueTemplateQuery:    true,
+				AutoMerge:             true,
+			},
+			wantErr: false,
+		},
 	}
 
 	for _, tt := range tests {

pkg/cmd/pr/create/create.go

@@ -588,9 +588,6 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
 			return nil, cmdutil.CancelError
 		} else {
 			// "Create a fork of ..."
-			if baseRepo.IsPrivate {
-				return nil, fmt.Errorf("cannot fork private repository %s", ghrepo.FullName(baseRepo))
-			}
 			headBranchLabel = fmt.Sprintf("%s:%s", currentLogin, headBranch)
 		}
 	}

pkg/cmd/repo/edit/edit.go

@@ -12,6 +12,7 @@ import (
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/v2/api"
+	fd "github.com/cli/cli/v2/internal/featuredetection"
 	"github.com/cli/cli/v2/internal/ghinstance"
 	"github.com/cli/cli/v2/internal/ghrepo"
 	"github.com/cli/cli/v2/pkg/cmdutil"
@@ -48,6 +49,7 @@ type EditOptions struct {
 	AddTopics       []string
 	RemoveTopics    []string
 	InteractiveMode bool
+	Detector        fd.Detector
 	// Cache of current repo topics to avoid retrieving them
 	// in multiple flows.
 	topicsCache []string
@@ -158,9 +160,17 @@ func editRun(ctx context.Context, opts *EditOptions) error {
 	repo := opts.Repository
 
 	if opts.InteractiveMode {
+		detector := opts.Detector
+		if detector == nil {
+			detector = fd.NewDetector(opts.HTTPClient, repo.RepoHost())
+		}
+		repoFeatures, err := detector.RepositoryFeatures()
+		if err != nil {
+			return err
+		}
+
 		apiClient := api.NewClientFromHTTP(opts.HTTPClient)
 		fieldsToRetrieve := []string{
-			"autoMergeAllowed",
 			"defaultBranchRef",
 			"deleteBranchOnMerge",
 			"description",
@@ -174,8 +184,14 @@ func editRun(ctx context.Context, opts *EditOptions) error {
 			"rebaseMergeAllowed",
 			"repositoryTopics",
 			"squashMergeAllowed",
-			"visibility",
 		}
+		if repoFeatures.VisibilityField {
+			fieldsToRetrieve = append(fieldsToRetrieve, "visibility")
+		}
+		if repoFeatures.AutoMerge {
+			fieldsToRetrieve = append(fieldsToRetrieve, "autoMergeAllowed")
+		}
+
 		opts.IO.StartProgressIndicator()
 		fetchedRepo, err := api.FetchRepository(apiClient, opts.Repository, fieldsToRetrieve)
 		opts.IO.StopProgressIndicator()

pkg/cmd/repo/list/list.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/config"
+	fd "github.com/cli/cli/v2/internal/featuredetection"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/pkg/text"
@@ -21,6 +22,7 @@ type ListOptions struct {
 	Config     func() (config.Config, error)
 	IO         *iostreams.IOStreams
 	Exporter   cmdutil.Exporter
+	Detector   fd.Detector
 
 	Limit int
 	Owner string
@@ -104,7 +106,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
 	return cmd
 }
 
-var defaultFields = []string{"nameWithOwner", "description", "isPrivate", "isFork", "isArchived", "createdAt", "pushedAt", "visibility"}
+var defaultFields = []string{"nameWithOwner", "description", "isPrivate", "isFork", "isArchived", "createdAt", "pushedAt"}
 
 func listRun(opts *ListOptions) error {
 	httpClient, err := opts.HttpClient()
@@ -112,20 +114,6 @@ func listRun(opts *ListOptions) error {
 		return err
 	}
 
-	filter := FilterOptions{
-		Visibility:  opts.Visibility,
-		Fork:        opts.Fork,
-		Source:      opts.Source,
-		Language:    opts.Language,
-		Topic:       opts.Topic,
-		Archived:    opts.Archived,
-		NonArchived: opts.NonArchived,
-		Fields:      defaultFields,
-	}
-	if opts.Exporter != nil {
-		filter.Fields = opts.Exporter.Fields()
-	}
-
 	cfg, err := opts.Config()
 	if err != nil {
 		return err
@@ -136,6 +124,33 @@ func listRun(opts *ListOptions) error {
 		return err
 	}
 
+	if opts.Detector == nil {
+		opts.Detector = fd.NewDetector(httpClient, host)
+	}
+	features, err := opts.Detector.RepositoryFeatures()
+	if err != nil {
+		return err
+	}
+
+	fields := defaultFields
+	if features.VisibilityField {
+		fields = append(defaultFields, "visibility")
+	}
+
+	filter := FilterOptions{
+		Visibility:  opts.Visibility,
+		Fork:        opts.Fork,
+		Source:      opts.Source,
+		Language:    opts.Language,
+		Topic:       opts.Topic,
+		Archived:    opts.Archived,
+		NonArchived: opts.NonArchived,
+		Fields:      fields,
+	}
+	if opts.Exporter != nil {
+		filter.Fields = opts.Exporter.Fields()
+	}
+
 	listResult, err := listRepos(httpClient, host, opts.Limit, opts.Owner, filter)
 	if err != nil {
 		return err
@@ -158,7 +173,7 @@ func listRun(opts *ListOptions) error {
 		info := repoInfo(repo)
 		infoColor := cs.Gray
 
-		if repo.Visibility != "PUBLIC" {
+		if repo.IsPrivate {
 			infoColor = cs.Yellow
 		}
 
@@ -208,9 +223,7 @@ func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool)
 }
 
 func repoInfo(r api.Repository) string {
-	tags := []string{
-		strings.ToLower(r.Visibility),
-	}
+	tags := []string{visibilityLabel(r)}
 
 	if r.IsFork {
 		tags = append(tags, "fork")
@@ -221,3 +234,12 @@ func repoInfo(r api.Repository) string {
 
 	return strings.Join(tags, ", ")
 }
+
+func visibilityLabel(repo api.Repository) string {
+	if repo.Visibility != "" {
+		return strings.ToLower(repo.Visibility)
+	} else if repo.IsPrivate {
+		return "private"
+	}
+	return "public"
+}

pkg/cmd/repo/list/list_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"io"
 	"net/http"
+	"strings"
 	"testing"
 	"time"
 
@@ -13,6 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/cli/cli/v2/internal/config"
+	fd "github.com/cli/cli/v2/internal/featuredetection"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
@@ -394,3 +396,44 @@ func TestRepoList_filtering(t *testing.T) {
 	assert.Equal(t, "", output.Stderr())
 	assert.Equal(t, "\nNo results match your search\n\n", output.String())
 }
+
+func TestRepoList_noVisibilityField(t *testing.T) {
+	ios, _, stdout, stderr := iostreams.Test()
+	ios.SetStdoutTTY(false)
+	ios.SetStdinTTY(false)
+	ios.SetStderrTTY(false)
+
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+
+	reg.Register(
+		httpmock.GraphQL(`query RepositoryList\b`),
+		httpmock.GraphQLQuery(`{"data":{"repositoryOwner":{"login":"octocat","repositories":{"totalCount":0}}}}`,
+			func(query string, params map[string]interface{}) {
+				assert.False(t, strings.Contains(query, "visibility"))
+			},
+		),
+	)
+
+	opts := ListOptions{
+		IO: ios,
+		HttpClient: func() (*http.Client, error) {
+			return &http.Client{Transport: reg}, nil
+		},
+		Config: func() (config.Config, error) {
+			return config.InheritEnv(config.NewBlankConfig()), nil
+		},
+		Now: func() time.Time {
+			t, _ := time.Parse(time.RFC822, "19 Feb 21 15:00 UTC")
+			return t
+		},
+		Limit:    30,
+		Detector: &fd.DisabledDetectorMock{},
+	}
+
+	err := listRun(&opts)
+
+	assert.NoError(t, err)
+	assert.Equal(t, "", stderr.String())
+	assert.Equal(t, "", stdout.String())
+}

pkg/cmd/search/repos/repos.go

@@ -154,7 +154,7 @@ func displayResults(io *iostreams.IOStreams, results search.RepositoriesResult)
 	cs := io.ColorScheme()
 	tp := utils.NewTablePrinter(io)
 	for _, repo := range results.Items {
-		tags := []string{repo.Visibility}
+		tags := []string{visibilityLabel(repo)}
 		if repo.IsFork {
 			tags = append(tags, "fork")
 		}
@@ -184,3 +184,12 @@ func displayResults(io *iostreams.IOStreams, results search.RepositoriesResult)
 	}
 	return tp.Render()
 }
+
+func visibilityLabel(repo search.Repository) string {
+	if repo.Visibility != "" {
+		return repo.Visibility
+	} else if repo.IsPrivate {
+		return "private"
+	}
+	return "public"
+}