Merge pull request #11514 from cli/github-cli-937-v1-project-ghes-deprecation

Ensure users can see v2 projects when viewing issues and PRs, avoid v1 projects on GHES 3.17 and newer
This commit is contained in:
Andy Feller 2025-08-18 16:15:54 -04:00 committed by GitHub
commit 9ae764b9fa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 268 additions and 62 deletions

View file

@ -166,7 +166,8 @@ type ProjectCards struct {
}
type ProjectItems struct {
Nodes []*ProjectV2Item
Nodes []*ProjectV2Item
TotalCount int
}
type ProjectInfo struct {

View file

@ -82,8 +82,9 @@ func ProjectsV2ItemsForIssue(client *Client, repo ghrepo.Interface, issue *Issue
Repository struct {
Issue struct {
ProjectItems struct {
Nodes []*projectV2Item
PageInfo struct {
TotalCount int
Nodes []*projectV2Item
PageInfo struct {
HasNextPage bool
EndCursor string
}
@ -149,8 +150,9 @@ func ProjectsV2ItemsForPullRequest(client *Client, repo ghrepo.Interface, pr *Pu
Repository struct {
PullRequest struct {
ProjectItems struct {
Nodes []*projectV2Item
PageInfo struct {
TotalCount int
Nodes []*projectV2Item
PageInfo struct {
HasNextPage bool
EndCursor string
}

View file

@ -5,6 +5,7 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
"github.com/hashicorp/go-version"
"golang.org/x/sync/errgroup"
ghauth "github.com/cli/go-gh/v2/pkg/auth"
@ -205,12 +206,35 @@ func (d *detector) RepositoryFeatures() (RepositoryFeatures, error) {
return features, nil
}
const (
enterpriseProjectsV1Removed = "3.17.0"
)
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) {
if !ghauth.IsEnterprise(d.host) {
return gh.ProjectsV1Unsupported
}
hostVersion, hostVersionErr := resolveEnterpriseVersion(d.httpClient, d.host)
v1ProjectCutoffVersion, v1ProjectCutoffVersionErr := version.NewVersion(enterpriseProjectsV1Removed)
if hostVersionErr == nil && v1ProjectCutoffVersionErr == nil && hostVersion.LessThan(v1ProjectCutoffVersion) {
return gh.ProjectsV1Supported
}
return gh.ProjectsV1Unsupported
}
func resolveEnterpriseVersion(httpClient *http.Client, host string) (*version.Version, error) {
var metaResponse struct {
InstalledVersion string `json:"installed_version"`
}
apiClient := api.NewClientFromHTTP(httpClient)
err := apiClient.REST(host, "GET", "meta", nil, &metaResponse)
if err != nil {
return nil, err
}
return version.NewVersion(metaResponse.InstalledVersion)
}

View file

@ -373,17 +373,69 @@ func TestRepositoryFeatures(t *testing.T) {
}
func TestProjectV1Support(t *testing.T) {
t.Parallel()
tests := []struct {
name string
hostname string
httpStubs func(*httpmock.Registry)
wantFeatures gh.ProjectsV1Support
}{
{
name: "github.com",
hostname: "github.com",
wantFeatures: gh.ProjectsV1Unsupported,
},
{
name: "ghec data residency (ghe.com)",
hostname: "stampname.ghe.com",
wantFeatures: gh.ProjectsV1Unsupported,
},
{
name: "GHE 3.16.0",
hostname: "git.my.org",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "api/v3/meta"),
httpmock.StringResponse(`{"installed_version":"3.16.0"}`),
)
},
wantFeatures: gh.ProjectsV1Supported,
},
{
name: "GHE 3.16.1",
hostname: "git.my.org",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "api/v3/meta"),
httpmock.StringResponse(`{"installed_version":"3.16.1"}`),
)
},
wantFeatures: gh.ProjectsV1Supported,
},
{
name: "GHE 3.17",
hostname: "git.my.org",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "api/v3/meta"),
httpmock.StringResponse(`{"installed_version":"3.17.0"}`),
)
},
wantFeatures: gh.ProjectsV1Unsupported,
},
}
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)
})
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
reg := &httpmock.Registry{}
if tt.httpStubs != nil {
tt.httpStubs(reg)
}
httpClient := &http.Client{}
httpmock.ReplaceTripper(httpClient, reg)
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)
})
detector := NewDetector(httpClient, tt.hostname)
require.Equal(t, tt.wantFeatures, detector.ProjectsV1())
})
}
}

View file

@ -124,6 +124,7 @@ func viewRun(opts *ViewOptions) error {
opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost())
}
lookupFields.Add("projectItems")
projectsV1Support := opts.Detector.ProjectsV1()
if projectsV1Support == gh.ProjectsV1Supported {
lookupFields.Add("projectCards")
@ -310,11 +311,24 @@ func issueAssigneeList(issue api.Issue) string {
}
func issueProjectList(issue api.Issue) string {
if len(issue.ProjectCards.Nodes) == 0 {
totalCount := issue.ProjectCards.TotalCount + issue.ProjectItems.TotalCount
count := len(issue.ProjectCards.Nodes) + len(issue.ProjectItems.Nodes)
if count == 0 {
return ""
}
projectNames := make([]string, 0, len(issue.ProjectCards.Nodes))
projectNames := make([]string, 0, count)
for _, project := range issue.ProjectItems.Nodes {
colName := project.Status.Name
if colName == "" {
colName = "No Status"
}
projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Title, colName))
}
// TODO: Remove v1 classic project logic when completely deprecated
for _, project := range issue.ProjectCards.Nodes {
colName := project.Column.Name
if colName == "" {
@ -324,7 +338,7 @@ func issueProjectList(issue api.Issue) string {
}
list := strings.Join(projectNames, ", ")
if issue.ProjectCards.TotalCount > len(issue.ProjectCards.Nodes) {
if totalCount > count {
list += ", …"
}
return list

View file

@ -2,7 +2,6 @@ package view
import (
"bytes"
"fmt"
"io"
"net/http"
"testing"
@ -137,11 +136,14 @@ func TestIssueView_web(t *testing.T) {
func TestIssueView_nontty_Preview(t *testing.T) {
tests := map[string]struct {
fixture string
httpStubs func(*httpmock.Registry)
expectedOutputs []string
}{
"Open issue without metadata": {
fixture: "./fixtures/issueView_preview.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_preview.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tOPEN`,
@ -153,7 +155,10 @@ func TestIssueView_nontty_Preview(t *testing.T) {
},
},
"Open issue with metadata": {
fixture: "./fixtures/issueView_previewWithMetadata.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewWithMetadata.json"))
mockV2ProjectItems(t, r)
},
expectedOutputs: []string{
`title:\tix of coins`,
`assignees:\tmarseilles, monaco`,
@ -161,14 +166,17 @@ func TestIssueView_nontty_Preview(t *testing.T) {
`state:\tOPEN`,
`comments:\t9`,
`labels:\tClosed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`projects:\tv2 Project 1 \(No Status\), v2 Project 2 \(Done\), Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`number:\t123\n`,
`\*\*bold story\*\*`,
},
},
"Open issue with empty body": {
fixture: "./fixtures/issueView_previewWithEmptyBody.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewWithEmptyBody.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tOPEN`,
@ -178,7 +186,10 @@ func TestIssueView_nontty_Preview(t *testing.T) {
},
},
"Closed issue": {
fixture: "./fixtures/issueView_previewClosedState.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewClosedState.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tCLOSED`,
@ -194,8 +205,9 @@ func TestIssueView_nontty_Preview(t *testing.T) {
t.Run(name, func(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture))
if tc.httpStubs != nil {
tc.httpStubs(http)
}
output, err := runCommand(http, false, "123")
if err != nil {
@ -212,11 +224,14 @@ func TestIssueView_nontty_Preview(t *testing.T) {
func TestIssueView_tty_Preview(t *testing.T) {
tests := map[string]struct {
fixture string
httpStubs func(*httpmock.Registry)
expectedOutputs []string
}{
"Open issue without metadata": {
fixture: "./fixtures/issueView_preview.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_preview.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`ix of coins OWNER/REPO#123`,
`Open.*marseilles opened about 9 years ago.*9 comments`,
@ -225,21 +240,27 @@ func TestIssueView_tty_Preview(t *testing.T) {
},
},
"Open issue with metadata": {
fixture: "./fixtures/issueView_previewWithMetadata.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewWithMetadata.json"))
mockV2ProjectItems(t, r)
},
expectedOutputs: []string{
`ix of coins OWNER/REPO#123`,
`Open.*marseilles opened about 9 years ago.*9 comments`,
`8 \x{1f615} • 7 \x{1f440} • 6 \x{2764}\x{fe0f} • 5 \x{1f389} • 4 \x{1f604} • 3 \x{1f680} • 2 \x{1f44e} • 1 \x{1f44d}`,
`Assignees:.*marseilles, monaco\n`,
`Labels:.*Closed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`,
`Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`Projects:.*v2 Project 1 \(No Status\), v2 Project 2 \(Done\), Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`Milestone:.*uluru\n`,
`bold story`,
`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`,
},
},
"Open issue with empty body": {
fixture: "./fixtures/issueView_previewWithEmptyBody.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewWithEmptyBody.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`ix of coins OWNER/REPO#123`,
`Open.*marseilles opened about 9 years ago.*9 comments`,
@ -248,7 +269,10 @@ func TestIssueView_tty_Preview(t *testing.T) {
},
},
"Closed issue": {
fixture: "./fixtures/issueView_previewClosedState.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewClosedState.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`ix of coins OWNER/REPO#123`,
`Closed.*marseilles opened about 9 years ago.*9 comments`,
@ -266,8 +290,9 @@ func TestIssueView_tty_Preview(t *testing.T) {
httpReg := &httpmock.Registry{}
defer httpReg.Verify(t)
httpReg.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture))
if tc.httpStubs != nil {
tc.httpStubs(httpReg)
}
opts := ViewOptions{
IO: ios,
@ -354,14 +379,15 @@ func TestIssueView_disabledIssues(t *testing.T) {
func TestIssueView_tty_Comments(t *testing.T) {
tests := map[string]struct {
cli string
fixtures map[string]string
httpStubs func(*httpmock.Registry)
expectedOutputs []string
wantsErr bool
}{
"without comments flag": {
cli: "123",
fixtures: map[string]string{
"IssueByNumber": "./fixtures/issueView_previewSingleComment.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewSingleComment.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`some title OWNER/REPO#123`,
@ -375,9 +401,10 @@ func TestIssueView_tty_Comments(t *testing.T) {
},
"with comments flag": {
cli: "123 --comments",
fixtures: map[string]string{
"IssueByNumber": "./fixtures/issueView_previewSingleComment.json",
"CommentsForIssue": "./fixtures/issueView_previewFullComments.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewSingleComment.json"))
r.Register(httpmock.GraphQL(`query CommentsForIssue\b`), httpmock.FileResponse("./fixtures/issueView_previewFullComments.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`some title OWNER/REPO#123`,
@ -406,9 +433,8 @@ func TestIssueView_tty_Comments(t *testing.T) {
t.Run(name, func(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
for name, file := range tc.fixtures {
name := fmt.Sprintf(`query %s\b`, name)
http.Register(httpmock.GraphQL(name), httpmock.FileResponse(file))
if tc.httpStubs != nil {
tc.httpStubs(http)
}
output, err := runCommand(http, true, tc.cli)
if tc.wantsErr {
@ -426,14 +452,15 @@ func TestIssueView_tty_Comments(t *testing.T) {
func TestIssueView_nontty_Comments(t *testing.T) {
tests := map[string]struct {
cli string
fixtures map[string]string
httpStubs func(*httpmock.Registry)
expectedOutputs []string
wantsErr bool
}{
"without comments flag": {
cli: "123",
fixtures: map[string]string{
"IssueByNumber": "./fixtures/issueView_previewSingleComment.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewSingleComment.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`title:\tsome title`,
@ -446,9 +473,10 @@ func TestIssueView_nontty_Comments(t *testing.T) {
},
"with comments flag": {
cli: "123 --comments",
fixtures: map[string]string{
"IssueByNumber": "./fixtures/issueView_previewSingleComment.json",
"CommentsForIssue": "./fixtures/issueView_previewFullComments.json",
httpStubs: func(r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse("./fixtures/issueView_previewSingleComment.json"))
r.Register(httpmock.GraphQL(`query CommentsForIssue\b`), httpmock.FileResponse("./fixtures/issueView_previewFullComments.json"))
mockEmptyV2ProjectItems(t, r)
},
expectedOutputs: []string{
`author:\tmonalisa`,
@ -482,9 +510,8 @@ func TestIssueView_nontty_Comments(t *testing.T) {
t.Run(name, func(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
for name, file := range tc.fixtures {
name := fmt.Sprintf(`query %s\b`, name)
http.Register(httpmock.GraphQL(name), httpmock.FileResponse(file))
if tc.httpStubs != nil {
tc.httpStubs(http)
}
output, err := runCommand(http, false, tc.cli)
if tc.wantsErr {
@ -561,3 +588,50 @@ func TestProjectsV1Deprecation(t *testing.T) {
reg.Verify(t)
})
}
// mockEmptyV2ProjectItems registers GraphQL queries to report an issue is not contained on any v2 projects.
func mockEmptyV2ProjectItems(t *testing.T, r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueProjectItems\b`), httpmock.StringResponse(`
{ "data": { "repository": { "issue": {
"projectItems": {
"totalCount": 0,
"nodes": []
} } } } }
`))
}
// mockV2ProjectItems registers GraphQL queries to report an issue on multiple v2 projects in various states
// - `NO_STATUS_ITEM`: emulates this issue is on a project but is not given a status
// - `DONE_STATUS_ITEM`: emulates this issue is on a project and considered done
func mockV2ProjectItems(t *testing.T, r *httpmock.Registry) {
r.Register(httpmock.GraphQL(`query IssueProjectItems\b`), httpmock.StringResponse(`
{ "data": { "repository": { "issue": {
"projectItems": {
"totalCount": 2,
"nodes": [
{
"id": "NO_STATUS_ITEM",
"project": {
"id": "PROJECT1",
"title": "v2 Project 1"
},
"status": {
"optionId": "",
"name": ""
}
},
{
"id": "DONE_STATUS_ITEM",
"project": {
"id": "PROJECT2",
"title": "v2 Project 2"
},
"status": {
"optionId": "PROJECTITEMFIELD1",
"name": "Done"
}
}
]
} } } } }
`))
}

View file

@ -54,6 +54,33 @@
],
"totalcount": 5
},
"projectitems": {
"totalCount": 2,
"nodes": [
{
"id": "NO_STATUS_ITEM",
"project": {
"id": "PROJECT1",
"title": "v2 Project 1"
},
"status": {
"optionId": "",
"name": ""
}
},
{
"id": "DONE_STATUS_ITEM",
"project": {
"id": "PROJECT2",
"title": "v2 Project 2"
},
"status": {
"optionId": "PROJECTITEMFIELD1",
"name": "Done"
}
}
]
},
"projectcards": {
"nodes": [
{

View file

@ -85,7 +85,7 @@ var defaultFields = []string{
"url", "number", "title", "state", "body", "author", "autoMergeRequest",
"isDraft", "maintainerCanModify", "mergeable", "additions", "deletions", "commitsCount",
"baseRefName", "headRefName", "headRepositoryOwner", "headRepository", "isCrossRepository",
"reviewRequests", "reviews", "assignees", "labels", "projectCards", "milestone",
"reviewRequests", "reviews", "assignees", "labels", "projectCards", "projectItems", "milestone",
"comments", "reactionGroups", "createdAt", "statusCheckRollup",
}
@ -439,11 +439,23 @@ func prLabelList(pr api.PullRequest, cs *iostreams.ColorScheme) string {
}
func prProjectList(pr api.PullRequest) string {
if len(pr.ProjectCards.Nodes) == 0 {
totalCount := pr.ProjectCards.TotalCount + pr.ProjectItems.TotalCount
count := len(pr.ProjectCards.Nodes) + len(pr.ProjectItems.Nodes)
if count == 0 {
return ""
}
projectNames := make([]string, 0, len(pr.ProjectCards.Nodes))
for _, project := range pr.ProjectItems.Nodes {
colName := project.Status.Name
if colName == "" {
colName = "No Status"
}
projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Title, colName))
}
for _, project := range pr.ProjectCards.Nodes {
if project == nil {
continue
@ -456,7 +468,7 @@ func prProjectList(pr api.PullRequest) string {
}
list := strings.Join(projectNames, ", ")
if pr.ProjectCards.TotalCount > len(pr.ProjectCards.Nodes) {
if totalCount > count {
list += ", …"
}
return list

View file

@ -286,7 +286,7 @@ func TestPRView_Preview_nontty(t *testing.T) {
`reviewers:\t1 \(Requested\)\n`,
`assignees:\tmarseilles, monaco\n`,
`labels:\tClosed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`projects:\tv2 Project 1 \(No Status\), v2 Project 2 \(Done\), Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`\*\*blueberries taste good\*\*`,
},
@ -457,7 +457,7 @@ func TestPRView_Preview(t *testing.T) {
`Reviewers:.*1 \(.*Requested.*\)\n`,
`Assignees:.*marseilles, monaco\n`,
`Labels:.*Closed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`,
`Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`Projects:.*v2 Project 1 \(No Status\), v2 Project 2 \(Done\), Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`Milestone:.*uluru\n`,
`blueberries taste good`,
`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`,