diff --git a/go.mod b/go.mod index 5e6d0724708..a9c86a672d0 100644 --- a/go.mod +++ b/go.mod @@ -89,7 +89,7 @@ require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 // indirect github.com/dlclark/regexp2 v1.11.5 // indirect - github.com/docker/cli v29.0.3+incompatible // indirect + github.com/docker/cli v29.2.0+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/dustin/go-humanize v1.0.1 // indirect diff --git a/go.sum b/go.sum index 576d84c65b8..e4c39bd3a92 100644 --- a/go.sum +++ b/go.sum @@ -179,8 +179,8 @@ github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5Qvfr github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/dlclark/regexp2 v1.11.5 h1:Q/sSnsKerHeCkc/jSTNq1oCm7KiVgUMZRDUoRu0JQZQ= github.com/dlclark/regexp2 v1.11.5/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= -github.com/docker/cli v29.0.3+incompatible h1:8J+PZIcF2xLd6h5sHPsp5pvvJA+Sr2wGQxHkRl53a1E= -github.com/docker/cli v29.0.3+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/cli v29.2.0+incompatible h1:9oBd9+YM7rxjZLfyMGxjraKBKE4/nVyvVfN4qNl9XRM= +github.com/docker/cli v29.2.0+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker-credential-helpers v0.9.3 h1:gAm/VtF9wgqJMoxzT3Gj5p4AqIjCBS4wrsOh9yRqcz8= diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index e40c134bc7b..9af4c5aeca5 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -23,15 +23,11 @@ type Detector interface { } type IssueFeatures struct { - StateReason bool - StateReasonDuplicate bool - ActorIsAssignable bool + ActorIsAssignable bool } var allIssueFeatures = IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: true, + ActorIsAssignable: true, } type PullRequestFeatures struct { @@ -139,47 +135,9 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { return allIssueFeatures, nil } - features := IssueFeatures{ - StateReason: false, - StateReasonDuplicate: false, - ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES - } - - var featureDetection struct { - Issue struct { - Fields []struct { - Name string - } `graphql:"fields(includeDeprecated: true)"` - } `graphql:"Issue: __type(name: \"Issue\")"` - IssueClosedStateReason struct { - EnumValues []struct { - Name string - } `graphql:"enumValues(includeDeprecated: true)"` - } `graphql:"IssueClosedStateReason: __type(name: \"IssueClosedStateReason\")"` - } - - gql := api.NewClientFromHTTP(d.httpClient) - err := gql.Query(d.host, "Issue_fields", &featureDetection, nil) - if err != nil { - return features, err - } - - for _, field := range featureDetection.Issue.Fields { - if field.Name == "stateReason" { - features.StateReason = true - } - } - - if features.StateReason { - for _, enumValue := range featureDetection.IssueClosedStateReason.EnumValues { - if enumValue.Name == "DUPLICATE" { - features.StateReasonDuplicate = true - break - } - } - } - - return features, nil + return IssueFeatures{ + ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES + }, nil } func (d *detector) PullRequestFeatures() (PullRequestFeatures, error) { diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index 7417cc7d8e8..82132ab83fd 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -23,9 +23,7 @@ func TestIssueFeatures(t *testing.T) { name: "github.com", hostname: "github.com", wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: true, + ActorIsAssignable: true, }, wantErr: false, }, @@ -33,63 +31,15 @@ func TestIssueFeatures(t *testing.T) { name: "ghec data residency (ghe.com)", hostname: "stampname.ghe.com", wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: true, + ActorIsAssignable: true, }, wantErr: false, }, { - name: "GHE empty response", - hostname: "git.my.org", - queryResponse: map[string]string{ - `query Issue_fields\b`: `{"data": {}}`, - }, - wantFeatures: IssueFeatures{ - StateReason: false, - StateReasonDuplicate: false, - ActorIsAssignable: false, - }, - wantErr: false, - }, - { - name: "GHE has state reason field without duplicate enum", + name: "GHE", hostname: "git.my.org", - queryResponse: map[string]string{ - `query Issue_fields\b`: heredoc.Doc(` - { "data": { "Issue": { "fields": [ - {"name": "stateReason"} - ] }, "IssueClosedStateReason": { "enumValues": [ - {"name": "COMPLETED"}, - {"name": "NOT_PLANNED"} - ] } } } - `), - }, - wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: false, - ActorIsAssignable: false, - }, - wantErr: false, - }, - { - name: "GHE has duplicate state reason enum value", - hostname: "git.my.org", - queryResponse: map[string]string{ - `query Issue_fields\b`: heredoc.Doc(` - { "data": { "Issue": { "fields": [ - {"name": "stateReason"} - ] }, "IssueClosedStateReason": { "enumValues": [ - {"name": "COMPLETED"}, - {"name": "NOT_PLANNED"}, - {"name": "DUPLICATE"} - ] } } } - `), - }, wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: false, + ActorIsAssignable: false, }, wantErr: false, }, diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index b7f1f1c0c07..de758f5a3d2 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -268,7 +268,7 @@ func (m *Manager) Install(repo ghrepo.Interface, target string) error { return err } if !hs { - return fmt.Errorf("extension is not installable: no usable release artifact or script found in %s", repo) + return fmt.Errorf("extension is not installable: no usable release artifact or script found in %s", ghrepo.FullName(repo)) } return m.installGit(repo, target) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index e933f0bdfab..1e8f8248324 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -867,6 +867,39 @@ func TestManager_Install_git(t *testing.T) { assert.NoDirExistsf(t, extensionUpdatePath, "update directory should be removed") } +func TestManager_Install_not_installable(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + + ios, _, _, _ := iostreams.Test() + + m := newTestManager(dataDir, updateDir, &client, nil, ios) + + reg.Register( + httpmock.REST("GET", "repos/owner/gh-some-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "not-a-binary", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-some-ext/contents/gh-some-ext"), + httpmock.StatusStringResponse(404, "not found")) + + repo := ghrepo.New("owner", "gh-some-ext") + + err := m.Install(repo, "") + assert.EqualError(t, err, "extension is not installable: no usable release artifact or script found in owner/gh-some-ext") +} + func TestManager_Install_git_pinned(t *testing.T) { dataDir := t.TempDir() updateDir := t.TempDir() diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index 4c1c7d382bf..d9b02b6a22f 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -3,11 +3,9 @@ package close 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/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -26,8 +24,6 @@ type CloseOptions struct { Comment string Reason string DuplicateOf string - - Detector fd.Detector } func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Command { @@ -165,7 +161,7 @@ func closeRun(opts *CloseOptions) error { } } - err = apiClose(httpClient, baseRepo, issue, opts.Detector, closeReason, duplicateIssueID) + err = apiClose(httpClient, baseRepo, issue, closeReason, duplicateIssueID) if err != nil { return err } @@ -175,36 +171,11 @@ func closeRun(opts *CloseOptions) error { return nil } -func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue, detector fd.Detector, reason string, duplicateIssueID string) error { +func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue, reason string, duplicateIssueID string) error { if issue.IsPullRequest() { return api.PullRequestClose(httpClient, repo, issue.ID) } - if reason != "" || duplicateIssueID != "" { - if detector == nil { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - detector = fd.NewDetector(cachedClient, repo.RepoHost()) - } - features, err := detector.IssueFeatures() - if err != nil { - return err - } - // TODO stateReasonCleanup - if !features.StateReason { - // If StateReason is not supported silently close issue without setting StateReason. - if duplicateIssueID != "" { - return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost()) - } - reason = "" - } else if reason == "duplicate" && !features.StateReasonDuplicate { - if duplicateIssueID != "" { - return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost()) - } - // If DUPLICATE is not supported silently close issue without setting StateReason. - reason = "" - } - } - switch reason { case "": // If no reason is specified do not set it. diff --git a/pkg/cmd/issue/close/close_test.go b/pkg/cmd/issue/close/close_test.go index ddab7121007..e7dfa162751 100644 --- a/pkg/cmd/issue/close/close_test.go +++ b/pkg/cmd/issue/close/close_test.go @@ -5,7 +5,6 @@ import ( "net/http" "testing" - fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/argparsetest" "github.com/cli/cli/v2/pkg/cmdutil" @@ -16,15 +15,6 @@ import ( "github.com/stretchr/testify/require" ) -type issueFeaturesDetectorMock struct { - fd.EnabledDetectorMock - issueFeatures fd.IssueFeatures -} - -func (md *issueFeaturesDetectorMock) IssueFeatures() (fd.IssueFeatures, error) { - return md.issueFeatures, nil -} - func TestNewCmdClose(t *testing.T) { // Test shared parsing of issue number / URL. argparsetest.TestArgParsing(t, NewCmdClose) @@ -194,7 +184,6 @@ func TestCloseRun(t *testing.T) { opts: &CloseOptions{ IssueNumber: 13, Reason: "not planned", - Detector: &fd.EnabledDetectorMock{}, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -222,7 +211,6 @@ func TestCloseRun(t *testing.T) { opts: &CloseOptions{ IssueNumber: 13, Reason: "duplicate", - Detector: &fd.EnabledDetectorMock{}, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -250,7 +238,6 @@ func TestCloseRun(t *testing.T) { opts: &CloseOptions{ IssueNumber: 13, DuplicateOf: "99", - Detector: &fd.EnabledDetectorMock{}, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -282,71 +269,6 @@ func TestCloseRun(t *testing.T) { }, wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n", }, - { - name: "close issue with duplicate reason when duplicate is not supported", - opts: &CloseOptions{ - IssueNumber: 13, - Reason: "duplicate", - Detector: &issueFeaturesDetectorMock{ - issueFeatures: fd.IssueFeatures{ - StateReason: true, - StateReasonDuplicate: false, - }, - }, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} - } } }`), - ) - reg.Register( - httpmock.GraphQL(`mutation IssueClose\b`), - httpmock.GraphQLMutation(`{"id": "THE-ID"}`, - func(inputs map[string]interface{}) { - assert.Equal(t, 1, len(inputs)) - assert.Equal(t, "THE-ID", inputs["issueId"]) - }), - ) - }, - wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n", - }, - { - name: "close issue as duplicate when duplicate is not supported", - opts: &CloseOptions{ - IssueNumber: 13, - DuplicateOf: "99", - Detector: &issueFeaturesDetectorMock{ - issueFeatures: fd.IssueFeatures{ - StateReason: true, - StateReasonDuplicate: false, - }, - }, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} - } } }`), - ) - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "DUPLICATE-ID", "number": 99} - } } }`), - ) - }, - wantErr: true, - errMsg: "closing as duplicate is not supported on github.com", - }, { name: "duplicate of cannot point to same issue", opts: &CloseOptions{ @@ -412,33 +334,6 @@ func TestCloseRun(t *testing.T) { wantErr: true, errMsg: "invalid value for `--duplicate-of`: invalid issue format: \"not-an-issue\"", }, - { - name: "close issue with reason when reason is not supported", - opts: &CloseOptions{ - IssueNumber: 13, - Reason: "not planned", - Detector: &fd.DisabledDetectorMock{}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} - } } }`), - ) - reg.Register( - httpmock.GraphQL(`mutation IssueClose\b`), - httpmock.GraphQLMutation(`{"id": "THE-ID"}`, - func(inputs map[string]interface{}) { - assert.Equal(t, 1, len(inputs)) - assert.Equal(t, "THE-ID", inputs["issueId"]) - }), - ) - }, - wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n", - }, { name: "issue already closed", opts: &CloseOptions{ diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index f5a0a442926..d58357ac475 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -147,15 +147,7 @@ func listRun(opts *ListOptions) error { cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) } - features, err := opts.Detector.IssueFeatures() - if err != nil { - return err - } - fields := defaultFields - // TODO stateReasonCleanup - if features.StateReason { - fields = append(defaultFields, "stateReason") - } + fields := append(defaultFields, "stateReason") filterOptions := prShared.FilterOptions{ Entity: "issue", diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index 2975ad48481..63efd61f72e 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -8,10 +8,8 @@ import ( "regexp" "strconv" "strings" - "time" "github.com/cli/cli/v2/api" - fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" o "github.com/cli/cli/v2/pkg/option" "github.com/cli/cli/v2/pkg/set" @@ -138,18 +136,6 @@ func FindIssuesOrPRs(httpClient *http.Client, repo ghrepo.Interface, issueNumber func FindIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.Issue, error) { 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 - } - // TODO stateReasonCleanup - if !features.StateReason { - fieldSet.Remove("stateReason") - } - } var getProjectItems bool if fieldSet.Contains("projectItems") {