Skip to content

Commit b851e6e

Browse files
owenniblockCopilot
owenniblock
and
Copilot
committed
Remove dead REST issue-field-value output path
The MinimalIssueFieldValue / MinimalIssueFieldValueSingleSelectOption types and their populator in convertToMinimalIssue produced a single_select_option output that is never emitted: GetIssue unconditionally cleared minimalIssue.IssueFieldValues before marshalling, and it was the only caller of convertToMinimalIssue. History: the REST extraction was introduced in #2551 and superseded one day later by #2558, which switched get_issue to the GraphQL field_values enrichment path ("The verbose REST IssueFieldValues is always cleared from the response") to stay consistent with list_issues/search_issues. The struct + populator were left behind as orphaned code. This also removes a latent footgun: the struct only modelled single_select_option (go-github's REST IssueFieldValue has no multi-select equivalent), so any future code that surfaced it would have silently dropped multi-select option data. - Remove MinimalIssueFieldValue + MinimalIssueFieldValueSingleSelectOption - Remove MinimalIssue.IssueFieldValues field (json: issue_field_values) - Remove the populator loop in convertToMinimalIssue - Drop the now-redundant nil assignment in GetIssue - Drop the two test assertions on the removed field (behaviour is now structurally guaranteed; the GraphQL field_values assertions remain) No output shape change: issue_field_values was omitempty and always empty. The GraphQL field_values path is unaffected. SearchIssueResult's MarshalJSON still suppresses go-github's own embedded issue_field_values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6586b84 commit b851e6e

3 files changed

Lines changed: 24 additions & 67 deletions

File tree

‎pkg/github/issues.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -874,9 +874,8 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
874874

875875
minimalIssue := convertToMinimalIssue(issue)
876876

877-
// Always drop the verbose REST IssueFieldValues; only enrich with the GraphQL
878-
// field_values view when the issue-fields feature flag is on.
879-
minimalIssue.IssueFieldValues = nil
877+
// Enrich with the GraphQL field_values view only when the issue-fields feature
878+
// flag is on. The verbose REST issue field values are not surfaced.
880879
if deps.IsFeatureEnabled(ctx, FeatureFlagIssueFields) {
881880
if issue != nil && issue.NodeID != nil && *issue.NodeID != "" {
882881
gqlClient, err := deps.GetGQLClient(ctx)

‎pkg/github/issues_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,7 @@ func Test_GetIssue_FieldValues(t *testing.T) {
459459
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
460460
require.NoError(t, err)
461461

462-
// Flag is off: raw REST IssueFieldValues must be cleared, enriched FieldValues absent.
463-
assert.Empty(t, returnedIssue.IssueFieldValues, "raw REST issue_field_values should not be exposed when flag is off")
462+
// Flag is off: enriched field_values absent.
464463
assert.Empty(t, returnedIssue.FieldValues, "enriched field_values should not be present when flag is off")
465464
}
466465

@@ -549,9 +548,6 @@ func Test_GetIssue_FieldValues_FlagOn(t *testing.T) {
549548
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
550549
require.NoError(t, err)
551550

552-
// Raw REST IssueFieldValues is always cleared, even when flag is on.
553-
assert.Empty(t, returnedIssue.IssueFieldValues, "raw REST issue_field_values should not be exposed even when flag is on")
554-
555551
// Enriched FieldValues comes from the GraphQL nodes() round-trip.
556552
require.Len(t, returnedIssue.FieldValues, 2, "field_values should be populated from GraphQL when flag is on")
557553
assert.Equal(t, "priority", returnedIssue.FieldValues[0].Field)

‎pkg/github/minimal_types.go‎

Lines changed: 21 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -293,23 +293,6 @@ type MinimalReactions struct {
293293
Eyes int `json:"eyes"`
294294
}
295295

296-
// MinimalIssueFieldValueSingleSelectOption is the trimmed output type for a single-select option of an issue field value.
297-
type MinimalIssueFieldValueSingleSelectOption struct {
298-
ID int64 `json:"id"`
299-
Name string `json:"name"`
300-
Color string `json:"color"`
301-
}
302-
303-
// MinimalIssueFieldValue is the trimmed output type for a custom field value attached to an issue,
304-
// populated from REST API responses (e.g. get_issue). For GraphQL-sourced field values see MinimalFieldValue.
305-
type MinimalIssueFieldValue struct {
306-
IssueFieldID int64 `json:"issue_field_id,omitempty"`
307-
NodeID string `json:"node_id,omitempty"`
308-
DataType string `json:"data_type,omitempty"`
309-
Value any `json:"value,omitempty"`
310-
SingleSelectOption *MinimalIssueFieldValueSingleSelectOption `json:"single_select_option,omitempty"`
311-
}
312-
313296
// MinimalFieldValue is the trimmed output type for a custom field value resolved via GraphQL
314297
// (e.g. list_issues, search_issues). Single-value variants populate Value; Values is reserved for multi-select.
315298
type MinimalFieldValue struct {
@@ -320,28 +303,27 @@ type MinimalFieldValue struct {
320303

321304
// MinimalIssue is the trimmed output type for issue objects to reduce verbosity.
322305
type MinimalIssue struct {
323-
Number int `json:"number"`
324-
Title string `json:"title"`
325-
Body string `json:"body,omitempty"`
326-
State string `json:"state"`
327-
StateReason string `json:"state_reason,omitempty"`
328-
Draft bool `json:"draft,omitempty"`
329-
Locked bool `json:"locked,omitempty"`
330-
HTMLURL string `json:"html_url,omitempty"`
331-
User *MinimalUser `json:"user,omitempty"`
332-
AuthorAssociation string `json:"author_association,omitempty"`
333-
Labels []string `json:"labels,omitempty"`
334-
Assignees []string `json:"assignees,omitempty"`
335-
Milestone string `json:"milestone,omitempty"`
336-
Comments int `json:"comments,omitempty"`
337-
Reactions *MinimalReactions `json:"reactions,omitempty"`
338-
CreatedAt string `json:"created_at,omitempty"`
339-
UpdatedAt string `json:"updated_at,omitempty"`
340-
ClosedAt string `json:"closed_at,omitempty"`
341-
ClosedBy string `json:"closed_by,omitempty"`
342-
IssueType string `json:"issue_type,omitempty"`
343-
IssueFieldValues []MinimalIssueFieldValue `json:"issue_field_values,omitempty"`
344-
FieldValues []MinimalFieldValue `json:"field_values,omitempty"`
306+
Number int `json:"number"`
307+
Title string `json:"title"`
308+
Body string `json:"body,omitempty"`
309+
State string `json:"state"`
310+
StateReason string `json:"state_reason,omitempty"`
311+
Draft bool `json:"draft,omitempty"`
312+
Locked bool `json:"locked,omitempty"`
313+
HTMLURL string `json:"html_url,omitempty"`
314+
User *MinimalUser `json:"user,omitempty"`
315+
AuthorAssociation string `json:"author_association,omitempty"`
316+
Labels []string `json:"labels,omitempty"`
317+
Assignees []string `json:"assignees,omitempty"`
318+
Milestone string `json:"milestone,omitempty"`
319+
Comments int `json:"comments,omitempty"`
320+
Reactions *MinimalReactions `json:"reactions,omitempty"`
321+
CreatedAt string `json:"created_at,omitempty"`
322+
UpdatedAt string `json:"updated_at,omitempty"`
323+
ClosedAt string `json:"closed_at,omitempty"`
324+
ClosedBy string `json:"closed_by,omitempty"`
325+
IssueType string `json:"issue_type,omitempty"`
326+
FieldValues []MinimalFieldValue `json:"field_values,omitempty"`
345327
}
346328

347329
// MinimalIssuesResponse is the trimmed output for a paginated list of issues.
@@ -526,26 +508,6 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
526508
m.IssueType = issueType.GetName()
527509
}
528510

529-
for _, fv := range issue.IssueFieldValues {
530-
if fv == nil {
531-
continue
532-
}
533-
mfv := MinimalIssueFieldValue{
534-
IssueFieldID: fv.IssueFieldID,
535-
NodeID: fv.NodeID,
536-
DataType: fv.DataType,
537-
Value: fv.Value,
538-
}
539-
if opt := fv.SingleSelectOption; opt != nil {
540-
mfv.SingleSelectOption = &MinimalIssueFieldValueSingleSelectOption{
541-
ID: opt.ID,
542-
Name: opt.Name,
543-
Color: opt.Color,
544-
}
545-
}
546-
m.IssueFieldValues = append(m.IssueFieldValues, mfv)
547-
}
548-
549511
if r := issue.Reactions; r != nil {
550512
m.Reactions = &MinimalReactions{
551513
TotalCount: r.GetTotalCount(),

0 commit comments

Comments
 (0)