Skip to content

Commit 975b4c8

Browse files
Address PR review feedback
- Extract dual-fetch logic into listProjectsFromBothOwnerTypes helper - Rename addProjectItemWithResolution to addProjectItem (old function removed) - Add GraphQL test coverage for add_project_item using githubv4mock - Tests cover both org/issue and user/pull_request success paths
1 parent 0196e0c commit 975b4c8

File tree

2 files changed

+244
-81
lines changed

2 files changed

+244
-81
lines changed

pkg/github/projects.go

Lines changed: 56 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,7 +1412,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
14121412
return utils.NewToolResultError("item_type must be either 'issue' or 'pull_request'"), nil, nil
14131413
}
14141414

1415-
return addProjectItemWithResolution(ctx, gqlClient, owner, ownerType, projectNumber, itemOwner, itemRepo, itemNumber, itemType)
1415+
return addProjectItem(ctx, gqlClient, owner, ownerType, projectNumber, itemOwner, itemRepo, itemNumber, itemType)
14161416
case projectsMethodUpdateProjectItem:
14171417
itemID, err := RequiredBigInt(args, "item_id")
14181418
if err != nil {
@@ -1472,56 +1472,7 @@ func listProjects(ctx context.Context, client *github.Client, args map[string]an
14721472
// If owner_type not provided, fetch from both user and org
14731473
switch ownerType {
14741474
case "":
1475-
// Fetch user projects
1476-
userProjects, userResp, userErr := client.Projects.ListUserProjects(ctx, owner, opts)
1477-
var userProjectsList []MinimalProject
1478-
if userErr == nil && userResp.StatusCode == http.StatusOK {
1479-
for _, project := range userProjects {
1480-
mp := convertToMinimalProject(project)
1481-
mp.OwnerType = "user" // Add owner type for clarity
1482-
userProjectsList = append(userProjectsList, *mp)
1483-
}
1484-
_ = userResp.Body.Close()
1485-
}
1486-
1487-
// Fetch org projects
1488-
orgProjects, orgResp, orgErr := client.Projects.ListOrganizationProjects(ctx, owner, opts)
1489-
var orgProjectsList []MinimalProject
1490-
if orgErr == nil && orgResp.StatusCode == http.StatusOK {
1491-
for _, project := range orgProjects {
1492-
mp := convertToMinimalProject(project)
1493-
mp.OwnerType = "org" // Add owner type for clarity
1494-
orgProjectsList = append(orgProjectsList, *mp)
1495-
}
1496-
resp = orgResp // Use org response for pagination info
1497-
} else if userResp != nil {
1498-
resp = userResp // Fallback to user response
1499-
}
1500-
1501-
// Combine results
1502-
minimalProjects = append(minimalProjects, userProjectsList...)
1503-
minimalProjects = append(minimalProjects, orgProjectsList...)
1504-
1505-
// If both failed, return error
1506-
if (userErr != nil || userResp.StatusCode != http.StatusOK) && (orgErr != nil || orgResp.StatusCode != http.StatusOK) {
1507-
return utils.NewToolResultError(fmt.Sprintf("failed to list projects for owner '%s': not found as user or organization", owner)), nil, nil
1508-
}
1509-
1510-
response := map[string]any{
1511-
"projects": minimalProjects,
1512-
"note": "Results include both user and org projects. Each project includes 'owner_type' field. Pagination is limited when owner_type is not specified - specify 'owner_type' for full pagination support.",
1513-
}
1514-
if resp != nil {
1515-
response["pageInfo"] = buildPageInfo(resp)
1516-
defer func() { _ = resp.Body.Close() }()
1517-
}
1518-
1519-
r, err := json.Marshal(response)
1520-
if err != nil {
1521-
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
1522-
}
1523-
return utils.NewToolResultText(string(r)), nil, nil
1524-
1475+
return listProjectsFromBothOwnerTypes(ctx, client, owner, opts)
15251476
case "org":
15261477
projects, resp, err = client.Projects.ListOrganizationProjects(ctx, owner, opts)
15271478
if err != nil {
@@ -1568,6 +1519,58 @@ func listProjects(ctx context.Context, client *github.Client, args map[string]an
15681519
return nil, nil, fmt.Errorf("unexpected state in listProjects")
15691520
}
15701521

1522+
// listProjectsFromBothOwnerTypes fetches projects from both user and org endpoints
1523+
// when owner_type is not specified, combining the results with owner_type labels.
1524+
func listProjectsFromBothOwnerTypes(ctx context.Context, client *github.Client, owner string, opts *github.ListProjectsOptions) (*mcp.CallToolResult, any, error) {
1525+
var minimalProjects []MinimalProject
1526+
var resp *github.Response
1527+
1528+
// Fetch user projects
1529+
userProjects, userResp, userErr := client.Projects.ListUserProjects(ctx, owner, opts)
1530+
if userErr == nil && userResp.StatusCode == http.StatusOK {
1531+
for _, project := range userProjects {
1532+
mp := convertToMinimalProject(project)
1533+
mp.OwnerType = "user"
1534+
minimalProjects = append(minimalProjects, *mp)
1535+
}
1536+
_ = userResp.Body.Close()
1537+
}
1538+
1539+
// Fetch org projects
1540+
orgProjects, orgResp, orgErr := client.Projects.ListOrganizationProjects(ctx, owner, opts)
1541+
if orgErr == nil && orgResp.StatusCode == http.StatusOK {
1542+
for _, project := range orgProjects {
1543+
mp := convertToMinimalProject(project)
1544+
mp.OwnerType = "org"
1545+
minimalProjects = append(minimalProjects, *mp)
1546+
}
1547+
resp = orgResp // Use org response for pagination info
1548+
} else if userResp != nil {
1549+
resp = userResp // Fallback to user response
1550+
}
1551+
1552+
// If both failed, return error
1553+
if (userErr != nil || userResp == nil || userResp.StatusCode != http.StatusOK) &&
1554+
(orgErr != nil || orgResp == nil || orgResp.StatusCode != http.StatusOK) {
1555+
return utils.NewToolResultError(fmt.Sprintf("failed to list projects for owner '%s': not found as user or organization", owner)), nil, nil
1556+
}
1557+
1558+
response := map[string]any{
1559+
"projects": minimalProjects,
1560+
"note": "Results include both user and org projects. Each project includes 'owner_type' field. Pagination is limited when owner_type is not specified - specify 'owner_type' for full pagination support.",
1561+
}
1562+
if resp != nil {
1563+
response["pageInfo"] = buildPageInfo(resp)
1564+
defer func() { _ = resp.Body.Close() }()
1565+
}
1566+
1567+
r, err := json.Marshal(response)
1568+
if err != nil {
1569+
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
1570+
}
1571+
return utils.NewToolResultText(string(r)), nil, nil
1572+
}
1573+
15711574
func listProjectFields(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) {
15721575
projectNumber, err := RequiredInt(args, "project_number")
15731576
if err != nil {
@@ -1861,8 +1864,8 @@ func deleteProjectItem(ctx context.Context, client *github.Client, owner, ownerT
18611864
return utils.NewToolResultText("project item successfully deleted"), nil, nil
18621865
}
18631866

1864-
// addProjectItemWithResolution adds an item to a project by resolving the issue/PR number to a node ID
1865-
func addProjectItemWithResolution(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, itemOwner, itemRepo string, itemNumber int, itemType string) (*mcp.CallToolResult, any, error) {
1867+
// addProjectItem adds an item to a project by resolving the issue/PR number to a node ID
1868+
func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, itemOwner, itemRepo string, itemNumber int, itemType string) (*mcp.CallToolResult, any, error) {
18661869
if itemType != "issue" && itemType != "pull_request" {
18671870
return utils.NewToolResultError("item_type must be either 'issue' or 'pull_request'"), nil, nil
18681871
}

0 commit comments

Comments
 (0)