feat(pr): add pr +edit to update an open pull request
Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
This commit is contained in:
parent
9749a4c832
commit
6e71ff9941
|
|
@ -0,0 +1,18 @@
|
|||
# PR Edit Shortcut
|
||||
|
||||
This change adds a `pr +edit` verb, bringing pull requests in line with `issue`, `label`, `release`, `milestone`, and `webhook`, which already expose an edit/update command.
|
||||
|
||||
## Commands
|
||||
|
||||
- Add `pr +edit` for `PUT /api/{owner}/{repo}/pulls/{index}.json`.
|
||||
|
||||
## Behavior
|
||||
|
||||
- `pr +edit` first GETs the current PR (the same `/pulls/{index}` path used by `pr +view`), then merges the requested changes onto its current values before the PUT. The update endpoint requires `title`, `body`, `head`, `base`, `issue_tag_ids`, and `receivers_login` together, so unspecified flags fall back to the existing values to avoid clobbering them.
|
||||
- Flags: `--title`, `--body`, `--base`, `--head`, and `--tag-ids` (comma-separated). At least one is required.
|
||||
- `--tag-ids` replaces the tag set when given; otherwise the PR's current tag IDs are preserved.
|
||||
- PR detail fields are read defensively across the top-level, `pull_request`, and `issue` scopes, matching how the health checks parse the same endpoint.
|
||||
|
||||
## Verification
|
||||
|
||||
- Unit tests cover field preservation, tag-id override, the nested `pull_request` response shape, the missing-field guard, and the HTTP error path.
|
||||
|
|
@ -56,6 +56,7 @@
|
|||
"cmd.pr.comment.short": "Add a comment to a pull request",
|
||||
"cmd.pr.create.short": "Create a pull request",
|
||||
"cmd.pr.diff.short": "Show diff for a pull request",
|
||||
"cmd.pr.edit.short": "Edit a pull request while preserving unspecified fields",
|
||||
"cmd.pr.files.short": "List changed files in a pull request",
|
||||
"cmd.pr.list.short": "List pull requests",
|
||||
"cmd.pr.merge.short": "Merge a pull request",
|
||||
|
|
@ -194,6 +195,7 @@
|
|||
"flag.pr.reviewer_id": "Reviewer user ID",
|
||||
"flag.pr.state": "Filter: open, merged, closed",
|
||||
"flag.pr.tag_id": "Issue tag ID",
|
||||
"flag.pr.tag_ids": "Comma-separated issue tag IDs",
|
||||
"flag.pr.title": "PR title",
|
||||
"flag.pr.version_id": "Patchset version ID",
|
||||
"flag.profile.end_time": "End time (Unix timestamp)",
|
||||
|
|
|
|||
|
|
@ -56,6 +56,7 @@
|
|||
"cmd.pr.comment.short": "给拉取请求添加评论",
|
||||
"cmd.pr.create.short": "创建拉取请求",
|
||||
"cmd.pr.diff.short": "显示拉取请求 diff",
|
||||
"cmd.pr.edit.short": "编辑拉取请求并保留未指定字段",
|
||||
"cmd.pr.files.short": "列出拉取请求中的变更文件",
|
||||
"cmd.pr.list.short": "列出拉取请求",
|
||||
"cmd.pr.merge.short": "合并拉取请求",
|
||||
|
|
@ -194,6 +195,7 @@
|
|||
"flag.pr.reviewer_id": "评审人用户 ID",
|
||||
"flag.pr.state": "筛选：open、merged、closed",
|
||||
"flag.pr.tag_id": "议题标签 ID",
|
||||
"flag.pr.tag_ids": "逗号分隔的议题标签 ID",
|
||||
"flag.pr.title": "PR 标题",
|
||||
"flag.pr.version_id": "补丁集版本 ID",
|
||||
"flag.profile.end_time": "结束时间（Unix 时间戳）",
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
package pr
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/gitlink-org/gitlink-cli/internal/i18n"
|
||||
|
|
@ -144,6 +146,52 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut {
|
|||
return ctx.Output(env)
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "edit",
|
||||
Description: tr.T("cmd.pr.edit.short"),
|
||||
Flags: []common.Flag{
|
||||
{Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true},
|
||||
{Name: "title", Short: "t", Usage: tr.T("flag.pr.title")},
|
||||
{Name: "body", Short: "b", Usage: tr.T("flag.pr.body")},
|
||||
{Name: "base", Usage: tr.T("flag.pr.base")},
|
||||
{Name: "head", Usage: tr.T("flag.pr.head")},
|
||||
{Name: "tag-ids", Usage: tr.T("flag.pr.tag_ids")},
|
||||
},
|
||||
Run: func(ctx *common.RuntimeContext) error {
|
||||
if err := ctx.ResolveOwnerRepo(); err != nil {
|
||||
return err
|
||||
}
|
||||
id, err := ctx.RequireArg("id")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if ctx.Arg("title") == "" && ctx.Arg("body") == "" && ctx.Arg("base") == "" &&
|
||||
ctx.Arg("head") == "" && ctx.Arg("tag-ids") == "" {
|
||||
return fmt.Errorf("at least one of --title, --body, --base, --head, or --tag-ids is required")
|
||||
}
|
||||
|
||||
// The update endpoint requires title, body, head and base together,
|
||||
// so merge the requested changes onto the PR's current values to
|
||||
// avoid clobbering fields the caller did not pass.
|
||||
path := fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id)
|
||||
current, err := ctx.CallAPI("GET", path, nil)
|
||||
if err != nil {
|
||||
return fmt.Errorf("fetch PR: %w", err)
|
||||
}
|
||||
payload := pullRequestEditPayload(ctx, current)
|
||||
if payload["title"] == "" {
|
||||
return fmt.Errorf("could not resolve PR title for #%s; pass --title explicitly", id)
|
||||
}
|
||||
if payload["head"] == "" || payload["base"] == "" {
|
||||
return fmt.Errorf("could not resolve PR head/base branch for #%s; pass --head and --base explicitly", id)
|
||||
}
|
||||
env, err := ctx.CallAPI("PUT", path, payload)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return ctx.Output(env)
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "merge",
|
||||
Description: tr.T("cmd.pr.merge.short"),
|
||||
|
|
@ -470,6 +518,100 @@ func extractIssueID(env *output.Envelope) (int64, error) {
|
|||
return int64(idFloat), nil
|
||||
}
|
||||
|
||||
// pullRequestEditPayload merges the caller's flags over the PR's current values.
|
||||
// The PUT endpoint requires every field, so unspecified flags fall back to the
|
||||
// values read from the prior GET to avoid wiping them.
|
||||
func pullRequestEditPayload(ctx *common.RuntimeContext, current *output.Envelope) map[string]interface{} {
|
||||
data, _ := current.Data.(map[string]interface{})
|
||||
return map[string]interface{}{
|
||||
"title": firstNonEmpty(ctx.Arg("title"), pullRequestField(data, "title", "subject", "name")),
|
||||
"body": firstNonEmpty(ctx.Arg("body"), pullRequestField(data, "body", "description")),
|
||||
"head": firstNonEmpty(ctx.Arg("head"), pullRequestField(data, "head", "pull_request_head")),
|
||||
"base": firstNonEmpty(ctx.Arg("base"), pullRequestField(data, "base", "pull_request_base")),
|
||||
"issue_tag_ids": pullRequestEditTagIDs(ctx, data),
|
||||
"receivers_login": []string{},
|
||||
}
|
||||
}
|
||||
|
||||
// pullRequestEditTagIDs uses the explicitly requested --tag-ids when present,
|
||||
// otherwise preserves the tag IDs already attached to the PR.
|
||||
func pullRequestEditTagIDs(ctx *common.RuntimeContext, data map[string]interface{}) []string {
|
||||
if raw := ctx.Arg("tag-ids"); raw != "" {
|
||||
ids := make([]string, 0)
|
||||
for _, part := range strings.Split(raw, ",") {
|
||||
if trimmed := strings.TrimSpace(part); trimmed != "" {
|
||||
ids = append(ids, trimmed)
|
||||
}
|
||||
}
|
||||
return ids
|
||||
}
|
||||
for _, scope := range pullRequestFieldScopes(data) {
|
||||
tags, ok := scope["issue_tags"].([]interface{})
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
ids := make([]string, 0, len(tags))
|
||||
for _, raw := range tags {
|
||||
tag, ok := raw.(map[string]interface{})
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if id := tagIDString(tag["id"]); id != "" {
|
||||
ids = append(ids, id)
|
||||
}
|
||||
}
|
||||
return ids
|
||||
}
|
||||
return []string{}
|
||||
}
|
||||
|
||||
// pullRequestField reads the first non-empty string among the given keys across
|
||||
// the possible response scopes. The detail endpoint sometimes nests PR fields
|
||||
// under pull_request/issue rather than at the top level.
|
||||
func pullRequestField(data map[string]interface{}, keys ...string) string {
|
||||
for _, scope := range pullRequestFieldScopes(data) {
|
||||
for _, key := range keys {
|
||||
if v := stringField(scope, key); v != "" {
|
||||
return v
|
||||
}
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func pullRequestFieldScopes(data map[string]interface{}) []map[string]interface{} {
|
||||
scopes := []map[string]interface{}{data}
|
||||
if pr, ok := data["pull_request"].(map[string]interface{}); ok {
|
||||
scopes = append(scopes, pr)
|
||||
}
|
||||
if issue, ok := data["issue"].(map[string]interface{}); ok {
|
||||
scopes = append(scopes, issue)
|
||||
}
|
||||
return scopes
|
||||
}
|
||||
|
||||
func firstNonEmpty(values ...string) string {
|
||||
for _, v := range values {
|
||||
if v != "" {
|
||||
return v
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func tagIDString(v interface{}) string {
|
||||
switch id := v.(type) {
|
||||
case string:
|
||||
return id
|
||||
case float64:
|
||||
return strconv.FormatInt(int64(id), 10)
|
||||
case json.Number:
|
||||
return id.String()
|
||||
default:
|
||||
return ""
|
||||
}
|
||||
}
|
||||
|
||||
func enrichPullRequestClosedAt(ctx *common.RuntimeContext, env *output.Envelope) error {
|
||||
data, ok := env.Data.(map[string]interface{})
|
||||
if !ok {
|
||||
|
|
|
|||
|
|
@ -351,6 +351,151 @@ func TestPRDiff(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// --- edit ---
|
||||
|
||||
func editServer(t *testing.T, current map[string]interface{}, put *map[string]interface{}) *httptest.Server {
|
||||
t.Helper()
|
||||
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/owner/repo/pulls/42.json" {
|
||||
t.Fatalf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
switch r.Method {
|
||||
case "GET":
|
||||
writeJSON(t, w, current)
|
||||
case "PUT":
|
||||
*put = decodeJSON(t, r)
|
||||
writeJSON(t, w, map[string]interface{}{})
|
||||
default:
|
||||
t.Fatalf("unexpected method: %s", r.Method)
|
||||
}
|
||||
}))
|
||||
}
|
||||
|
||||
func TestPREditPreservesUnspecifiedFields(t *testing.T) {
|
||||
var put map[string]interface{}
|
||||
server := editServer(t, map[string]interface{}{
|
||||
"id": float64(42),
|
||||
"title": "original title",
|
||||
"body": "original body",
|
||||
"head": "feature/x",
|
||||
"base": "master",
|
||||
"issue": map[string]interface{}{
|
||||
"id": float64(105),
|
||||
"issue_tags": []interface{}{
|
||||
map[string]interface{}{"id": float64(7), "name": "bug"},
|
||||
map[string]interface{}{"id": float64(9), "name": "urgent"},
|
||||
},
|
||||
},
|
||||
}, &put)
|
||||
defer server.Close()
|
||||
|
||||
err := runPRShortcut(t, server, "edit", map[string]string{
|
||||
"id": "42",
|
||||
"body": "updated body",
|
||||
"base": "develop",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("edit failed: %v", err)
|
||||
}
|
||||
if put == nil {
|
||||
t.Fatal("PUT was not called")
|
||||
}
|
||||
assertEqual(t, put["title"], "original title")
|
||||
assertEqual(t, put["body"], "updated body")
|
||||
assertEqual(t, put["head"], "feature/x")
|
||||
assertEqual(t, put["base"], "develop")
|
||||
|
||||
tags, ok := put["issue_tag_ids"].([]interface{})
|
||||
if !ok || len(tags) != 2 || fmt.Sprintf("%v", tags[0]) != "7" || fmt.Sprintf("%v", tags[1]) != "9" {
|
||||
t.Fatalf("expected preserved tag ids [7 9], got %v", put["issue_tag_ids"])
|
||||
}
|
||||
if _, ok := put["receivers_login"].([]interface{}); !ok {
|
||||
t.Fatalf("expected receivers_login array, got %v", put["receivers_login"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestPREditTagIDsOverride(t *testing.T) {
|
||||
var put map[string]interface{}
|
||||
server := editServer(t, map[string]interface{}{
|
||||
"title": "keep",
|
||||
"body": "keep",
|
||||
"head": "src",
|
||||
"base": "master",
|
||||
"issue": map[string]interface{}{
|
||||
"issue_tags": []interface{}{
|
||||
map[string]interface{}{"id": float64(7)},
|
||||
},
|
||||
},
|
||||
}, &put)
|
||||
defer server.Close()
|
||||
|
||||
err := runPRShortcut(t, server, "edit", map[string]string{
|
||||
"id": "42",
|
||||
"tag-ids": "3, 5",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("edit failed: %v", err)
|
||||
}
|
||||
tags, ok := put["issue_tag_ids"].([]interface{})
|
||||
if !ok || len(tags) != 2 || fmt.Sprintf("%v", tags[0]) != "3" || fmt.Sprintf("%v", tags[1]) != "5" {
|
||||
t.Fatalf("expected tag ids [3 5], got %v", put["issue_tag_ids"])
|
||||
}
|
||||
}
|
||||
|
||||
// The non-v1 detail endpoint nests the branch names under pull_request.head /
|
||||
// pull_request.base (branch-name strings), which is the shape the live API
|
||||
// actually returns; editing title-only must still preserve them.
|
||||
func TestPREditReadsNestedPullRequestFields(t *testing.T) {
|
||||
var put map[string]interface{}
|
||||
server := editServer(t, map[string]interface{}{
|
||||
"pull_request": map[string]interface{}{
|
||||
"title": "nested title",
|
||||
"body": "nested body",
|
||||
"head": "topic",
|
||||
"base": "main",
|
||||
},
|
||||
"issue": map[string]interface{}{"id": float64(1)},
|
||||
}, &put)
|
||||
defer server.Close()
|
||||
|
||||
err := runPRShortcut(t, server, "edit", map[string]string{
|
||||
"id": "42",
|
||||
"title": "new title",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("edit failed: %v", err)
|
||||
}
|
||||
assertEqual(t, put["title"], "new title")
|
||||
assertEqual(t, put["body"], "nested body")
|
||||
assertEqual(t, put["head"], "topic")
|
||||
assertEqual(t, put["base"], "main")
|
||||
}
|
||||
|
||||
func TestPREditRequiresAtLeastOneField(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
t.Fatalf("no request expected, got %s %s", r.Method, r.URL.Path)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
err := runPRShortcut(t, server, "edit", map[string]string{"id": "42"})
|
||||
if err == nil {
|
||||
t.Fatal("expected error when no edit fields are provided")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPREditHTTPError(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte("server error"))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
err := runPRShortcut(t, server, "edit", map[string]string{"id": "42", "title": "x"})
|
||||
if err == nil {
|
||||
t.Fatal("expected error for HTTP 500")
|
||||
}
|
||||
}
|
||||
|
||||
// --- extractIssueID ---
|
||||
|
||||
func TestExtractIssueID(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue