fix(label): paginate label lookup in +update to avoid clobbering fields

Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>

doc/changes/label-update-pagination.md

@@ -0,0 +1,12 @@
+# label +update 分页取值修复
+
+修复 `label +update` 在标签数量超过一页时静默覆盖服务端数据的问题。
+
+`+update` 需要先取标签当前的 `name`/`description`/`color`（更新接口要求三者同时提交），此前 `fetchLabel` 只对列表接口做单次 `GET`，没有翻页；虽然注释声称“分页匹配 id”，实际当目标标签落在第二页及以后时返回“未找到”。随后 `+update` 用空描述与缺省颜色 `#1E90FF` 回填，PATCH 便把服务端真实的描述与颜色抹掉，造成数据丢失。
+
+变更：
+
+- `fetchLabel` 改为真正翻页（按 `page`/`limit=50` 循环，页内条数少于一页即视为末页），与仓库其他列表接口的翻页方式一致。
+- 只有用户显式传入的字段才会覆盖，未传字段一律保留服务端现值，不再用缺省值静默重置。
+- 全量翻页后仍找不到该 id 时直接返回明确错误，不再带缺省值发起 PATCH。
+- 单元测试新增“目标标签在第二页”用例，断言 PATCH 报文保留原描述与颜色；并补充“标签不存在时报错、不 PATCH”用例。

shortcuts/label/label.go

@@ -14,6 +14,10 @@ import (
 // defaultLabelColor is used when the caller does not provide a color.
 const defaultLabelColor = "#1E90FF"
 
+// labelListPageSize is the page size fetchLabel requests while paging the list
+// endpoint. A short page that returns fewer rows than this marks the last page.
+const labelListPageSize = 50
+
 // hexColorPattern matches #RGB and #RRGGBB hex color values.
 var hexColorPattern = regexp.MustCompile(`^#(?:[0-9a-fA-F]{3}|[0-9a-fA-F]{6})$`)
 
@@ -142,7 +146,7 @@ func runUpdate(ctx *common.RuntimeContext) error {
 	if name == "" {
 		return fmt.Errorf("could not resolve label name for id %s; pass --name explicitly", id)
 	}
-	color := firstNonEmpty(ctx.Arg("color"), stringFromMap(current, "color"), defaultLabelColor)
+	color := firstNonEmpty(ctx.Arg("color"), stringFromMap(current, "color"))
 	if err := validateColor(color); err != nil {
 		return err
 	}
@@ -164,32 +168,41 @@ func runUpdate(ctx *common.RuntimeContext) error {
 }
 
 // fetchLabel looks up a single label by id from the list endpoint. GitLink does
-// not expose a single-label GET, so we page through the list and match by id.
-// A nil result (label not found) is not an error: the caller falls back to the
-// flags it was given.
+// not expose a single-label GET, so we page through the list and match by id. A
+// label beyond the first page must still be found, otherwise update would PATCH
+// the server's real name/description/color away with defaults, so an id that is
+// absent after the whole list is exhausted is reported as an error.
 func fetchLabel(ctx *common.RuntimeContext, id string) (map[string]interface{}, error) {
-	env, err := ctx.CallAPI("GET", labelPath(ctx), nil)
-	if err != nil {
-		return nil, err
-	}
-	data, ok := env.Data.(map[string]interface{})
-	if !ok {
-		return nil, nil
-	}
-	rawTags, ok := data["issue_tags"].([]interface{})
-	if !ok {
-		return nil, nil
-	}
-	for _, raw := range rawTags {
-		tag, ok := raw.(map[string]interface{})
+	for page := 1; ; page++ {
+		q := url.Values{}
+		q.Set("page", strconv.Itoa(page))
+		q.Set("limit", strconv.Itoa(labelListPageSize))
+		env, err := ctx.CallAPIWithQuery("GET", labelPath(ctx), q)
+		if err != nil {
+			return nil, err
+		}
+		data, ok := env.Data.(map[string]interface{})
 		if !ok {
-			continue
+			break
 		}
-		if labelIDString(tag["id"]) == id {
-			return tag, nil
+		rawTags, ok := data["issue_tags"].([]interface{})
+		if !ok {
+			break
+		}
+		for _, raw := range rawTags {
+			tag, ok := raw.(map[string]interface{})
+			if !ok {
+				continue
+			}
+			if labelIDString(tag["id"]) == id {
+				return tag, nil
+			}
+		}
+		if len(rawTags) < labelListPageSize {
+			break
 		}
 	}
-	return nil, nil
+	return nil, fmt.Errorf("label id %s not found in this repository's issue labels", id)
 }
 
 func labelPath(ctx *common.RuntimeContext) string {

shortcuts/label/label_test.go

@@ -2,6 +2,7 @@ package label
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -124,6 +125,92 @@ func TestLabelUpdatePreservesCurrentFields(t *testing.T) {
 	assertEqual(t, payload["color"], "#00FF00")
 }
 
+func TestLabelUpdatePreservesFieldsWhenLabelOnSecondPage(t *testing.T) {
+	var payload map[string]interface{}
+	var pagesFetched []string
+	server := newLabelTestServer(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issue_tags.json":
+			page := r.URL.Query().Get("page")
+			pagesFetched = append(pagesFetched, page)
+			switch page {
+			case "1":
+				writeJSON(t, w, map[string]interface{}{
+					"total_count": labelListPageSize + 1,
+					"issue_tags":  fillerLabels(labelListPageSize),
+				})
+			case "2":
+				writeJSON(t, w, map[string]interface{}{
+					"total_count": labelListPageSize + 1,
+					"issue_tags": []interface{}{
+						map[string]interface{}{
+							"id":          float64(7),
+							"name":        "bug",
+							"description": "old description",
+							"color":       "#FF0000",
+						},
+					},
+				})
+			default:
+				t.Fatalf("unexpected page %q", page)
+			}
+		case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issue_tags/7.json":
+			payload = decodeJSON(t, r)
+			writeJSON(t, w, map[string]interface{}{"status": 0, "message": "success"})
+		default:
+			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
+		}
+	})
+	defer server.Close()
+
+	err := runLabelShortcut(t, server, "update", map[string]string{
+		"id":   "7",
+		"name": "renamed",
+	})
+	if err != nil {
+		t.Fatalf("update shortcut failed: %v", err)
+	}
+
+	if len(pagesFetched) != 2 || pagesFetched[0] != "1" || pagesFetched[1] != "2" {
+		t.Fatalf("expected pages [1 2] to be fetched, got %v", pagesFetched)
+	}
+	// The label lives on page 2; only --name was passed, so the description and
+	// color the server already holds must survive the PATCH untouched.
+	assertEqual(t, payload["name"], "renamed")
+	assertEqual(t, payload["description"], "old description")
+	assertEqual(t, payload["color"], "#FF0000")
+}
+
+func TestLabelUpdateErrorsWhenLabelNotFound(t *testing.T) {
+	server := newLabelTestServer(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issue_tags.json":
+			writeJSON(t, w, map[string]interface{}{
+				"total_count": 1,
+				"issue_tags": []interface{}{
+					map[string]interface{}{
+						"id":          float64(3),
+						"name":        "docs",
+						"description": "documentation",
+						"color":       "#00FF00",
+					},
+				},
+			})
+		default:
+			t.Fatalf("missing label must not PATCH, got: %s %s", r.Method, r.URL.Path)
+		}
+	})
+	defer server.Close()
+
+	err := runLabelShortcut(t, server, "update", map[string]string{
+		"id":    "7",
+		"color": "#123456",
+	})
+	if err == nil {
+		t.Fatal("expected update of a missing label id to return an error")
+	}
+}
+
 func TestLabelUpdateRequiresAtLeastOneField(t *testing.T) {
 	server := newLabelTestServer(t, func(w http.ResponseWriter, r *http.Request) {
 		t.Fatalf("update with no fields should not call API, got: %s %s", r.Method, r.URL.Path)
@@ -205,6 +292,21 @@ func newLabelTestServer(t *testing.T, handler http.HandlerFunc) *httptest.Server
 	return httptest.NewServer(handler)
 }
 
+// fillerLabels builds n distinct labels whose ids never collide with the target
+// ids used in the update tests, so a full page forces fetchLabel onto the next.
+func fillerLabels(n int) []interface{} {
+	labels := make([]interface{}, 0, n)
+	for i := 0; i < n; i++ {
+		labels = append(labels, map[string]interface{}{
+			"id":          float64(1000 + i),
+			"name":        fmt.Sprintf("filler-%d", i),
+			"description": "filler",
+			"color":       "#123456",
+		})
+	}
+	return labels
+}
+
 func assertRequest(t *testing.T, r *http.Request, method, path string) {
 	t.Helper()
 	if r.Method != method || r.URL.Path != path {