Skip to content

Commit 6b56a23

Browse files
BagToadCopilot
andcommitted
Remove unnecessary StateReasonDuplicate feature detection
The DUPLICATE enum variant for IssueClosedStateReason was added in GHES 3.16, which is older than the earliest supported GHES version. The feature detection check is therefore unnecessary. Addresses: cli#12811 (comment) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d594c5e commit 6b56a23

4 files changed

Lines changed: 14 additions & 140 deletions

File tree

internal/featuredetection/feature_detection.go

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,13 @@ type Detector interface {
2323
}
2424

2525
type IssueFeatures struct {
26-
StateReason bool
27-
StateReasonDuplicate bool
28-
ActorIsAssignable bool
26+
StateReason bool
27+
ActorIsAssignable bool
2928
}
3029

3130
var allIssueFeatures = IssueFeatures{
32-
StateReason: true,
33-
StateReasonDuplicate: true,
34-
ActorIsAssignable: true,
31+
StateReason: true,
32+
ActorIsAssignable: true,
3533
}
3634

3735
type PullRequestFeatures struct {
@@ -140,9 +138,8 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
140138
}
141139

142140
features := IssueFeatures{
143-
StateReason: false,
144-
StateReasonDuplicate: false,
145-
ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
141+
StateReason: false,
142+
ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
146143
}
147144

148145
var featureDetection struct {
@@ -151,11 +148,6 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
151148
Name string
152149
} `graphql:"fields(includeDeprecated: true)"`
153150
} `graphql:"Issue: __type(name: \"Issue\")"`
154-
IssueClosedStateReason struct {
155-
EnumValues []struct {
156-
Name string
157-
} `graphql:"enumValues(includeDeprecated: true)"`
158-
} `graphql:"IssueClosedStateReason: __type(name: \"IssueClosedStateReason\")"`
159151
}
160152

161153
gql := api.NewClientFromHTTP(d.httpClient)
@@ -170,15 +162,6 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
170162
}
171163
}
172164

173-
if features.StateReason {
174-
for _, enumValue := range featureDetection.IssueClosedStateReason.EnumValues {
175-
if enumValue.Name == "DUPLICATE" {
176-
features.StateReasonDuplicate = true
177-
break
178-
}
179-
}
180-
}
181-
182165
return features, nil
183166
}
184167

internal/featuredetection/feature_detection_test.go

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,17 @@ func TestIssueFeatures(t *testing.T) {
2323
name: "github.com",
2424
hostname: "github.com",
2525
wantFeatures: IssueFeatures{
26-
StateReason: true,
27-
StateReasonDuplicate: true,
28-
ActorIsAssignable: true,
26+
StateReason: true,
27+
ActorIsAssignable: true,
2928
},
3029
wantErr: false,
3130
},
3231
{
3332
name: "ghec data residency (ghe.com)",
3433
hostname: "stampname.ghe.com",
3534
wantFeatures: IssueFeatures{
36-
StateReason: true,
37-
StateReasonDuplicate: true,
38-
ActorIsAssignable: true,
35+
StateReason: true,
36+
ActorIsAssignable: true,
3937
},
4038
wantErr: false,
4139
},
@@ -46,50 +44,23 @@ func TestIssueFeatures(t *testing.T) {
4644
`query Issue_fields\b`: `{"data": {}}`,
4745
},
4846
wantFeatures: IssueFeatures{
49-
StateReason: false,
50-
StateReasonDuplicate: false,
51-
ActorIsAssignable: false,
47+
StateReason: false,
48+
ActorIsAssignable: false,
5249
},
5350
wantErr: false,
5451
},
5552
{
56-
name: "GHE has state reason field without duplicate enum",
53+
name: "GHE has state reason field",
5754
hostname: "git.my.org",
5855
queryResponse: map[string]string{
5956
`query Issue_fields\b`: heredoc.Doc(`
6057
{ "data": { "Issue": { "fields": [
6158
{"name": "stateReason"}
62-
] }, "IssueClosedStateReason": { "enumValues": [
63-
{"name": "COMPLETED"},
64-
{"name": "NOT_PLANNED"}
6559
] } } }
6660
`),
6761
},
6862
wantFeatures: IssueFeatures{
69-
StateReason: true,
70-
StateReasonDuplicate: false,
71-
ActorIsAssignable: false,
72-
},
73-
wantErr: false,
74-
},
75-
{
76-
name: "GHE has duplicate state reason enum value",
77-
hostname: "git.my.org",
78-
queryResponse: map[string]string{
79-
`query Issue_fields\b`: heredoc.Doc(`
80-
{ "data": { "Issue": { "fields": [
81-
{"name": "stateReason"}
82-
] }, "IssueClosedStateReason": { "enumValues": [
83-
{"name": "COMPLETED"},
84-
{"name": "NOT_PLANNED"},
85-
{"name": "DUPLICATE"}
86-
] } } }
87-
`),
88-
},
89-
wantFeatures: IssueFeatures{
90-
StateReason: true,
91-
StateReasonDuplicate: true,
92-
ActorIsAssignable: false,
63+
StateReason: true,
9364
},
9465
wantErr: false,
9566
},

pkg/cmd/issue/close/close.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,6 @@ func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue,
196196
return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost())
197197
}
198198
reason = ""
199-
} else if reason == "duplicate" && !features.StateReasonDuplicate {
200-
if duplicateIssueID != "" {
201-
return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost())
202-
}
203-
// If DUPLICATE is not supported silently close issue without setting StateReason.
204-
reason = ""
205199
}
206200
}
207201

pkg/cmd/issue/close/close_test.go

Lines changed: 0 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,6 @@ import (
1616
"github.com/stretchr/testify/require"
1717
)
1818

19-
type issueFeaturesDetectorMock struct {
20-
fd.EnabledDetectorMock
21-
issueFeatures fd.IssueFeatures
22-
}
23-
24-
func (md *issueFeaturesDetectorMock) IssueFeatures() (fd.IssueFeatures, error) {
25-
return md.issueFeatures, nil
26-
}
27-
2819
func TestNewCmdClose(t *testing.T) {
2920
// Test shared parsing of issue number / URL.
3021
argparsetest.TestArgParsing(t, NewCmdClose)
@@ -282,71 +273,6 @@ func TestCloseRun(t *testing.T) {
282273
},
283274
wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n",
284275
},
285-
{
286-
name: "close issue with duplicate reason when duplicate is not supported",
287-
opts: &CloseOptions{
288-
IssueNumber: 13,
289-
Reason: "duplicate",
290-
Detector: &issueFeaturesDetectorMock{
291-
issueFeatures: fd.IssueFeatures{
292-
StateReason: true,
293-
StateReasonDuplicate: false,
294-
},
295-
},
296-
},
297-
httpStubs: func(reg *httpmock.Registry) {
298-
reg.Register(
299-
httpmock.GraphQL(`query IssueByNumber\b`),
300-
httpmock.StringResponse(`
301-
{ "data": { "repository": {
302-
"hasIssuesEnabled": true,
303-
"issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"}
304-
} } }`),
305-
)
306-
reg.Register(
307-
httpmock.GraphQL(`mutation IssueClose\b`),
308-
httpmock.GraphQLMutation(`{"id": "THE-ID"}`,
309-
func(inputs map[string]interface{}) {
310-
assert.Equal(t, 1, len(inputs))
311-
assert.Equal(t, "THE-ID", inputs["issueId"])
312-
}),
313-
)
314-
},
315-
wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n",
316-
},
317-
{
318-
name: "close issue as duplicate when duplicate is not supported",
319-
opts: &CloseOptions{
320-
IssueNumber: 13,
321-
DuplicateOf: "99",
322-
Detector: &issueFeaturesDetectorMock{
323-
issueFeatures: fd.IssueFeatures{
324-
StateReason: true,
325-
StateReasonDuplicate: false,
326-
},
327-
},
328-
},
329-
httpStubs: func(reg *httpmock.Registry) {
330-
reg.Register(
331-
httpmock.GraphQL(`query IssueByNumber\b`),
332-
httpmock.StringResponse(`
333-
{ "data": { "repository": {
334-
"hasIssuesEnabled": true,
335-
"issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"}
336-
} } }`),
337-
)
338-
reg.Register(
339-
httpmock.GraphQL(`query IssueByNumber\b`),
340-
httpmock.StringResponse(`
341-
{ "data": { "repository": {
342-
"hasIssuesEnabled": true,
343-
"issue": { "id": "DUPLICATE-ID", "number": 99}
344-
} } }`),
345-
)
346-
},
347-
wantErr: true,
348-
errMsg: "closing as duplicate is not supported on github.com",
349-
},
350276
{
351277
name: "duplicate of cannot point to same issue",
352278
opts: &CloseOptions{

0 commit comments

Comments
 (0)