Skip to content

Commit 6ed0475

Browse files
committed
Add tests and tool snaps
1 parent b221d70 commit 6ed0475

File tree

2 files changed

+282
-6
lines changed

2 files changed

+282
-6
lines changed

pkg/github/__toolsnaps__/update_project_item.snap

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@
66
"description": "Update a specific Project item for a user or org",
77
"inputSchema": {
88
"properties": {
9-
"fields": {
10-
"description": "A list of field updates to apply.",
11-
"type": "array"
12-
},
139
"item_id": {
14-
"description": "The numeric ID of the project item to update (not the issue or pull request ID).",
10+
"description": "The numeric ID of the issue or pull request to add to the project.",
1511
"type": "number"
1612
},
13+
"new_field": {
14+
"description": "Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set this to null. Example: {\"id\": 123456, \"value\": \"New Value\"}",
15+
"properties": {},
16+
"type": "object"
17+
},
1718
"owner": {
1819
"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.",
1920
"type": "string"
@@ -36,7 +37,7 @@
3637
"owner",
3738
"project_number",
3839
"item_id",
39-
"fields"
40+
"new_field"
4041
],
4142
"type": "object"
4243
},

pkg/github/projects_test.go

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,281 @@ func Test_AddProjectItem(t *testing.T) {
11621162
}
11631163
}
11641164

1165+
func Test_UpdateProjectItem(t *testing.T) {
1166+
mockClient := gh.NewClient(nil)
1167+
tool, _ := UpdateProjectItem(stubGetClientFn(mockClient), translations.NullTranslationHelper)
1168+
require.NoError(t, toolsnaps.Test(tool.Name, tool))
1169+
1170+
assert.Equal(t, "update_project_item", tool.Name)
1171+
assert.NotEmpty(t, tool.Description)
1172+
assert.Contains(t, tool.InputSchema.Properties, "owner_type")
1173+
assert.Contains(t, tool.InputSchema.Properties, "owner")
1174+
assert.Contains(t, tool.InputSchema.Properties, "project_number")
1175+
assert.Contains(t, tool.InputSchema.Properties, "item_id")
1176+
assert.Contains(t, tool.InputSchema.Properties, "new_field")
1177+
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner_type", "owner", "project_number", "item_id", "new_field"})
1178+
1179+
orgUpdatedItem := map[string]any{
1180+
"id": 801,
1181+
"content_type": "Issue",
1182+
}
1183+
userUpdatedItem := map[string]any{
1184+
"id": 802,
1185+
"content_type": "PullRequest",
1186+
}
1187+
1188+
tests := []struct {
1189+
name string
1190+
mockedClient *http.Client
1191+
requestArgs map[string]any
1192+
expectError bool
1193+
expectedErrMsg string
1194+
expectedID int
1195+
}{
1196+
{
1197+
name: "success organization update",
1198+
mockedClient: mock.NewMockedHTTPClient(
1199+
mock.WithRequestMatchHandler(
1200+
mock.EndpointPattern{Pattern: "/orgs/{org}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch},
1201+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1202+
body, err := io.ReadAll(r.Body)
1203+
assert.NoError(t, err)
1204+
var payload struct {
1205+
Fields []struct {
1206+
ID int `json:"id"`
1207+
Value interface{} `json:"value"`
1208+
} `json:"fields"`
1209+
}
1210+
assert.NoError(t, json.Unmarshal(body, &payload))
1211+
require.Len(t, payload.Fields, 1)
1212+
assert.Equal(t, 101, payload.Fields[0].ID)
1213+
assert.Equal(t, "Done", payload.Fields[0].Value)
1214+
w.WriteHeader(http.StatusOK)
1215+
_, _ = w.Write(mock.MustMarshal(orgUpdatedItem))
1216+
}),
1217+
),
1218+
),
1219+
requestArgs: map[string]any{
1220+
"owner": "octo-org",
1221+
"owner_type": "org",
1222+
"project_number": float64(1001),
1223+
"item_id": float64(5555),
1224+
"new_field": map[string]any{
1225+
"id": float64(101),
1226+
"value": "Done",
1227+
},
1228+
},
1229+
expectedID: 801,
1230+
},
1231+
{
1232+
name: "success user update",
1233+
mockedClient: mock.NewMockedHTTPClient(
1234+
mock.WithRequestMatchHandler(
1235+
mock.EndpointPattern{Pattern: "/users/{user}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch},
1236+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1237+
body, err := io.ReadAll(r.Body)
1238+
assert.NoError(t, err)
1239+
var payload struct {
1240+
Fields []struct {
1241+
ID int `json:"id"`
1242+
Value interface{} `json:"value"`
1243+
} `json:"fields"`
1244+
}
1245+
assert.NoError(t, json.Unmarshal(body, &payload))
1246+
require.Len(t, payload.Fields, 1)
1247+
assert.Equal(t, 202, payload.Fields[0].ID)
1248+
assert.Equal(t, 42.0, payload.Fields[0].Value) // number value example
1249+
w.WriteHeader(http.StatusCreated)
1250+
_, _ = w.Write(mock.MustMarshal(userUpdatedItem))
1251+
}),
1252+
),
1253+
),
1254+
requestArgs: map[string]any{
1255+
"owner": "octocat",
1256+
"owner_type": "user",
1257+
"project_number": float64(2002),
1258+
"item_id": float64(6666),
1259+
"new_field": map[string]any{
1260+
"id": float64(202),
1261+
"value": float64(42),
1262+
},
1263+
},
1264+
expectedID: 802,
1265+
},
1266+
{
1267+
name: "api error",
1268+
mockedClient: mock.NewMockedHTTPClient(
1269+
mock.WithRequestMatchHandler(
1270+
mock.EndpointPattern{Pattern: "/orgs/{org}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch},
1271+
mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}),
1272+
),
1273+
),
1274+
requestArgs: map[string]any{
1275+
"owner": "octo-org",
1276+
"owner_type": "org",
1277+
"project_number": float64(3003),
1278+
"item_id": float64(7777),
1279+
"new_field": map[string]any{
1280+
"id": float64(303),
1281+
"value": "In Progress",
1282+
},
1283+
},
1284+
expectError: true,
1285+
expectedErrMsg: "failed to add a project item", // implementation uses this message for update failures
1286+
},
1287+
{
1288+
name: "missing owner",
1289+
mockedClient: mock.NewMockedHTTPClient(),
1290+
requestArgs: map[string]any{
1291+
"owner_type": "org",
1292+
"project_number": float64(1),
1293+
"item_id": float64(2),
1294+
"new_field": map[string]any{
1295+
"id": float64(1),
1296+
"value": "X",
1297+
},
1298+
},
1299+
expectError: true,
1300+
},
1301+
{
1302+
name: "missing owner_type",
1303+
mockedClient: mock.NewMockedHTTPClient(),
1304+
requestArgs: map[string]any{
1305+
"owner": "octo-org",
1306+
"project_number": float64(1),
1307+
"item_id": float64(2),
1308+
"new_field": map[string]any{
1309+
"id": float64(1),
1310+
"value": "X",
1311+
},
1312+
},
1313+
expectError: true,
1314+
},
1315+
{
1316+
name: "missing project_number",
1317+
mockedClient: mock.NewMockedHTTPClient(),
1318+
requestArgs: map[string]any{
1319+
"owner": "octo-org",
1320+
"owner_type": "org",
1321+
"item_id": float64(2),
1322+
"new_field": map[string]any{
1323+
"id": float64(1),
1324+
"value": "X",
1325+
},
1326+
},
1327+
expectError: true,
1328+
},
1329+
{
1330+
name: "missing item_id",
1331+
mockedClient: mock.NewMockedHTTPClient(),
1332+
requestArgs: map[string]any{
1333+
"owner": "octo-org",
1334+
"owner_type": "org",
1335+
"project_number": float64(1),
1336+
"new_field": map[string]any{
1337+
"id": float64(1),
1338+
"value": "X",
1339+
},
1340+
},
1341+
expectError: true,
1342+
},
1343+
{
1344+
name: "missing new_field",
1345+
mockedClient: mock.NewMockedHTTPClient(),
1346+
requestArgs: map[string]any{
1347+
"owner": "octo-org",
1348+
"owner_type": "org",
1349+
"project_number": float64(1),
1350+
"item_id": float64(2),
1351+
},
1352+
expectError: true,
1353+
},
1354+
{
1355+
name: "new_field not object",
1356+
mockedClient: mock.NewMockedHTTPClient(),
1357+
requestArgs: map[string]any{
1358+
"owner": "octo-org",
1359+
"owner_type": "org",
1360+
"project_number": float64(1),
1361+
"item_id": float64(2),
1362+
"new_field": "not-an-object",
1363+
},
1364+
expectError: true,
1365+
},
1366+
{
1367+
name: "new_field missing id",
1368+
mockedClient: mock.NewMockedHTTPClient(),
1369+
requestArgs: map[string]any{
1370+
"owner": "octo-org",
1371+
"owner_type": "org",
1372+
"project_number": float64(1),
1373+
"item_id": float64(2),
1374+
"new_field": map[string]any{},
1375+
},
1376+
expectError: true,
1377+
},
1378+
{
1379+
name: "new_field missing value",
1380+
mockedClient: mock.NewMockedHTTPClient(),
1381+
requestArgs: map[string]any{
1382+
"owner": "octo-org",
1383+
"owner_type": "org",
1384+
"project_number": float64(1),
1385+
"item_id": float64(2),
1386+
"new_field": map[string]any{
1387+
"id": float64(9),
1388+
},
1389+
},
1390+
expectError: true,
1391+
},
1392+
}
1393+
1394+
for _, tc := range tests {
1395+
t.Run(tc.name, func(t *testing.T) {
1396+
client := gh.NewClient(tc.mockedClient)
1397+
_, handler := UpdateProjectItem(stubGetClientFn(client), translations.NullTranslationHelper)
1398+
request := createMCPRequest(tc.requestArgs)
1399+
result, err := handler(context.Background(), request)
1400+
1401+
require.NoError(t, err)
1402+
if tc.expectError {
1403+
require.True(t, result.IsError)
1404+
text := getTextResult(t, result).Text
1405+
if tc.expectedErrMsg != "" {
1406+
assert.Contains(t, text, tc.expectedErrMsg)
1407+
}
1408+
switch tc.name {
1409+
case "missing owner":
1410+
assert.Contains(t, text, "missing required parameter: owner")
1411+
case "missing owner_type":
1412+
assert.Contains(t, text, "missing required parameter: owner_type")
1413+
case "missing project_number":
1414+
assert.Contains(t, text, "missing required parameter: project_number")
1415+
case "missing item_id":
1416+
assert.Contains(t, text, "missing required parameter: item_id")
1417+
case "missing new_field":
1418+
assert.Contains(t, text, "missing required parameter: new_field")
1419+
case "new_field not object":
1420+
assert.Contains(t, text, "new_field must be an object")
1421+
case "new_field missing id":
1422+
assert.Contains(t, text, "new_field.id is required")
1423+
case "new_field missing value":
1424+
assert.Contains(t, text, "new_field.value is required")
1425+
}
1426+
return
1427+
}
1428+
1429+
require.False(t, result.IsError)
1430+
textContent := getTextResult(t, result)
1431+
var item map[string]any
1432+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &item))
1433+
if tc.expectedID != 0 {
1434+
assert.Equal(t, float64(tc.expectedID), item["id"])
1435+
}
1436+
})
1437+
}
1438+
}
1439+
11651440
func Test_DeleteProjectItem(t *testing.T) {
11661441
mockClient := gh.NewClient(nil)
11671442
tool, _ := DeleteProjectItem(stubGetClientFn(mockClient), translations.NullTranslationHelper)

0 commit comments

Comments
 (0)