diff --git a/api/queries_issue.go b/api/queries_issue.go index 094b6b198c7..813dab4961f 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -92,12 +92,16 @@ type ProjectItems struct { } type ProjectInfo struct { - Project struct { - Name string `json:"name"` - } `json:"project"` - Column struct { - Name string `json:"name"` - } `json:"column"` + Project ProjectV1ProjectName `json:"project"` + Column ProjectV1ProjectColumn `json:"column"` +} + +type ProjectV1ProjectName struct { + Name string `json:"name"` +} + +type ProjectV1ProjectColumn struct { + Name string `json:"name"` } type ProjectV2Item struct { diff --git a/api/queries_repo.go b/api/queries_repo.go index 53e6d879a47..0ea6378215d 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghinstance" "golang.org/x/sync/errgroup" @@ -868,7 +869,7 @@ type RepoMetadataInput struct { } // RepoMetadata pre-fetches the metadata for attaching to issues and pull requests -func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput) (*RepoMetadataResult, error) { +func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput, projectsV1Support gh.ProjectsV1Support) (*RepoMetadataResult, error) { var result RepoMetadataResult var g errgroup.Group @@ -917,7 +918,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput if input.Projects { g.Go(func() error { var err error - result.Projects, result.ProjectsV2, err = relevantProjects(client, repo) + result.Projects, result.ProjectsV2, err = relevantProjects(client, repo, projectsV1Support) return err }) } @@ -948,7 +949,7 @@ type RepoResolveInput struct { } // RepoResolveMetadataIDs looks up GraphQL node IDs in bulk -func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoResolveInput) (*RepoMetadataResult, error) { +func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoResolveInput, projectsV1Support gh.ProjectsV1Support) (*RepoMetadataResult, error) { users := input.Assignees hasUser := func(target string) bool { for _, u := range users { @@ -973,7 +974,7 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes Projects: len(input.Projects) > 0, Milestones: len(input.Milestones) > 0, } - result, err := RepoMetadata(client, repo, mi) + result, err := RepoMetadata(client, repo, mi, projectsV1Support) if err != nil { return result, err } @@ -1237,8 +1238,8 @@ func RepoMilestones(client *Client, repo ghrepo.Interface, state string) ([]Repo return milestones, nil } -func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string) ([]string, error) { - projects, projectsV2, err := relevantProjects(client, repo) +func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string, projectsV1Support gh.ProjectsV1Support) ([]string, error) { + projects, projectsV2, err := relevantProjects(client, repo, projectsV1Support) if err != nil { return nil, err } @@ -1251,7 +1252,7 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s // - ProjectsV2 owned by current user // - ProjectsV2 linked to repository // - ProjectsV2 owned by repository organization, if it belongs to one -func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []ProjectV2, error) { +func relevantProjects(client *Client, repo ghrepo.Interface, projectsV1Support gh.ProjectsV1Support) ([]RepoProject, []ProjectV2, error) { var repoProjects []RepoProject var orgProjects []RepoProject var userProjectsV2 []ProjectV2 @@ -1260,23 +1261,26 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P g, _ := errgroup.WithContext(context.Background()) - g.Go(func() error { - var err error - repoProjects, err = RepoProjects(client, repo) - if err != nil { - err = fmt.Errorf("error fetching repo projects (classic): %w", err) - } - return err - }) - g.Go(func() error { - var err error - orgProjects, err = OrganizationProjects(client, repo) - if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) { - err = fmt.Errorf("error fetching organization projects (classic): %w", err) + if projectsV1Support == gh.ProjectsV1Supported { + g.Go(func() error { + var err error + repoProjects, err = RepoProjects(client, repo) + if err != nil { + err = fmt.Errorf("error fetching repo projects (classic): %w", err) + } return err - } - return nil - }) + }) + g.Go(func() error { + var err error + orgProjects, err = OrganizationProjects(client, repo) + if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) { + err = fmt.Errorf("error fetching organization projects (classic): %w", err) + return err + } + return nil + }) + } + g.Go(func() error { var err error userProjectsV2, err = CurrentUserProjectsV2(client, repo.RepoHost()) diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 13aee459a1e..922090c6c09 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/assert" @@ -151,7 +152,7 @@ func Test_RepoMetadata(t *testing.T) { { "data": { "viewer": { "login": "monalisa" } } } `)) - result, err := RepoMetadata(client, repo, input) + result, err := RepoMetadata(client, repo, input, gh.ProjectsV1Supported) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -292,7 +293,7 @@ func Test_ProjectNamesToPaths(t *testing.T) { } } } } `)) - projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"}) + projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Supported) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -352,7 +353,7 @@ t001: team(slug:"robots"){id,slug} } })) - result, err := RepoResolveMetadataIDs(client, repo, input) + result, err := RepoResolveMetadataIDs(client, repo, input, gh.ProjectsV1Supported) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/featuredetection/detector_mock.go b/internal/featuredetection/detector_mock.go index 6f36dd3fc03..4ba51a3c8d8 100644 --- a/internal/featuredetection/detector_mock.go +++ b/internal/featuredetection/detector_mock.go @@ -1,5 +1,7 @@ package featuredetection +import "github.com/cli/cli/v2/internal/gh" + type DisabledDetectorMock struct{} func (md *DisabledDetectorMock) IssueFeatures() (IssueFeatures, error) { @@ -14,6 +16,10 @@ func (md *DisabledDetectorMock) RepositoryFeatures() (RepositoryFeatures, error) return RepositoryFeatures{}, nil } +func (md *DisabledDetectorMock) ProjectsV1() (gh.ProjectsV1Support, error) { + return gh.ProjectsV1Unsupported, nil +} + type EnabledDetectorMock struct{} func (md *EnabledDetectorMock) IssueFeatures() (IssueFeatures, error) { @@ -27,3 +33,7 @@ func (md *EnabledDetectorMock) PullRequestFeatures() (PullRequestFeatures, error func (md *EnabledDetectorMock) RepositoryFeatures() (RepositoryFeatures, error) { return allRepositoryFeatures, nil } + +func (md *EnabledDetectorMock) ProjectsV1() (gh.ProjectsV1Support, error) { + return gh.ProjectsV1Supported, nil +} diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index a9bbe25f851..cc17032158e 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -2,8 +2,11 @@ package featuredetection import ( "net/http" + "os" + "strconv" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "golang.org/x/sync/errgroup" ghauth "github.com/cli/go-gh/v2/pkg/auth" @@ -13,6 +16,7 @@ type Detector interface { IssueFeatures() (IssueFeatures, error) PullRequestFeatures() (PullRequestFeatures, error) RepositoryFeatures() (RepositoryFeatures, error) + ProjectsV1() (gh.ProjectsV1Support, error) } type IssueFeatures struct { @@ -199,3 +203,35 @@ func (d *detector) RepositoryFeatures() (RepositoryFeatures, error) { return features, nil } + +func (d *detector) ProjectsV1() (gh.ProjectsV1Support, error) { + // Bypass feature detection logic for testing purposes + if env := os.Getenv("PROJECTS_V1_SUPPORTED"); env != "" { + b, err := strconv.ParseBool(env) + if err != nil { + return nil, err + } + return gh.ParseProjectsV1Support(b), nil + } + + var featureDetection struct { + Repository struct { + Fields []struct { + Name string + } `graphql:"fields(includeDeprecated: true)"` + } `graphql:"Repository: __type(name: \"Repository\")"` + } + + gql := api.NewClientFromHTTP(d.httpClient) + if err := gql.Query(d.host, "ProjectsV1FeatureDetection", &featureDetection, nil); err != nil { + return nil, err + } + + for _, field := range featureDetection.Repository.Fields { + if field.Name == "projects" { + return gh.ProjectsV1Supported, nil + } + } + + return gh.ProjectsV1Unsupported, nil +} diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index 8af091c3f01..ee31b511020 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -5,8 +5,10 @@ import ( "testing" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIssueFeatures(t *testing.T) { @@ -366,3 +368,83 @@ func TestRepositoryFeatures(t *testing.T) { }) } } + +func TestProjectV1SupportEnvOverride(t *testing.T) { + tt := []struct { + name string + envVal string + expectedSupport gh.ProjectsV1Support + expectedErr bool + }{ + { + name: "PROJECTS_V1_SUPPORTED=true", + envVal: "true", + expectedSupport: gh.ProjectsV1Supported, + }, + { + name: "PROJECTS_V1_SUPPORTED=false", + envVal: "false", + expectedSupport: gh.ProjectsV1Unsupported, + }, + { + name: "PROJECTS_V1_SUPPORTED=nonboolean", + envVal: "nonboolean", + expectedErr: true, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("PROJECTS_V1_SUPPORTED", tc.envVal) + + detector := detector{} + support, err := detector.ProjectsV1() + + if tc.expectedErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + require.Equal(t, tc.expectedSupport, support) + }) + } +} + +func TestProjectV1APINotSupported(t *testing.T) { + // Given the API does not include projects in the returned fields + reg := &httpmock.Registry{} + httpClient := &http.Client{} + httpmock.ReplaceTripper(httpClient, reg) + reg.Register( + httpmock.GraphQL(`query ProjectsV1FeatureDetection\b`), + httpmock.StringResponse(heredoc.Doc(`{ "data": { "Repository": { "fields": [] } } }`)), + ) + + // When we call the api to detect for ProjectV1 support + detector := detector{httpClient: httpClient} + actualSupport, err := detector.ProjectsV1() + + // Then it succeeds and reports ProjectV1 is not supported + require.NoError(t, err) + require.Equal(t, gh.ProjectsV1Unsupported, actualSupport) +} + +func TestProjectV1APISupported(t *testing.T) { + // Given the API includes projects in the returned fields + reg := &httpmock.Registry{} + httpClient := &http.Client{} + httpmock.ReplaceTripper(httpClient, reg) + reg.Register( + httpmock.GraphQL(`query ProjectsV1FeatureDetection\b`), + httpmock.StringResponse(heredoc.Doc(`{ "data": { "Repository": { "fields": [ {"name": "projects"} ] } } }`)), + ) + + // When we call the api to detect for ProjectV1 support + detector := detector{httpClient: httpClient} + actualSupport, err := detector.ProjectsV1() + + // Then it succeeds and reports ProjectV1 is supported + require.NoError(t, err) + require.Equal(t, gh.ProjectsV1Supported, actualSupport) +} diff --git a/internal/gh/projects.go b/internal/gh/projects.go new file mode 100644 index 00000000000..042a35843c6 --- /dev/null +++ b/internal/gh/projects.go @@ -0,0 +1,30 @@ +package gh + +// ProjectsV1Support provides type safety and readability around whether or not Projects v1 is supported +// by the targeted host. +// +// It is a sealed type to ensure that consumers must use the exported ProjectsV1Supported and ProjectsV1Unsupported +// variables to get an instance of the type. +type ProjectsV1Support interface { + sealed() +} + +type projectsV1Supported struct{} + +func (projectsV1Supported) sealed() {} + +type projectsV1Unsupported struct{} + +func (projectsV1Unsupported) sealed() {} + +var ( + ProjectsV1Supported ProjectsV1Support = projectsV1Supported{} + ProjectsV1Unsupported ProjectsV1Support = projectsV1Unsupported{} +) + +func ParseProjectsV1Support(b bool) ProjectsV1Support { + if b { + return ProjectsV1Supported + } + return ProjectsV1Unsupported +} diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index 9197abff661..5e4845d48c8 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -67,7 +67,7 @@ func closeRun(opts *CloseOptions) error { return err } - issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"}) + issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"}, opts.Detector) if err != nil { return err } @@ -109,6 +109,7 @@ func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue, } if reason != "" { + // Should this be moved up into closeRun()? if detector == nil { cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) detector = fd.NewDetector(cachedClient, repo.RepoHost()) diff --git a/pkg/cmd/issue/comment/comment.go b/pkg/cmd/issue/comment/comment.go index 090b0748c4b..414dfaccf9a 100644 --- a/pkg/cmd/issue/comment/comment.go +++ b/pkg/cmd/issue/comment/comment.go @@ -45,7 +45,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e if opts.EditLast { fields = append(fields, "comments") } - return issueShared.IssueFromArgWithFields(httpClient, f.BaseRepo, args[0], fields) + return issueShared.IssueFromArgWithFields(httpClient, f.BaseRepo, args[0], fields, opts.Detector) } return prShared.CommentablePreRun(cmd, opts) }, diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 2e3e0de519a..e6c28cce2d9 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -4,10 +4,12 @@ import ( "errors" "fmt" "net/http" + "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/text" @@ -25,6 +27,7 @@ type CreateOptions struct { Browser browser.Browser Prompter prShared.Prompt TitledEditSurvey func(string, string) (string, string, error) + Detector fd.Detector RootDirOverride string @@ -146,6 +149,16 @@ func createRun(opts *CreateOptions) (err error) { return } + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) + } + + projectsV1Support, err := opts.Detector.ProjectsV1() + if err != nil { + return err + } + isTerminal := opts.IO.IsStdoutTTY() var milestones []string @@ -182,7 +195,7 @@ func createRun(opts *CreateOptions) (err error) { if opts.WebMode { var openURL string if opts.Title != "" || opts.Body != "" || tb.HasMetadata() { - openURL, err = generatePreviewURL(apiClient, baseRepo, tb) + openURL, err = generatePreviewURL(apiClient, baseRepo, tb, projectsV1Support) if err != nil { return } @@ -260,7 +273,7 @@ func createRun(opts *CreateOptions) (err error) { } } - openURL, err = generatePreviewURL(apiClient, baseRepo, tb) + openURL, err = generatePreviewURL(apiClient, baseRepo, tb, projectsV1Support) if err != nil { return } @@ -279,7 +292,7 @@ func createRun(opts *CreateOptions) (err error) { Repo: baseRepo, State: &tb, } - err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb) + err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support) if err != nil { return } @@ -335,7 +348,7 @@ func createRun(opts *CreateOptions) (err error) { params["issueTemplate"] = templateNameForSubmit } - err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb) + err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb, projectsV1Support) if err != nil { return } @@ -354,7 +367,7 @@ func createRun(opts *CreateOptions) (err error) { return } -func generatePreviewURL(apiClient *api.Client, baseRepo ghrepo.Interface, tb prShared.IssueMetadataState) (string, error) { +func generatePreviewURL(apiClient *api.Client, baseRepo ghrepo.Interface, tb prShared.IssueMetadataState, projectsV1Support gh.ProjectsV1Support) (string, error) { openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") - return prShared.WithPrAndIssueQueryParams(apiClient, baseRepo, openURL, tb) + return prShared.WithPrAndIssueQueryParams(apiClient, baseRepo, openURL, tb, projectsV1Support) } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 8e49700a012..2295c4c50f7 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -14,6 +14,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" @@ -475,6 +476,7 @@ func Test_createRun(t *testing.T) { } browser := &browser.Stub{} opts.Browser = browser + opts.Detector = &featuredetection.EnabledDetectorMock{} err := createRun(opts) if tt.wantsErr == "" { @@ -521,6 +523,7 @@ func runCommandWithRootDirOverridden(rt http.RoundTripper, isTTY bool, cli strin cmd := NewCmdCreate(factory, func(opts *CreateOptions) error { opts.RootDirOverride = rootDir + opts.Detector = &featuredetection.EnabledDetectorMock{} return createRun(opts) }) diff --git a/pkg/cmd/issue/delete/delete.go b/pkg/cmd/issue/delete/delete.go index fb41f288e56..6976b7e04a5 100644 --- a/pkg/cmd/issue/delete/delete.go +++ b/pkg/cmd/issue/delete/delete.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -20,6 +21,7 @@ type DeleteOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Prompter iprompter + Detector fd.Detector SelectorArg string Confirmed bool @@ -71,7 +73,7 @@ func deleteRun(opts *DeleteOptions) error { return err } - issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title"}) + issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title"}, opts.Detector) if err != nil { return err } diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 1536800f072..a37d35a421a 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -23,6 +24,7 @@ type DevelopOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Remotes func() (context.Remotes, error) + Detector fd.Detector IssueSelector string Name string @@ -132,7 +134,7 @@ func developRun(opts *DevelopOptions) error { } opts.IO.StartProgressIndicator() - issue, issueRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number"}) + issue, issueRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number"}, opts.Detector) opts.IO.StopProgressIndicator() if err != nil { return err diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 18067319f40..42bbbb340c2 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -5,9 +5,12 @@ import ( "net/http" "sort" "sync" + "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/text" shared "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -22,11 +25,12 @@ type EditOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Prompter prShared.EditPrompter + Detector fd.Detector DetermineEditor func() (string, error) FieldsToEditSurvey func(prShared.EditPrompter, *prShared.Editable) error EditFieldsSurvey func(prShared.EditPrompter, *prShared.Editable, string) error - FetchOptions func(*api.Client, ghrepo.Interface, *prShared.Editable) error + FetchOptions func(*api.Client, ghrepo.Interface, *prShared.Editable, gh.ProjectsV1Support) error SelectorArgs []string Interactive bool @@ -167,6 +171,21 @@ func editRun(opts *EditOptions) error { return err } + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) + } + + projectsV1Support, err := opts.Detector.ProjectsV1() + if err != nil { + return err + } + // Prompt the user which fields they'd like to edit. editable := opts.Editable if opts.Interactive { @@ -192,7 +211,7 @@ func editRun(opts *EditOptions) error { } // Get all specified issues and make sure they are within the same repo. - issues, repo, err := shared.IssuesFromArgsWithFields(httpClient, opts.BaseRepo, opts.SelectorArgs, lookupFields) + issues, repo, err := shared.IssuesFromArgsWithFields(httpClient, opts.BaseRepo, opts.SelectorArgs, lookupFields, opts.Detector) if err != nil { return err } @@ -200,7 +219,7 @@ func editRun(opts *EditOptions) error { // Fetch editable shared fields once for all issues. apiClient := api.NewClientFromHTTP(httpClient) opts.IO.StartProgressIndicatorWithLabel("Fetching repository information") - err = opts.FetchOptions(apiClient, repo, &editable) + err = opts.FetchOptions(apiClient, repo, &editable, projectsV1Support) opts.IO.StopProgressIndicator() if err != nil { return err diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 40fe6491cbe..2f74944dbe2 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -10,6 +10,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -577,6 +578,7 @@ func Test_editRun(t *testing.T) { tt.input.IO = ios tt.input.HttpClient = httpClient tt.input.BaseRepo = baseRepo + tt.input.Detector = &featuredetection.EnabledDetectorMock{} err := editRun(tt.input) if tt.wantErr { diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 4e0dac05815..d7df78afca4 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -17,6 +17,7 @@ import ( "strings" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -96,6 +97,7 @@ type LockOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Prompter iprompter + Detector fd.Detector ParentCmd string Reason string @@ -214,7 +216,7 @@ func lockRun(state string, opts *LockOptions) error { return err } - issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, fields()) + issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, fields(), opts.Detector) parent := alias[opts.ParentCmd] diff --git a/pkg/cmd/issue/pin/pin.go b/pkg/cmd/issue/pin/pin.go index dfb11a8811c..6488e43028e 100644 --- a/pkg/cmd/issue/pin/pin.go +++ b/pkg/cmd/issue/pin/pin.go @@ -73,7 +73,7 @@ func pinRun(opts *PinOptions) error { return err } - issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "isPinned"}) + issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "isPinned"}, nil) if err != nil { return err } diff --git a/pkg/cmd/issue/reopen/reopen.go b/pkg/cmd/issue/reopen/reopen.go index 92f18a7d979..c39e6076455 100644 --- a/pkg/cmd/issue/reopen/reopen.go +++ b/pkg/cmd/issue/reopen/reopen.go @@ -64,7 +64,7 @@ func reopenRun(opts *ReopenOptions) error { return err } - issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"}) + issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"}, nil) if err != nil { return err } diff --git a/pkg/cmd/issue/shared/display.go b/pkg/cmd/issue/shared/display.go index 0c56ffd2cae..7d8eea42454 100644 --- a/pkg/cmd/issue/shared/display.go +++ b/pkg/cmd/issue/shared/display.go @@ -32,7 +32,7 @@ func PrintIssues(io *iostreams.IOStreams, now time.Time, prefix string, totalCou issueNum = "#" + issueNum } issueNum = prefix + issueNum - table.AddField(issueNum, tableprinter.WithColor(cs.ColorFromString(prShared.ColorForIssueState(issue)))) + table.AddField(issueNum, tableprinter.WithColor(cs.ColorFromString(prShared.ColorForIssueState(issue.State, issue.StateReason)))) if !isTTY { table.AddField(issue.State) } diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index be79f9a73e6..c80759aef87 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -12,6 +12,7 @@ import ( "github.com/cli/cli/v2/api" fd "github.com/cli/cli/v2/internal/featuredetection" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/set" "golang.org/x/sync/errgroup" @@ -19,7 +20,7 @@ import ( // IssueFromArgWithFields loads an issue or pull request with the specified fields. If some of the fields // could not be fetched by GraphQL, this returns a non-nil issue and a *PartialLoadError. -func IssueFromArgWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), arg string, fields []string) (*api.Issue, ghrepo.Interface, error) { +func IssueFromArgWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), arg string, fields []string, detector fd.Detector) (*api.Issue, ghrepo.Interface, error) { issueNumber, baseRepo, err := IssueNumberAndRepoFromArg(arg) if err != nil { return nil, nil, err @@ -32,13 +33,13 @@ func IssueFromArgWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.I } } - issue, err := findIssueOrPR(httpClient, baseRepo, issueNumber, fields) + issue, err := findIssueOrPR(httpClient, baseRepo, issueNumber, fields, detector) return issue, baseRepo, err } // IssuesFromArgsWithFields loads 1 or more issues or pull requests with the specified fields. If some of the fields // could not be fetched by GraphQL, this returns non-nil issues and a *PartialLoadError. -func IssuesFromArgsWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), args []string, fields []string) ([]*api.Issue, ghrepo.Interface, error) { +func IssuesFromArgsWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), args []string, fields []string, detector fd.Detector) ([]*api.Issue, ghrepo.Interface, error) { var issuesRepo ghrepo.Interface issueNumbers := make([]int, 0, len(args)) @@ -75,7 +76,7 @@ func IssuesFromArgsWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo for _, num := range issueNumbers { issueNumber := num g.Go(func() error { - issue, err := findIssueOrPR(httpClient, issuesRepo, issueNumber, fields) + issue, err := findIssueOrPR(httpClient, issuesRepo, issueNumber, fields, detector) if err != nil { return err } @@ -142,12 +143,15 @@ type PartialLoadError struct { error } -func findIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.Issue, error) { +func findIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string, detector fd.Detector) (*api.Issue, error) { + if detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + detector = fd.NewDetector(cachedClient, repo.RepoHost()) + } + fieldSet := set.NewStringSet() fieldSet.AddValues(fields) if fieldSet.Contains("stateReason") { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - detector := fd.NewDetector(cachedClient, repo.RepoHost()) features, err := detector.IssueFeatures() if err != nil { return nil, err @@ -163,6 +167,15 @@ func findIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, f fieldSet.Remove("projectItems") fieldSet.Add("number") } + if fieldSet.Contains("projectCards") { + projectsV1Support, err := detector.ProjectsV1() + if err != nil { + return nil, err + } + if projectsV1Support == gh.ProjectsV1Unsupported { + fieldSet.Remove("projectCards") + } + } fields = fieldSet.ToSlice() diff --git a/pkg/cmd/issue/shared/lookup_test.go b/pkg/cmd/issue/shared/lookup_test.go index 44f496de44a..1261d3bf44b 100644 --- a/pkg/cmd/issue/shared/lookup_test.go +++ b/pkg/cmd/issue/shared/lookup_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/assert" @@ -193,7 +194,7 @@ func TestIssueFromArgWithFields(t *testing.T) { tt.httpStub(reg) } httpClient := &http.Client{Transport: reg} - issue, repo, err := IssueFromArgWithFields(httpClient, tt.args.baseRepoFn, tt.args.selector, []string{"number"}) + issue, repo, err := IssueFromArgWithFields(httpClient, tt.args.baseRepoFn, tt.args.selector, []string{"number"}, &fd.EnabledDetectorMock{}) if (err != nil) != tt.wantErr { t.Errorf("IssueFromArgWithFields() error = %v, wantErr %v", err, tt.wantErr) if issue == nil { @@ -289,7 +290,7 @@ func TestIssuesFromArgsWithFields(t *testing.T) { tt.httpStub(reg) } httpClient := &http.Client{Transport: reg} - issues, repo, err := IssuesFromArgsWithFields(httpClient, tt.args.baseRepoFn, tt.args.selectors, []string{"number"}) + issues, repo, err := IssuesFromArgsWithFields(httpClient, tt.args.baseRepoFn, tt.args.selectors, []string{"number"}, &fd.EnabledDetectorMock{}) if (err != nil) != tt.wantErr { t.Errorf("IssuesFromArgsWithFields() error = %v, wantErr %v", err, tt.wantErr) if issues == nil { diff --git a/pkg/cmd/issue/transfer/transfer.go b/pkg/cmd/issue/transfer/transfer.go index 140d02b9194..f2c70181445 100644 --- a/pkg/cmd/issue/transfer/transfer.go +++ b/pkg/cmd/issue/transfer/transfer.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -19,6 +20,7 @@ type TransferOptions struct { Config func() (gh.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Detector fd.Detector IssueSelector string DestRepoSelector string @@ -57,7 +59,7 @@ func transferRun(opts *TransferOptions) error { return err } - issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number"}) + issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number"}, opts.Detector) if err != nil { return err } diff --git a/pkg/cmd/issue/unpin/unpin.go b/pkg/cmd/issue/unpin/unpin.go index 3ac28d47cf5..8e820ed54bb 100644 --- a/pkg/cmd/issue/unpin/unpin.go +++ b/pkg/cmd/issue/unpin/unpin.go @@ -6,6 +6,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -21,6 +22,7 @@ type UnpinOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) SelectorArg string + Detector fd.Detector } func NewCmdUnpin(f *cmdutil.Factory, runF func(*UnpinOptions) error) *cobra.Command { @@ -73,7 +75,7 @@ func unpinRun(opts *UnpinOptions) error { return err } - issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "isPinned"}) + issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "isPinned"}, opts.Detector) if err != nil { return err } diff --git a/pkg/cmd/issue/view/presentation_issue.go b/pkg/cmd/issue/view/presentation_issue.go new file mode 100644 index 00000000000..e7147a27c74 --- /dev/null +++ b/pkg/cmd/issue/view/presentation_issue.go @@ -0,0 +1,122 @@ +package view + +import ( + "fmt" + "slices" + "strings" + "time" + + "github.com/cli/cli/v2/api" + prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" + "github.com/cli/cli/v2/pkg/iostreams" +) + +type PresentationIssue struct { + Title string + Number int + CreatedAt time.Time + Comments api.Comments + Author string + State string + StateReason string + Reactions string + AssigneesList string + LabelsList string + ProjectsList string + MilestoneTitle string + Body string + URL string +} + +// Creates a new PresentationIssue from an api.Issue. +// The PresentationIssue is what the issue printers need to display an +// issue to the user. +func MapApiIssueToPresentationIssue(issue *api.Issue, colorScheme *iostreams.ColorScheme) (PresentationIssue, error) { + presentationIssue := PresentationIssue{ + Title: issue.Title, + Number: issue.Number, + CreatedAt: issue.CreatedAt, + Comments: issue.Comments, + Author: issue.Author.Login, + State: issue.State, + StateReason: issue.StateReason, + Reactions: prShared.ReactionGroupList(issue.ReactionGroups), + AssigneesList: stringifyAssignees(issue.Assignees), + LabelsList: stringifyAndColorizeLabels(sortAlphabeticallyIgnoreCase(issue.Labels), colorScheme), + ProjectsList: stringifyProjects(issue.ProjectCards, issue.ProjectItems), + Body: issue.Body, + URL: issue.URL, + } + + if issue.Milestone != nil { + presentationIssue.MilestoneTitle = issue.Milestone.Title + } + + return presentationIssue, nil +} + +func stringifyProjects(projectCards api.ProjectCards, projectItems api.ProjectItems) string { + if len(projectCards.Nodes) == 0 && len(projectItems.Nodes) == 0 { + return "" + } + + projectNames := make([]string, len(projectCards.Nodes)+len(projectItems.Nodes)) + for i, project := range projectCards.Nodes { + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames[i] = fmt.Sprintf("%s (%s)", project.Project.Name, colName) + } + + for i, project := range projectItems.Nodes { + statusName := project.Status.Name + if statusName == "" { + statusName = "Backlog" + } + projectNames[i+len(projectCards.Nodes)] = fmt.Sprintf("%s (%s)", project.Project.Title, statusName) + } + + list := strings.Join(projectNames, ", ") + if projectCards.TotalCount > len(projectCards.Nodes) { + list += ", …" + } + return list +} + +func stringifyAssignees(issueAssignees api.Assignees) string { + if len(issueAssignees.Nodes) == 0 { + return "" + } + + AssigneeNames := make([]string, 0, len(issueAssignees.Nodes)) + for _, assignee := range issueAssignees.Nodes { + AssigneeNames = append(AssigneeNames, assignee.Login) + } + + list := strings.Join(AssigneeNames, ", ") + if issueAssignees.TotalCount > len(issueAssignees.Nodes) { + list += ", …" + } + return list +} + +func stringifyAndColorizeLabels(issueLabels api.Labels, colorScheme *iostreams.ColorScheme) string { + labelNames := make([]string, len(issueLabels.Nodes)) + for j, label := range issueLabels.Nodes { + if colorScheme == nil { + labelNames[j] = label.Name + } else { + labelNames[j] = colorScheme.HexToRGB(label.Color, label.Name) + } + } + + return strings.Join(labelNames, ", ") +} + +func sortAlphabeticallyIgnoreCase(l api.Labels) api.Labels { + slices.SortStableFunc(l.Nodes, func(a, b api.IssueLabel) int { + return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name)) + }) + return l +} diff --git a/pkg/cmd/issue/view/presentation_issue_test.go b/pkg/cmd/issue/view/presentation_issue_test.go new file mode 100644 index 00000000000..46d7bad470d --- /dev/null +++ b/pkg/cmd/issue/view/presentation_issue_test.go @@ -0,0 +1,398 @@ +package view + +import ( + "testing" + "time" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" +) + +func Test_MapApiIssueToPresentationIssue(t *testing.T) { + createdAt, err := time.Parse(time.RFC3339, "2022-01-01T00:00:00Z") + if err != nil { + t.Fatal(err) + } + + tests := map[string]struct { + issue *api.Issue + expect PresentationIssue + }{ + "basic integration test": { + issue: &api.Issue{ + Title: "Title", + Number: 123, + Comments: api.Comments{ + Nodes: []api.Comment{ + { + Author: api.CommentAuthor{ + Login: "monalisa", + }, + Body: "comment body 1", + ReactionGroups: api.ReactionGroups{}, + }, + }, + TotalCount: 1, + }, + State: "OPEN", + StateReason: "", + URL: "github.com/OWNER/REPO/issues/123", + Author: api.Author{ + Login: "author", + Name: "Octo Cat", + ID: "321", + }, + Assignees: api.Assignees{ + Nodes: []api.GitHubUser{ + { + Login: "assignee", + Name: "Octo Cat", + ID: "321", + }, + }, + TotalCount: 1, + }, + Labels: api.Labels{ + Nodes: []api.IssueLabel{ + { + Name: "bug", + Color: "fc0303", + }, + }, + TotalCount: 1, + }, + ProjectCards: api.ProjectCards{ + Nodes: []*api.ProjectInfo{ + { + Project: api.ProjectV1ProjectName{ + Name: "ProjectCardName", + }, + Column: api.ProjectV1ProjectColumn{ + Name: "ProjectCardColumn", + }, + }, + }, + TotalCount: 1, + }, + ProjectItems: api.ProjectItems{ + Nodes: []*api.ProjectV2Item{ + { + ID: "projectItemID", + Project: api.ProjectV2ItemProject{ + ID: "projectID", + Title: "V2 Project Title", + }, + Status: api.ProjectV2ItemStatus{ + OptionID: "statusID", + Name: "STATUS", + }, + }, + }, + }, + Milestone: &api.Milestone{ + Title: "MilestoneTitle", + }, + ReactionGroups: api.ReactionGroups{ + { + Content: "THUMBS_UP", + Users: api.ReactionGroupUsers{ + TotalCount: 1, + }, + }, + }, + CreatedAt: createdAt, + }, + expect: PresentationIssue{ + Title: "Title", + Number: 123, + CreatedAt: createdAt, + Comments: api.Comments{ + Nodes: []api.Comment{ + { + Author: api.CommentAuthor{ + Login: "monalisa", + }, + Body: "comment body 1", + ReactionGroups: api.ReactionGroups{}, + }, + }, + TotalCount: 1, + }, + State: "OPEN", + StateReason: "", + Reactions: "1 \U0001f44d", + Author: "author", + AssigneesList: "assignee", + LabelsList: "bug", + ProjectsList: "ProjectCardName (ProjectCardColumn), V2 Project Title (STATUS)", + MilestoneTitle: "MilestoneTitle", + Body: "", + URL: "github.com/OWNER/REPO/issues/123", + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + presentationIssue, err := MapApiIssueToPresentationIssue(tc.issue, nil) + if err != nil { + t.Fatal(err) + } + + // These are here to aid development and debugging of the individual pieces + assert.Equal(t, presentationIssue.Title, tc.expect.Title) + assert.Equal(t, presentationIssue.Number, tc.expect.Number) + assert.Equal(t, presentationIssue.CreatedAt, tc.expect.CreatedAt) + assert.Equal(t, presentationIssue.Comments, tc.expect.Comments) + assert.Equal(t, presentationIssue.State, tc.expect.State) + assert.Equal(t, presentationIssue.Reactions, tc.expect.Reactions) + assert.Equal(t, presentationIssue.Author, tc.expect.Author) + assert.Equal(t, presentationIssue.AssigneesList, tc.expect.AssigneesList) + assert.Equal(t, presentationIssue.LabelsList, tc.expect.LabelsList) + assert.Equal(t, presentationIssue.ProjectsList, tc.expect.ProjectsList) + assert.Equal(t, presentationIssue.MilestoneTitle, tc.expect.MilestoneTitle) + assert.Equal(t, presentationIssue.Body, tc.expect.Body) + assert.Equal(t, presentationIssue.URL, tc.expect.URL) + + // This tests the entire struct + assert.Equal(t, presentationIssue, tc.expect) + }) + } +} + +func Test_stringifyAssignees(t *testing.T) { + tests := map[string]struct { + assignees api.Assignees + expected string + }{ + "two assignees": { + assignees: api.Assignees{ + Nodes: []api.GitHubUser{ + {Login: "monalisa"}, + {Login: "hubot"}, + }, + TotalCount: 2, + }, + expected: "monalisa, hubot", + }, + "no assignees": { + assignees: api.Assignees{}, + expected: "", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, stringifyAssignees(tc.assignees)) + }) + } +} + +func Test_stringifyAndColorizeLabels(t *testing.T) { + tests := map[string]struct { + labels api.Labels + isColorSchemeEnabled bool + expected string + }{ + "no labels": { + labels: api.Labels{}, + expected: "", + }, + "single label no colorScheme": { + labels: api.Labels{ + Nodes: []api.IssueLabel{ + { + Name: "bug", + Color: "fc0303", + }, + }, + }, + isColorSchemeEnabled: false, + expected: "bug", + }, + "single label with colorScheme": { + labels: api.Labels{ + Nodes: []api.IssueLabel{ + { + Name: "bug", + Color: "fc0303", + }, + }, + }, + isColorSchemeEnabled: true, + expected: "\033[38;2;252;3;3mbug\033[0m", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + ios.SetColorEnabled(tc.isColorSchemeEnabled) + + assert.Equal(t, stringifyAndColorizeLabels(tc.labels, ios.ColorScheme()), tc.expected) + }) + } +} + +func Test_stringifyProjects(t *testing.T) { + tests := map[string]struct { + projectCards api.ProjectCards + projectItems api.ProjectItems + expected string + }{ + "no projects": { + projectCards: api.ProjectCards{ + Nodes: []*api.ProjectInfo{}, + TotalCount: 0, + }, + projectItems: api.ProjectItems{ + Nodes: []*api.ProjectV2Item{}, + }, + expected: "", + }, + "two v1 projects and no v2 projects": { + projectCards: api.ProjectCards{ + Nodes: []*api.ProjectInfo{ + {Project: api.ProjectV1ProjectName{Name: "Project 1"}, Column: api.ProjectV1ProjectColumn{Name: "Column 1"}}, + {Project: api.ProjectV1ProjectName{Name: "Project 2"}, Column: api.ProjectV1ProjectColumn{Name: "Column 2"}}, + }, + TotalCount: 2, + }, + projectItems: api.ProjectItems{ + Nodes: []*api.ProjectV2Item{}, + }, + expected: "Project 1 (Column 1), Project 2 (Column 2)", + }, + "two v1 projects without columns and no v2 projects": { + projectCards: api.ProjectCards{ + Nodes: []*api.ProjectInfo{ + {Project: api.ProjectV1ProjectName{Name: "Project 1"}, Column: api.ProjectV1ProjectColumn{Name: ""}}, + {Project: api.ProjectV1ProjectName{Name: "Project 2"}, Column: api.ProjectV1ProjectColumn{Name: ""}}, + }, + TotalCount: 2, + }, + projectItems: api.ProjectItems{ + Nodes: []*api.ProjectV2Item{}, + }, + expected: "Project 1 (Awaiting triage), Project 2 (Awaiting triage)", + }, + "no v1 projects and 2 v2 projects": { + projectCards: api.ProjectCards{ + Nodes: []*api.ProjectInfo{}, + TotalCount: 0, + }, + projectItems: api.ProjectItems{ + Nodes: []*api.ProjectV2Item{ + { + ID: "projectItemID", + Project: api.ProjectV2ItemProject{ + ID: "projectID", + Title: "V2 Project Title", + }, + Status: api.ProjectV2ItemStatus{ + OptionID: "statusID", + Name: "STATUS", + }, + }, + { + ID: "projectItemID2", + Project: api.ProjectV2ItemProject{ + ID: "projectID2", + Title: "V2 Project Title 2", + }, + Status: api.ProjectV2ItemStatus{ + OptionID: "statusID2", + Name: "", + }, + }, + }, + }, + expected: "V2 Project Title (STATUS), V2 Project Title 2 (Backlog)", + }, + "1 v1 project and 1 v2 project": { + projectCards: api.ProjectCards{ + Nodes: []*api.ProjectInfo{ + {Project: api.ProjectV1ProjectName{Name: "V1 Project Name"}, Column: api.ProjectV1ProjectColumn{Name: "COLUMN"}}, + }, + TotalCount: 1, + }, + projectItems: api.ProjectItems{ + Nodes: []*api.ProjectV2Item{ + { + ID: "projectItemID", + Project: api.ProjectV2ItemProject{ + ID: "projectID", + Title: "V2 Project Title", + }, + Status: api.ProjectV2ItemStatus{ + OptionID: "statusID", + Name: "STATUS", + }, + }, + }, + }, + expected: "V1 Project Name (COLUMN), V2 Project Title (STATUS)", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, stringifyProjects(tc.projectCards, tc.projectItems), tc.expected) + }) + } +} + +func Test_sortAlphabeticallyIgnoreCase(t *testing.T) { + tests := map[string]struct { + labels api.Labels + expected api.Labels + }{ + "no repeat labels": { + labels: api.Labels{ + Nodes: []api.IssueLabel{ + {Name: "c"}, + {Name: "B"}, + {Name: "a"}, + }, + }, + expected: api.Labels{ + Nodes: []api.IssueLabel{ + {Name: "a"}, + {Name: "B"}, + {Name: "c"}, + }, + }, + }, + "repeat labels case insensitive": { + labels: api.Labels{ + Nodes: []api.IssueLabel{ + {Name: "c"}, + {Name: "B"}, + {Name: "C"}, + }, + }, + expected: api.Labels{ + Nodes: []api.IssueLabel{ + {Name: "B"}, + {Name: "c"}, + {Name: "C"}, + }, + }, + }, + "no labels": { + labels: api.Labels{ + Nodes: []api.IssueLabel{}, + }, + expected: api.Labels{ + Nodes: []api.IssueLabel{}, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, sortAlphabeticallyIgnoreCase(tc.labels)) + }) + } +} diff --git a/pkg/cmd/issue/view/raw_issue_printer.go b/pkg/cmd/issue/view/raw_issue_printer.go new file mode 100644 index 00000000000..8f0d6610246 --- /dev/null +++ b/pkg/cmd/issue/view/raw_issue_printer.go @@ -0,0 +1,39 @@ +package view + +import ( + "fmt" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" + "github.com/cli/cli/v2/pkg/iostreams" +) + +type RawIssuePrinter struct { + IO *iostreams.IOStreams + Comments bool +} + +// Print outputs the issue to the terminal for non-TTY use cases. +func (p *RawIssuePrinter) Print(pi PresentationIssue, repo ghrepo.Interface) error { + if p.Comments { + fmt.Fprint(p.IO.Out, prShared.RawCommentList(pi.Comments, api.PullRequestReviews{})) + return nil + } + + // Print empty strings for empty values so the number of metadata lines is consistent when + // processing many issues with head and grep. + fmt.Fprintf(p.IO.Out, "title:\t%s\n", pi.Title) + fmt.Fprintf(p.IO.Out, "state:\t%s\n", pi.State) + fmt.Fprintf(p.IO.Out, "author:\t%s\n", pi.Author) + fmt.Fprintf(p.IO.Out, "labels:\t%s\n", pi.LabelsList) + fmt.Fprintf(p.IO.Out, "comments:\t%d\n", pi.Comments.TotalCount) + fmt.Fprintf(p.IO.Out, "assignees:\t%s\n", pi.AssigneesList) + fmt.Fprintf(p.IO.Out, "projects:\t%s\n", pi.ProjectsList) + fmt.Fprintf(p.IO.Out, "milestone:\t%s\n", pi.MilestoneTitle) + fmt.Fprintf(p.IO.Out, "number:\t%d\n", pi.Number) + fmt.Fprintln(p.IO.Out, "--") + fmt.Fprintln(p.IO.Out, pi.Body) + + return nil +} diff --git a/pkg/cmd/issue/view/raw_issue_printer_test.go b/pkg/cmd/issue/view/raw_issue_printer_test.go new file mode 100644 index 00000000000..b0d3465cb7b --- /dev/null +++ b/pkg/cmd/issue/view/raw_issue_printer_test.go @@ -0,0 +1,70 @@ +package view + +import ( + "testing" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" +) + +func TestRawIssuePrinting(t *testing.T) { + tests := map[string]struct { + comments bool + presentationIssue PresentationIssue + expectedOutput string + }{ + "basic issue, no comments requested": { + comments: false, + presentationIssue: PresentationIssue{ + Title: "issueTitle", + State: "issueState", + Author: "authorLogin", + LabelsList: "labelsList", + Comments: api.Comments{ + TotalCount: 1, + }, + AssigneesList: "assigneesList", + ProjectsList: "projectsList", + MilestoneTitle: "milestoneTitle", + Number: 123, + Body: "issueBody", + }, + expectedOutput: "title:\tissueTitle\nstate:\tissueState\nauthor:\tauthorLogin\nlabels:\tlabelsList\ncomments:\t1\nassignees:\tassigneesList\nprojects:\tprojectsList\nmilestone:\tmilestoneTitle\nnumber:\t123\n--\nissueBody\n", + }, + "basic issue, displays only comments when requested": { + comments: true, + presentationIssue: PresentationIssue{ + Comments: api.Comments{ + TotalCount: 1, + Nodes: []api.Comment{ + { + Author: api.CommentAuthor{ + Login: "test-author", + }, + AuthorAssociation: "member", + Body: "comment body", + IncludesCreatedEdit: false, + }, + }, + }, + }, + expectedOutput: "author:\ttest-author\nassociation:\tmember\nedited:\tfalse\nstatus:\tnone\n--\ncomment body\n--\n", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + + rip := &RawIssuePrinter{ + IO: ios, + Comments: tc.comments, + } + + rip.Print(tc.presentationIssue, ghrepo.New("OWNER", "REPO")) + assert.Equal(t, tc.expectedOutput, stdout.String()) + }) + } +} diff --git a/pkg/cmd/issue/view/rich_issue_printer.go b/pkg/cmd/issue/view/rich_issue_printer.go new file mode 100644 index 00000000000..1b6827f269a --- /dev/null +++ b/pkg/cmd/issue/view/rich_issue_printer.go @@ -0,0 +1,143 @@ +package view + +import ( + "fmt" + "time" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/text" + prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/markdown" +) + +type RichIssuePrinter struct { + IO *iostreams.IOStreams + TimeNow time.Time + Comments bool +} + +// Print outputs an issue to the terminal for TTY use cases. +func (p *RichIssuePrinter) Print(pi PresentationIssue, repo ghrepo.Interface) error { + + // header (Title and State) + p.header(pi, repo) + // Reactions + p.reactions(pi) + // Metadata + p.assigneeList(pi) + p.labelList(pi) + p.projectList(pi) + + p.milestone(pi) + + // Body + err := p.body(pi) + if err != nil { + return err + } + + // Comments + isCommentsPreview := !p.Comments + err = p.comments(pi, isCommentsPreview) + if err != nil { + return err + } + + // Footer + p.footer(pi) + + return nil +} + +func (p *RichIssuePrinter) header(pi PresentationIssue, repo ghrepo.Interface) { + fmt.Fprintf(p.IO.Out, "%s %s#%d\n", p.IO.ColorScheme().Bold(pi.Title), ghrepo.FullName(repo), pi.Number) + fmt.Fprintf(p.IO.Out, + "%s • %s opened %s • %s\n", + p.issueStateTitleWithColor(pi.State, pi.StateReason), + pi.Author, + text.FuzzyAgo(p.TimeNow, pi.CreatedAt), + text.Pluralize(pi.Comments.TotalCount, "comment"), + ) +} + +func (p *RichIssuePrinter) issueStateTitleWithColor(state string, stateReason string) string { + colorFunc := p.IO.ColorScheme().ColorFromString(prShared.ColorForIssueState(state, stateReason)) + formattedState := "Open" + if state == "CLOSED" { + formattedState = "Closed" + } + return colorFunc(formattedState) +} + +func (p *RichIssuePrinter) reactions(pi PresentationIssue) { + if pi.Reactions != "" { + fmt.Fprint(p.IO.Out, pi.Reactions) + fmt.Fprintln(p.IO.Out) + } +} + +func (p *RichIssuePrinter) assigneeList(pi PresentationIssue) { + assignees := pi.AssigneesList + if assignees != "" { + fmt.Fprint(p.IO.Out, p.IO.ColorScheme().Bold("Assignees: ")) + fmt.Fprintln(p.IO.Out, assignees) + } +} + +func (p *RichIssuePrinter) labelList(pi PresentationIssue) { + labels := pi.LabelsList + if labels != "" { + fmt.Fprint(p.IO.Out, p.IO.ColorScheme().Bold("Labels: ")) + fmt.Fprintln(p.IO.Out, labels) + } +} + +func (p *RichIssuePrinter) projectList(pi PresentationIssue) { + projects := pi.ProjectsList + if projects != "" { + fmt.Fprint(p.IO.Out, p.IO.ColorScheme().Bold("Projects: ")) + fmt.Fprintln(p.IO.Out, projects) + } +} + +func (p *RichIssuePrinter) milestone(pi PresentationIssue) { + if pi.MilestoneTitle != "" { + fmt.Fprint(p.IO.Out, p.IO.ColorScheme().Bold("Milestone: ")) + fmt.Fprintln(p.IO.Out, pi.MilestoneTitle) + } +} + +func (p *RichIssuePrinter) body(pi PresentationIssue) error { + var md string + var err error + body := pi.Body + if body == "" { + md = fmt.Sprintf("\n %s\n\n", p.IO.ColorScheme().Gray("No description provided")) + } else { + md, err = markdown.Render(body, + markdown.WithTheme(p.IO.TerminalTheme()), + markdown.WithWrap(p.IO.TerminalWidth())) + if err != nil { + return err + } + } + fmt.Fprintf(p.IO.Out, "\n%s\n", md) + return nil +} + +func (p *RichIssuePrinter) comments(pi PresentationIssue, isPreview bool) error { + if pi.Comments.TotalCount > 0 { + comments, err := prShared.CommentList(p.IO, pi.Comments, api.PullRequestReviews{}, isPreview) + if err != nil { + return err + } + fmt.Fprint(p.IO.Out, comments) + } + return nil +} + +func (p *RichIssuePrinter) footer(pi PresentationIssue) { + fmt.Fprintf(p.IO.Out, p.IO.ColorScheme().Gray("View this issue on GitHub: %s\n"), pi.URL) +} diff --git a/pkg/cmd/issue/view/rich_issue_printer_test.go b/pkg/cmd/issue/view/rich_issue_printer_test.go new file mode 100644 index 00000000000..68640106fbb --- /dev/null +++ b/pkg/cmd/issue/view/rich_issue_printer_test.go @@ -0,0 +1,404 @@ +package view + +import ( + "testing" + "time" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" +) + +// This does not test color, just output +func Test_header(t *testing.T) { + tests := map[string]struct { + title string + number int + state string + stateReason string + createdAt string + author string + baseRepo ghrepo.Interface + expected string + }{ + "simple open issue": { + title: "Simple Issue Test", + baseRepo: ghrepo.New("OWNER", "REPO"), + number: 123, + state: "OPEN", + stateReason: "", + author: "monalisa", + createdAt: "2022-01-01T00:00:00Z", + expected: "Simple Issue Test OWNER/REPO#123\nOpen • monalisa opened about 1 day ago • 1 comment\n", + }, + "simple closed issue": { + title: "Simple Issue Test", + baseRepo: ghrepo.New("OWNER", "REPO"), + number: 123, + state: "CLOSED", + stateReason: "COMPLETED", + author: "monalisa", + createdAt: "2022-01-01T00:00:00Z", + expected: "Simple Issue Test OWNER/REPO#123\nClosed • monalisa opened about 1 day ago • 1 comment\n", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + createdAtTime, err := time.Parse(time.RFC3339, tc.createdAt) + if err != nil { + t.Fatal(err) + } + + presentationIssue := PresentationIssue{ + Title: tc.title, + Number: tc.number, + Comments: api.Comments{ + TotalCount: 1, + }, + State: tc.state, + StateReason: tc.stateReason, + CreatedAt: createdAtTime, + Author: tc.author, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + TimeNow: createdAtTime.AddDate(0, 0, 1), + } + richIssuePrinter.header(presentationIssue, tc.baseRepo) + assert.Equal(t, tc.expected, stdout.String()) + }) + } +} + +func Test_reactions(t *testing.T) { + tests := map[string]struct { + reactions string + expected string + }{ + "no reactions": { + reactions: "", + expected: "", + }, + "reactions": { + reactions: "a reaction", + expected: "a reaction\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := PresentationIssue{ + Reactions: tc.reactions, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + richIssuePrinter.reactions(presentationIssue) + assert.Equal(t, stdout.String(), tc.expected) + }) + } +} + +func Test_assigneeList(t *testing.T) { + tests := map[string]struct { + assignees string + expected string + }{ + "no assignees": { + assignees: "", + expected: "", + }, + "assignees": { + assignees: "monalisa, octocat", + expected: "Assignees: monalisa, octocat\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := PresentationIssue{ + AssigneesList: tc.assignees, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + + richIssuePrinter.assigneeList(presentationIssue) + assert.Equal(t, stdout.String(), tc.expected) + }) + } +} + +func Test_labelList(t *testing.T) { + tests := map[string]struct { + labels string + expected string + }{ + "no labels": { + labels: "", + expected: "", + }, + "labels": { + labels: "bug, enhancement", + expected: "Labels: bug, enhancement\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := PresentationIssue{ + LabelsList: tc.labels, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + + richIssuePrinter.labelList(presentationIssue) + assert.Equal(t, stdout.String(), tc.expected) + }) + } +} + +func Test_projectList(t *testing.T) { + tests := map[string]struct { + projectList string + expected string + }{ + "no projects": { + projectList: "", + expected: "", + }, + "some projects": { + projectList: "ProjectV1 1 (Column 1), ProjectV1 2 (Awaiting triage)", + expected: "Projects: ProjectV1 1 (Column 1), ProjectV1 2 (Awaiting triage)\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := PresentationIssue{ + ProjectsList: tc.projectList, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + + richIssuePrinter.projectList(presentationIssue) + assert.Equal(t, stdout.String(), tc.expected) + }) + } +} + +func Test_milestone(t *testing.T) { + tests := map[string]struct { + milestone string + expected string + }{ + "no milestone": { + milestone: "", + expected: "", + }, + "milestone": { + milestone: "milestoneTitle", + expected: "Milestone: milestoneTitle\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := PresentationIssue{ + MilestoneTitle: tc.milestone, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + + richIssuePrinter.milestone(presentationIssue) + assert.Equal(t, stdout.String(), tc.expected) + }) + } +} + +func Test_body(t *testing.T) { + tests := map[string]struct { + body string + expected string + }{ + "no body": { + body: "", + expected: "No description provided", + }, + "with body": { + body: "This is a body", + expected: "This is a body", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := PresentationIssue{ + Body: tc.body, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + + err := richIssuePrinter.body(presentationIssue) + if err != nil { + t.Fatal(err) + } + // This is getting around whitespace issues I was having with assert.Equal + assert.Contains(t, stdout.String(), tc.expected) + }) + } +} + +func Test_comments(t *testing.T) { + test := map[string]struct { + commentNodes []api.Comment + expected string + isPreview bool + }{ + "no comments": { + commentNodes: []api.Comment{}, + expected: "", + isPreview: false, + }, + "comments": { + commentNodes: []api.Comment{ + { + Author: api.CommentAuthor{ + Login: "monalisa", + }, + Body: "body 1", + ReactionGroups: api.ReactionGroups{}, + }, + }, + expected: "monalisa () • Dec 31, 2021 • Newest comment\n\n body 1", + isPreview: false, + }, + "preview": { + commentNodes: []api.Comment{ + { + Author: api.CommentAuthor{ + Login: "monalisa", + }, + Body: "body 1", + ReactionGroups: api.ReactionGroups{}, + }, + }, + expected: "monalisa () • Dec 31, 2021 • Newest comment\n\n body 1", + isPreview: true, + }, + } + for name, tc := range test { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + timeNow, err := time.Parse(time.RFC3339, "2022-01-01T00:00:00Z") + if err != nil { + t.Fatal(err) + } + + for i := range tc.commentNodes { + // subtract a day + tc.commentNodes[i].CreatedAt = timeNow.AddDate(0, 0, -1) + } + + presentationIssue := PresentationIssue{ + Comments: api.Comments{ + Nodes: tc.commentNodes, + TotalCount: len(tc.commentNodes), + }, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + TimeNow: timeNow, + } + + err = richIssuePrinter.comments(presentationIssue, tc.isPreview) + if err != nil { + t.Fatal(err) + } + + assert.Contains(t, stdout.String(), tc.expected) + }) + } +} + +func Test_footer(t *testing.T) { + tests := map[string]struct { + url string + expected string + }{ + "no url": { + url: "", + expected: "View this issue on GitHub: \n", + }, + "with url": { + url: "github.com/OWNER/REPO/issues/123", + expected: "View this issue on GitHub: github.com/OWNER/REPO/issues/123\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + presentationIssue := &PresentationIssue{ + URL: tc.url, + } + + richIssuePrinter := &RichIssuePrinter{ + IO: ios, + } + + richIssuePrinter.footer(*presentationIssue) + assert.Equal(t, stdout.String(), tc.expected) + }) + } +} diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index b188c6a4ca3..cd9c7a25874 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -3,22 +3,18 @@ package view import ( "errors" "fmt" - "io" "net/http" - "sort" - "strings" "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/text" issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared" - prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/markdown" "github.com/cli/cli/v2/pkg/set" "github.com/spf13/cobra" ) @@ -28,6 +24,7 @@ type ViewOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser + Detector fd.Detector SelectorArg string WebMode bool @@ -35,6 +32,12 @@ type ViewOptions struct { Exporter cmdutil.Exporter Now func() time.Time + + IssuePrinter IssuePrinter +} + +type IssuePrinter interface { + Print(PresentationIssue, ghrepo.Interface) error } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -62,6 +65,19 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman opts.SelectorArg = args[0] } + if opts.IO.IsStdoutTTY() { + opts.IssuePrinter = &RichIssuePrinter{ + IO: opts.IO, + TimeNow: opts.Now(), + Comments: opts.Comments, + } + } else { + opts.IssuePrinter = &RawIssuePrinter{ + IO: opts.IO, + Comments: opts.Comments, + } + } + if runF != nil { return runF(opts) } @@ -103,7 +119,7 @@ func viewRun(opts *ViewOptions) error { opts.IO.DetectTerminalTheme() opts.IO.StartProgressIndicator() - issue, baseRepo, err := findIssue(httpClient, opts.BaseRepo, opts.SelectorArg, lookupFields.ToSlice()) + issue, baseRepo, err := findIssue(httpClient, opts.BaseRepo, opts.SelectorArg, lookupFields.ToSlice(), opts.Detector) opts.IO.StopProgressIndicator() if err != nil { var loadErr *issueShared.PartialLoadError @@ -131,24 +147,20 @@ func viewRun(opts *ViewOptions) error { return opts.Exporter.Write(opts.IO, issue) } - if opts.IO.IsStdoutTTY() { - return printHumanIssuePreview(opts, baseRepo, issue) - } - - if opts.Comments { - fmt.Fprint(opts.IO.Out, prShared.RawCommentList(issue.Comments, api.PullRequestReviews{})) - return nil + presentationIssue, err := MapApiIssueToPresentationIssue(issue, opts.IO.ColorScheme()) + if err != nil { + return err } - return printRawIssuePreview(opts.IO.Out, issue) + return opts.IssuePrinter.Print(presentationIssue, baseRepo) } -func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), selector string, fields []string) (*api.Issue, ghrepo.Interface, error) { +func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), selector string, fields []string, detector fd.Detector) (*api.Issue, ghrepo.Interface, error) { fieldSet := set.NewStringSet() fieldSet.AddValues(fields) fieldSet.Add("id") - issue, repo, err := issueShared.IssueFromArgWithFields(client, baseRepoFn, selector, fieldSet.ToSlice()) + issue, repo, err := issueShared.IssueFromArgWithFields(client, baseRepoFn, selector, fieldSet.ToSlice(), detector) if err != nil { return issue, repo, err } @@ -160,166 +172,3 @@ func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), } return issue, repo, err } - -func printRawIssuePreview(out io.Writer, issue *api.Issue) error { - assignees := issueAssigneeList(*issue) - labels := issueLabelList(issue, nil) - projects := issueProjectList(*issue) - - // Print empty strings for empty values so the number of metadata lines is consistent when - // processing many issues with head and grep. - fmt.Fprintf(out, "title:\t%s\n", issue.Title) - fmt.Fprintf(out, "state:\t%s\n", issue.State) - fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login) - fmt.Fprintf(out, "labels:\t%s\n", labels) - fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount) - fmt.Fprintf(out, "assignees:\t%s\n", assignees) - fmt.Fprintf(out, "projects:\t%s\n", projects) - var milestoneTitle string - if issue.Milestone != nil { - milestoneTitle = issue.Milestone.Title - } - fmt.Fprintf(out, "milestone:\t%s\n", milestoneTitle) - fmt.Fprintf(out, "number:\t%d\n", issue.Number) - fmt.Fprintln(out, "--") - fmt.Fprintln(out, issue.Body) - return nil -} - -func printHumanIssuePreview(opts *ViewOptions, baseRepo ghrepo.Interface, issue *api.Issue) error { - out := opts.IO.Out - cs := opts.IO.ColorScheme() - - // Header (Title and State) - fmt.Fprintf(out, "%s %s#%d\n", cs.Bold(issue.Title), ghrepo.FullName(baseRepo), issue.Number) - fmt.Fprintf(out, - "%s • %s opened %s • %s\n", - issueStateTitleWithColor(cs, issue), - issue.Author.Login, - text.FuzzyAgo(opts.Now(), issue.CreatedAt), - text.Pluralize(issue.Comments.TotalCount, "comment"), - ) - - // Reactions - if reactions := prShared.ReactionGroupList(issue.ReactionGroups); reactions != "" { - fmt.Fprint(out, reactions) - fmt.Fprintln(out) - } - - // Metadata - if assignees := issueAssigneeList(*issue); assignees != "" { - fmt.Fprint(out, cs.Bold("Assignees: ")) - fmt.Fprintln(out, assignees) - } - if labels := issueLabelList(issue, cs); labels != "" { - fmt.Fprint(out, cs.Bold("Labels: ")) - fmt.Fprintln(out, labels) - } - if projects := issueProjectList(*issue); projects != "" { - fmt.Fprint(out, cs.Bold("Projects: ")) - fmt.Fprintln(out, projects) - } - if issue.Milestone != nil { - fmt.Fprint(out, cs.Bold("Milestone: ")) - fmt.Fprintln(out, issue.Milestone.Title) - } - - // Body - var md string - var err error - if issue.Body == "" { - md = fmt.Sprintf("\n %s\n\n", cs.Gray("No description provided")) - } else { - md, err = markdown.Render(issue.Body, - markdown.WithTheme(opts.IO.TerminalTheme()), - markdown.WithWrap(opts.IO.TerminalWidth())) - if err != nil { - return err - } - } - fmt.Fprintf(out, "\n%s\n", md) - - // Comments - if issue.Comments.TotalCount > 0 { - preview := !opts.Comments - comments, err := prShared.CommentList(opts.IO, issue.Comments, api.PullRequestReviews{}, preview) - if err != nil { - return err - } - fmt.Fprint(out, comments) - } - - // Footer - fmt.Fprintf(out, cs.Gray("View this issue on GitHub: %s\n"), issue.URL) - - return nil -} - -func issueStateTitleWithColor(cs *iostreams.ColorScheme, issue *api.Issue) string { - colorFunc := cs.ColorFromString(prShared.ColorForIssueState(*issue)) - state := "Open" - if issue.State == "CLOSED" { - state = "Closed" - } - return colorFunc(state) -} - -func issueAssigneeList(issue api.Issue) string { - if len(issue.Assignees.Nodes) == 0 { - return "" - } - - AssigneeNames := make([]string, 0, len(issue.Assignees.Nodes)) - for _, assignee := range issue.Assignees.Nodes { - AssigneeNames = append(AssigneeNames, assignee.Login) - } - - list := strings.Join(AssigneeNames, ", ") - if issue.Assignees.TotalCount > len(issue.Assignees.Nodes) { - list += ", …" - } - return list -} - -func issueProjectList(issue api.Issue) string { - if len(issue.ProjectCards.Nodes) == 0 { - return "" - } - - projectNames := make([]string, 0, len(issue.ProjectCards.Nodes)) - for _, project := range issue.ProjectCards.Nodes { - colName := project.Column.Name - if colName == "" { - colName = "Awaiting triage" - } - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) - } - - list := strings.Join(projectNames, ", ") - if issue.ProjectCards.TotalCount > len(issue.ProjectCards.Nodes) { - list += ", …" - } - return list -} - -func issueLabelList(issue *api.Issue, cs *iostreams.ColorScheme) string { - if len(issue.Labels.Nodes) == 0 { - return "" - } - - // ignore case sort - sort.SliceStable(issue.Labels.Nodes, func(i, j int) bool { - return strings.ToLower(issue.Labels.Nodes[i].Name) < strings.ToLower(issue.Labels.Nodes[j].Name) - }) - - labelNames := make([]string, len(issue.Labels.Nodes)) - for i, label := range issue.Labels.Nodes { - if cs == nil { - labelNames[i] = label.Name - } else { - labelNames[i] = cs.HexToRGB(label.Color, label.Name) - } - } - - return strings.Join(labelNames, ", ") -} diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index e1798af9f83..5df91d5abeb 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" @@ -66,7 +67,10 @@ func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, err }, } - cmd := NewCmdView(factory, nil) + cmd := NewCmdView(factory, func(opts *ViewOptions) error { + opts.Detector = &featuredetection.EnabledDetectorMock{} + return viewRun(opts) + }) argv, err := shlex.Split(cli) if err != nil { @@ -261,11 +265,11 @@ func TestIssueView_tty_Preview(t *testing.T) { httpReg.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + stubbedTime, _ := time.Parse(time.RFC822, "03 Nov 20 15:04 UTC") opts := ViewOptions{ IO: ios, Now: func() time.Time { - t, _ := time.Parse(time.RFC822, "03 Nov 20 15:04 UTC") - return t + return stubbedTime }, HttpClient: func() (*http.Client, error) { return &http.Client{Transport: httpReg}, nil @@ -274,6 +278,9 @@ func TestIssueView_tty_Preview(t *testing.T) { return ghrepo.New("OWNER", "REPO"), nil }, SelectorArg: "123", + Detector: &featuredetection.EnabledDetectorMock{}, + + IssuePrinter: &RichIssuePrinter{IO: ios, TimeNow: stubbedTime}, } err := viewRun(&opts) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 0066bbc6e02..40a34e8fa1a 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -18,6 +18,7 @@ import ( ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/browser" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/text" @@ -40,6 +41,7 @@ type CreateOptions struct { Prompter shared.Prompt Finder shared.PRFinder TitledEditSurvey func(string, string) (string, string, error) + Detector fd.Detector TitleProvided bool BodyProvided bool @@ -86,6 +88,7 @@ type CreateContext struct { IsPushEnabled bool Client *api.Client GitClient *git.Client + ProjectsV1Support gh.ProjectsV1Support } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { @@ -442,7 +445,7 @@ func createRun(opts *CreateOptions) error { Repo: ctx.BaseRepo, State: state, } - err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.BaseRepo, fetcher, state) + err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.BaseRepo, fetcher, state, ctx.ProjectsV1Support) if err != nil { return err } @@ -777,6 +780,15 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { baseTrackingBranch = fmt.Sprintf("%s/%s", baseRemote.Name, baseBranch) } + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) + } + projectsV1Support, err := opts.Detector.ProjectsV1() + if err != nil { + return nil, err + } + return &CreateContext{ BaseRepo: baseRepo, HeadRepo: headRepo, @@ -789,6 +801,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { RepoContext: repoContext, Client: client, GitClient: gitClient, + ProjectsV1Support: projectsV1Support, }, nil } @@ -821,7 +834,7 @@ func submitPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataS return errors.New("pull request title must not be blank") } - err := shared.AddMetadataToIssueParams(client, ctx.BaseRepo, params, &state) + err := shared.AddMetadataToIssueParams(client, ctx.BaseRepo, params, &state, ctx.ProjectsV1Support) if err != nil { return err } @@ -1058,7 +1071,7 @@ func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState) (str ctx.BaseRepo, "compare/%s...%s?expand=1", url.PathEscape(ctx.BaseBranch), url.PathEscape(ctx.HeadBranchLabel)) - url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.BaseRepo, u, state) + url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.BaseRepo, u, state, ctx.ProjectsV1Support) if err != nil { return "", err } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 55012d7ddd9..4404298ae20 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -16,6 +16,7 @@ import ( "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" @@ -1593,6 +1594,7 @@ func Test_createRun(t *testing.T) { GhPath: "some/path/gh", GitPath: "some/path/git", } + opts.Detector = &featuredetection.EnabledDetectorMock{} cleanSetup := func() {} if tt.setup != nil { cleanSetup = tt.setup(&opts, t) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 3c8d73ad393..49bb26ee2ee 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -3,9 +3,11 @@ package edit import ( "fmt" "net/http" + "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" shared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -25,6 +27,7 @@ type EditOptions struct { Fetcher EditableOptionsFetcher EditorRetriever EditorRetriever Prompter shared.EditPrompter + Detector fd.Detector SelectorArg string Interactive bool @@ -230,8 +233,17 @@ func editRun(opts *EditOptions) error { } apiClient := api.NewClientFromHTTP(httpClient) + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, repo.RepoHost()) + } + projectsV1Support, err := opts.Detector.ProjectsV1() + if err != nil { + return err + } + opts.IO.StartProgressIndicator() - err = opts.Fetcher.EditableOptionsFetch(apiClient, repo, &editable) + err = opts.Fetcher.EditableOptionsFetch(apiClient, repo, &editable, projectsV1Support) opts.IO.StopProgressIndicator() if err != nil { return err @@ -311,13 +323,13 @@ func (s surveyor) EditFields(editable *shared.Editable, editorCmd string) error } type EditableOptionsFetcher interface { - EditableOptionsFetch(*api.Client, ghrepo.Interface, *shared.Editable) error + EditableOptionsFetch(*api.Client, ghrepo.Interface, *shared.Editable, gh.ProjectsV1Support) error } type fetcher struct{} -func (f fetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error { - return shared.FetchOptions(client, repo, opts) +func (f fetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable, projectsV1Support gh.ProjectsV1Support) error { + return shared.FetchOptions(client, repo, opts, projectsV1Support) } type EditorRetriever interface { diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 3c4882961ad..e64e039671e 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -9,6 +9,8 @@ import ( "testing" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/featuredetection" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" shared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -502,6 +504,7 @@ func Test_editRun(t *testing.T) { tt.input.IO = ios tt.input.HttpClient = httpClient + tt.input.Detector = &featuredetection.EnabledDetectorMock{} err := editRun(tt.input) assert.NoError(t, err) @@ -661,8 +664,8 @@ type testSurveyor struct { } type testEditorRetriever struct{} -func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error { - return shared.FetchOptions(client, repo, opts) +func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable, projectsV1Support gh.ProjectsV1Support) error { + return shared.FetchOptions(client, repo, opts, projectsV1Support) } func (s testSurveyor) FieldsToEdit(e *shared.Editable) error { diff --git a/pkg/cmd/pr/shared/commentable.go b/pkg/cmd/pr/shared/commentable.go index f909c755934..0bb38652065 100644 --- a/pkg/cmd/pr/shared/commentable.go +++ b/pkg/cmd/pr/shared/commentable.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/text" @@ -36,6 +37,7 @@ type Commentable interface { type CommentableOptions struct { IO *iostreams.IOStreams HttpClient func() (*http.Client, error) + Detector fd.Detector RetrieveCommentable func() (Commentable, ghrepo.Interface, error) EditSurvey func(string) (string, error) InteractiveEditSurvey func(string) (string, error) diff --git a/pkg/cmd/pr/shared/completion.go b/pkg/cmd/pr/shared/completion.go index e07abc5a73f..99bdae406d5 100644 --- a/pkg/cmd/pr/shared/completion.go +++ b/pkg/cmd/pr/shared/completion.go @@ -8,13 +8,14 @@ import ( "time" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" ) func RequestableReviewersForCompletion(httpClient *http.Client, repo ghrepo.Interface) ([]string, error) { client := api.NewClientFromHTTP(api.NewCachedHTTPClient(httpClient, time.Minute*2)) - metadata, err := api.RepoMetadata(client, repo, api.RepoMetadataInput{Reviewers: true}) + metadata, err := api.RepoMetadata(client, repo, api.RepoMetadataInput{Reviewers: true}, gh.ProjectsV1Unsupported) if err != nil { return nil, err } diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go index 02482951c5c..46ee9b9f0ec 100644 --- a/pkg/cmd/pr/shared/display.go +++ b/pkg/cmd/pr/shared/display.go @@ -41,12 +41,12 @@ func ColorForPRState(pr api.PullRequest) string { } } -func ColorForIssueState(issue api.Issue) string { - switch issue.State { +func ColorForIssueState(state string, stateReason string) string { + switch state { case "OPEN": return "green" case "CLOSED": - if issue.StateReason == "NOT_PLANNED" { + if stateReason == "NOT_PLANNED" { return "gray" } return "magenta" diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index cec3bfe8c9c..072cf5bd9ac 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/set" ) @@ -376,7 +377,7 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error { return nil } -func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) error { +func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, projectsV1Support gh.ProjectsV1Support) error { input := api.RepoMetadataInput{ Reviewers: editable.Reviewers.Edited, Assignees: editable.Assignees.Edited, @@ -384,7 +385,7 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) Projects: editable.Projects.Edited, Milestones: editable.Milestone.Edited, } - metadata, err := api.RepoMetadata(client, repo, input) + metadata, err := api.RepoMetadata(client, repo, input, projectsV1Support) if err != nil { return err } diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index a5452852788..57888574d4f 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -16,6 +16,7 @@ import ( remotes "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" fd "github.com/cli/cli/v2/internal/featuredetection" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/set" @@ -95,6 +96,8 @@ type FindOptions struct { BaseBranch string // States lists the possible PR states to scope the PR-for-branch lookup to. States []string + // Detector determines which features are available. + Detector fd.Detector } // TODO: Does this also need the BaseBranchName? @@ -214,6 +217,11 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err return nil, nil, err } + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost()) + } + // TODO(josebalius): Should we be guarding here? if f.progress != nil { f.progress.StartProgressIndicator() @@ -226,9 +234,7 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err fields.AddValues([]string{"id", "number"}) // for additional preload queries below if fields.Contains("isInMergeQueue") || fields.Contains("isMergeQueueEnabled") { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - detector := fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost()) - prFeatures, err := detector.PullRequestFeatures() + prFeatures, err := opts.Detector.PullRequestFeatures() if err != nil { return nil, nil, err } @@ -243,6 +249,15 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err getProjectItems = true fields.Remove("projectItems") } + if fields.Contains("projectCards") { + projectsV1Support, err := opts.Detector.ProjectsV1() + if err != nil { + return nil, nil, err + } + if projectsV1Support == gh.ProjectsV1Unsupported { + fields.Remove("projectCards") + } + } var pr *api.PullRequest if f.prNumber > 0 { diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 128c51068a0..b96ff57f4ef 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -6,12 +6,13 @@ import ( "strings" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/search" "github.com/google/shlex" ) -func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, baseURL string, state IssueMetadataState) (string, error) { +func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, baseURL string, state IssueMetadataState, projectsV1Support gh.ProjectsV1Support) (string, error) { u, err := url.Parse(baseURL) if err != nil { return "", err @@ -35,7 +36,7 @@ func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, ba q.Set("labels", strings.Join(state.Labels, ",")) } if len(state.Projects) > 0 { - projectPaths, err := api.ProjectNamesToPaths(client, baseRepo, state.Projects) + projectPaths, err := api.ProjectNamesToPaths(client, baseRepo, state.Projects, projectsV1Support) if err != nil { return "", fmt.Errorf("could not add to project: %w", err) } @@ -56,7 +57,7 @@ func ValidURL(urlStr string) bool { // Ensure that tb.MetadataResult object exists and contains enough pre-fetched API data to be able // to resolve all object listed in tb to GraphQL IDs. -func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetadataState) error { +func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetadataState, projectsV1Support gh.ProjectsV1Support) error { resolveInput := api.RepoResolveInput{} if len(tb.Assignees) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { @@ -79,7 +80,7 @@ func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetada resolveInput.Milestones = tb.Milestones } - metadataResult, err := api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) + metadataResult, err := api.RepoResolveMetadataIDs(client, baseRepo, resolveInput, projectsV1Support) if err != nil { return err } @@ -93,12 +94,12 @@ func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetada return nil } -func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState) error { +func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState, projectsV1Support gh.ProjectsV1Support) error { if !tb.HasMetadata() { return nil } - if err := fillMetadata(client, baseRepo, tb); err != nil { + if err := fillMetadata(client, baseRepo, tb, projectsV1Support); err != nil { return err } diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 5f5e674cc0f..fe02649d87e 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/assert" @@ -265,7 +266,7 @@ func Test_WithPrAndIssueQueryParams(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := WithPrAndIssueQueryParams(nil, nil, tt.args.baseURL, tt.args.state) + got, err := WithPrAndIssueQueryParams(nil, nil, tt.args.baseURL, tt.args.state, gh.ProjectsV1Unsupported) if (err != nil) != tt.wantErr { t.Errorf("WithPrAndIssueQueryParams() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index ce38535d97b..e2e1635769b 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -139,19 +139,19 @@ type MetadataFetcher struct { State *IssueMetadataState } -func (mf *MetadataFetcher) RepoMetadataFetch(input api.RepoMetadataInput) (*api.RepoMetadataResult, error) { +func (mf *MetadataFetcher) RepoMetadataFetch(input api.RepoMetadataInput, projectsV1Support gh.ProjectsV1Support) (*api.RepoMetadataResult, error) { mf.IO.StartProgressIndicator() - metadataResult, err := api.RepoMetadata(mf.APIClient, mf.Repo, input) + metadataResult, err := api.RepoMetadata(mf.APIClient, mf.Repo, input, projectsV1Support) mf.IO.StopProgressIndicator() mf.State.MetadataResult = metadataResult return metadataResult, err } type RepoMetadataFetcher interface { - RepoMetadataFetch(api.RepoMetadataInput) (*api.RepoMetadataResult, error) + RepoMetadataFetch(api.RepoMetadataInput, gh.ProjectsV1Support) (*api.RepoMetadataResult, error) } -func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState) error { +func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState, projectsV1Support gh.ProjectsV1Support) error { isChosen := func(m string) bool { for _, c := range state.Metadata { if m == c { @@ -184,7 +184,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface Projects: isChosen("Projects"), Milestones: isChosen("Milestone"), } - metadataResult, err := fetcher.RepoMetadataFetch(metadataInput) + metadataResult, err := fetcher.RepoMetadataFetch(metadataInput, projectsV1Support) if err != nil { return fmt.Errorf("error fetching metadata options: %w", err) } diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index d74696460a2..4a76b8032ac 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/iostreams" @@ -14,7 +15,7 @@ type metadataFetcher struct { metadataResult *api.RepoMetadataResult } -func (mf *metadataFetcher) RepoMetadataFetch(input api.RepoMetadataInput) (*api.RepoMetadataResult, error) { +func (mf *metadataFetcher) RepoMetadataFetch(input api.RepoMetadataInput, projectsV1Support gh.ProjectsV1Support) (*api.RepoMetadataResult, error) { return mf.metadataResult, nil } @@ -68,7 +69,7 @@ func TestMetadataSurvey_selectAll(t *testing.T) { Assignees: []string{"hubot"}, Type: PRMetadata, } - err := MetadataSurvey(pm, ios, repo, fetcher, state) + err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) @@ -113,7 +114,7 @@ func TestMetadataSurvey_keepExisting(t *testing.T) { state := &IssueMetadataState{ Assignees: []string{"hubot"}, } - err := MetadataSurvey(pm, ios, repo, fetcher, state) + err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported) assert.NoError(t, err) assert.Equal(t, "", stdout.String())