Skip to content

Commit cf67252

Browse files
zwickzwick
committed
Unify project view visible fields
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000
1 parent 588abca commit cf67252

5 files changed

Lines changed: 108 additions & 83 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,8 +1138,7 @@ 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_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)
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)
11431142

11441143
</details>
11451144

‎pkg/github/__toolsnaps__/projects_write.snap‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,8 @@
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-
},
164157
"visible_fields": {
165-
"description": "Field database IDs for table or board creation; mutually exclusive with visible_field_names.",
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.",
166159
"items": {
167160
"type": "string"
168161
},

‎pkg/github/projects.go‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -677,14 +677,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
677677
},
678678
"visible_fields": {
679679
Type: "array",
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.",
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.",
688681
Items: &jsonschema.Schema{
689682
Type: "string",
690683
},
@@ -815,18 +808,18 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
815808
}
816809

817810
if method == projectsMethodCreateProjectView {
818-
visibleFieldNames, namesErr := OptionalStringArrayParam(args, "visible_field_names")
819-
if namesErr != nil {
820-
return utils.NewToolResultError(namesErr.Error()), nil, nil
811+
visibleFields, fieldsErr := OptionalStringArrayParam(args, "visible_fields")
812+
if fieldsErr != nil {
813+
return utils.NewToolResultError(fieldsErr.Error()), nil, nil
821814
}
822815
var gqlClient *githubv4.Client
823-
if len(visibleFieldNames) > 0 {
816+
if hasProjectViewVisibleFieldNames(visibleFields) {
824817
gqlClient, err = deps.GetGQLClient(ctx)
825818
if err != nil {
826819
return utils.NewToolResultError(err.Error()), nil, nil
827820
}
828821
}
829-
return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFieldNames)
822+
return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFields)
830823
}
831824

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

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) {
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) {
18351828
name, err := RequiredParam[string](args, "name")
18361829
if err != nil {
18371830
return utils.NewToolResultError(err.Error()), nil, nil
@@ -1851,23 +1844,13 @@ func createProjectView(ctx context.Context, client *github.Client, gqlClient *gi
18511844
if err != nil {
18521845
return utils.NewToolResultError(err.Error()), nil, nil
18531846
}
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
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
18691852
}
1870-
visibleFields = resolvedIDs
1853+
return utils.NewToolResultError(resolveErr.Error()), nil, nil
18711854
}
18721855
if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && len(visibleFields) > 0 {
18731856
return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil
@@ -1920,6 +1903,38 @@ func createProjectView(ctx context.Context, client *github.Client, gqlClient *gi
19201903
return MarshalledTextResult(view), nil, nil
19211904
}
19221905

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+
19231938
func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) error {
19241939
expectedProjectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber)
19251940
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.Contains(t, inputSchema.Properties, "visible_field_names")
890+
assert.NotContains(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: 60 additions & 42 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_field_names": []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_fields": []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_field_names": []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_fields": []any{"Status"},
890890
})
891891

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

954-
t.Run("rejects visible fields and names together", func(t *testing.T) {
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+
})
955975
deps := BaseDeps{
956-
Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})),
957-
GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()),
976+
Client: mustNewGHClient(t, restClient),
977+
GQLClient: gqlClient,
958978
}
959979
handler := toolDef.Handler(deps)
960980
request := createMCPRequest(map[string]any{
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"},
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"},
969988
})
970989

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

977995
for _, tc := range []struct {
@@ -985,7 +1003,7 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
9851003
nodes: []map[string]any{
9861004
statusFieldNode("PVTSSF_status", 101, "Status", nil),
9871005
},
988-
requestedName: "Priority",
1006+
requestedName: "123invalid",
9891007
expectedError: "field_not_found",
9901008
},
9911009
{
@@ -1008,13 +1026,13 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
10081026
}
10091027
handler := toolDef.Handler(deps)
10101028
request := createMCPRequest(map[string]any{
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},
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},
10181036
})
10191037

10201038
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
@@ -1058,13 +1076,13 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) {
10581076
}
10591077
handler := toolDef.Handler(deps)
10601078
request := createMCPRequest(map[string]any{
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"},
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"},
10681086
})
10691087

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

0 commit comments

Comments
 (0)