fix(pr): 补齐 pr +view 的合并与关闭时间字段
This commit is contained in:
parent
52b7093846
commit
104162b480
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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`
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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": "<b>合并了</b> 合并请求",
|
||||
"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": "<b>关闭了</b>合并请求",
|
||||
"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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue