From b851e6ed6472942ddce0e623289a1971a3249bc2 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 16 Jun 2026 13:59:39 +0100 Subject: [PATCH] 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> --- pkg/github/issues.go | 5 +-- pkg/github/issues_test.go | 6 +-- pkg/github/minimal_types.go | 80 ++++++++++--------------------------- 3 files changed, 24 insertions(+), 67 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 79b8b23ad2..e4d0a1b9e3 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -874,9 +874,8 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies, minimalIssue := convertToMinimalIssue(issue) - // Always drop the verbose REST IssueFieldValues; only enrich with the GraphQL - // field_values view when the issue-fields feature flag is on. - minimalIssue.IssueFieldValues = nil + // Enrich with the GraphQL field_values view only when the issue-fields feature + // flag is on. The verbose REST issue field values are not surfaced. if deps.IsFeatureEnabled(ctx, FeatureFlagIssueFields) { if issue != nil && issue.NodeID != nil && *issue.NodeID != "" { gqlClient, err := deps.GetGQLClient(ctx) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 5378ff62b7..e2e4fb54a2 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -459,8 +459,7 @@ func Test_GetIssue_FieldValues(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) - // Flag is off: raw REST IssueFieldValues must be cleared, enriched FieldValues absent. - assert.Empty(t, returnedIssue.IssueFieldValues, "raw REST issue_field_values should not be exposed when flag is off") + // Flag is off: enriched field_values absent. assert.Empty(t, returnedIssue.FieldValues, "enriched field_values should not be present when flag is off") } @@ -549,9 +548,6 @@ func Test_GetIssue_FieldValues_FlagOn(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) - // Raw REST IssueFieldValues is always cleared, even when flag is on. - assert.Empty(t, returnedIssue.IssueFieldValues, "raw REST issue_field_values should not be exposed even when flag is on") - // Enriched FieldValues comes from the GraphQL nodes() round-trip. require.Len(t, returnedIssue.FieldValues, 2, "field_values should be populated from GraphQL when flag is on") assert.Equal(t, "priority", returnedIssue.FieldValues[0].Field) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index eff6edc133..3451ebb7a6 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -293,23 +293,6 @@ type MinimalReactions struct { Eyes int `json:"eyes"` } -// MinimalIssueFieldValueSingleSelectOption is the trimmed output type for a single-select option of an issue field value. -type MinimalIssueFieldValueSingleSelectOption struct { - ID int64 `json:"id"` - Name string `json:"name"` - Color string `json:"color"` -} - -// MinimalIssueFieldValue is the trimmed output type for a custom field value attached to an issue, -// populated from REST API responses (e.g. get_issue). For GraphQL-sourced field values see MinimalFieldValue. -type MinimalIssueFieldValue struct { - IssueFieldID int64 `json:"issue_field_id,omitempty"` - NodeID string `json:"node_id,omitempty"` - DataType string `json:"data_type,omitempty"` - Value any `json:"value,omitempty"` - SingleSelectOption *MinimalIssueFieldValueSingleSelectOption `json:"single_select_option,omitempty"` -} - // MinimalFieldValue is the trimmed output type for a custom field value resolved via GraphQL // (e.g. list_issues, search_issues). Single-value variants populate Value; Values is reserved for multi-select. type MinimalFieldValue struct { @@ -320,28 +303,27 @@ type MinimalFieldValue struct { // MinimalIssue is the trimmed output type for issue objects to reduce verbosity. type MinimalIssue struct { - Number int `json:"number"` - Title string `json:"title"` - Body string `json:"body,omitempty"` - State string `json:"state"` - StateReason string `json:"state_reason,omitempty"` - Draft bool `json:"draft,omitempty"` - Locked bool `json:"locked,omitempty"` - HTMLURL string `json:"html_url,omitempty"` - User *MinimalUser `json:"user,omitempty"` - AuthorAssociation string `json:"author_association,omitempty"` - Labels []string `json:"labels,omitempty"` - Assignees []string `json:"assignees,omitempty"` - Milestone string `json:"milestone,omitempty"` - Comments int `json:"comments,omitempty"` - Reactions *MinimalReactions `json:"reactions,omitempty"` - CreatedAt string `json:"created_at,omitempty"` - UpdatedAt string `json:"updated_at,omitempty"` - ClosedAt string `json:"closed_at,omitempty"` - ClosedBy string `json:"closed_by,omitempty"` - IssueType string `json:"issue_type,omitempty"` - IssueFieldValues []MinimalIssueFieldValue `json:"issue_field_values,omitempty"` - FieldValues []MinimalFieldValue `json:"field_values,omitempty"` + Number int `json:"number"` + Title string `json:"title"` + Body string `json:"body,omitempty"` + State string `json:"state"` + StateReason string `json:"state_reason,omitempty"` + Draft bool `json:"draft,omitempty"` + Locked bool `json:"locked,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + User *MinimalUser `json:"user,omitempty"` + AuthorAssociation string `json:"author_association,omitempty"` + Labels []string `json:"labels,omitempty"` + Assignees []string `json:"assignees,omitempty"` + Milestone string `json:"milestone,omitempty"` + Comments int `json:"comments,omitempty"` + Reactions *MinimalReactions `json:"reactions,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` + ClosedAt string `json:"closed_at,omitempty"` + ClosedBy string `json:"closed_by,omitempty"` + IssueType string `json:"issue_type,omitempty"` + FieldValues []MinimalFieldValue `json:"field_values,omitempty"` } // MinimalIssuesResponse is the trimmed output for a paginated list of issues. @@ -526,26 +508,6 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue { m.IssueType = issueType.GetName() } - for _, fv := range issue.IssueFieldValues { - if fv == nil { - continue - } - mfv := MinimalIssueFieldValue{ - IssueFieldID: fv.IssueFieldID, - NodeID: fv.NodeID, - DataType: fv.DataType, - Value: fv.Value, - } - if opt := fv.SingleSelectOption; opt != nil { - mfv.SingleSelectOption = &MinimalIssueFieldValueSingleSelectOption{ - ID: opt.ID, - Name: opt.Name, - Color: opt.Color, - } - } - m.IssueFieldValues = append(m.IssueFieldValues, mfv) - } - if r := issue.Reactions; r != nil { m.Reactions = &MinimalReactions{ TotalCount: r.GetTotalCount(),