From 0a5bad01c85b5adacebc50890756a0ff73061ced Mon Sep 17 00:00:00 2001 From: Mengz <2567587994@qq.com> Date: Wed, 10 Jun 2026 19:33:56 +0800 Subject: [PATCH] =?UTF-8?q?fix(issue):=20=E4=BF=AE=E5=A4=8D=E8=AF=A6?= =?UTF-8?q?=E6=83=85=E7=BC=BA=E5=A4=B1=E5=B9=B6=E4=BF=9D=E6=8A=A4=E6=9B=B4?= =?UTF-8?q?=E6=96=B0=E5=85=83=E6=95=B0=E6=8D=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 8 + doc/changes/issue-view-update-safeguards.md | 9 + shortcuts/issue/batch.go | 2 + shortcuts/issue/issue.go | 268 ++++++++++++++-- shortcuts/issue/issue_test.go | 324 ++++++++++++++++++-- 5 files changed, 574 insertions(+), 37 deletions(-) create mode 100644 doc/changes/issue-view-update-safeguards.md diff --git a/README.md b/README.md index b43dcd1..ba425cc 100644 --- a/README.md +++ b/README.md @@ -336,6 +336,14 @@ gitlink-cli issue +statuses --owner Gitlink --repo forgeplus is accepted as a compatibility alias for the same web issue number, not the global database ID. +`issue +view` now returns both `number` and `database_id`, and it enriches the +response with tracker, priority, status, and issue tag metadata when the legacy +issue detail endpoints provide those fields. + +`issue +update`, `issue +close`, and `issue +batch-close` preserve existing +tracker, version, assignee, tag, and schedule metadata before sending updates, +which avoids clearing required fields on the server by accident. + ### Label Management ```bash diff --git a/doc/changes/issue-view-update-safeguards.md b/doc/changes/issue-view-update-safeguards.md new file mode 100644 index 0000000..124cfae --- /dev/null +++ b/doc/changes/issue-view-update-safeguards.md @@ -0,0 +1,9 @@ +# issue 详情增强与更新保护 + +这个变更聚焦修复 `issue` 快捷命令里两个容易影响实际使用的问题。 + +`issue +view` 之前只读取 v1 详情接口，返回结果里缺少网页端常见的状态、优先级、跟踪器和标签信息，用户很难直接把 CLI 输出和网页上的 issue 页面对应起来。这次调整后，命令会继续以 v1 接口为主，再补充读取旧版详情与编辑接口，在不影响主流程可用性的前提下，把 `number`、`database_id`、`tracker_id`、`issue_type`、`issue_tag_ids`、`issue_tag_names` 等信息一起带出来。 + +`issue +update`、`issue +close` 和 `issue +batch-close` 之前只保留了部分字段，更新时可能把现有 issue 的 `tracker_id`、`fixed_version_id`、`assigned_to_id`、`issue_type` 等服务端依赖字段丢掉，导致网页上出现状态异常或字段被误清空。现在这些命令会先读取 issue 的编辑元数据，再把关键字段一并回写；如果编辑元数据拉取失败，就直接终止更新，避免发送不完整的 PATCH 请求。 + +为了防止这类问题回归，这次补充了 `shortcuts/issue` 的单元测试，覆盖了详情增强、元数据保留、编辑元数据失败时停止写入，以及批量关闭复用同一套保护逻辑的场景。 diff --git a/shortcuts/issue/batch.go b/shortcuts/issue/batch.go index 2345b6c..32b9175 100644 --- a/shortcuts/issue/batch.go +++ b/shortcuts/issue/batch.go @@ -102,6 +102,8 @@ func closeIssue(ctx *common.RuntimeContext, number string) error { "description": current.Description, "status_id": closedIssueStatusID, } + preserveIssueMetadata(body, current) + body["status_id"] = closedIssueStatusID if _, err := ctx.CallAPI("PATCH", fmt.Sprintf("%s/issues/%s", v1RepoPath(ctx), number), body); err != nil { return fmt.Errorf("close issue: %w", err) } diff --git a/shortcuts/issue/issue.go b/shortcuts/issue/issue.go index 9e5a117..538cd50 100644 --- a/shortcuts/issue/issue.go +++ b/shortcuts/issue/issue.go @@ -1,6 +1,7 @@ package issue import ( + "errors" "fmt" "net/url" "strconv" @@ -30,15 +31,27 @@ func normalizeIssueListState(state string) string { } type existingIssue struct { - Subject string - Description string - StatusID interface{} - PriorityID interface{} - TagIDs []interface{} - AssignerIDs []interface{} - BranchName string - StartDate string - DueDate string + Subject string + Description string + StatusID interface{} + PriorityID interface{} + TagIDs []interface{} + AssignerIDs []interface{} + AssignedToID interface{} + FixedVersionID interface{} + TrackerID interface{} + IssueType interface{} + BranchName string + StartDate string + DueDate string +} + +func legacyIssuePath(ctx *common.RuntimeContext, number string) string { + return fmt.Sprintf("%s/issues/%s", ctx.RepoPath(), number) +} + +func legacyIssueEditPath(ctx *common.RuntimeContext, number string) string { + return legacyIssuePath(ctx, number) + "/edit" } func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { @@ -172,6 +185,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { if err != nil { return err } + enrichIssueView(ctx, number, env) return ctx.Output(env) }, }, @@ -487,16 +501,24 @@ func fetchExistingIssue(ctx *common.RuntimeContext, number string) (*existingIss return nil, fmt.Errorf("failed to parse issue subject") } description, _ := issueData["description"].(string) + editData, err := fetchLegacyIssueEdit(ctx, number) + if err != nil { + return nil, fmt.Errorf("fetch issue edit metadata: %w", err) + } return &existingIssue{ - Subject: subject, - Description: description, - StatusID: nestedIssueID(issueData, "status"), - PriorityID: nestedIssueID(issueData, "priority"), - TagIDs: issueObjectIDs(issueData, "tags", "issue_tags"), - AssignerIDs: issueObjectIDs(issueData, "assigners"), - BranchName: stringField(issueData, "branch_name"), - StartDate: stringField(issueData, "start_date"), - DueDate: stringField(issueData, "due_date"), + Subject: subject, + Description: description, + StatusID: firstNonNil(nestedIssueID(issueData, "status"), editData["status_id"]), + PriorityID: firstNonNil(nestedIssueID(issueData, "priority"), editData["priority_id"]), + TagIDs: firstNonEmptyIDs(issueObjectIDs(issueData, "tags", "issue_tags"), issueValueIDs(editData, "issue_tags")), + AssignerIDs: issueObjectIDs(issueData, "assigners"), + AssignedToID: firstNonNil(issueData["assigned_to_id"], editData["assigned_to_id"]), + FixedVersionID: firstNonNil(issueData["fixed_version_id"], editData["fixed_version_id"]), + TrackerID: firstNonNil(issueData["tracker_id"], editData["tracker_id"], nestedIssueID(issueData, "tracker")), + IssueType: firstNonNil(issueData["issue_type"], editData["issue_type"]), + BranchName: firstNonEmptyString(stringField(issueData, "branch_name"), stringField(editData, "branch_name")), + StartDate: firstNonEmptyString(stringField(issueData, "start_date"), stringField(editData, "start_date")), + DueDate: firstNonEmptyString(stringField(issueData, "due_date"), stringField(editData, "due_date")), }, nil } @@ -513,6 +535,18 @@ func preserveIssueMetadata(body map[string]interface{}, issue *existingIssue) { if len(issue.AssignerIDs) > 0 { body["assigner_ids"] = issue.AssignerIDs } + if issue.AssignedToID != nil { + body["assigned_to_id"] = issue.AssignedToID + } + if issue.FixedVersionID != nil { + body["fixed_version_id"] = issue.FixedVersionID + } + if issue.TrackerID != nil { + body["tracker_id"] = issue.TrackerID + } + if issue.IssueType != nil { + body["issue_type"] = issue.IssueType + } if issue.BranchName != "" { body["branch_name"] = issue.BranchName } @@ -534,12 +568,17 @@ func nestedIssueID(data map[string]interface{}, key string) interface{} { func issueObjectIDs(data map[string]interface{}, keys ...string) []interface{} { for _, key := range keys { - items, ok := data[key].([]interface{}) + items, ok := interfaceSlice(data[key]) if !ok { continue } ids := make([]interface{}, 0, len(items)) for _, item := range items { + switch value := item.(type) { + case float64, int, int64, string: + ids = append(ids, value) + continue + } obj, ok := item.(map[string]interface{}) if !ok { continue @@ -555,11 +594,199 @@ func issueObjectIDs(data map[string]interface{}, keys ...string) []interface{} { return nil } +func issueObjectNames(data map[string]interface{}, key string) []string { + items, ok := interfaceSlice(data[key]) + if !ok { + return nil + } + names := make([]string, 0, len(items)) + for _, item := range items { + obj, ok := item.(map[string]interface{}) + if !ok { + continue + } + if name, ok := obj["name"].(string); ok && name != "" { + names = append(names, name) + } + } + if len(names) == 0 { + return nil + } + return names +} + func stringField(data map[string]interface{}, key string) string { value, _ := data[key].(string) return value } +func mapField(data map[string]interface{}, key string) map[string]interface{} { + item, _ := data[key].(map[string]interface{}) + return item +} + +func interfaceSlice(value interface{}) ([]interface{}, bool) { + items, ok := value.([]interface{}) + if ok { + return items, true + } + switch typed := value.(type) { + case []map[string]interface{}: + items = make([]interface{}, 0, len(typed)) + for _, item := range typed { + items = append(items, item) + } + return items, true + } + return nil, false +} + +func issueValueIDs(data map[string]interface{}, keys ...string) []interface{} { + return issueObjectIDs(data, keys...) +} + +func firstNonNil(values ...interface{}) interface{} { + for _, value := range values { + if !isNilValue(value) { + return value + } + } + return nil +} + +func firstNonEmptyString(values ...string) string { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return value + } + } + return "" +} + +func firstNonEmptyIDs(values ...[]interface{}) []interface{} { + for _, ids := range values { + if len(ids) > 0 { + return ids + } + } + return nil +} + +func isNilValue(value interface{}) bool { + if value == nil { + return true + } + switch typed := value.(type) { + case map[string]interface{}: + return len(typed) == 0 + } + return false +} + +func fetchLegacyIssueDetail(ctx *common.RuntimeContext, number string) (map[string]interface{}, error) { + return fetchIssueMap(ctx, legacyIssuePath(ctx, number), "failed to parse legacy issue detail") +} + +func fetchLegacyIssueEdit(ctx *common.RuntimeContext, number string) (map[string]interface{}, error) { + return fetchIssueMap(ctx, legacyIssueEditPath(ctx, number), "failed to parse legacy issue edit data") +} + +func fetchIssueMap(ctx *common.RuntimeContext, path, parseErr string) (map[string]interface{}, error) { + env, err := ctx.CallAPI("GET", path, nil) + if err != nil { + return nil, err + } + data, ok := env.Data.(map[string]interface{}) + if !ok { + return nil, errors.New(parseErr) + } + return data, nil +} + +func enrichIssueView(ctx *common.RuntimeContext, number string, env *output.Envelope) { + if env == nil { + return + } + issueData, ok := env.Data.(map[string]interface{}) + if !ok { + return + } + legacyDetail, _ := fetchLegacyIssueDetail(ctx, number) + legacyEdit, _ := fetchLegacyIssueEdit(ctx, number) + env.Data = mergeIssueViewData(issueData, legacyDetail, legacyEdit) +} + +func mergeIssueViewData(v1Data, legacyDetail, legacyEdit map[string]interface{}) map[string]interface{} { + issue := cloneIssueMap(v1Data) + if issue == nil { + return v1Data + } + + if number := firstNonNil(issue["number"], issue["project_issues_index"], legacyDetail["project_issues_index"]); number != nil { + issue["number"] = number + } + if databaseID := firstNonNil(issue["id"], legacyDetail["id"]); databaseID != nil { + issue["database_id"] = databaseID + delete(issue, "id") + } + + status := firstNonNil(mapField(issue, "status"), mapField(legacyDetail, "issue_status")) + if status != nil { + issue["status"] = status + if statusMap, ok := status.(map[string]interface{}); ok { + if name := stringField(statusMap, "name"); name != "" { + issue["status_name"] = name + } + } + } + if priority := firstNonNil(mapField(issue, "priority"), mapField(legacyDetail, "priority")); priority != nil { + issue["priority"] = priority + if priorityMap, ok := priority.(map[string]interface{}); ok { + if name := stringField(priorityMap, "name"); name != "" { + issue["priority_name"] = name + } + } + } + if tracker := firstNonNil(mapField(issue, "tracker"), mapField(legacyDetail, "tracker")); tracker != nil { + issue["tracker"] = tracker + } + if trackerID := firstNonNil(issue["tracker_id"], nestedIssueID(issue, "tracker"), nestedIssueID(legacyDetail, "tracker"), legacyEdit["tracker_id"]); trackerID != nil { + issue["tracker_id"] = trackerID + } + if issueType := firstNonNil(issue["issue_type"], legacyDetail["issue_type"], legacyEdit["issue_type"]); issueType != nil { + issue["issue_type"] = issueType + } + if assignedToID := firstNonNil(issue["assigned_to_id"], legacyDetail["assigned_to_id"], legacyEdit["assigned_to_id"]); assignedToID != nil { + issue["assigned_to_id"] = assignedToID + } + if fixedVersionID := firstNonNil(issue["fixed_version_id"], legacyDetail["fixed_version_id"], legacyDetail["version_id"], legacyEdit["fixed_version_id"]); fixedVersionID != nil { + issue["fixed_version_id"] = fixedVersionID + } + if versionID := firstNonNil(issue["version_id"], legacyDetail["version_id"], legacyEdit["fixed_version_id"]); versionID != nil { + issue["version_id"] = versionID + } + + if tagIDs := firstNonEmptyIDs(issueObjectIDs(issue, "tags", "issue_tags"), issueObjectIDs(legacyDetail, "issue_tags"), issueValueIDs(legacyEdit, "issue_tags")); len(tagIDs) > 0 { + issue["issue_tag_ids"] = tagIDs + } + if tagNames := issueObjectNames(legacyDetail, "issue_tags"); len(tagNames) > 0 { + issue["issue_tag_names"] = tagNames + } + + return issue +} + +func cloneIssueMap(data map[string]interface{}) map[string]interface{} { + if data == nil { + return nil + } + cloned := make(map[string]interface{}, len(data)) + for key, value := range data { + cloned[key] = value + } + return cloned +} + func normalizeIssueStatus(state string) (interface{}, error) { switch strings.ToLower(strings.TrimSpace(state)) { case "open": @@ -611,6 +838,9 @@ func applyIssueMetadataArgs(ctx *common.RuntimeContext, body map[string]interfac return err } body["assigner_ids"] = ids + if len(ids) == 1 { + body["assigned_to_id"] = ids[0] + } } if branch := ctx.Arg("branch"); branch != "" { body["branch_name"] = branch diff --git a/shortcuts/issue/issue_test.go b/shortcuts/issue/issue_test.go index 48be057..3f7dcb2 100644 --- a/shortcuts/issue/issue_test.go +++ b/shortcuts/issue/issue_test.go @@ -80,8 +80,26 @@ func assertEqual(t *testing.T, got interface{}, want interface{}) { func assertNumberSlice(t *testing.T, got interface{}, want []float64) { t.Helper() - values, ok := got.([]interface{}) - if !ok { + var values []float64 + switch typed := got.(type) { + case []interface{}: + values = make([]float64, 0, len(typed)) + for _, value := range typed { + switch number := value.(type) { + case float64: + values = append(values, number) + case int: + values = append(values, float64(number)) + default: + t.Fatalf("got %v (%T), want numeric slice", got, got) + } + } + case []int: + values = make([]float64, 0, len(typed)) + for _, value := range typed { + values = append(values, float64(value)) + } + default: t.Fatalf("got %v (%T), want numeric slice", got, got) } if len(values) != len(want) { @@ -94,6 +112,48 @@ func assertNumberSlice(t *testing.T, got interface{}, want []float64) { } } +func issueLegacyPath(number string) string { + return "/owner/repo/issues/" + number + ".json" +} + +func issueLegacyEditPath(number string) string { + return "/owner/repo/issues/" + number + "/edit.json" +} + +func writeLegacyIssueDetail(t *testing.T, w http.ResponseWriter, number string) { + t.Helper() + writeJSON(t, w, map[string]interface{}{ + "id": 9001, + "project_issues_index": 42, + "tracker": map[string]interface{}{"id": 11, "name": "Bug"}, + "priority": map[string]interface{}{"id": 2, "name": "Normal"}, + "issue_status": map[string]interface{}{"id": 1, "name": "Open"}, + "issue_tags": []map[string]interface{}{ + {"id": 7, "name": "backend"}, + }, + "version_id": 13, + "branch_name": "main", + "start_date": "2026-05-01", + "due_date": "2026-05-31", + }) +} + +func writeLegacyIssueEdit(t *testing.T, w http.ResponseWriter) { + t.Helper() + writeJSON(t, w, map[string]interface{}{ + "status_id": 1, + "priority_id": 2, + "tracker_id": 11, + "issue_type": "bug", + "issue_tags": []interface{}{7, 8}, + "assigned_to_id": 9, + "fixed_version_id": 13, + "branch_name": "main", + "start_date": "2026-05-01", + "due_date": "2026-05-31", + }) +} + // --- list --- func TestIssueList(t *testing.T) { @@ -251,13 +311,21 @@ func TestIssueCreateMissingTitle(t *testing.T) { func TestIssueView(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" { - t.Fatalf("expected GET, got %s", r.Method) + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": + writeJSON(t, w, map[string]interface{}{ + "id": float64(42), + "project_issues_index": float64(42), + "subject": "bug", + "status": nil, + }) + case r.Method == "GET" && r.URL.Path == issueLegacyPath("42"): + writeLegacyIssueDetail(t, w, "42") + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) + default: + t.Fatalf("unexpected path: %s %s", r.Method, r.URL.Path) } - if r.URL.Path != "/v1/owner/repo/issues/42.json" { - t.Fatalf("unexpected path: %s", r.URL.Path) - } - writeJSON(t, w, map[string]interface{}{"id": float64(42), "subject": "bug"}) }) defer server.Close() @@ -282,14 +350,20 @@ func TestIssueViewMissingNumber(t *testing.T) { func TestIssueViewAcceptsIDAlias(t *testing.T) { var requestedPath string server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - requestedPath = r.URL.Path - if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/42.json" { + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": + requestedPath = r.URL.Path + writeJSON(t, w, map[string]interface{}{ + "project_issues_index": 42, + "subject": "Issue from web URL", + }) + case r.Method == "GET" && r.URL.Path == issueLegacyPath("42"): + writeLegacyIssueDetail(t, w, "42") + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) + default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } - writeJSON(t, w, map[string]interface{}{ - "project_issues_index": 42, - "subject": "Issue from web URL", - }) }) defer server.Close() @@ -302,10 +376,16 @@ func TestIssueViewAcceptsIDAlias(t *testing.T) { func TestIssueNumberTakesPrecedenceOverIDAlias(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/42.json" { + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": + writeJSON(t, w, map[string]interface{}{"subject": "Existing title"}) + case r.Method == "GET" && r.URL.Path == issueLegacyPath("42"): + writeLegacyIssueDetail(t, w, "42") + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) + default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } - writeJSON(t, w, map[string]interface{}{"subject": "Existing title"}) }) defer server.Close() @@ -330,6 +410,8 @@ func TestIssueClose(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, patchPayload) @@ -357,6 +439,8 @@ func TestIssueCloseAcceptsIDAlias(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -385,6 +469,19 @@ func TestIssueClosePreservesCurrentMetadata(t *testing.T) { {"id": 4}, }, }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeJSON(t, w, map[string]interface{}{ + "status_id": 1, + "priority_id": 3, + "tracker_id": 11, + "issue_type": "bug", + "issue_tags": []interface{}{4}, + "assigned_to_id": 9, + "fixed_version_id": 13, + "branch_name": "main", + "start_date": "2026-05-01", + "due_date": "2026-05-31", + }) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -402,6 +499,10 @@ func TestIssueClosePreservesCurrentMetadata(t *testing.T) { assertEqual(t, updatePayload["status_id"], float64(5)) assertEqual(t, updatePayload["priority_id"], float64(3)) assertNumberSlice(t, updatePayload["issue_tag_ids"], []float64{4}) + assertEqual(t, updatePayload["tracker_id"], float64(11)) + assertEqual(t, updatePayload["issue_type"], "bug") + assertEqual(t, updatePayload["assigned_to_id"], float64(9)) + assertEqual(t, updatePayload["fixed_version_id"], float64(13)) } func TestIssueCloseFetchFails(t *testing.T) { @@ -429,6 +530,8 @@ func TestIssueUpdateTitle(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, patchPayload) @@ -457,6 +560,8 @@ func TestIssueUpdateDescription(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, patchPayload) @@ -484,6 +589,8 @@ func TestIssueUpdateNumericState(t *testing.T) { "subject": "bug", "description": "desc", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, map[string]interface{}{"id": float64(42)}) @@ -509,6 +616,8 @@ func TestIssueUpdateAcceptsIDAlias(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -550,6 +659,19 @@ func TestIssueUpdatePreservesCurrentMetadata(t *testing.T) { "start_date": "2026-05-01", "due_date": "2026-05-31", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeJSON(t, w, map[string]interface{}{ + "status_id": 1, + "priority_id": 2, + "tracker_id": 11, + "issue_type": "bug", + "issue_tags": []interface{}{7, 8}, + "assigned_to_id": 9, + "fixed_version_id": 13, + "branch_name": "main", + "start_date": "2026-05-01", + "due_date": "2026-05-31", + }) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -575,6 +697,10 @@ func TestIssueUpdatePreservesCurrentMetadata(t *testing.T) { assertEqual(t, updatePayload["branch_name"], "main") assertEqual(t, updatePayload["start_date"], "2026-05-01") assertEqual(t, updatePayload["due_date"], "2026-05-31") + assertEqual(t, updatePayload["tracker_id"], float64(11)) + assertEqual(t, updatePayload["issue_type"], "bug") + assertEqual(t, updatePayload["assigned_to_id"], float64(9)) + assertEqual(t, updatePayload["fixed_version_id"], float64(13)) } func TestIssueUpdateSupportsMetadataFields(t *testing.T) { @@ -586,6 +712,8 @@ func TestIssueUpdateSupportsMetadataFields(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -612,6 +740,7 @@ func TestIssueUpdateSupportsMetadataFields(t *testing.T) { assertEqual(t, updatePayload["priority_id"], float64(4)) assertNumberSlice(t, updatePayload["issue_tag_ids"], []float64{6, 7}) assertNumberSlice(t, updatePayload["assigner_ids"], []float64{8}) + assertEqual(t, updatePayload["assigned_to_id"], float64(8)) assertEqual(t, updatePayload["branch_name"], "bugfix/metadata") assertEqual(t, updatePayload["start_date"], "2026-06-01") assertEqual(t, updatePayload["due_date"], "2026-06-15") @@ -626,6 +755,8 @@ func TestIssueUpdateInvalidState(t *testing.T) { "subject": "bug", "description": "desc", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } @@ -768,6 +899,8 @@ func TestBatchClosePreservesCurrentDescription(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -833,8 +966,12 @@ func TestBatchCloseWithFailedClose(t *testing.T) { switch { case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": writeJSON(t, w, map[string]interface{}{"subject": "Issue 1", "description": "desc1"}) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): + writeLegacyIssueEdit(t, w) case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/2.json": writeJSON(t, w, map[string]interface{}{"subject": "Issue 2", "description": "desc2"}) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("2"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/1.json": writeJSON(t, w, map[string]interface{}{"subject": "Issue 1", "description": "desc1", "status_id": float64(5)}) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/2.json": @@ -1005,6 +1142,54 @@ func TestIssueViewHTTPError(t *testing.T) { } } +func TestMergeIssueViewDataAddsLegacyFields(t *testing.T) { + merged := mergeIssueViewData( + map[string]interface{}{ + "id": float64(42), + "project_issues_index": float64(18), + "subject": "Issue from v1", + "status": nil, + }, + map[string]interface{}{ + "id": float64(2048), + "project_issues_index": float64(18), + "tracker": map[string]interface{}{"id": 5, "name": "Feature"}, + "priority": map[string]interface{}{"id": 2, "name": "Normal"}, + "issue_status": map[string]interface{}{"id": 1, "name": "Open"}, + "issue_tags": []map[string]interface{}{ + {"id": 7, "name": "backend"}, + {"id": 8, "name": "urgent"}, + }, + "version_id": 13, + }, + map[string]interface{}{ + "tracker_id": 5, + "issue_type": "feature", + "assigned_to_id": 9, + "fixed_version_id": 13, + "issue_tags": []interface{}{7, 8}, + }, + ) + + assertEqual(t, merged["number"], float64(18)) + assertEqual(t, merged["database_id"], float64(42)) + assertEqual(t, merged["tracker_id"], 5) + assertEqual(t, merged["issue_type"], "feature") + assertEqual(t, merged["assigned_to_id"], 9) + assertEqual(t, merged["fixed_version_id"], 13) + assertEqual(t, merged["version_id"], 13) + assertEqual(t, merged["status_name"], "Open") + assertEqual(t, merged["priority_name"], "Normal") + assertNumberSlice(t, merged["issue_tag_ids"], []float64{7, 8}) + tagNames, ok := merged["issue_tag_names"].([]string) + if !ok { + t.Fatalf("expected issue_tag_names, got %T", merged["issue_tag_names"]) + } + if len(tagNames) != 2 || tagNames[0] != "backend" || tagNames[1] != "urgent" { + t.Fatalf("unexpected issue_tag_names: %v", tagNames) + } +} + func TestIssueCommentHTTPError(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { writeText(t, w, http.StatusInternalServerError, "server error") @@ -1024,6 +1209,8 @@ func TestIssueUpdateHTTPError(t *testing.T) { writeJSON(t, w, map[string]interface{}{ "id": float64(42), "subject": "bug", "description": "desc", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": writeText(t, w, http.StatusInternalServerError, "server error") default: @@ -1045,6 +1232,8 @@ func TestIssueCloseHTTPError(t *testing.T) { writeJSON(t, w, map[string]interface{}{ "id": float64(42), "subject": "bug", "description": "desc", }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": writeText(t, w, http.StatusInternalServerError, "server error") default: @@ -1061,7 +1250,14 @@ func TestIssueCloseHTTPError(t *testing.T) { func TestFetchExistingIssueBadData(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - writeJSON(t, w, "not a map") + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": + writeJSON(t, w, "not a map") + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): + writeLegacyIssueEdit(t, w) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } }) defer server.Close() @@ -1078,7 +1274,14 @@ func TestFetchExistingIssueBadData(t *testing.T) { func TestFetchExistingIssueNoSubject(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - writeJSON(t, w, map[string]interface{}{"id": float64(1)}) + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": + writeJSON(t, w, map[string]interface{}{"id": float64(1)}) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): + writeLegacyIssueEdit(t, w) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } }) defer server.Close() @@ -1093,6 +1296,91 @@ func TestFetchExistingIssueNoSubject(t *testing.T) { } } +func TestFetchExistingIssueRequiresEditMetadata(t *testing.T) { + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": + writeJSON(t, w, map[string]interface{}{ + "subject": "issue", + "description": "desc", + }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): + writeText(t, w, http.StatusInternalServerError, "boom") + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + ctx := &common.RuntimeContext{ + Client: &client.Client{HTTP: server.Client(), BaseURL: server.URL}, + Owner: "owner", + Repo: "repo", + } + _, err := fetchExistingIssue(ctx, "1") + if err == nil || !strings.Contains(err.Error(), "edit metadata") { + t.Fatalf("expected edit metadata error, got %v", err) + } +} + +func TestIssueUpdateStopsWhenEditMetadataFails(t *testing.T) { + patchCalled := false + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": + writeJSON(t, w, map[string]interface{}{ + "subject": "Existing title", + "description": "Existing description", + }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeText(t, w, http.StatusInternalServerError, "server error") + case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": + patchCalled = true + t.Fatal("PATCH should not be sent when edit metadata fetch fails") + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runShortcut(t, server, "update", map[string]string{"number": "42", "title": "New title"}) + if err == nil { + t.Fatal("expected error when edit metadata fetch fails") + } + if patchCalled { + t.Fatal("patch should not have been called") + } +} + +func TestIssueCloseStopsWhenEditMetadataFails(t *testing.T) { + patchCalled := false + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": + writeJSON(t, w, map[string]interface{}{ + "subject": "Existing title", + "description": "Existing description", + }) + case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): + writeText(t, w, http.StatusInternalServerError, "server error") + case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": + patchCalled = true + t.Fatal("PATCH should not be sent when edit metadata fetch fails") + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runShortcut(t, server, "close", map[string]string{"number": "42"}) + if err == nil { + t.Fatal("expected error when edit metadata fetch fails") + } + if patchCalled { + t.Fatal("patch should not have been called") + } +} + // --- normalizeIssueStatus --- func TestNormalizeIssueStatus(t *testing.T) {