Merge PR #361: feat(pr): add pr +edit to update an open pull request

# Conflicts:
#	shortcuts/pr/pr.go
This commit is contained in:
wbtiger 2026-07-14 22:49:53 +08:00
commit d465ac2427
5 changed files with 312 additions and 12 deletions

18
doc/changes/pr-edit.md Normal file
View File

@ -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.

View File

@ -75,6 +75,7 @@
"cmd.pr.comments.short": "List comments (journals) of 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",
@ -242,6 +243,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)",

View File

@ -75,6 +75,7 @@
"cmd.pr.comments.short": "列出合并请求的评论journals",
"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": "合并拉取请求",
@ -242,6 +243,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 时间戳)",

View File

@ -1,8 +1,10 @@
package pr
import (
"encoding/json"
"fmt"
"net/url"
"strconv"
"strings"
"github.com/gitlink-org/gitlink-cli/internal/i18n"
@ -138,7 +140,53 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut {
if err != nil {
return err
}
if err := enrichPullRequestTimestamps(ctx, env); err != nil {
if err := enrichPullRequestClosedAt(ctx, env); err != nil {
return err
}
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)
@ -470,22 +518,107 @@ func extractIssueID(env *output.Envelope) (int64, error) {
return int64(idFloat), nil
}
func enrichPullRequestTimestamps(ctx *common.RuntimeContext, env *output.Envelope) error {
// 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 {
return nil
}
pr, ok := data["pull_request"].(map[string]interface{})
if !ok {
return nil
}
if mergedAt := stringField(pr, "merged_at"); mergedAt != "" {
data["merged_at"] = mergedAt
if merged, ok := pr["merged"].(bool); ok {
data["merged"] = merged
}
}
if !isClosedPullRequest(pr) || stringField(pr, "closed_at") != "" {
if !ok || !isClosedPullRequest(pr) || stringField(pr, "closed_at") != "" {
return nil
}
issue, ok := data["issue"].(map[string]interface{})

View File

@ -440,6 +440,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) {