From 104162b48009e2348076a7869e00568a4264c3ed Mon Sep 17 00:00:00 2001 From: Mengz <2567587994@qq.com> Date: Wed, 10 Jun 2026 16:12:13 +0800 Subject: [PATCH] =?UTF-8?q?fix(pr):=20=E8=A1=A5=E9=BD=90=20pr=20+view=20?= =?UTF-8?q?=E7=9A=84=E5=90=88=E5=B9=B6=E4=B8=8E=E5=85=B3=E9=97=AD=E6=97=B6?= =?UTF-8?q?=E9=97=B4=E5=AD=97=E6=AE=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 1 + README.zh-CN.md | 1 + doc/changes/pr-view-timestamps.md | 26 ++++ shortcuts/pr/pr.go | 174 +++++++++++++++++++++----- shortcuts/pr/pr_test.go | 195 ++++++++++++++++++++++++++++++ 5 files changed, 366 insertions(+), 31 deletions(-) create mode 100644 doc/changes/pr-view-timestamps.md diff --git a/README.md b/README.md index b43dcd1..8902ada 100644 --- a/README.md +++ b/README.md @@ -369,6 +369,7 @@ gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: New feature" - # View a PR gitlink-cli pr +view --owner Gitlink --repo forgeplus -i 42 +# For merged or closed PRs, JSON output also normalizes `created_at`, `merged_at`, `closed_at`, and `closed_on` when GitLink provides or journals can infer them. # Merge a PR gitlink-cli pr +merge --owner Gitlink --repo forgeplus -i 42 diff --git a/README.zh-CN.md b/README.zh-CN.md index 265ac98..fe39889 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -379,6 +379,7 @@ gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: 新功能" --h # 查看 PR gitlink-cli pr +view --owner Gitlink --repo forgeplus -i 42 +# 对于已合并或已关闭的 PR,JSON 输出会尽量补齐 `created_at`、`merged_at`、`closed_at` 和 `closed_on`。 # 合并 PR gitlink-cli pr +merge --owner Gitlink --repo forgeplus -i 42 diff --git a/doc/changes/pr-view-timestamps.md b/doc/changes/pr-view-timestamps.md new file mode 100644 index 0000000..7707f26 --- /dev/null +++ b/doc/changes/pr-view-timestamps.md @@ -0,0 +1,26 @@ +# PR View Timestamp Normalization + +## Summary + +`gitlink-cli pr +view` now normalizes pull request lifecycle timestamps so merged and closed pull requests expose stable top-level time fields in JSON output. + +## Command + +| Command | Purpose | +|---------|---------| +| `gitlink-cli pr +view` | Return PR detail and normalize `created_at`, `merged_at`, `closed_at`, and `closed_on` when GitLink provides them directly or journals can infer them. | + +## Behavior + +- Promote `created_at` and `merged_at` from nested response objects to the top-level payload. +- Backfill `closed_at` and `closed_on` for merged pull requests when GitLink omits an explicit close timestamp. +- Read issue journals only when a merged or closed pull request is still missing lifecycle timestamps. +- Recognize merge and close journal operations after stripping HTML tags and whitespace. +- Support both `pull_request_status` and `pull_request_staus` status shapes returned by GitLink APIs. + +## Tests + +- `go test ./shortcuts/pr/...` +- `go build ./...` +- `go test ./...` +- `go run . pr +view --owner Gitlink --repo gitlink-cli -i 15 --format json` diff --git a/shortcuts/pr/pr.go b/shortcuts/pr/pr.go index 03f537f..083b641 100644 --- a/shortcuts/pr/pr.go +++ b/shortcuts/pr/pr.go @@ -2,7 +2,9 @@ package pr import ( "fmt" + "html" "net/url" + "regexp" "strings" "github.com/gitlink-org/gitlink-cli/internal/i18n" @@ -10,6 +12,8 @@ import ( "github.com/gitlink-org/gitlink-cli/shortcuts/common" ) +var pullRequestJournalHTMLTagPattern = regexp.MustCompile(`<[^>]+>`) + func v1RepoPath(ctx *common.RuntimeContext) string { return fmt.Sprintf("/v1/%s/%s", ctx.Owner, ctx.Repo) } @@ -138,7 +142,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { if err != nil { return err } - if err := enrichPullRequestClosedAt(ctx, env); err != nil { + if err := enrichPullRequestTimestamps(ctx, env); err != nil { return err } return ctx.Output(env) @@ -470,79 +474,187 @@ func extractIssueID(env *output.Envelope) (int64, error) { return int64(idFloat), nil } -func enrichPullRequestClosedAt(ctx *common.RuntimeContext, env *output.Envelope) error { +func enrichPullRequestTimestamps(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 || !isClosedPullRequest(pr) || stringField(pr, "closed_at") != "" { + if !ok { return nil } issue, ok := data["issue"].(map[string]interface{}) - if !ok { - return nil + if createdAt := firstNonEmptyString( + stringField(data, "created_at"), + stringField(pr, "created_at"), + stringField(issue, "created_at"), + ); createdAt != "" { + data["created_at"] = createdAt } - issueID, ok := numberField(issue, "id") - if !ok { - return nil + + mergedAt := firstNonEmptyString( + stringField(data, "merged_at"), + stringField(pr, "merged_at"), + stringField(issue, "merged_at"), + ) + closedAt := firstNonEmptyString( + stringField(data, "closed_at"), + stringField(data, "closed_on"), + stringField(pr, "closed_at"), + stringField(issue, "closed_on"), + ) + + if shouldFetchPullRequestTimestamps(pr, mergedAt, closedAt) { + issueID, ok := numberField(issue, "id") + if ok { + journalsEnv, err := ctx.CallAPI("GET", fmt.Sprintf("/v1/%s/%s/issues/%d/journals", ctx.Owner, ctx.Repo, int64(issueID)), nil) + if err != nil { + return err + } + journalMergedAt, journalClosedAt := extractPullRequestJournalTimes(journalsEnv) + mergedAt = firstNonEmptyString(mergedAt, journalMergedAt) + closedAt = firstNonEmptyString(closedAt, journalClosedAt) + } } - journalsEnv, err := ctx.CallAPI("GET", fmt.Sprintf("/v1/%s/%s/issues/%d/journals", ctx.Owner, ctx.Repo, int64(issueID)), nil) - if err != nil { - return err + + if closedAt == "" && isMergedPullRequest(pr) { + closedAt = mergedAt } - closedAt := extractPullRequestClosedAt(journalsEnv) - if closedAt == "" { - return nil + + if mergedAt != "" { + pr["merged_at"] = mergedAt + data["merged_at"] = mergedAt + } + if closedAt != "" { + pr["closed_at"] = closedAt + data["closed_at"] = closedAt + data["closed_on"] = closedAt + if issue != nil { + issue["closed_on"] = closedAt + } } - pr["closed_at"] = closedAt - data["closed_at"] = closedAt return nil } func isClosedPullRequest(pr map[string]interface{}) bool { + if isMergedPullRequest(pr) { + return true + } if stringField(pr, "pull_request_staus") == "closed" || stringField(pr, "state") == "closed" { return true } - status, ok := numberField(pr, "status") + status, ok := numberField(pr, "pull_request_status") + if ok { + return int(status) == 2 + } + status, ok = numberField(pr, "status") return ok && int(status) == 2 } -func extractPullRequestClosedAt(env *output.Envelope) string { +func isMergedPullRequest(pr map[string]interface{}) bool { + if merged, ok := pr["merged"].(bool); ok && merged { + return true + } + if stringField(pr, "pull_request_staus") == "merged" || stringField(pr, "state") == "merged" { + return true + } + status, ok := numberField(pr, "pull_request_status") + if ok { + return int(status) == 1 + } + status, ok = numberField(pr, "status") + return ok && int(status) == 1 +} + +func shouldFetchPullRequestTimestamps(pr map[string]interface{}, mergedAt, closedAt string) bool { + if !isClosedPullRequest(pr) { + return false + } + if isMergedPullRequest(pr) { + return mergedAt == "" || closedAt == "" + } + return closedAt == "" +} + +func extractPullRequestJournalTimes(env *output.Envelope) (string, string) { data, ok := env.Data.(map[string]interface{}) if !ok { - return "" + return "", "" } rawJournals, ok := data["journals"].([]interface{}) if !ok { - return "" + return "", "" } + var mergedAt string + var closedAt string for i := len(rawJournals) - 1; i >= 0; i-- { journal, ok := rawJournals[i].(map[string]interface{}) if !ok || stringField(journal, "operate_category") != "status" { continue } content := stringField(journal, "operate_content") - if !isPullRequestCloseOperation(content) { + timestamp := firstNonEmptyString( + stringField(journal, "updated_at"), + stringField(journal, "created_at"), + ) + if timestamp == "" { continue } - if updatedAt := stringField(journal, "updated_at"); updatedAt != "" { - return updatedAt + if mergedAt == "" && isPullRequestMergeOperation(content) { + mergedAt = timestamp } - if createdAt := stringField(journal, "created_at"); createdAt != "" { - return createdAt + if closedAt == "" && isPullRequestCloseOperation(content) { + closedAt = timestamp + } + if mergedAt != "" && closedAt != "" { + break + } + } + return mergedAt, closedAt +} + +func isPullRequestMergeOperation(content string) bool { + content = normalizePullRequestJournalContent(content) + return containsPullRequestMarker(content) && + (strings.Contains(content, "\u5408\u5e76\u4e86") || + strings.Contains(content, "\u5df2\u5408\u5e76") || + strings.Contains(content, "merged")) +} + +func isPullRequestCloseOperation(content string) bool { + content = normalizePullRequestJournalContent(content) + return containsPullRequestMarker(content) && + (strings.Contains(content, "\u62d2\u7edd") || + strings.Contains(content, "rejected") || + strings.Contains(content, "refused") || + strings.Contains(content, "\u5173\u95ed") || + strings.Contains(content, "closed")) +} + +func normalizePullRequestJournalContent(content string) string { + content = html.UnescapeString(content) + content = pullRequestJournalHTMLTagPattern.ReplaceAllString(content, "") + content = strings.ToLower(strings.TrimSpace(content)) + return strings.Join(strings.Fields(content), "") +} + +func containsPullRequestMarker(content string) bool { + return strings.Contains(content, "\u5408\u5e76\u8bf7\u6c42") || strings.Contains(content, "pullrequest") +} + +func firstNonEmptyString(values ...string) string { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return strings.TrimSpace(value) } } return "" } -func isPullRequestCloseOperation(content string) bool { - content = strings.ToLower(content) - return strings.Contains(content, "合并请求") && - (strings.Contains(content, "拒绝") || strings.Contains(content, "关闭") || strings.Contains(content, "closed")) -} - func stringField(m map[string]interface{}, key string) string { + if m == nil { + return "" + } v, _ := m[key].(string) return v } diff --git a/shortcuts/pr/pr_test.go b/shortcuts/pr/pr_test.go index eece6d9..2b4160d 100644 --- a/shortcuts/pr/pr_test.go +++ b/shortcuts/pr/pr_test.go @@ -249,6 +249,201 @@ func TestPRView(t *testing.T) { } } +func TestEnrichPullRequestTimestampsPromotesMergedAndClosedAt(t *testing.T) { + var journalCalls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Fatalf("expected GET, got %s", r.Method) + } + if r.URL.Path != "/v1/owner/repo/issues/142349/journals.json" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + journalCalls++ + writeJSON(t, w, map[string]interface{}{"journals": []interface{}{}}) + })) + defer server.Close() + + ctx := &common.RuntimeContext{ + Client: &client.Client{ + HTTP: server.Client(), + BaseURL: server.URL, + }, + Owner: "owner", + Repo: "repo", + } + env := &output.Envelope{Data: map[string]interface{}{ + "pull_request": map[string]interface{}{ + "pull_request_status": float64(1), + "merged_at": "2026-05-14T14:26:27+08:00", + "created_at": "2026-05-10T09:00:00+08:00", + }, + "issue": map[string]interface{}{ + "id": float64(142349), + "created_at": "2026-05-10T09:00:00+08:00", + }, + }} + + if err := enrichPullRequestTimestamps(ctx, env); err != nil { + t.Fatalf("enrichPullRequestTimestamps returned error: %v", err) + } + + data := env.Data.(map[string]interface{}) + pr := data["pull_request"].(map[string]interface{}) + issue := data["issue"].(map[string]interface{}) + assertEqual(t, data["created_at"], "2026-05-10T09:00:00+08:00") + assertEqual(t, data["merged_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, data["closed_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, data["closed_on"], "2026-05-14T14:26:27+08:00") + assertEqual(t, pr["merged_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, pr["closed_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, issue["closed_on"], "2026-05-14T14:26:27+08:00") + if journalCalls != 1 { + t.Fatalf("journalCalls = %d, want 1", journalCalls) + } +} + +func TestEnrichPullRequestTimestampsReadsMergeTimeFromJournals(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Fatalf("expected GET, got %s", r.Method) + } + if r.URL.Path != "/v1/owner/repo/issues/142349/journals.json" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{ + "journals": []interface{}{ + map[string]interface{}{ + "operate_category": "status", + "operate_content": "合并了 合并请求", + "updated_at": "2026-05-14T14:26:27+08:00", + }, + }, + }) + })) + defer server.Close() + + ctx := &common.RuntimeContext{ + Client: &client.Client{ + HTTP: server.Client(), + BaseURL: server.URL, + }, + Owner: "owner", + Repo: "repo", + } + env := &output.Envelope{Data: map[string]interface{}{ + "pull_request": map[string]interface{}{ + "pull_request_staus": "merged", + }, + "issue": map[string]interface{}{ + "id": float64(142349), + }, + }} + + if err := enrichPullRequestTimestamps(ctx, env); err != nil { + t.Fatalf("enrichPullRequestTimestamps returned error: %v", err) + } + + data := env.Data.(map[string]interface{}) + pr := data["pull_request"].(map[string]interface{}) + issue := data["issue"].(map[string]interface{}) + assertEqual(t, data["merged_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, data["closed_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, data["closed_on"], "2026-05-14T14:26:27+08:00") + assertEqual(t, pr["merged_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, pr["closed_at"], "2026-05-14T14:26:27+08:00") + assertEqual(t, issue["closed_on"], "2026-05-14T14:26:27+08:00") +} + +func TestEnrichPullRequestTimestampsReadsClosedTimeFromJournals(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Fatalf("expected GET, got %s", r.Method) + } + if r.URL.Path != "/v1/owner/repo/issues/142350/journals.json" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{ + "journals": []interface{}{ + map[string]interface{}{ + "operate_category": "status", + "operate_content": "关闭了合并请求", + "created_at": "2026-05-15T10:30:00+08:00", + }, + }, + }) + })) + defer server.Close() + + ctx := &common.RuntimeContext{ + Client: &client.Client{ + HTTP: server.Client(), + BaseURL: server.URL, + }, + Owner: "owner", + Repo: "repo", + } + env := &output.Envelope{Data: map[string]interface{}{ + "pull_request": map[string]interface{}{ + "pull_request_status": float64(2), + }, + "issue": map[string]interface{}{ + "id": float64(142350), + }, + }} + + if err := enrichPullRequestTimestamps(ctx, env); err != nil { + t.Fatalf("enrichPullRequestTimestamps returned error: %v", err) + } + + data := env.Data.(map[string]interface{}) + pr := data["pull_request"].(map[string]interface{}) + issue := data["issue"].(map[string]interface{}) + assertEqual(t, data["closed_at"], "2026-05-15T10:30:00+08:00") + assertEqual(t, data["closed_on"], "2026-05-15T10:30:00+08:00") + assertEqual(t, pr["closed_at"], "2026-05-15T10:30:00+08:00") + assertEqual(t, issue["closed_on"], "2026-05-15T10:30:00+08:00") + if _, ok := data["merged_at"]; ok { + t.Fatalf("merged_at should stay empty for closed pull requests, got %v", data["merged_at"]) + } +} + +func TestEnrichPullRequestTimestampsSkipsJournalsForOpenPR(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + return + })) + defer server.Close() + + ctx := &common.RuntimeContext{ + Client: &client.Client{ + HTTP: server.Client(), + BaseURL: server.URL, + }, + Owner: "owner", + Repo: "repo", + } + env := &output.Envelope{Data: map[string]interface{}{ + "pull_request": map[string]interface{}{ + "pull_request_status": float64(0), + }, + "issue": map[string]interface{}{ + "id": float64(142351), + }, + }} + + if err := enrichPullRequestTimestamps(ctx, env); err != nil { + t.Fatalf("enrichPullRequestTimestamps returned error: %v", err) + } + + data := env.Data.(map[string]interface{}) + if _, ok := data["closed_at"]; ok { + t.Fatalf("closed_at should not be set for open pull requests, got %v", data["closed_at"]) + } + if _, ok := data["merged_at"]; ok { + t.Fatalf("merged_at should not be set for open pull requests, got %v", data["merged_at"]) + } +} + // --- merge --- func TestPRMerge(t *testing.T) {