Avoid requesting projectCards for issue view

This commit is contained in:
William Martin 2025-04-16 16:46:37 +02:00
parent fff0e70259
commit 5ec2160bc6
6 changed files with 143 additions and 1 deletions

View file

@ -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 {
return gh.ProjectsV1Unsupported
}
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 {
return gh.ProjectsV1Supported
}

View file

@ -4,6 +4,7 @@ import (
"net/http"
"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 +14,7 @@ type Detector interface {
IssueFeatures() (IssueFeatures, error)
PullRequestFeatures() (PullRequestFeatures, error)
RepositoryFeatures() (RepositoryFeatures, error)
ProjectsV1() gh.ProjectsV1Support
}
type IssueFeatures struct {
@ -199,3 +201,13 @@ func (d *detector) RepositoryFeatures() (RepositoryFeatures, error) {
return features, nil
}
func (d *detector) ProjectsV1() gh.ProjectsV1Support {
// Currently, projects v1 support is entirely dependent on the host. As this is deprecated in GHES,
// we will do feature detection on whether the GHES version has support.
if ghauth.IsEnterprise(d.host) {
return gh.ProjectsV1Supported
}
return gh.ProjectsV1Unsupported
}

View file

@ -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,19 @@ func TestRepositoryFeatures(t *testing.T) {
})
}
}
func TestProjectV1Support(t *testing.T) {
t.Parallel()
t.Run("when the host is enterprise, project v1 is supported", func(t *testing.T) {
detector := detector{host: "my.ghes.com"}
isProjectV1Supported := detector.ProjectsV1()
require.Equal(t, gh.ProjectsV1Supported, isProjectV1Supported)
})
t.Run("when the host is not enterprise, project v1 is not supported", func(t *testing.T) {
detector := detector{host: "github.com"}
isProjectV1Supported := detector.ProjectsV1()
require.Equal(t, gh.ProjectsV1Unsupported, isProjectV1Supported)
})
}

23
internal/gh/projects.go Normal file
View file

@ -0,0 +1,23 @@
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{}
)

View file

@ -12,6 +12,8 @@ import (
"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"
"github.com/cli/cli/v2/pkg/cmd/issue/shared"
@ -29,6 +31,7 @@ type ViewOptions struct {
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
Detector fd.Detector
IssueNumber int
WebMode bool
@ -89,7 +92,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
var defaultFields = []string{
"number", "url", "state", "createdAt", "title", "body", "author", "milestone",
"assignees", "labels", "projectCards", "reactionGroups", "lastComment", "stateReason",
"assignees", "labels", "reactionGroups", "lastComment", "stateReason",
}
func viewRun(opts *ViewOptions) error {
@ -114,6 +117,18 @@ func viewRun(opts *ViewOptions) error {
lookupFields.Add("comments")
lookupFields.Remove("lastComment")
}
// TODO projectsV1Deprecation
// Remove this section as we should no longer add projectCards
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost())
}
projectsV1Support := opts.Detector.ProjectsV1()
if projectsV1Support == gh.ProjectsV1Supported {
lookupFields.Add("projectCards")
}
}
opts.IO.DetectTerminalTheme()

View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/config"
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/run"
@ -496,3 +497,66 @@ func TestIssueView_nontty_Comments(t *testing.T) {
})
}
}
// TODO projectsV1Deprecation
// Remove this test.
func TestProjectsV1Deprecation(t *testing.T) {
t.Run("when projects v1 is supported, is included in query", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.Register(
httpmock.GraphQL(`projectCards`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = viewRun(&ViewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Detector: &fd.EnabledDetectorMock{},
IssueNumber: 123,
})
// Verify that our request contained projectCards
reg.Verify(t)
})
t.Run("when projects v1 is not supported, is not included in query", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.Exclude(t, httpmock.GraphQL(`projectCards`))
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
// Ignore the error because we're not really interested in it.
_ = viewRun(&ViewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Detector: &fd.DisabledDetectorMock{},
IssueNumber: 123,
})
// Verify that our request contained projectCards
reg.Verify(t)
})
}