Skip to content

Commit cd30e93

Browse files
zwickzwick
committed
Restore separate project view field inputs
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000
1 parent cf67252 commit cd30e93

5 files changed

Lines changed: 83 additions & 108 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1138,7 +1138,8 @@ The following sets of tools are available:
11381138
- `title`: The project title. Required for 'create_project' method. (string, optional)
11391139
- `updated_field`: Object describing the field to update and its new value. Required for 'update_project_item'. Two shapes are accepted: (1) by ID — {"id": 123456, "value": "..."}; (2) by name — {"name": "Status", "value": "In Progress"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field. (object, optional)
11401140
- `view_id`: Project view node ID for update or delete; must belong to owner/project_number. (string, optional)
1141-
- `visible_fields`: Field database ID strings or case-insensitive names for table or board creation; valid base-10 int64 strings are IDs. Unsupported for roadmap. (string[], optional)
1141+
- `visible_field_names`: Field names for table or board creation; mutually exclusive with visible_fields. (string[], optional)
1142+
- `visible_fields`: Field database IDs for table or board creation; mutually exclusive with visible_field_names. (string[], optional)
11421143

11431144
</details>
11441145

‎pkg/github/__toolsnaps__/projects_write.snap‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,15 @@
154154
"description": "Project view node ID for update or delete; must belong to owner/project_number.",
155155
"type": "string"
156156
},
157+
"visible_field_names": {
158+
"description": "Field names for table or board creation; mutually exclusive with visible_fields.",
159+
"items": {
160+
"type": "string"
161+
},
162+
"type": "array"
163+
},
157164
"visible_fields": {
158-
"description": "Field database ID strings or case-insensitive names for table or board creation; valid base-10 int64 strings are IDs. Unsupported for roadmap.",
165+
"description": "Field database IDs for table or board creation; mutually exclusive with visible_field_names.",
159166
"items": {
160167
"type": "string"
161168
},

‎pkg/github/projects.go‎

Lines changed: 30 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,14 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
677677
},
678678
"visible_fields": {
679679
Type: "array",
680-
Description: "Field database ID strings or case-insensitive names for table or board creation; valid base-10 int64 strings are IDs. Unsupported for roadmap.",
680+
Description: "Field database IDs for table or board creation; mutually exclusive with visible_field_names.",
681+
Items: &jsonschema.Schema{
682+
Type: "string",
683+
},
684+
},
685+
"visible_field_names": {
686+
Type: "array",
687+
Description: "Field names for table or board creation; mutually exclusive with visible_fields.",
681688
Items: &jsonschema.Schema{
682689
Type: "string",
683690
},
@@ -808,18 +815,18 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
808815
}
809816

810817
if method == projectsMethodCreateProjectView {
811-
visibleFields, fieldsErr := OptionalStringArrayParam(args, "visible_fields")
812-
if fieldsErr != nil {
813-
return utils.NewToolResultError(fieldsErr.Error()), nil, nil
818+
visibleFieldNames, namesErr := OptionalStringArrayParam(args, "visible_field_names")
819+
if namesErr != nil {
820+
return utils.NewToolResultError(namesErr.Error()), nil, nil
814821
}
815822
var gqlClient *githubv4.Client
816-
if hasProjectViewVisibleFieldNames(visibleFields) {
823+
if len(visibleFieldNames) > 0 {
817824
gqlClient, err = deps.GetGQLClient(ctx)
818825
if err != nil {
819826
return utils.NewToolResultError(err.Error()), nil, nil
820827
}
821828
}
822-
return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFields)
829+
return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFieldNames)
823830
}
824831

825832
gqlClient, err := deps.GetGQLClient(ctx)
@@ -1824,7 +1831,7 @@ func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID stri
18241831
return utils.NewToolResultText(string(result)), !bool(query.Node.ProjectView.Project.Public), nil, nil
18251832
}
18261833

1827-
func createProjectView(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int, visibleFieldValues []string) (*mcp.CallToolResult, any, error) {
1834+
func createProjectView(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int, visibleFieldNames []string) (*mcp.CallToolResult, any, error) {
18281835
name, err := RequiredParam[string](args, "name")
18291836
if err != nil {
18301837
return utils.NewToolResultError(err.Error()), nil, nil
@@ -1844,13 +1851,23 @@ func createProjectView(ctx context.Context, client *github.Client, gqlClient *gi
18441851
if err != nil {
18451852
return utils.NewToolResultError(err.Error()), nil, nil
18461853
}
1847-
visibleFields, resolveErr := resolveProjectViewVisibleFields(ctx, gqlClient, owner, ownerType, projectNumber, visibleFieldValues)
1848-
if resolveErr != nil {
1849-
var structured *ghErrors.StructuredResolutionError
1850-
if errors.As(resolveErr, &structured) {
1851-
return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil
1854+
visibleFields, err := OptionalBigIntArrayParam(args, "visible_fields")
1855+
if err != nil {
1856+
return utils.NewToolResultError(err.Error()), nil, nil
1857+
}
1858+
if len(visibleFields) > 0 && len(visibleFieldNames) > 0 {
1859+
return utils.NewToolResultError("provide either 'visible_fields' or 'visible_field_names', not both"), nil, nil
1860+
}
1861+
if len(visibleFieldNames) > 0 {
1862+
resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, visibleFieldNames)
1863+
if resolveErr != nil {
1864+
var structured *ghErrors.StructuredResolutionError
1865+
if errors.As(resolveErr, &structured) {
1866+
return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil
1867+
}
1868+
return utils.NewToolResultError(resolveErr.Error()), nil, nil
18521869
}
1853-
return utils.NewToolResultError(resolveErr.Error()), nil, nil
1870+
visibleFields = resolvedIDs
18541871
}
18551872
if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && len(visibleFields) > 0 {
18561873
return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil
@@ -1903,38 +1920,6 @@ func createProjectView(ctx context.Context, client *github.Client, gqlClient *gi
19031920
return MarshalledTextResult(view), nil, nil
19041921
}
19051922

1906-
func hasProjectViewVisibleFieldNames(fields []string) bool {
1907-
for _, field := range fields {
1908-
if _, err := parseInt64(field); err != nil {
1909-
return true
1910-
}
1911-
}
1912-
return false
1913-
}
1914-
1915-
func resolveProjectViewVisibleFields(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, fields []string) ([]int64, error) {
1916-
resolved := make([]int64, len(fields))
1917-
names := make([]string, 0, len(fields))
1918-
namePositions := make([]int, 0, len(fields))
1919-
for i, field := range fields {
1920-
if id, err := parseInt64(field); err == nil {
1921-
resolved[i] = id
1922-
} else {
1923-
names = append(names, field)
1924-
namePositions = append(namePositions, i)
1925-
}
1926-
}
1927-
1928-
nameIDs, err := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, names)
1929-
if err != nil {
1930-
return nil, err
1931-
}
1932-
for i, position := range namePositions {
1933-
resolved[position] = nameIDs[i]
1934-
}
1935-
return resolved, nil
1936-
}
1937-
19381923
func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) error {
19391924
expectedProjectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber)
19401925
if err != nil {

‎pkg/github/projects_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ func Test_ProjectsWrite(t *testing.T) {
887887
assert.Contains(t, inputSchema.Properties, "layout")
888888
assert.Contains(t, inputSchema.Properties, "filter")
889889
assert.Contains(t, inputSchema.Properties, "visible_fields")
890-
assert.NotContains(t, inputSchema.Properties, "visible_field_names")
890+
assert.Contains(t, inputSchema.Properties, "visible_field_names")
891891
assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodCreateProjectView)
892892
assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodUpdateProjectView)
893893
assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodDeleteProjectView)

‎pkg/github/projects_v2_test.go‎

Lines changed: 42 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -839,13 +839,13 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
839839
}
840840
handler := toolDef.Handler(deps)
841841
request := createMCPRequest(map[string]any{
842-
"method": "create_project_view",
843-
"owner": "octo-org",
844-
"owner_type": "org",
845-
"project_number": float64(7),
846-
"name": "Named fields",
847-
"layout": "table",
848-
"visible_fields": []any{"Priority", "status"},
842+
"method": "create_project_view",
843+
"owner": "octo-org",
844+
"owner_type": "org",
845+
"project_number": float64(7),
846+
"name": "Named fields",
847+
"layout": "table",
848+
"visible_field_names": []any{"Priority", "status"},
849849
})
850850

851851
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
@@ -880,13 +880,13 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
880880
}
881881
handler := toolDef.Handler(deps)
882882
request := createMCPRequest(map[string]any{
883-
"method": "create_project_view",
884-
"owner": "octocat",
885-
"owner_type": "user",
886-
"project_number": float64(8),
887-
"name": "User fields",
888-
"layout": "board",
889-
"visible_fields": []any{"Status"},
883+
"method": "create_project_view",
884+
"owner": "octocat",
885+
"owner_type": "user",
886+
"project_number": float64(8),
887+
"name": "User fields",
888+
"layout": "board",
889+
"visible_field_names": []any{"Status"},
890890
})
891891

892892
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
@@ -951,45 +951,27 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
951951
require.False(t, result.IsError)
952952
})
953953

954-
t.Run("preserves mixed visible field ID and name order", func(t *testing.T) {
955-
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(
956-
projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{
957-
statusFieldNode("PVTSSF_status", 303, "Status", nil),
958-
statusFieldNode("PVTSSF_priority", 404, "Priority", nil),
959-
}),
960-
))
961-
restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
962-
"POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) {
963-
var body map[string]any
964-
require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
965-
assert.Equal(t, []any{float64(101), float64(404), float64(202), float64(303)}, body["visible_fields"])
966-
mockResponse(t, http.StatusCreated, map[string]any{
967-
"node_id": "PVTV_mixed",
968-
"number": 4,
969-
"name": "Mixed fields",
970-
"layout": "table",
971-
"visible_fields": []int64{101, 404, 202, 303},
972-
})(w, r)
973-
},
974-
})
954+
t.Run("rejects visible fields and names together", func(t *testing.T) {
975955
deps := BaseDeps{
976-
Client: mustNewGHClient(t, restClient),
977-
GQLClient: gqlClient,
956+
Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})),
957+
GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()),
978958
}
979959
handler := toolDef.Handler(deps)
980960
request := createMCPRequest(map[string]any{
981-
"method": "create_project_view",
982-
"owner": "octo-org",
983-
"owner_type": "org",
984-
"project_number": float64(7),
985-
"name": "Mixed fields",
986-
"layout": "table",
987-
"visible_fields": []any{"101", "Priority", "202", "status"},
961+
"method": "create_project_view",
962+
"owner": "octo-org",
963+
"owner_type": "org",
964+
"project_number": float64(7),
965+
"name": "Table",
966+
"layout": "table",
967+
"visible_fields": []any{"101"},
968+
"visible_field_names": []any{"Status"},
988969
})
989970

990971
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
991972
require.NoError(t, err)
992-
require.False(t, result.IsError)
973+
require.True(t, result.IsError)
974+
assert.Contains(t, getTextResult(t, result).Text, "provide either 'visible_fields' or 'visible_field_names', not both")
993975
})
994976

995977
for _, tc := range []struct {
@@ -1003,7 +985,7 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
1003985
nodes: []map[string]any{
1004986
statusFieldNode("PVTSSF_status", 101, "Status", nil),
1005987
},
1006-
requestedName: "123invalid",
988+
requestedName: "Priority",
1007989
expectedError: "field_not_found",
1008990
},
1009991
{
@@ -1026,13 +1008,13 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
10261008
}
10271009
handler := toolDef.Handler(deps)
10281010
request := createMCPRequest(map[string]any{
1029-
"method": "create_project_view",
1030-
"owner": "octo-org",
1031-
"owner_type": "org",
1032-
"project_number": float64(7),
1033-
"name": "Table",
1034-
"layout": "table",
1035-
"visible_fields": []any{tc.requestedName},
1011+
"method": "create_project_view",
1012+
"owner": "octo-org",
1013+
"owner_type": "org",
1014+
"project_number": float64(7),
1015+
"name": "Table",
1016+
"layout": "table",
1017+
"visible_field_names": []any{tc.requestedName},
10361018
})
10371019

10381020
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
@@ -1076,13 +1058,13 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
10761058
}
10771059
handler := toolDef.Handler(deps)
10781060
request := createMCPRequest(map[string]any{
1079-
"method": "create_project_view",
1080-
"owner": "octo-org",
1081-
"owner_type": "org",
1082-
"project_number": float64(7),
1083-
"name": "Roadmap",
1084-
"layout": "roadmap",
1085-
"visible_fields": []any{"Status"},
1061+
"method": "create_project_view",
1062+
"owner": "octo-org",
1063+
"owner_type": "org",
1064+
"project_number": float64(7),
1065+
"name": "Roadmap",
1066+
"layout": "roadmap",
1067+
"visible_field_names": []any{"Status"},
10861068
})
10871069

10881070
result, err := handler(ContextWithDeps(context.Background(), deps), &request)

0 commit comments

Comments
 (0)