8000 Feature detect v1 projects on `issue view` by williammartin · Pull Request #10813 · cli/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feature detect v1 projects on issue view #10813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions internal/featuredetection/detector_mock.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package featuredetection

import "github.com/cli/cli/v2/internal/gh"

type DisabledDetectorMock struct{}

func (md *DisabledDetectorMock) IssueFeatures() (IssueFeatures, error) {
Expand All @@ -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) {
Expand All @@ -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
}
12 changes: 12 additions & 0 deletions internal/featuredetection/feature_detection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -13,6 +14,7 @@ type Detector interface {
IssueFeatures() (IssueFeatures, error)
PullRequestFeatures() (PullRequestFeatures, error)
RepositoryFeatures() (RepositoryFeatures, error)
ProjectsV1() gh.ProjectsV1Support
}

type IssueFeatures struct {
Expand Down Expand Up @@ -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
}
18 changes: 18 additions & 0 deletions internal/featuredetection/feature_detection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 changes: 23 additions & 0 deletions internal/gh/projects.go
Original file line number Diff line number Diff line change
@@ -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{}
)
17 changes: 16 additions & 1 deletion pkg/cmd/issue/view/view.go
ED48
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand Down
64 changes: 64 additions & 0 deletions pkg/cmd/issue/view/view_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
15 changes: 12 additions & 3 deletions pkg/httpmock/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"strings"
"sync"
"testing"

"github.com/stretchr/testify/assert"
)

// Replace http.Client transport layer with registry so all requests get
Expand All @@ -32,10 +30,21 @@ func (r *Registry) Register(m Matcher, resp Responder) {
}

func (r *Registry) Exclude(t *testing.T, m Matcher) {
registrationStack := string(debug.Stack())

excludedStub := &Stub{
Matcher: m,
Responder: func(req *http.Request) (*http.Response, error) {
assert.FailNowf(t, "Exclude error", "API called when excluded: %v", req.URL)
callStack := string(debug.Stack())

var errMsg strings.Builder
errMsg.WriteString("HTTP call was made when it should have been excluded:\n")
errMsg.WriteString(fmt.Sprintf("Request URL: %s\n", req.URL))
errMsg.WriteString(fmt.Sprintf("Was excluded by: %s\n", registrationStack))
errMsg.WriteString(fmt.Sprintf("Was called from: %s\n", callStack))

t.Error(errMsg.String())
t.FailNow()
return nil, nil
},
exclude: true,
Expand Down
Loading
0