diff --git a/README.md b/README.md index fd45e12..3e6cc29 100644 --- a/README.md +++ b/README.md @@ -429,6 +429,10 @@ gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: Search feature # Create a PR (from a fork) gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: New feature" --head your_username/forgeplus:feature/my-feature --base master +# The fork form auto-resolves GitLink fork metadata +# so you do not need to pass merge_user_login or fork_project_id yourself +gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "fix: CLI bug" --head your_username/forgeplus:fix/bug --base master + # View a PR gitlink-cli pr +view --owner Gitlink --repo forgeplus -i 42 diff --git a/README.zh-CN.md b/README.zh-CN.md index 9bbfda5..2ef6256 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -439,6 +439,10 @@ gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: 搜索功能" # 创建 PR(从 Fork 仓库) gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: 新功能" --head your_username/forgeplus:feature/my-feature --base master +# Fork 写法会自动补齐 GitLink 所需的 fork 元数据 +# 不需要再手动传 merge_user_login 或 fork_project_id +gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "fix: CLI bug" --head your_username/forgeplus:fix/bug --base master + # 查看 PR gitlink-cli pr +view --owner Gitlink --repo forgeplus -i 42 diff --git a/doc/changes/pr-create-fork-support.md b/doc/changes/pr-create-fork-support.md new file mode 100644 index 0000000..cef805f --- /dev/null +++ b/doc/changes/pr-create-fork-support.md @@ -0,0 +1,30 @@ +# PR Create Fork Support + +## Summary + +This change makes `gitlink-cli pr +create` work with the fork syntax already documented in the README: + +```bash +gitlink-cli pr +create --owner Gitlink --repo forgeplus \ + -t "feat: New feature" \ + --head your_username/forgeplus:feature/my-feature \ + --base master +``` + +## What changed + +- Parse `owner/repo:branch` fork heads in `pr +create` +- Auto-resolve the fork repository metadata required by GitLink: + - `merge_user_login` + - `merge_project_identifier` + - `fork_project_id` +- Auto-fill compare counts when available so the request matches GitLink's real PR create flow more closely +- Keep same-repo PR creation behavior unchanged + +## Validation + +- `go test ./...` +- Unit tests for: + - same-repo PR creation payload + - fork PR creation payload + - invalid fork head syntax diff --git a/shortcuts/pr/pr.go b/shortcuts/pr/pr.go index 0837ba6..4983f1b 100644 --- a/shortcuts/pr/pr.go +++ b/shortcuts/pr/pr.go @@ -1,6 +1,7 @@ package pr import ( + "encoding/base64" "fmt" "net/url" "strings" @@ -55,13 +56,9 @@ func Shortcuts() []*common.Shortcut { if base == "" { base = "master" } - payload := map[string]interface{}{ - "title": title, - "head": head, - "base": base, - } - if b := ctx.Arg("body"); b != "" { - payload["body"] = b + payload, err := buildCreatePRPayload(ctx, title, head, base, ctx.Arg("body")) + if err != nil { + return err } env, err := ctx.CallAPI("POST", ctx.RepoPath()+"/pulls", payload) if err != nil { @@ -551,3 +548,146 @@ func numberField(m map[string]interface{}, key string) (float64, bool) { return 0, false } } + +type prHeadSpec struct { + Branch string + ForkOwner string + ForkRepo string + IsFork bool + CompareHead string +} + +func buildCreatePRPayload(ctx *common.RuntimeContext, title, head, base, body string) (map[string]interface{}, error) { + spec, err := parsePRHead(head) + if err != nil { + return nil, err + } + + payload := map[string]interface{}{ + "title": title, + "head": spec.Branch, + "base": base, + "assigned_to_id": "", + "fixed_version_id": "", + "issue_tag_ids": []string{}, + "reviewer_ids": []string{}, + "receivers_login": []string{}, + "priority_id": "2", + "is_original": spec.IsFork, + } + if body != "" { + payload["body"] = body + } + + if spec.IsFork { + repoInfo, err := fetchProjectInfo(ctx, spec.ForkOwner, spec.ForkRepo) + if err != nil { + return nil, err + } + projectID, err := extractFloatField(repoInfo, "project_id", "id") + if err != nil { + return nil, fmt.Errorf("resolve fork project id: %w", err) + } + identifier, err := extractStringField(repoInfo, "project_identifier", "identifier") + if err != nil { + return nil, fmt.Errorf("resolve fork project identifier: %w", err) + } + payload["merge_user_login"] = spec.ForkOwner + payload["merge_project_identifier"] = identifier + payload["fork_project_id"] = int(projectID) + } + + compareCounts, err := fetchPRCompareCounts(ctx, spec.CompareHead, base) + if err == nil { + if commits, ok := compareCounts["commits_count"]; ok { + payload["commits_count"] = commits + } + if files, ok := compareCounts["files_count"]; ok { + payload["files_count"] = files + } + } + + return payload, nil +} + +func parsePRHead(head string) (*prHeadSpec, error) { + if head == "" { + return nil, fmt.Errorf("source branch cannot be empty") + } + if !strings.Contains(head, ":") { + return &prHeadSpec{ + Branch: head, + IsFork: false, + CompareHead: head, + }, nil + } + + parts := strings.SplitN(head, ":", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return nil, fmt.Errorf("invalid --head %q, expected owner/repo:branch", head) + } + + repoParts := strings.Split(parts[0], "/") + if len(repoParts) != 2 || repoParts[0] == "" || repoParts[1] == "" { + return nil, fmt.Errorf("invalid --head %q, expected owner/repo:branch", head) + } + + return &prHeadSpec{ + Branch: parts[1], + ForkOwner: repoParts[0], + ForkRepo: repoParts[1], + IsFork: true, + CompareHead: repoParts[0] + ":" + parts[1], + }, nil +} + +func fetchProjectInfo(ctx *common.RuntimeContext, owner, repo string) (map[string]interface{}, error) { + env, err := ctx.CallAPI("GET", fmt.Sprintf("/%s/%s", owner, repo), nil) + if err != nil { + return nil, fmt.Errorf("fetch project info: %w", err) + } + data, ok := env.Data.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("unexpected project info response format") + } + return data, nil +} + +func fetchPRCompareCounts(ctx *common.RuntimeContext, head, base string) (map[string]int, error) { + encodedHead := base64.RawURLEncoding.EncodeToString([]byte(head)) + encodedBase := base64.RawURLEncoding.EncodeToString([]byte(base)) + env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/compare/%s...%s", ctx.RepoPath(), encodedHead, encodedBase), nil) + if err != nil { + return nil, err + } + data, ok := env.Data.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("unexpected compare response format") + } + result := map[string]int{} + if v, ok := data["commits_count"].(float64); ok { + result["commits_count"] = int(v) + } + if v, ok := data["files_count"].(float64); ok { + result["files_count"] = int(v) + } + return result, nil +} + +func extractFloatField(data map[string]interface{}, keys ...string) (float64, error) { + for _, key := range keys { + if v, ok := data[key].(float64); ok { + return v, nil + } + } + return 0, fmt.Errorf("missing numeric field %v", keys) +} + +func extractStringField(data map[string]interface{}, keys ...string) (string, error) { + for _, key := range keys { + if v, ok := data[key].(string); ok && v != "" { + return v, nil + } + } + return "", fmt.Errorf("missing string field %v", keys) +} diff --git a/shortcuts/pr/pr_test.go b/shortcuts/pr/pr_test.go index 75bffde..438f6ca 100644 --- a/shortcuts/pr/pr_test.go +++ b/shortcuts/pr/pr_test.go @@ -1,6 +1,7 @@ package pr import ( + "encoding/base64" "encoding/json" "fmt" "net/http" @@ -54,6 +55,135 @@ func TestPRCommentPostsToCorrectIssueJournal(t *testing.T) { assertEqual(t, journalPayload["notes"], "LGTM, looks good!") } +func TestPRCreateSameRepoBranchUsesSimplePayload(t *testing.T) { + var payload map[string]interface{} + encodedHead := base64.RawURLEncoding.EncodeToString([]byte("feature/search")) + encodedBase := base64.RawURLEncoding.EncodeToString([]byte("master")) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/owner/repo/compare/"+encodedHead+"..."+encodedBase+".json": + writeJSON(t, w, map[string]interface{}{ + "commits_count": float64(2), + "files_count": float64(5), + }) + case r.Method == "POST" && r.URL.Path == "/owner/repo/pulls.json": + payload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{"status": float64(0), "pull_request_number": float64(22)}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer server.Close() + + err := runPRShortcut(t, server, "create", map[string]string{ + "title": "feat: search", + "head": "feature/search", + "base": "master", + "body": "Add search support", + }) + if err != nil { + t.Fatalf("create shortcut failed: %v", err) + } + + assertEqual(t, payload["head"], "feature/search") + assertEqual(t, payload["base"], "master") + assertEqual(t, payload["is_original"], false) + assertEqual(t, payload["commits_count"], float64(2)) + assertEqual(t, payload["files_count"], float64(5)) + if _, ok := payload["merge_user_login"]; ok { + t.Fatal("same-repo PR should not include merge_user_login") + } +} + +func TestPRCreateForkBranchAddsGitLinkForkFields(t *testing.T) { + var payload map[string]interface{} + encodedHead := base64.RawURLEncoding.EncodeToString([]byte("alice:feature/JIRA-123/fix")) + encodedBase := base64.RawURLEncoding.EncodeToString([]byte("master")) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/alice/fork-repo.json": + writeJSON(t, w, map[string]interface{}{ + "project_id": float64(1546652), + "project_identifier": "fork-repo", + }) + case r.Method == "GET" && r.URL.Path == "/owner/repo/compare/"+encodedHead+"..."+encodedBase+".json": + writeJSON(t, w, map[string]interface{}{ + "commits_count": float64(3), + "files_count": float64(7), + }) + case r.Method == "POST" && r.URL.Path == "/owner/repo/pulls.json": + payload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{"status": float64(0), "pull_request_number": float64(23)}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer server.Close() + + err := runPRShortcut(t, server, "create", map[string]string{ + "title": "feat: fork support", + "head": "alice/fork-repo:feature/JIRA-123/fix", + "base": "master", + }) + if err != nil { + t.Fatalf("create shortcut failed: %v", err) + } + + assertEqual(t, payload["head"], "feature/JIRA-123/fix") + assertEqual(t, payload["is_original"], true) + assertEqual(t, payload["merge_user_login"], "alice") + assertEqual(t, payload["merge_project_identifier"], "fork-repo") + assertEqual(t, payload["fork_project_id"], float64(1546652)) + assertEqual(t, payload["commits_count"], float64(3)) + assertEqual(t, payload["files_count"], float64(7)) +} + +func TestFetchPRCompareCountsUsesURLSafeBase64(t *testing.T) { + encodedHead := base64.RawURLEncoding.EncodeToString([]byte("alice:feature/fork")) + encodedBase := base64.RawURLEncoding.EncodeToString([]byte("master")) + + if encodedHead != "YWxpY2U6ZmVhdHVyZS9mb3Jr" { + t.Fatalf("unexpected encoded head: %s", encodedHead) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wantPath := "/owner/repo/compare/" + encodedHead + "..." + encodedBase + ".json" + if r.Method != "GET" || r.URL.Path != wantPath { + t.Fatalf("unexpected request: %s %s, want GET %s", r.Method, r.URL.Path, wantPath) + } + writeJSON(t, w, map[string]interface{}{ + "commits_count": float64(4), + "files_count": float64(9), + }) + })) + defer server.Close() + + ctx := &common.RuntimeContext{ + Client: &client.Client{ + HTTP: server.Client(), + BaseURL: server.URL, + }, + Owner: "owner", + Repo: "repo", + } + + counts, err := fetchPRCompareCounts(ctx, "alice:feature/fork", "master") + if err != nil { + t.Fatalf("fetchPRCompareCounts failed: %v", err) + } + + assertEqual(t, counts["commits_count"], 4) + assertEqual(t, counts["files_count"], 9) +} + +func TestParsePRHeadRejectsInvalidForkSyntax(t *testing.T) { + _, err := parsePRHead("alice:feature/fork") + if err == nil { + t.Fatal("expected invalid head syntax to fail") + } +} + func TestPRCommentFailsWhenPRNotFound(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) @@ -328,15 +458,18 @@ func TestPRListStateAllOmitsStatus(t *testing.T) { func TestPRCreate(t *testing.T) { var payload map[string]interface{} + encodedHead := base64.RawURLEncoding.EncodeToString([]byte("feature/x")) + encodedBase := base64.RawURLEncoding.EncodeToString([]byte("master")) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "POST" { - t.Fatalf("expected POST, got %s", r.Method) + switch { + case r.Method == "GET" && r.URL.Path == "/owner/repo/compare/"+encodedHead+"..."+encodedBase+".json": + writeJSON(t, w, map[string]interface{}{"commits_count": float64(1), "files_count": float64(2)}) + case r.Method == "POST" && r.URL.Path == "/owner/repo/pulls.json": + payload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{"id": float64(42), "title": "feat: new"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } - if r.URL.Path != "/owner/repo/pulls.json" { - t.Fatalf("unexpected path: %s", r.URL.Path) - } - payload = decodeJSON(t, r) - writeJSON(t, w, map[string]interface{}{"id": float64(42), "title": "feat: new"}) })) defer server.Close() @@ -357,9 +490,18 @@ func TestPRCreate(t *testing.T) { func TestPRCreateNoBody(t *testing.T) { var payload map[string]interface{} + encodedHead := base64.RawURLEncoding.EncodeToString([]byte("feature/y")) + encodedBase := base64.RawURLEncoding.EncodeToString([]byte("master")) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - payload = decodeJSON(t, r) - writeJSON(t, w, map[string]interface{}{"id": float64(43), "title": "feat: nob"}) + switch { + case r.Method == "GET" && r.URL.Path == "/owner/repo/compare/"+encodedHead+"..."+encodedBase+".json": + writeJSON(t, w, map[string]interface{}{"commits_count": float64(0), "files_count": float64(0)}) + case r.Method == "POST" && r.URL.Path == "/owner/repo/pulls.json": + payload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{"id": float64(43), "title": "feat: nob"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } })) defer server.Close()