Skip to content

Commit 9f4af24

Browse files
zwickzwick
committed
Preserve standard project field resolution
Keep read and general field resolution independent of Issue Field schema features, and narrowly fall back for standard writes on hosts without the attachment bridge. Correct Issue Field error classification and align batch single-select option precedence with singular updates. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d574277-3735-4356-a753-8354aa6b537c
1 parent ef62fe3 commit 9f4af24

8 files changed

Lines changed: 322 additions & 69 deletions

‎pkg/github/projects.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1705,7 +1705,7 @@ func buildUpdateProjectItem(ctx context.Context, gqlClient *githubv4.Client, own
17051705
return nil, fmt.Errorf("internal error: gqlClient is required to resolve updated_field.name")
17061706
}
17071707
var err error
1708-
resolved, err = resolveProjectFieldByName(ctx, gqlClient, owner, ownerType, projectNumber, fieldName, "")
1708+
resolved, err = resolveProjectFieldForUpdateByName(ctx, gqlClient, owner, ownerType, projectNumber, fieldName, "")
17091709
if err != nil {
17101710
return nil, err
17111711
}

‎pkg/github/projects_batch.go‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -611,10 +611,10 @@ func parseBatchFieldSpec(raw any) (batchFieldSpec, error) {
611611

612612
func resolveBatchProjectField(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, spec batchFieldSpec) (*ResolvedField, error) {
613613
if spec.name != "" {
614-
return resolveProjectFieldByName(ctx, gqlClient, owner, ownerType, projectNumber, spec.name, "")
614+
return resolveProjectFieldForUpdateByName(ctx, gqlClient, owner, ownerType, projectNumber, spec.name, "")
615615
}
616616

617-
fields, err := listAllProjectFields(ctx, gqlClient, owner, ownerType, projectNumber)
617+
fields, err := listAllProjectFieldsForUpdate(ctx, gqlClient, owner, ownerType, projectNumber)
618618
if err != nil {
619619
return nil, err
620620
}
@@ -681,20 +681,9 @@ func convertProjectFieldValue(field *ResolvedField, raw any) (githubv4.ProjectV2
681681
if !ok || s == "" {
682682
return zero, fmt.Errorf("field %q is SINGLE_SELECT; value must be a non-empty string (option name or ID)", field.Name)
683683
}
684-
optID := s
685-
if resolvedID, optErr := resolveSingleSelectOptionByName(field, s); optErr == nil {
686-
optID = resolvedID
687-
} else {
688-
known := false
689-
for _, opt := range field.Options {
690-
if opt.ID == s {
691-
known = true
692-
break
693-
}
694-
}
695-
if !known {
696-
return zero, optErr
697-
}
684+
optID, err := resolveSingleSelectOptionByNameOrID(field, s)
685+
if err != nil {
686+
return zero, err
698687
}
699688
v := githubv4.String(optID)
700689
return githubv4.ProjectV2FieldValue{SingleSelectOptionID: &v}, nil

‎pkg/github/projects_batch_test.go‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ func Test_UpdateProjectItemsBatch_InvalidSharedValueIsTopLevelError(t *testing.T
277277
queryTransport := githubv4mock.NewMockedHTTPClient(
278278
projectIDMatcher("octo-org", 1, "PVT_project1"),
279279
githubv4mock.NewQueryMatcher(
280-
projectFieldsTestQuery{},
280+
projectFieldsWithIssueFieldsTestQuery{},
281281
fieldsQueryVars("octo-org", 1),
282282
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
283283
statusFieldNode("PVTSSF_status", 101, "Status", []map[string]any{
@@ -323,7 +323,7 @@ func Test_ProjectsWrite_UpdateProjectItems_NodeIDBypassesRESTLookup(t *testing.T
323323
queryTransport := githubv4mock.NewMockedHTTPClient(
324324
projectIDMatcher("octo-org", 1, "PVT_project1"),
325325
githubv4mock.NewQueryMatcher(
326-
projectFieldsTestQuery{},
326+
projectFieldsWithIssueFieldsTestQuery{},
327327
fieldsQueryVars("octo-org", 1),
328328
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
329329
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -376,7 +376,7 @@ func Test_ProjectsWrite_UpdateProjectItems_NumericItemIDDeduplicatesRESTLookup(t
376376
queryTransport := githubv4mock.NewMockedHTTPClient(
377377
projectIDMatcher("octo-org", 1, "PVT_project1"),
378378
githubv4mock.NewQueryMatcher(
379-
projectFieldsTestQuery{},
379+
projectFieldsWithIssueFieldsTestQuery{},
380380
fieldsQueryVars("octo-org", 1),
381381
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
382382
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -493,7 +493,7 @@ func Test_ProjectsWrite_UpdateProjectItems_IssueRefPaginationIsDeduplicated(t *t
493493
}),
494494
),
495495
githubv4mock.NewQueryMatcher(
496-
projectFieldsTestQuery{},
496+
projectFieldsWithIssueFieldsTestQuery{},
497497
fieldsQueryVars("octo-org", 1),
498498
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
499499
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -561,7 +561,7 @@ func Test_ProjectsWrite_UpdateProjectItems_DuplicateTargetRejected(t *testing.T)
561561
queryTransport := githubv4mock.NewMockedHTTPClient(
562562
projectIDMatcher("octo-org", 1, "PVT_project1"),
563563
githubv4mock.NewQueryMatcher(
564-
projectFieldsTestQuery{},
564+
projectFieldsWithIssueFieldsTestQuery{},
565565
fieldsQueryVars("octo-org", 1),
566566
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
567567
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -647,7 +647,7 @@ func chunkSizeTestRun(t *testing.T, toolDef inventory.ServerTool, itemCount int)
647647
queryTransport := githubv4mock.NewMockedHTTPClient(
648648
projectIDMatcher("octo-org", 1, "PVT_project1"),
649649
githubv4mock.NewQueryMatcher(
650-
projectFieldsTestQuery{},
650+
projectFieldsWithIssueFieldsTestQuery{},
651651
fieldsQueryVars("octo-org", 1),
652652
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
653653
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -705,7 +705,7 @@ func Test_ProjectsWrite_UpdateProjectItems_SharedNullClearsAllItemsInOrder(t *te
705705
queryTransport := githubv4mock.NewMockedHTTPClient(
706706
projectIDMatcher("octo-org", 1, "PVT_project1"),
707707
githubv4mock.NewQueryMatcher(
708-
projectFieldsTestQuery{},
708+
projectFieldsWithIssueFieldsTestQuery{},
709709
fieldsQueryVars("octo-org", 1),
710710
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
711711
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -770,7 +770,7 @@ func Test_ProjectsWrite_UpdateProjectItems_TransportFailureAbortsLaterChunks(t *
770770
queryTransport := githubv4mock.NewMockedHTTPClient(
771771
projectIDMatcher("octo-org", 1, "PVT_project1"),
772772
githubv4mock.NewQueryMatcher(
773-
projectFieldsTestQuery{},
773+
projectFieldsWithIssueFieldsTestQuery{},
774774
fieldsQueryVars("octo-org", 1),
775775
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
776776
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -866,7 +866,7 @@ func Test_ProjectsWrite_UpdateProjectItems_MixedOutcomeKeepsIsErrorFalse(t *test
866866
queryTransport := githubv4mock.NewMockedHTTPClient(
867867
projectIDMatcher("octo-org", 1, "PVT_project1"),
868868
githubv4mock.NewQueryMatcher(
869-
projectFieldsTestQuery{},
869+
projectFieldsWithIssueFieldsTestQuery{},
870870
fieldsQueryVars("octo-org", 1),
871871
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
872872
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -919,7 +919,7 @@ func Test_ProjectsWrite_UpdateProjectItems_EnterpriseClientWiring(t *testing.T)
919919
queryTransport := githubv4mock.NewMockedHTTPClient(
920920
projectIDMatcher("octo-org", 1, "PVT_project1"),
921921
githubv4mock.NewQueryMatcher(
922-
projectFieldsTestQuery{},
922+
projectFieldsWithIssueFieldsTestQuery{},
923923
fieldsQueryVars("octo-org", 1),
924924
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
925925
fieldNode("PVTF_notes", 101, "Notes", "TEXT"),
@@ -1144,6 +1144,22 @@ func Test_ConvertProjectFieldValue_SingleSelect_ByOptionID(t *testing.T) {
11441144
assert.Equal(t, "OPT_1", string(*v.SingleSelectOptionID))
11451145
}
11461146

1147+
func Test_ConvertProjectFieldValue_SingleSelect_IDPrecedesName(t *testing.T) {
1148+
field := &ResolvedField{
1149+
Name: "Status",
1150+
DataType: "SINGLE_SELECT",
1151+
Options: []ResolvedFieldOption{
1152+
{ID: "OPT_other", Name: "OPT_target"},
1153+
{ID: "OPT_target", Name: "Target"},
1154+
},
1155+
}
1156+
1157+
v, err := convertProjectFieldValue(field, "OPT_target")
1158+
require.NoError(t, err)
1159+
require.NotNil(t, v.SingleSelectOptionID)
1160+
assert.Equal(t, "OPT_target", string(*v.SingleSelectOptionID))
1161+
}
1162+
11471163
func Test_ConvertProjectFieldValue_SingleSelect_Unknown(t *testing.T) {
11481164
field := &ResolvedField{
11491165
Name: "Status",
@@ -1190,7 +1206,7 @@ func Test_ResolveBatchProjectField_ByIDAndName(t *testing.T) {
11901206
t.Run(tt.name, func(t *testing.T) {
11911207
mocked := githubv4mock.NewMockedHTTPClient(
11921208
githubv4mock.NewQueryMatcher(
1193-
projectFieldsTestQuery{},
1209+
projectFieldsWithIssueFieldsTestQuery{},
11941210
fieldsQueryVars("octo-org", 7),
11951211
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
11961212
statusFieldNode("PVTF_status", 101, "Status", nil),
@@ -1209,7 +1225,7 @@ func Test_ResolveBatchProjectField_ByIDAndName(t *testing.T) {
12091225
func Test_ResolveBatchProjectField_AmbiguousName(t *testing.T) {
12101226
mocked := githubv4mock.NewMockedHTTPClient(
12111227
githubv4mock.NewQueryMatcher(
1212-
projectFieldsTestQuery{},
1228+
projectFieldsWithIssueFieldsTestQuery{},
12131229
fieldsQueryVars("octo-org", 7),
12141230
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
12151231
statusFieldNode("PVTSSF_status1", 101, "Status", nil),

‎pkg/github/projects_issue_fields.go‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,31 @@ import (
1111
)
1212

1313
func buildIssueFieldUpdate(field *ResolvedField, raw any) (*IssueFieldCreateOrUpdateInput, error) {
14-
if field == nil || field.IssueFieldNodeID == "" {
15-
name := ""
16-
if field != nil {
17-
name = field.Name
18-
}
14+
if field == nil {
15+
return nil, ghErrors.NewStructuredResolutionError(
16+
"issue_field_metadata_unavailable",
17+
"",
18+
"the attached Project field metadata is unavailable",
19+
nil,
20+
)
21+
}
22+
23+
switch field.DataType {
24+
case "TEXT", "NUMBER", "DATE", "SINGLE_SELECT":
25+
default:
26+
return nil, ghErrors.NewStructuredResolutionError(
27+
"unsupported_field_type",
28+
field.Name,
29+
fmt.Sprintf("Issue Field %q has unsupported data type %q; supported types are TEXT, NUMBER, DATE, and SINGLE_SELECT", field.Name, field.DataType),
30+
nil,
31+
)
32+
}
33+
34+
if field.IssueFieldNodeID == "" {
1935
return nil, ghErrors.NewStructuredResolutionError(
2036
"issue_field_metadata_unavailable",
21-
name,
22-
"the attached Project field did not include the underlying Issue Field node ID; refresh field metadata and retry",
37+
field.Name,
38+
"the attached Project field did not include the underlying Issue Field node ID",
2339
nil,
2440
)
2541
}
@@ -69,13 +85,6 @@ func buildIssueFieldUpdate(field *ResolvedField, raw any) (*IssueFieldCreateOrUp
6985
}
7086
id := githubv4.ID(optionID)
7187
input.SingleSelectOptionID = &id
72-
default:
73-
return nil, ghErrors.NewStructuredResolutionError(
74-
"unsupported_field_type",
75-
field.Name,
76-
fmt.Sprintf("Issue Field %q has unsupported data type %q; supported types are TEXT, NUMBER, DATE, and SINGLE_SELECT", field.Name, field.DataType),
77-
nil,
78-
)
7988
}
8089
return input, nil
8190
}

‎pkg/github/projects_issue_fields_test.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func projectItemIssueByNodeIDMatcher(itemNodeID string, itemID int, issueNodeID
8686
func Test_ResolveProjectFieldByID_AttachedIssueFieldMetadata(t *testing.T) {
8787
queryTransport := githubv4mock.NewMockedHTTPClient(
8888
githubv4mock.NewQueryMatcher(
89-
projectFieldsTestQuery{},
89+
projectFieldsWithIssueFieldsTestQuery{},
9090
fieldsQueryVars("octo-org", 7),
9191
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
9292
attachedIssueFieldNode("PVTSSF_risk", 702, "IF_risk", "Risk", "SINGLE_SELECT", []map[string]any{
@@ -154,6 +154,7 @@ func Test_BuildIssueFieldUpdate_StructuredErrors(t *testing.T) {
154154
}{
155155
{"missing metadata", &ResolvedField{Name: "Customer", DataType: "TEXT", IsIssueField: true}, "Acme", "issue_field_metadata_unavailable"},
156156
{"wrong value type", &ResolvedField{Name: "Customer", DataType: "TEXT", IsIssueField: true, IssueFieldNodeID: "IF_customer"}, 42.0, "invalid_field_value"},
157+
{"unsupported type clear", &ResolvedField{Name: "Reviewers", DataType: "USER", IsIssueField: true}, nil, "unsupported_field_type"},
157158
}
158159
for _, tt := range tests {
159160
t.Run(tt.name, func(t *testing.T) {
@@ -169,7 +170,7 @@ func Test_BuildIssueFieldUpdate_StructuredErrors(t *testing.T) {
169170
func Test_UpdateProjectItem_AttachedIssueField(t *testing.T) {
170171
queryTransport := githubv4mock.NewMockedHTTPClient(
171172
githubv4mock.NewQueryMatcher(
172-
projectFieldsTestQuery{},
173+
projectFieldsWithIssueFieldsTestQuery{},
173174
fieldsQueryVars("octo-org", 1),
174175
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
175176
attachedIssueFieldNode("PVTSSF_risk", 704, "IF_risk", "Risk", "SINGLE_SELECT", []map[string]any{{"id": "IFO_high", "name": "High"}}),
@@ -225,15 +226,19 @@ func Test_ProjectItemReads_FieldNamesIncludeAttachedIssueFieldValues(t *testing.
225226

226227
for _, tt := range tests {
227228
t.Run(tt.name, func(t *testing.T) {
228-
gql := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(
229+
queryTransport := githubv4mock.NewMockedHTTPClient(
229230
githubv4mock.NewQueryMatcher(
230231
projectFieldsTestQuery{},
231232
fieldsQueryVars("octo-org", 1),
232233
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
233-
attachedIssueFieldNode("PVTF_customer", 701, "IF_customer", "Customer", "TEXT", nil),
234+
genericFieldNode("PVTF_customer", 701, "Customer", "TEXT"),
234235
})),
235236
),
236-
))
237+
)
238+
transport := &mutationAwareTransport{t: t, queries: queryTransport.Transport}
239+
gql := githubv4.NewClient(&http.Client{
240+
Transport: &transportpkg.GraphQLFeaturesTransport{Transport: transport},
241+
})
237242
rest := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
238243
tt.route: func(w http.ResponseWriter, r *http.Request) {
239244
assert.Equal(t, "701", r.URL.Query().Get("fields"))
@@ -255,6 +260,10 @@ func Test_ProjectItemReads_FieldNamesIncludeAttachedIssueFieldValues(t *testing.
255260
require.False(t, result.IsError, getTextResult(t, result).Text)
256261
assert.Contains(t, getTextResult(t, result).Text, `"name":"Customer"`)
257262
assert.Contains(t, getTextResult(t, result).Text, `"value":"Acme"`)
263+
require.Len(t, transport.queryCalls, 1)
264+
assert.Empty(t, transport.queryCalls[0].Headers.Get(headers.GraphQLFeaturesHeader))
265+
assert.NotContains(t, transport.queryCalls[0].Query, "isIssueField")
266+
assert.NotContains(t, transport.queryCalls[0].Query, "issueField")
258267
})
259268
}
260269
}
@@ -329,7 +338,7 @@ func Test_UpdateProjectItemsBatch_AttachedIssueFields(t *testing.T) {
329338
matchers := []githubv4mock.Matcher{
330339
projectIDMatcher("octo-org", 1, "PVT_project1"),
331340
githubv4mock.NewQueryMatcher(
332-
projectFieldsTestQuery{}, fieldsQueryVars("octo-org", 1),
341+
projectFieldsWithIssueFieldsTestQuery{}, fieldsQueryVars("octo-org", 1),
333342
githubv4mock.DataResponse(fieldsResponse([]map[string]any{tt.fieldNode})),
334343
),
335344
}

0 commit comments

Comments
 (0)