Skip to content

Commit 5355584

Browse files
JoannaaKLJoannaaKL
committed
Change schema of fields
1 parent 6793b9d commit 5355584

File tree

4 files changed

+190
-42
lines changed

4 files changed

+190
-42
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,7 @@ The following sets of tools are available (all are on by default):
708708
- `query`: Filter projects by a search query (matches title and description) (string, optional)
709709

710710
- **update_project_item** - Update project item
711-
- `fields`: A list of field updates to apply. (array, required)
711+
- `fields`: A list of field updates to apply. (object[], required)
712712
- `item_id`: The numeric ID of the project item to update (not the issue or pull request ID). (number, required)
713713
- `owner`: If owner_type == user it is the handle for the GitHub user account. If owner_type == org it is the name of the organization. The name is not case sensitive. (string, required)
714714
- `owner_type`: Owner type (string, required)

‎pkg/github/__toolsnaps__/update_project_item.snap‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,24 @@
88
"properties": {
99
"fields": {
1010
"description": "A list of field updates to apply.",
11+
"items": {
12+
"additionalProperties": false,
13+
"properties": {
14+
"id": {
15+
"description": "ID of the project field",
16+
"type": "int"
17+
},
18+
"value": {
19+
"description": "value of the project field",
20+
"type": "any"
21+
}
22+
},
23+
"required": [
24+
"id",
25+
"value"
26+
],
27+
"type": "object"
28+
},
1129
"type": "array"
1230
},
1331
"item_id": {

‎pkg/github/projects.go‎

Lines changed: 65 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -624,12 +624,44 @@ func DeleteProjectItem(getClient GetClientFn, t translations.TranslationHelperFu
624624
func UpdateProjectItem(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
625625
return mcp.NewTool("update_project_item",
626626
mcp.WithDescription(t("TOOL_UPDATE_PROJECT_ITEM_DESCRIPTION", "Update a specific Project item for a user or org")),
627-
mcp.WithToolAnnotation(mcp.ToolAnnotation{Title: t("TOOL_UPDATE_PROJECT_ITEM_USER_TITLE", "Update project item"), ReadOnlyHint: ToBoolPtr(false)}),
628-
mcp.WithString("owner_type", mcp.Required(), mcp.Description("Owner type"), mcp.Enum("user", "org")),
629-
mcp.WithString("owner", mcp.Required(), mcp.Description("If owner_type == user it is the handle for the GitHub user account. If owner_type == org it is the name of the organization. The name is not case sensitive.")),
630-
mcp.WithNumber("project_number", mcp.Required(), mcp.Description("The project's number.")),
631-
mcp.WithNumber("item_id", mcp.Required(), mcp.Description("The numeric ID of the project item to update (not the issue or pull request ID).")),
632-
mcp.WithArray("fields", mcp.Required(), mcp.Description("A list of field updates to apply.")),
627+
mcp.WithToolAnnotation(mcp.ToolAnnotation{Title: t("TOOL_UPDATE_PROJECT_ITEM_USER_TITLE", "Update project item"),
628+
ReadOnlyHint: ToBoolPtr(false)},
629+
),
630+
mcp.WithString("owner_type",
631+
mcp.Required(), mcp.Description("Owner type"), mcp.Enum("user", "org"),
632+
),
633+
mcp.WithString("owner",
634+
mcp.Required(),
635+
mcp.Description("If owner_type == user it is the handle for the GitHub user account. If owner_type == org it is the name of the organization. The name is not case sensitive."),
636+
),
637+
mcp.WithNumber("project_number",
638+
mcp.Required(), mcp.Description("The project's number."),
639+
),
640+
mcp.WithNumber("item_id",
641+
mcp.Required(),
642+
mcp.Description("The numeric ID of the project item to update (not the issue or pull request ID)."),
643+
),
644+
mcp.WithArray("fields",
645+
mcp.Required(),
646+
mcp.Description("A list of field updates to apply."),
647+
mcp.Items(
648+
map[string]interface{}{
649+
"type": "object",
650+
"additionalProperties": false,
651+
"required": []string{"id", "value"},
652+
"properties": map[string]interface{}{
653+
"id": map[string]interface{}{
654+
"type": "int",
655+
"description": "ID of the project field",
656+
},
657+
"value": map[string]interface{}{
658+
"type": "any",
659+
// intentionally left without a specific JSON schema type to allow any value
660+
"description": "value of the project field",
661+
},
662+
},
663+
}),
664+
),
633665
), func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
634666
owner, err := RequiredParam[string](req, "owner")
635667
if err != nil {
@@ -651,16 +683,18 @@ func UpdateProjectItem(getClient GetClientFn, t translations.TranslationHelperFu
651683
if err != nil {
652684
return mcp.NewToolResultError(err.Error()), nil
653685
}
654-
fieldsParam, ok := req.GetArguments()["fields"]
655-
if !ok {
686+
687+
// Extract and validate fields parameter (required & non-empty)
688+
args := req.GetArguments()
689+
rawFields, present := args["fields"]
690+
if !present {
656691
return mcp.NewToolResultError("missing required parameter: fields"), nil
657692
}
658-
659-
rawFields, ok := fieldsParam.([]any)
693+
fields, ok := rawFields.([]interface{})
660694
if !ok {
661-
return mcp.NewToolResultError("parameter fields must be an array of objects"), nil
695+
return mcp.NewToolResultError("fields parameter must be an array of objects with id and value"), nil
662696
}
663-
if len(rawFields) == 0 {
697+
if len(fields) == 0 {
664698
return mcp.NewToolResultError("fields must contain at least one field update"), nil
665699
}
666700

@@ -671,43 +705,32 @@ func UpdateProjectItem(getClient GetClientFn, t translations.TranslationHelperFu
671705
projectsURL = fmt.Sprintf("users/%s/projectsV2/%d/items/%d", owner, projectNumber, itemID)
672706
}
673707

674-
updateFields := make([]*newProjectV2Field, 0, len(rawFields))
675-
for idx, rawField := range rawFields {
676-
fieldMap, ok := rawField.(map[string]any)
708+
updateFields := make([]*newProjectV2Field, 0, len(fields))
709+
for _, rawField := range fields {
710+
m, ok := rawField.(map[string]any)
677711
if !ok {
678-
return mcp.NewToolResultError(fmt.Sprintf("fields[%d] must be an object", idx)), nil
712+
return mcp.NewToolResultError("each element of fields must be an object"), nil
679713
}
680-
681-
rawID, ok := fieldMap["id"]
714+
idVal, ok := m["id"]
682715
if !ok {
683-
return mcp.NewToolResultError(fmt.Sprintf("fields[%d] is missing 'id'", idx)), nil
716+
return mcp.NewToolResultError("each field update must include an 'id'"), nil
684717
}
685-
686-
var fieldID int64
687-
switch v := rawID.(type) {
718+
var idInt64 int64
719+
switch v := idVal.(type) {
688720
case float64:
689-
fieldID = int64(v)
721+
idInt64 = int64(v)
722+
case int:
723+
idInt64 = int64(v)
690724
case int64:
691-
fieldID = v
692-
case json.Number:
693-
n, convErr := v.Int64()
694-
if convErr != nil {
695-
return mcp.NewToolResultError(fmt.Sprintf("fields[%d].id must be a numeric value", idx)), nil
696-
}
697-
fieldID = n
725+
idInt64 = v
698726
default:
699-
return mcp.NewToolResultError(fmt.Sprintf("fields[%d].id must be a numeric value", idx)), nil
727+
return mcp.NewToolResultError("field 'id' must be a number"), nil
700728
}
701-
702-
value, ok := fieldMap["value"]
729+
value, ok := m["value"]
703730
if !ok {
704-
return mcp.NewToolResultError(fmt.Sprintf("fields[%d] is missing 'value'", idx)), nil
731+
return mcp.NewToolResultError("each field update must include a 'value'"), nil
705732
}
706-
707-
updateFields = append(updateFields, &newProjectV2Field{
708-
ID: github.Ptr(fieldID),
709-
Value: value,
710-
})
733+
updateFields = append(updateFields, &newProjectV2Field{ID: github.Ptr(idInt64), Value: value})
711734
}
712735

713736
updateProjectItemOptions := &updateProjectItemOptions{Fields: updateFields}
@@ -749,8 +772,9 @@ type updateProjectItemOptions struct {
749772
}
750773

751774
type newProjectV2Field struct {
752-
ID *int64 `json:"id,omitempty"`
753-
Value any `json:"value,omitempty"`
775+
ID *int64 `json:"id,omitempty"`
776+
// Value should be sent as explicit null if user supplies null, so do NOT use omitempty
777+
Value any `json:"value"`
754778
}
755779

756780
type newProjectItem struct {

‎pkg/github/projects_test.go‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,6 +1383,82 @@ func Test_UpdateProjectItem(t *testing.T) {
13831383
expectedID: 801,
13841384
expectedCreatorLogin: "octocat",
13851385
},
1386+
{
1387+
name: "success organization multi-field update",
1388+
mockedClient: mock.NewMockedHTTPClient(
1389+
mock.WithRequestMatchHandler(
1390+
mock.EndpointPattern{Pattern: "/orgs/{org}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch},
1391+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1392+
body, err := io.ReadAll(r.Body)
1393+
assert.NoError(t, err)
1394+
var payload struct {
1395+
Fields []struct {
1396+
ID int `json:"id"`
1397+
Value interface{} `json:"value"`
1398+
} `json:"fields"`
1399+
}
1400+
assert.NoError(t, json.Unmarshal(body, &payload))
1401+
assert.Len(t, payload.Fields, 2)
1402+
if len(payload.Fields) == 2 {
1403+
assert.Equal(t, 1111, payload.Fields[0].ID)
1404+
assert.Equal(t, "Backlog", payload.Fields[0].Value)
1405+
assert.Equal(t, 2222, payload.Fields[1].ID)
1406+
assert.Equal(t, 5.0, payload.Fields[1].Value) // JSON numbers -> float64
1407+
}
1408+
w.WriteHeader(http.StatusOK)
1409+
_, _ = w.Write(mock.MustMarshal(orgUpdated))
1410+
}),
1411+
),
1412+
),
1413+
requestArgs: map[string]any{
1414+
"owner": "octo-org",
1415+
"owner_type": "org",
1416+
"project_number": float64(9999),
1417+
"item_id": float64(8888),
1418+
"fields": []any{
1419+
map[string]any{"id": float64(1111), "value": "Backlog"},
1420+
map[string]any{"id": float64(2222), "value": float64(5)},
1421+
},
1422+
},
1423+
expectedID: 801,
1424+
expectedCreatorLogin: "octocat",
1425+
},
1426+
{
1427+
name: "success organization update null value",
1428+
mockedClient: mock.NewMockedHTTPClient(
1429+
mock.WithRequestMatchHandler(
1430+
mock.EndpointPattern{Pattern: "/orgs/{org}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch},
1431+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1432+
body, err := io.ReadAll(r.Body)
1433+
assert.NoError(t, err)
1434+
var raw map[string]any
1435+
assert.NoError(t, json.Unmarshal(body, &raw))
1436+
fieldsRaw, ok := raw["fields"].([]any)
1437+
assert.True(t, ok)
1438+
assert.Len(t, fieldsRaw, 1)
1439+
first, ok := fieldsRaw[0].(map[string]any)
1440+
assert.True(t, ok)
1441+
// Ensure key exists and is explicitly null (presence matters)
1442+
val, present := first["value"]
1443+
assert.True(t, present)
1444+
assert.Nil(t, val)
1445+
w.WriteHeader(http.StatusOK)
1446+
_, _ = w.Write(mock.MustMarshal(orgUpdated))
1447+
}),
1448+
),
1449+
),
1450+
requestArgs: map[string]any{
1451+
"owner": "octo-org",
1452+
"owner_type": "org",
1453+
"project_number": float64(1010),
1454+
"item_id": float64(2020),
1455+
"fields": []any{
1456+
map[string]any{"id": float64(999), "value": nil},
1457+
},
1458+
},
1459+
expectedID: 801,
1460+
expectedCreatorLogin: "octocat",
1461+
},
13861462
{
13871463
name: "success user update",
13881464
mockedClient: mock.NewMockedHTTPClient(
@@ -1502,6 +1578,36 @@ func Test_UpdateProjectItem(t *testing.T) {
15021578
expectError: true,
15031579
expectedErrMsg: "fields must contain at least one field update",
15041580
},
1581+
{
1582+
name: "field missing id",
1583+
mockedClient: mock.NewMockedHTTPClient(),
1584+
requestArgs: map[string]any{
1585+
"owner": "octo-org",
1586+
"owner_type": "org",
1587+
"project_number": float64(1),
1588+
"item_id": float64(1),
1589+
"fields": []any{
1590+
map[string]any{"value": "X"},
1591+
},
1592+
},
1593+
expectError: true,
1594+
expectedErrMsg: "each field update must include an 'id'",
1595+
},
1596+
{
1597+
name: "field missing value",
1598+
mockedClient: mock.NewMockedHTTPClient(),
1599+
requestArgs: map[string]any{
1600+
"owner": "octo-org",
1601+
"owner_type": "org",
1602+
"project_number": float64(1),
1603+
"item_id": float64(1),
1604+
"fields": []any{
1605+
map[string]any{"id": float64(55)},
1606+
},
1607+
},
1608+
expectError: true,
1609+
expectedErrMsg: "each field update must include a 'value'",
1610+
},
15051611
}
15061612

15071613
for _, tc := range tests {

0 commit comments

Comments
 (0)