mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 18:06:45 +08:00
fix(issues): derive a usable status key from non-ASCII display names (MUL-6749) (#7638)
Creating a custom issue status whose display name is written entirely in a
non-Latin script — 客户确认, 고객 확인, 確認待ち — failed with "cannot derive a
status key from that name; provide one explicitly". The key alphabet is ASCII,
so such a name slugified to nothing, and the settings form exposes no field for
an explicit key: there was no way to create the status through the UI at all.
The key stays an ASCII machine handle — it is what issue.status stores, what the
CLI takes as an argument, and what the column CHECK permits — so there is NO
migration. When the name has nothing to slug the key falls back to the status's
CATEGORY plus an ordinal (in_review_2), which keeps saying which platform
behavior the status inherits.
Also fixed in the same path:
- Two names that collapse onto one slug ("待客户 Review" and "待供应商 Review"
both slug to "review") no longer make the second create fail with a conflict
blaming a display name nobody took. Disambiguation counts ARCHIVED rows, since
idx_issue_status_workspace_key is not partial.
- Every create takes the exclusive catalog lock, not just the deriving one, so
an explicit-key insert cannot land between a derive's catalog read and its
insert and hand a 409 to a form with no key field.
- The ordinal scan is bounded by the catalog rather than a constant.
A derived key is opaque, so the places that printed a key alone now carry the
name: issue payloads gain status_name (HTTP and background events alike, from
one catalog read), the unknown-status 400 lists `key (Name)`, creating a status
surfaces the key it minted, and `multica issue status --help` says the argument
is a key rather than a display name.
Fixes #7627. Supersedes #7636.
This commit is contained in:
@@ -167,6 +167,35 @@ describe("ChatSessionSchema", () => {
|
||||
});
|
||||
});
|
||||
describe("IssueSchema (via ListIssuesResponseSchema)", () => {
|
||||
// A custom status key can be derived rather than readable — "客户确认" becomes
|
||||
// `in_review_2` — so the display name travels with it. The field has to
|
||||
// survive a server that predates it, since an issue that fails validation
|
||||
// degrades to a stub rather than losing one field. (MUL-6749)
|
||||
it("carries a custom status's display name", () => {
|
||||
const parsed = ListIssuesResponseSchema.parse({
|
||||
issues: [{ ...baseIssue, status: "in_review_2", status_name: "客户确认" }],
|
||||
total: 1,
|
||||
});
|
||||
expect(parsed.issues[0]?.status_name).toBe("客户确认");
|
||||
});
|
||||
it("drops only a malformed status_name, keeping the issue and the list", () => {
|
||||
for (const bad of [42, { name: "x" }, ["x"], true]) {
|
||||
const parsed = ListIssuesResponseSchema.parse({
|
||||
issues: [{ ...baseIssue, status: "in_review_2", status_name: bad }],
|
||||
total: 1,
|
||||
});
|
||||
expect(parsed.issues).toHaveLength(1);
|
||||
expect(parsed.issues[0]?.id).toBe(baseIssue.id);
|
||||
expect(parsed.issues[0]?.status).toBe("in_review_2");
|
||||
expect(parsed.issues[0]?.status_name).toBeUndefined();
|
||||
}
|
||||
});
|
||||
it("still parses an issue from a server that does not send status_name", () => {
|
||||
const { status_name: _omitted, ...withoutName } = { ...baseIssue, status_name: "x" };
|
||||
const parsed = ListIssuesResponseSchema.parse({ issues: [withoutName], total: 1 });
|
||||
expect(parsed.issues[0]?.id).toBe(baseIssue.id);
|
||||
expect(parsed.issues[0]?.status_name).toBeUndefined();
|
||||
});
|
||||
it("keeps the issue while independently dropping a malformed source context", () => {
|
||||
const parsed = ListIssuesResponseSchema.parse({
|
||||
issues: [{ ...baseIssue, source_context: { snapshot: "bad" } }],
|
||||
|
||||
@@ -1206,6 +1206,18 @@ export const IssueSchema = z.object({
|
||||
// consumers must fall back to `status` rather than treat "" as a category.
|
||||
// (MUL-6243)
|
||||
status_category: z.string().optional(),
|
||||
// A CUSTOM status's display name; "" for a built-in, which clients localize
|
||||
// from the key. Optional so a response from a server that predates the field
|
||||
// still validates.
|
||||
//
|
||||
// .catch(undefined) because this is ADDITIVE display data and the failure it
|
||||
// guards against is disproportionate: a parse failure anywhere in IssueSchema
|
||||
// takes the whole response through parseWithFallback to EMPTY_LIST_ISSUES_RESPONSE,
|
||||
// so one malformed name from a mixed-version deploy would blank an entire
|
||||
// issue list. Same treatment as source_context and labels above. Nothing reads
|
||||
// this field to make a decision — useStatusLabel resolves the label from the
|
||||
// catalog — so dropping it costs a fallback to the key. (MUL-6749)
|
||||
status_name: z.string().optional().catch(undefined),
|
||||
priority: z.string(),
|
||||
assignee_type: z.string().nullable(),
|
||||
assignee_id: z.string().nullable(),
|
||||
|
||||
@@ -172,6 +172,15 @@ export interface Issue {
|
||||
* (MUL-6243)
|
||||
*/
|
||||
status_category?: IssueStatusCategory;
|
||||
/**
|
||||
* A CUSTOM status's display name, carried beside the key. Empty for the 7
|
||||
* built-ins, which are localized from the key — prefer `useStatusLabel`,
|
||||
* which handles both and stays correct when an admin renames a status.
|
||||
*
|
||||
* Optional only for compatibility with a server that predates it; a current
|
||||
* server always sends the field. (MUL-6749)
|
||||
*/
|
||||
status_name?: string;
|
||||
priority: IssuePriority;
|
||||
assignee_type: IssueAssigneeType | null;
|
||||
assignee_id: string | null;
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
"name": "Name",
|
||||
"name_placeholder": "e.g. Code Review",
|
||||
"key_hint": "Referred to as {{key}} by the API and the CLI.",
|
||||
"created": "Status created. The API and the CLI refer to it as {{key}}.",
|
||||
"category": "Category",
|
||||
"category_locked": "Category cannot be changed — issues already on this status depend on it.",
|
||||
"description": "Description",
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
"name": "名前",
|
||||
"name_placeholder": "例: Code Review",
|
||||
"key_hint": "API と CLI では {{key}} として参照されます。",
|
||||
"created": "ステータスを作成しました。API と CLI では {{key}} として参照されます。",
|
||||
"category": "カテゴリ",
|
||||
"category_locked": "カテゴリは変更できません。すでにこのステータスのタスクが依存しています。",
|
||||
"description": "説明",
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
"name": "이름",
|
||||
"name_placeholder": "예: Code Review",
|
||||
"key_hint": "API와 CLI에서는 {{key}}로 참조합니다.",
|
||||
"created": "상태를 만들었습니다. API와 CLI에서는 {{key}}로 참조합니다.",
|
||||
"category": "카테고리",
|
||||
"category_locked": "카테고리는 변경할 수 없습니다. 이미 이 상태인 태스크가 이에 의존합니다.",
|
||||
"description": "설명",
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
"name": "名称",
|
||||
"name_placeholder": "例如:Code Review",
|
||||
"key_hint": "API 和 CLI 中通过 {{key}} 引用。",
|
||||
"created": "状态已创建。API 和 CLI 中通过 {{key}} 引用。",
|
||||
"category": "分类",
|
||||
"category_locked": "分类不可修改 —— 已经处于该状态的任务依赖它。",
|
||||
"description": "描述",
|
||||
|
||||
@@ -524,7 +524,22 @@ function StatusEditorDialog({
|
||||
category: draft.category,
|
||||
color: draft.color,
|
||||
},
|
||||
{ onSuccess: () => onOpenChange(false), onError },
|
||||
{
|
||||
onSuccess: (created) => {
|
||||
onOpenChange(false);
|
||||
// The key is derived server-side and is the only handle the API and
|
||||
// the CLI accept, so creation has to say what it minted. A name with
|
||||
// no ASCII to slug gets one that cannot be guessed back from the name
|
||||
// — "客户确认" becomes `in_review_2` (MUL-6749) — so staying silent
|
||||
// would leave the admin no way to learn it short of reopening the
|
||||
// row. The dialog is already closing; a toast is the one surface
|
||||
// still visible.
|
||||
if (created?.key) {
|
||||
toast.success(t(($) => $.issue_statuses.editor.created, { key: created.key }));
|
||||
}
|
||||
},
|
||||
onError,
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
|
||||
@@ -220,8 +220,10 @@ var issueAssignCmd = &cobra.Command{
|
||||
var issueStatusCmd = &cobra.Command{
|
||||
Use: "status <id> <status>",
|
||||
Short: "Change issue status",
|
||||
Long: "Change an issue's status. Valid statuses: " +
|
||||
"backlog, todo, in_progress, in_review, done, blocked, cancelled.",
|
||||
Long: "Change an issue's status. The argument is a status KEY, not its display name.\n" +
|
||||
"Built-in keys: backlog, todo, in_progress, in_review, done, blocked, cancelled.\n" +
|
||||
"A workspace may define custom statuses on top of these; their keys are shown in\n" +
|
||||
"Workspace Settings > Issue Statuses, and an unknown value errors with the full list.",
|
||||
Args: exactArgs(2),
|
||||
RunE: runIssueStatus,
|
||||
}
|
||||
|
||||
@@ -46,15 +46,29 @@ type IssueResponse struct {
|
||||
// category for a custom status. Omitted when the endpoint does not resolve
|
||||
// it, so consumers must fall back to Status rather than assume a blank
|
||||
// value means "no category". (MUL-6243)
|
||||
StatusCategory string `json:"status_category,omitempty"`
|
||||
Priority string `json:"priority"`
|
||||
AssigneeType *string `json:"assignee_type"`
|
||||
AssigneeID *string `json:"assignee_id"`
|
||||
CreatorType string `json:"creator_type"`
|
||||
CreatorID string `json:"creator_id"`
|
||||
ParentIssueID *string `json:"parent_issue_id"`
|
||||
ProjectID *string `json:"project_id"`
|
||||
Position float64 `json:"position"`
|
||||
StatusCategory string `json:"status_category,omitempty"`
|
||||
// StatusName is a CUSTOM status's display name, carried beside the key so a
|
||||
// consumer that only ever sees `status` is not left holding a bare handle.
|
||||
// A key derived from a non-Latin name is opaque by construction
|
||||
// (`in_review_2`), and an agent reading an issue has nothing else to match
|
||||
// against the status a human named for it.
|
||||
//
|
||||
// Always emitted, unlike StatusCategory. Empty is a MEANING here — "this is
|
||||
// a built-in, localize it from the key" — not the "this endpoint did not
|
||||
// resolve it" that an absent status_category signals. Keeping the key
|
||||
// present is also what lets TestIssueToMap_KeysMatchIssueResponse see the
|
||||
// field at all: with omitempty a built-in fixture hides it from BOTH
|
||||
// renderings, and the drift guard goes green on a payload that has drifted.
|
||||
// (MUL-6749)
|
||||
StatusName string `json:"status_name"`
|
||||
Priority string `json:"priority"`
|
||||
AssigneeType *string `json:"assignee_type"`
|
||||
AssigneeID *string `json:"assignee_id"`
|
||||
CreatorType string `json:"creator_type"`
|
||||
CreatorID string `json:"creator_id"`
|
||||
ParentIssueID *string `json:"parent_issue_id"`
|
||||
ProjectID *string `json:"project_id"`
|
||||
Position float64 `json:"position"`
|
||||
// Stage groups sub-issues under the same parent into ordered barrier
|
||||
// groups (null = unstaged). See issue_child_done.go for how a closed
|
||||
// stage gates the child-done -> parent wake.
|
||||
@@ -129,7 +143,10 @@ func (h *Handler) resolveIssueStatusKeyKind(w http.ResponseWriter, r *http.Reque
|
||||
entry, err := issuestatus.Resolve(r.Context(), h.Queries, workspaceID, status)
|
||||
if err != nil {
|
||||
if errors.Is(err, issuestatus.ErrUnknownStatus) {
|
||||
allowed, listErr := issuestatus.ActiveKeys(r.Context(), h.Queries, workspaceID)
|
||||
// Labels, not bare keys: a derived key says nothing about what the
|
||||
// status means, so listing `in_review_2` alone leaves the caller no
|
||||
// way to find the one they were told to use. (MUL-6749)
|
||||
allowed, listErr := issuestatus.ActiveKeyLabels(r.Context(), h.Queries, workspaceID)
|
||||
if listErr != nil || len(allowed) == 0 {
|
||||
allowed = issuestatus.Canonical()
|
||||
}
|
||||
@@ -260,6 +277,9 @@ func (h *Handler) newStatusCategoryFiller(ctx context.Context, wsID pgtype.UUID)
|
||||
return
|
||||
}
|
||||
resp.StatusCategory = resolver.Effective(ctx, h.Queries, resp.Status)
|
||||
// Same Resolver, same single catalog read, so the name rides along for
|
||||
// free. Built-ins return "" and stay omitted. (MUL-6749)
|
||||
resp.StatusName = resolver.Name(ctx, h.Queries, resp.Status)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
@@ -155,28 +156,31 @@ func (h *Handler) CreateIssueStatus(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// An explicit key wins; otherwise derive one from the name so the common
|
||||
// case is a single field. Either way it is validated against the reserved
|
||||
// built-in keys and the storage pattern.
|
||||
var key string
|
||||
// An explicit key wins and is checked here, against the reserved built-in
|
||||
// names and the storage pattern. Without one the key is DERIVED from the
|
||||
// display name, which needs the catalog and so happens under the lock in
|
||||
// createIssueStatusEntry.
|
||||
var explicitKey string
|
||||
if strings.TrimSpace(req.Key) != "" {
|
||||
key, err = issuestatus.ValidateKey(req.Key)
|
||||
} else {
|
||||
key, err = issuestatus.SlugifyKey(name)
|
||||
}
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
explicitKey, err = issuestatus.ValidateKey(req.Key)
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
entry, err := h.Queries.CreateIssueStatusEntry(r.Context(), db.CreateIssueStatusEntryParams{
|
||||
entry, badRequest, err := h.createIssueStatusEntry(r.Context(), wsUUID, db.CreateIssueStatusEntryParams{
|
||||
WorkspaceID: wsUUID,
|
||||
Key: key,
|
||||
Key: explicitKey,
|
||||
Name: name,
|
||||
Description: req.Description,
|
||||
Category: req.Category,
|
||||
Color: strings.ToLower(color),
|
||||
})
|
||||
if badRequest != "" {
|
||||
writeError(w, http.StatusBadRequest, badRequest)
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
if isUniqueViolation(err) {
|
||||
writeError(w, http.StatusConflict, "a status with this key or name already exists")
|
||||
@@ -190,6 +194,69 @@ func (h *Handler) CreateIssueStatus(w http.ResponseWriter, r *http.Request) {
|
||||
writeJSON(w, http.StatusCreated, issueStatusToResponse(entry))
|
||||
}
|
||||
|
||||
// createIssueStatusEntry writes one catalog row, deriving arg.Key from the
|
||||
// display name when it arrives empty.
|
||||
//
|
||||
// Derivation READS the catalog to choose a key nothing already owns, so the
|
||||
// read and the insert have to be a single atomic step: two admins creating a
|
||||
// Chinese-named in_review status at the same instant would otherwise both
|
||||
// compute `in_review_2`, and the loser would be told a key they never typed was
|
||||
// already taken. The EXCLUSIVE catalog lock — the same one archive takes —
|
||||
// serializes them.
|
||||
//
|
||||
// EVERY create takes that lock, including one that supplies its own key.
|
||||
// Excluding those would leave the race half-closed: an explicit-key insert of
|
||||
// `in_review_2` could still land between a derive's catalog read and its
|
||||
// insert, and the derive — a UI request with no key field to blame — would come
|
||||
// back 409. The lock is only contended by catalog writes, which are rare admin
|
||||
// actions, so serializing them costs nothing worth keeping the hole for.
|
||||
//
|
||||
// A non-empty second return is a caller error the handler reports as 400,
|
||||
// distinct from a nil-error success and from an infrastructure failure.
|
||||
func (h *Handler) createIssueStatusEntry(ctx context.Context, workspaceID pgtype.UUID, arg db.CreateIssueStatusEntryParams) (db.IssueStatus, string, error) {
|
||||
tx, err := h.TxStarter.Begin(ctx)
|
||||
if err != nil {
|
||||
return db.IssueStatus{}, "", err
|
||||
}
|
||||
defer tx.Rollback(ctx)
|
||||
qtx := h.Queries.WithTx(tx)
|
||||
|
||||
if err := qtx.LockIssueStatusCatalog(ctx, workspaceID); err != nil {
|
||||
return db.IssueStatus{}, "", err
|
||||
}
|
||||
|
||||
if arg.Key == "" {
|
||||
// IncludeArchived, because idx_issue_status_workspace_key is NOT a
|
||||
// partial index: a retired status still owns its key, so reusing it
|
||||
// would fail on insert instead of producing a second candidate.
|
||||
entries, err := qtx.ListIssueStatusEntries(ctx, db.ListIssueStatusEntriesParams{
|
||||
WorkspaceID: workspaceID,
|
||||
IncludeArchived: true,
|
||||
})
|
||||
if err != nil {
|
||||
return db.IssueStatus{}, "", err
|
||||
}
|
||||
taken := make(map[string]bool, len(entries))
|
||||
for _, e := range entries {
|
||||
taken[e.Key] = true
|
||||
}
|
||||
key, err := issuestatus.DeriveKey(arg.Name, arg.Category, taken)
|
||||
if err != nil {
|
||||
return db.IssueStatus{}, err.Error(), nil
|
||||
}
|
||||
arg.Key = key
|
||||
}
|
||||
|
||||
entry, err := qtx.CreateIssueStatusEntry(ctx, arg)
|
||||
if err != nil {
|
||||
return db.IssueStatus{}, "", err
|
||||
}
|
||||
if err := tx.Commit(ctx); err != nil {
|
||||
return db.IssueStatus{}, "", err
|
||||
}
|
||||
return entry, "", nil
|
||||
}
|
||||
|
||||
// UpdateIssueStatus edits a custom status's presentation. Built-in statuses are
|
||||
// immutable in v1 — name and color included — so the default workspace looks
|
||||
// and behaves identically for everyone who never opens this settings page.
|
||||
|
||||
@@ -7,6 +7,8 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -1196,7 +1198,7 @@ func TestListEndpointsCarryStatusCategory(t *testing.T) {
|
||||
|
||||
// TestBackgroundEventCarriesCustomStatusCategory pins the background-event
|
||||
// payload. IssueToMap fills a category only for built-ins; the publishers that
|
||||
// can emit a CUSTOM status go through IssueToMapWithCategory so clients receive
|
||||
// can emit a CUSTOM status go through IssueToMapResolved so clients receive
|
||||
// an authoritative one instead of a blank they would have to refetch to resolve.
|
||||
func TestBackgroundEventCarriesCustomStatusCategory(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
@@ -1216,9 +1218,9 @@ func TestBackgroundEventCarriesCustomStatusCategory(t *testing.T) {
|
||||
plain["status_category"])
|
||||
}
|
||||
|
||||
authoritative := service.IssueToMapWithCategory(ctx, testHandler.Queries, issue, "MUL")
|
||||
authoritative := service.IssueToMapResolved(ctx, testHandler.Queries, issue, "MUL")
|
||||
if authoritative["status_category"] != "in_review" {
|
||||
t.Errorf("IssueToMapWithCategory status_category = %v, want in_review",
|
||||
t.Errorf("IssueToMapResolved status_category = %v, want in_review",
|
||||
authoritative["status_category"])
|
||||
}
|
||||
if authoritative["status"] != "human_review_bg" {
|
||||
@@ -1330,3 +1332,449 @@ func TestCatalogWritesAnnounceThemselves(t *testing.T) {
|
||||
}
|
||||
expectNoChange(t)
|
||||
}
|
||||
|
||||
// TestCreateIssueStatusAcceptsANonLatinName is the regression for MUL-6749 /
|
||||
// GitHub #7627. A display name written entirely in a non-Latin script has no
|
||||
// characters in the key alphabet, so key derivation used to fail the create
|
||||
// outright — and the settings form has no field for an explicit key, which left
|
||||
// no way to create the status at all.
|
||||
func TestCreateIssueStatusAcceptsANonLatinName(t *testing.T) {
|
||||
seedTestCatalog(t)
|
||||
|
||||
create := func(t *testing.T, name, category string) IssueStatusResponse {
|
||||
t.Helper()
|
||||
var created IssueStatusResponse
|
||||
testutil.Call(t, testHandler.CreateIssueStatus,
|
||||
newRequest(http.MethodPost, "/api/issue-statuses", map[string]any{
|
||||
"name": name, "category": category, "color": "#123456",
|
||||
})).Want(http.StatusCreated).JSON(&created)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, parseUUID(created.ID))
|
||||
})
|
||||
return created
|
||||
}
|
||||
|
||||
first := create(t, "客户确认", issuestatus.InReview)
|
||||
if first.Key != "in_review_2" {
|
||||
t.Errorf("derived key = %q, want %q", first.Key, "in_review_2")
|
||||
}
|
||||
if first.Name != "客户确认" {
|
||||
t.Errorf("display name = %q, want it stored verbatim", first.Name)
|
||||
}
|
||||
|
||||
// A second one in the same category takes the next ordinal instead of
|
||||
// colliding with the first.
|
||||
second := create(t, "供应商确认", issuestatus.InReview)
|
||||
if second.Key != "in_review_3" {
|
||||
t.Errorf("second derived key = %q, want %q", second.Key, "in_review_3")
|
||||
}
|
||||
|
||||
// A name that CAN be slugged is untouched by the fallback.
|
||||
english := create(t, "Human Review Zh", issuestatus.InReview)
|
||||
if english.Key != "human_review_zh" {
|
||||
t.Errorf("sluggable name derived %q, want %q", english.Key, "human_review_zh")
|
||||
}
|
||||
}
|
||||
|
||||
// TestCreateIssueStatusDisambiguatesCollidingSlugs covers the sharper half of
|
||||
// #7627. The bug report's suggested workaround was to mix ASCII into the
|
||||
// display name, but the non-ASCII part is dropped, so two DIFFERENT names
|
||||
// collapse onto one key — and the second create used to fail with a conflict
|
||||
// that blamed a display name nobody had taken.
|
||||
func TestCreateIssueStatusDisambiguatesCollidingSlugs(t *testing.T) {
|
||||
seedTestCatalog(t)
|
||||
|
||||
create := func(t *testing.T, name string) IssueStatusResponse {
|
||||
t.Helper()
|
||||
var created IssueStatusResponse
|
||||
testutil.Call(t, testHandler.CreateIssueStatus,
|
||||
newRequest(http.MethodPost, "/api/issue-statuses", map[string]any{
|
||||
"name": name, "category": issuestatus.Todo, "color": "#123456",
|
||||
})).Want(http.StatusCreated).JSON(&created)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, parseUUID(created.ID))
|
||||
})
|
||||
return created
|
||||
}
|
||||
|
||||
if got := create(t, "待客户 Zzreview").Key; got != "zzreview" {
|
||||
t.Fatalf("first key = %q, want %q", got, "zzreview")
|
||||
}
|
||||
if got := create(t, "待供应商 Zzreview").Key; got != "zzreview_2" {
|
||||
t.Errorf("colliding key = %q, want %q", got, "zzreview_2")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDerivedKeyAvoidsAnArchivedKey pins the storage constraint that makes the
|
||||
// "taken" set wider than the visible catalog: idx_issue_status_workspace_key is
|
||||
// NOT partial, so an archived status still owns its key and handing it out
|
||||
// again would fail on insert.
|
||||
func TestDerivedKeyAvoidsAnArchivedKey(t *testing.T) {
|
||||
entry := createTestCustomStatus(t, "zzarchived", issuestatus.Todo)
|
||||
if _, err := testHandler.Queries.ArchiveIssueStatusEntry(context.Background(), db.ArchiveIssueStatusEntryParams{
|
||||
ID: entry.ID,
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
}); err != nil {
|
||||
t.Fatalf("archive: %v", err)
|
||||
}
|
||||
|
||||
var created IssueStatusResponse
|
||||
testutil.Call(t, testHandler.CreateIssueStatus,
|
||||
newRequest(http.MethodPost, "/api/issue-statuses", map[string]any{
|
||||
"name": "Zzarchived", "category": issuestatus.Todo, "color": "#123456",
|
||||
})).Want(http.StatusCreated).JSON(&created)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, parseUUID(created.ID))
|
||||
})
|
||||
|
||||
if created.Key == "zzarchived" {
|
||||
t.Fatal("derivation reused an archived status's key")
|
||||
}
|
||||
if created.Key != "zzarchived_2" {
|
||||
t.Errorf("derived key = %q, want %q", created.Key, "zzarchived_2")
|
||||
}
|
||||
}
|
||||
|
||||
// TestUnknownStatusErrorNamesCustomStatuses covers the other half of what makes
|
||||
// a derived key usable: on its own `in_review_2` says nothing, so the error a
|
||||
// caller gets after writing a bad status has to carry the display name or there
|
||||
// is no way to find the status they were told to use. (MUL-6749)
|
||||
func TestUnknownStatusErrorNamesCustomStatuses(t *testing.T) {
|
||||
seedTestCatalog(t)
|
||||
entry, err := testHandler.Queries.CreateIssueStatusEntry(context.Background(), db.CreateIssueStatusEntryParams{
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
Key: "in_review_9",
|
||||
Name: "客户确认",
|
||||
Description: "",
|
||||
Category: issuestatus.InReview,
|
||||
Color: "#123456",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create custom status: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, entry.ID)
|
||||
})
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
testHandler.CreateIssue(rec, newRequest(http.MethodPost, "/api/issues", map[string]any{
|
||||
"title": "unknown status error body",
|
||||
"status": "not_a_status",
|
||||
}))
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
body := rec.Body.String()
|
||||
if !strings.Contains(body, "in_review_9 (客户确认)") {
|
||||
t.Errorf("error body does not pair the key with its name: %s", body)
|
||||
}
|
||||
// Built-ins stay bare — clients localize those from the key, so echoing the
|
||||
// seeded English name would be the one string a Chinese workspace ignores.
|
||||
if strings.Contains(body, "todo (Todo)") {
|
||||
t.Errorf("built-in statuses should be listed as bare keys: %s", body)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssueResponseCarriesCustomStatusName pins the field an agent reads an
|
||||
// issue through. `status` alone is a bare handle, and a derived key carries no
|
||||
// meaning, so the display name travels beside it. (MUL-6749)
|
||||
func TestIssueResponseCarriesCustomStatusName(t *testing.T) {
|
||||
seedTestCatalog(t)
|
||||
entry, err := testHandler.Queries.CreateIssueStatusEntry(context.Background(), db.CreateIssueStatusEntryParams{
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
Key: "in_review_8",
|
||||
Name: "客户确认",
|
||||
Description: "",
|
||||
Category: issuestatus.InReview,
|
||||
Color: "#123456",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create custom status: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, entry.ID)
|
||||
})
|
||||
|
||||
var custom IssueResponse
|
||||
testutil.Call(t, testHandler.CreateIssue,
|
||||
newRequest(http.MethodPost, "/api/issues", map[string]any{
|
||||
"title": "custom status name", "status": "in_review_8",
|
||||
})).Want(http.StatusCreated).JSON(&custom)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, parseUUID(custom.ID))
|
||||
})
|
||||
if custom.StatusName != "客户确认" {
|
||||
t.Errorf("status_name = %q, want %q", custom.StatusName, "客户确认")
|
||||
}
|
||||
if custom.StatusCategory != issuestatus.InReview {
|
||||
t.Errorf("status_category = %q, want %q", custom.StatusCategory, issuestatus.InReview)
|
||||
}
|
||||
|
||||
// A built-in carries no name: every client renders those from the key
|
||||
// through i18n, so the seeded English one would be noise at best.
|
||||
var builtIn IssueResponse
|
||||
testutil.Call(t, testHandler.CreateIssue,
|
||||
newRequest(http.MethodPost, "/api/issues", map[string]any{
|
||||
"title": "built-in status name", "status": issuestatus.Todo,
|
||||
})).Want(http.StatusCreated).JSON(&builtIn)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, parseUUID(builtIn.ID))
|
||||
})
|
||||
if builtIn.StatusName != "" {
|
||||
t.Errorf("built-in status_name = %q, want it empty", builtIn.StatusName)
|
||||
}
|
||||
}
|
||||
|
||||
// TestExplicitKeyCreateAlsoTakesTheCatalogLock closes the half-open race found
|
||||
// in review of MUL-6749. Derivation reads the catalog and then inserts; if a
|
||||
// create that supplies its OWN key were allowed to skip the lock, it could land
|
||||
// on the key the derive just chose in exactly that gap. The derive would then
|
||||
// fail on the unique index and return 409 to a settings form that has no key
|
||||
// field — the same dead end this issue exists to remove.
|
||||
//
|
||||
// The assertion is that an explicit-key create PARKS while the lock is held.
|
||||
// Before the fix it completed immediately.
|
||||
func TestExplicitKeyCreateAlsoTakesTheCatalogLock(t *testing.T) {
|
||||
seedTestCatalog(t)
|
||||
ctx := context.Background()
|
||||
|
||||
tx, err := testPool.Begin(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("begin: %v", err)
|
||||
}
|
||||
defer tx.Rollback(ctx)
|
||||
if _, err := tx.Exec(ctx,
|
||||
`SELECT pg_advisory_xact_lock(hashtextextended($1::uuid::text || ':issue_status', 0))`,
|
||||
parseUUID(testWorkspaceID)); err != nil {
|
||||
t.Fatalf("take exclusive lock: %v", err)
|
||||
}
|
||||
|
||||
done := make(chan *httptest.ResponseRecorder, 1)
|
||||
go func() {
|
||||
rec := httptest.NewRecorder()
|
||||
testHandler.CreateIssueStatus(rec, newRequest(http.MethodPost, "/api/issue-statuses", map[string]any{
|
||||
"name": "Zzlockprobe", "key": "zzlockprobe", "category": issuestatus.Todo, "color": "#123456",
|
||||
}))
|
||||
done <- rec
|
||||
}()
|
||||
|
||||
select {
|
||||
case rec := <-done:
|
||||
t.Fatalf("an explicit-key create completed (%d) while the catalog lock was held; "+
|
||||
"it can still insert between a derived create's catalog read and its insert", rec.Code)
|
||||
case <-time.After(400 * time.Millisecond):
|
||||
// Parked on the lock, which is the point.
|
||||
}
|
||||
|
||||
if err := tx.Commit(ctx); err != nil {
|
||||
t.Fatalf("release lock: %v", err)
|
||||
}
|
||||
|
||||
select {
|
||||
case rec := <-done:
|
||||
if rec.Code != http.StatusCreated {
|
||||
t.Fatalf("explicit-key create after the lock released: %d %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var created IssueStatusResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &created); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, parseUUID(created.ID))
|
||||
})
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("explicit-key create never completed after the lock released")
|
||||
}
|
||||
}
|
||||
|
||||
// TestConcurrentDerivedCreatesNeverConflict is the behavioral half of the same
|
||||
// fix: whatever else is writing the catalog, a create that supplied no key must
|
||||
// never come back 409. A caller who typed a key CAN legitimately be told it is
|
||||
// taken; a caller who typed only a display name has nothing to correct.
|
||||
//
|
||||
// Every result carries WHICH kind of writer produced it. Counting successes
|
||||
// without that — the first version of this test did — lets an explicit writer's
|
||||
// 201 stand in for a derived writer's 409 and passes on exactly the regression
|
||||
// it exists to catch.
|
||||
//
|
||||
// This is a probabilistic net, not a proof: the window it hunts for is the few
|
||||
// microseconds between a derive's catalog read and its insert, and in-process
|
||||
// it does not reliably open. TestExplicitKeyCreateAlsoTakesTheCatalogLock is
|
||||
// the deterministic guard on the lock itself — this one guards the OUTCOME, so
|
||||
// that any future path which lets a keyless create conflict shows up here even
|
||||
// if the lock is still nominally taken. Do not delete one for the other.
|
||||
func TestConcurrentDerivedCreatesNeverConflict(t *testing.T) {
|
||||
seedTestCatalog(t)
|
||||
|
||||
const derivedWriters = 6
|
||||
const rounds = 4
|
||||
type result struct {
|
||||
derived bool
|
||||
label string
|
||||
code int
|
||||
key string
|
||||
body string
|
||||
}
|
||||
|
||||
// Every derived name must be ALL non-ASCII, or it slugs to something and
|
||||
// never reaches the fallback the test is about: "客户确认0-1" keeps its ASCII
|
||||
// digits and derives `0_1`, which contends with nothing. Uniqueness comes
|
||||
// from a distinct base per writer and a repeated character per round, so the
|
||||
// names stay collision-free without smuggling in a digit.
|
||||
bases := []string{"客户确认", "供应商确认", "财务确认", "法务确认", "安全确认", "运营确认"}
|
||||
if len(bases) != derivedWriters {
|
||||
t.Fatalf("need one non-ASCII base per derived writer, got %d for %d", len(bases), derivedWriters)
|
||||
}
|
||||
|
||||
for round := range rounds {
|
||||
t.Run(fmt.Sprintf("round-%d", round), func(t *testing.T) {
|
||||
results := make(chan result, derivedWriters+2)
|
||||
start := make(chan struct{})
|
||||
var wg sync.WaitGroup
|
||||
|
||||
post := func(derived bool, label string, body map[string]any) {
|
||||
defer wg.Done()
|
||||
<-start
|
||||
rec := httptest.NewRecorder()
|
||||
testHandler.CreateIssueStatus(rec, newRequest(http.MethodPost, "/api/issue-statuses", body))
|
||||
var created IssueStatusResponse
|
||||
json.Unmarshal(rec.Body.Bytes(), &created)
|
||||
results <- result{derived, label, rec.Code, created.Key, rec.Body.String()}
|
||||
}
|
||||
|
||||
// Admins naming a status in a non-Latin script, all in one category,
|
||||
// so every one of them derives from the same base.
|
||||
for i := range derivedWriters {
|
||||
name := bases[i] + strings.Repeat("确", round+1)
|
||||
wg.Add(1)
|
||||
go post(true, name, map[string]any{
|
||||
"name": name, "category": issuestatus.Blocked, "color": "#123456",
|
||||
})
|
||||
}
|
||||
// Explicit writers aiming straight at the ordinals the derives want.
|
||||
for _, key := range []string{"blocked_2", "blocked_3"} {
|
||||
wg.Add(1)
|
||||
go post(false, key, map[string]any{
|
||||
"name": fmt.Sprintf("Zz %s %d", key, round), "key": key,
|
||||
"category": issuestatus.Blocked, "color": "#123456",
|
||||
})
|
||||
}
|
||||
|
||||
close(start)
|
||||
wg.Wait()
|
||||
close(results)
|
||||
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(),
|
||||
`DELETE FROM issue_status WHERE workspace_id = $1 AND category = 'blocked' AND is_system = FALSE`,
|
||||
parseUUID(testWorkspaceID))
|
||||
})
|
||||
|
||||
seen := map[string]bool{}
|
||||
derivedSucceeded := 0
|
||||
for r := range results {
|
||||
switch {
|
||||
case r.derived && r.code != http.StatusCreated:
|
||||
// The regression this test exists for: a writer with no key
|
||||
// field to correct was told to correct one.
|
||||
t.Errorf("derived create %q returned %d, want 201 — a create with no key must never conflict: %s",
|
||||
r.label, r.code, r.body)
|
||||
continue
|
||||
case !r.derived && r.code != http.StatusCreated && r.code != http.StatusConflict:
|
||||
t.Errorf("explicit create %q returned %d, want 201 or 409: %s", r.label, r.code, r.body)
|
||||
continue
|
||||
case r.code == http.StatusConflict:
|
||||
// Only reachable for an explicit writer, by the branch above.
|
||||
if !strings.Contains(r.body, "already exists") {
|
||||
t.Errorf("unexpected 409 body for %q: %s", r.label, r.body)
|
||||
}
|
||||
continue
|
||||
}
|
||||
if seen[r.key] {
|
||||
t.Errorf("two statuses were created with the same key %q", r.key)
|
||||
}
|
||||
seen[r.key] = true
|
||||
if issuestatus.IsBuiltIn(r.key) {
|
||||
t.Errorf("a custom status shadowed the built-in key %q", r.key)
|
||||
}
|
||||
if r.derived {
|
||||
// Pins the PREMISE: a derived name that slugs to anything at
|
||||
// all takes the slug path and contends with nothing, which
|
||||
// would leave this test green while exercising nothing.
|
||||
if !strings.HasPrefix(r.key, issuestatus.Blocked+"_") {
|
||||
t.Errorf("derived create %q produced key %q; the name must slug to nothing "+
|
||||
"so it lands on the <category>_<n> fallback these writers contend for",
|
||||
r.label, r.key)
|
||||
}
|
||||
derivedSucceeded++
|
||||
}
|
||||
}
|
||||
if derivedSucceeded != derivedWriters {
|
||||
t.Errorf("%d of %d DERIVED creates succeeded", derivedSucceeded, derivedWriters)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCustomStatusPayloadsAgreeAcrossRenderings pins the contract
|
||||
// TestIssueToMap_KeysMatchIssueResponse cannot reach with a built-in fixture:
|
||||
// an issue on a CUSTOM status must describe itself identically whether it
|
||||
// arrives over HTTP or on a background event. A field present in one and blank
|
||||
// in the other reads back undefined depending on which entry point produced the
|
||||
// issue. (MUL-6749)
|
||||
func TestCustomStatusPayloadsAgreeAcrossRenderings(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
seedTestCatalog(t)
|
||||
entry, err := testHandler.Queries.CreateIssueStatusEntry(ctx, db.CreateIssueStatusEntryParams{
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
Key: "in_review_7",
|
||||
Name: "客户确认",
|
||||
Description: "",
|
||||
Category: issuestatus.InReview,
|
||||
Color: "#123456",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create custom status: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue_status WHERE id = $1`, entry.ID)
|
||||
})
|
||||
|
||||
var fromHTTP IssueResponse
|
||||
testutil.Call(t, testHandler.CreateIssue,
|
||||
newRequest(http.MethodPost, "/api/issues", map[string]any{
|
||||
"title": "payload parity", "status": "in_review_7",
|
||||
})).Want(http.StatusCreated).JSON(&fromHTTP)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, parseUUID(fromHTTP.ID))
|
||||
})
|
||||
|
||||
issue, err := testHandler.Queries.GetIssue(ctx, parseUUID(fromHTTP.ID))
|
||||
if err != nil {
|
||||
t.Fatalf("reload issue: %v", err)
|
||||
}
|
||||
fromEvent := service.IssueToMapResolved(ctx, testHandler.Queries, issue, "MUL")
|
||||
|
||||
for field, want := range map[string]string{
|
||||
"status": "in_review_7",
|
||||
"status_category": issuestatus.InReview,
|
||||
"status_name": "客户确认",
|
||||
} {
|
||||
got, ok := fromEvent[field].(string)
|
||||
if !ok {
|
||||
t.Errorf("event payload is missing %q; clients treat it as a complete issue", field)
|
||||
continue
|
||||
}
|
||||
if got != want {
|
||||
t.Errorf("event %s = %q, want %q", field, got, want)
|
||||
}
|
||||
}
|
||||
if fromHTTP.StatusName != fromEvent["status_name"] {
|
||||
t.Errorf("status_name differs by entry point: HTTP %q, event %q",
|
||||
fromHTTP.StatusName, fromEvent["status_name"])
|
||||
}
|
||||
if fromHTTP.StatusCategory != fromEvent["status_category"] {
|
||||
t.Errorf("status_category differs by entry point: HTTP %q, event %q",
|
||||
fromHTTP.StatusCategory, fromEvent["status_category"])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,6 +27,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
@@ -131,9 +132,17 @@ func ValidateKey(key string) (string, error) {
|
||||
return key, nil
|
||||
}
|
||||
|
||||
// SlugifyKey derives a candidate key from a display name, for callers that let
|
||||
// an admin type only a name. Returns an error when nothing usable survives.
|
||||
func SlugifyKey(name string) (string, error) {
|
||||
// maxKeyLen mirrors the 32-character ceiling in keyPattern and the issue_status
|
||||
// CHECK. Suffixing a key has to stay under it, so the base gets truncated
|
||||
// rather than the suffix dropped — a truncated base is still unique-able, a
|
||||
// dropped suffix is not.
|
||||
const maxKeyLen = 32
|
||||
|
||||
// slugify reduces a display name to the ASCII key alphabet, returning "" when
|
||||
// nothing survives. Lowercase because `multica issue status <id> human_review`
|
||||
// has to be unambiguous to type; runs of everything else collapse to a single
|
||||
// underscore so "Gate — Approved!" does not become "gate___approved".
|
||||
func slugify(name string) string {
|
||||
var b strings.Builder
|
||||
lastUnderscore := false
|
||||
for _, r := range strings.ToLower(strings.TrimSpace(name)) {
|
||||
@@ -147,13 +156,109 @@ func SlugifyKey(name string) (string, error) {
|
||||
}
|
||||
}
|
||||
slug := strings.Trim(b.String(), "_")
|
||||
if len(slug) > 32 {
|
||||
slug = strings.Trim(slug[:32], "_")
|
||||
if len(slug) > maxKeyLen {
|
||||
slug = strings.Trim(slug[:maxKeyLen], "_")
|
||||
}
|
||||
if slug == "" {
|
||||
return "", errors.New("cannot derive a status key from that name; provide one explicitly")
|
||||
return slug
|
||||
}
|
||||
|
||||
// DeriveKey picks the stored key for a status whose creator supplied only a
|
||||
// display name (MUL-6749 / GitHub #7627).
|
||||
//
|
||||
// The key is an ASCII machine handle — it is what issue.status stores, what the
|
||||
// CLI takes as an argument, and what the column's CHECK constraint permits —
|
||||
// while the display name is free Unicode. Deriving one from the other therefore
|
||||
// has to survive names that share no alphabet with it:
|
||||
//
|
||||
// - a name that slugs to something usable keeps that slug, so an
|
||||
// English-named status still gets the readable `human_review` it always
|
||||
// got;
|
||||
// - a name written entirely in a non-Latin script slugs to nothing, and falls
|
||||
// back to its CATEGORY plus an ordinal. Category, not a random suffix: it
|
||||
// is already the anchor an agent reasons from, so `in_review_2` still says
|
||||
// which platform behavior the status inherits, and it degrades into the
|
||||
// right neighborhood when a model confuses it with its category. The
|
||||
// meaning of the status travels with its name and description, which the
|
||||
// agent brief prints beside the key.
|
||||
//
|
||||
// Either way the result is disambiguated against keys the workspace already
|
||||
// owns, so two names that collapse onto one slug ("待客户 Review" and
|
||||
// "待供应商 Review" both slug to "review") no longer make the second create
|
||||
// fail with a conflict that blames the display name.
|
||||
//
|
||||
// taken reports whether a key already exists in this workspace and MUST count
|
||||
// ARCHIVED statuses: idx_issue_status_workspace_key is not partial, so an
|
||||
// archived row still owns its key. Built-in keys are treated as taken here
|
||||
// regardless of what taken says, which both stops a custom status from
|
||||
// shadowing one and is what makes the fallback start at `<category>_2`.
|
||||
//
|
||||
// Derivation is expected to succeed for any name that reaches it. The one
|
||||
// refusal left is a name that slugs ONTO a built-in key: that is a naming
|
||||
// collision the admin should see rather than have quietly renamed, and it is
|
||||
// unchanged from before.
|
||||
func DeriveKey(name, category string, taken map[string]bool) (string, error) {
|
||||
if slug := slugify(name); slug != "" {
|
||||
if IsBuiltIn(slug) {
|
||||
return "", fmt.Errorf("%q is a built-in status key and cannot be reused; rename the status or pass an explicit key", slug)
|
||||
}
|
||||
return firstFreeKey(slug, taken)
|
||||
}
|
||||
return ValidateKey(slug)
|
||||
// Nothing in the name belongs to the key alphabet. Category is a valid key
|
||||
// by construction, and its built-in always occupies it, so this lands on
|
||||
// <category>_2 for the first such status in that category.
|
||||
if !IsCategory(category) {
|
||||
return "", fmt.Errorf("category must be one of: %s", strings.Join(canonicalOrder, ", "))
|
||||
}
|
||||
return firstFreeKey(category, taken)
|
||||
}
|
||||
|
||||
// firstFreeKey returns base when it is unclaimed, otherwise the first
|
||||
// base_<n> that is. n starts at 2 so the series reads as "the second one".
|
||||
//
|
||||
// The scan is bounded by the CATALOG, not by a policy number. Every candidate
|
||||
// it tests is distinct, and at most len(taken)+7 keys can be occupied (the
|
||||
// workspace's own plus the built-ins), so by the pigeonhole principle a free
|
||||
// one has to turn up within that many attempts plus one. Picking a round
|
||||
// constant instead would invent a cap on custom statuses that exists nowhere
|
||||
// else in the product, and would fail with "provide one explicitly" — the very
|
||||
// error this package was changed to stop showing a UI that has no key field.
|
||||
func firstFreeKey(base string, taken map[string]bool) (string, error) {
|
||||
if !keyOccupied(base, taken) {
|
||||
return ValidateKey(base)
|
||||
}
|
||||
limit := len(taken) + len(canonicalOrder) + 2
|
||||
for n := 2; n <= limit; n++ {
|
||||
suffix := "_" + strconv.Itoa(n)
|
||||
candidate := truncateForSuffix(base, len(suffix)) + suffix
|
||||
if !keyOccupied(candidate, taken) {
|
||||
return ValidateKey(candidate)
|
||||
}
|
||||
}
|
||||
// Unreachable by the argument above; kept so a future change to candidate
|
||||
// generation surfaces as an error rather than an infinite loop.
|
||||
return "", errors.New("could not derive an unused status key from that name; provide one explicitly")
|
||||
}
|
||||
|
||||
// keyOccupied folds the built-in set into the workspace's own keys. A built-in
|
||||
// counts as occupied even in a workspace whose catalog rows have not been
|
||||
// seeded yet, so an unseeded workspace cannot mint a custom status that
|
||||
// shadows one.
|
||||
func keyOccupied(key string, taken map[string]bool) bool {
|
||||
if IsBuiltIn(key) {
|
||||
return true
|
||||
}
|
||||
return taken[key]
|
||||
}
|
||||
|
||||
// truncateForSuffix shortens base so base+suffix fits maxKeyLen. The trailing
|
||||
// underscore trim keeps "human_review" from becoming "human_" + "_2"; base
|
||||
// always starts with an alphanumeric, so trimming can never empty it.
|
||||
func truncateForSuffix(base string, suffixLen int) string {
|
||||
limit := maxKeyLen - suffixLen
|
||||
if len(base) <= limit {
|
||||
return base
|
||||
}
|
||||
return strings.TrimRight(base[:limit], "_")
|
||||
}
|
||||
|
||||
// Ensure idempotently seeds a workspace's 7 built-in statuses. Safe to call
|
||||
@@ -192,6 +297,33 @@ func Effective(ctx context.Context, q Querier, workspaceID pgtype.UUID, status s
|
||||
return entry.Category
|
||||
}
|
||||
|
||||
// EffectiveAndName resolves a status to BOTH its category and its display name
|
||||
// in one catalog read, for payload builders that need the pair.
|
||||
//
|
||||
// Two separate calls would double the query on every background event carrying
|
||||
// a custom status. Same fail-safe direction as Effective: an unresolvable key
|
||||
// yields the key unchanged and an empty name.
|
||||
//
|
||||
// The name is empty for a built-in on purpose — clients localize those from the
|
||||
// key, so echoing the seeded English one would be the single string a
|
||||
// non-English workspace has to ignore. (MUL-6749)
|
||||
func EffectiveAndName(ctx context.Context, q Querier, workspaceID pgtype.UUID, status string) (string, string) {
|
||||
if IsBuiltIn(status) {
|
||||
return status, ""
|
||||
}
|
||||
entry, err := q.GetIssueStatusEntryByKey(ctx, db.GetIssueStatusEntryByKeyParams{
|
||||
WorkspaceID: workspaceID,
|
||||
Key: status,
|
||||
})
|
||||
if err != nil {
|
||||
return status, ""
|
||||
}
|
||||
if !IsCategory(entry.Category) {
|
||||
return status, entry.Name
|
||||
}
|
||||
return entry.Category, entry.Name
|
||||
}
|
||||
|
||||
// Resolve validates that status is usable in this workspace, returning the
|
||||
// catalog entry. Write paths use this; it is the application-layer replacement
|
||||
// for the enum CHECK that migration 337 dropped.
|
||||
@@ -267,6 +399,42 @@ func ActiveKeys(ctx context.Context, q Querier, workspaceID pgtype.UUID) ([]stri
|
||||
return keys, nil
|
||||
}
|
||||
|
||||
// ActiveKeyLabels is ActiveKeys with each CUSTOM status's display name attached,
|
||||
// as `key (Name)`. It backs the error a caller sees after writing an unknown
|
||||
// status, where a bare key list is the least useful thing to hand back: a key
|
||||
// derived from a non-Latin name carries no meaning on its own (`in_review_2`),
|
||||
// so the reader — a person or an agent — has nothing to match against the name
|
||||
// they were told to use.
|
||||
//
|
||||
// Built-ins stay bare. Their key IS their category, their seeded name is
|
||||
// English, and every client localizes them from the key anyway, so printing
|
||||
// `todo (Todo)` would add noise rather than information.
|
||||
func ActiveKeyLabels(ctx context.Context, q Querier, workspaceID pgtype.UUID) ([]string, error) {
|
||||
entries, err := q.ListIssueStatusEntries(ctx, db.ListIssueStatusEntriesParams{
|
||||
WorkspaceID: workspaceID,
|
||||
IncludeArchived: false,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
labels := make([]string, 0, len(entries)+len(canonicalOrder))
|
||||
seen := make(map[string]bool, len(entries))
|
||||
for _, e := range entries {
|
||||
seen[e.Key] = true
|
||||
if IsBuiltIn(e.Key) || strings.TrimSpace(e.Name) == "" {
|
||||
labels = append(labels, e.Key)
|
||||
continue
|
||||
}
|
||||
labels = append(labels, e.Key+" ("+e.Name+")")
|
||||
}
|
||||
for _, key := range canonicalOrder {
|
||||
if !seen[key] {
|
||||
labels = append(labels, key)
|
||||
}
|
||||
}
|
||||
return labels, nil
|
||||
}
|
||||
|
||||
// Resolver resolves many statuses against ONE workspace's catalog using at most
|
||||
// one query, for list endpoints that would otherwise issue a lookup per row.
|
||||
//
|
||||
@@ -281,6 +449,7 @@ func ActiveKeys(ctx context.Context, q Querier, workspaceID pgtype.UUID) ([]stri
|
||||
type Resolver struct {
|
||||
workspaceID pgtype.UUID
|
||||
categories map[string]string
|
||||
names map[string]string
|
||||
loaded bool
|
||||
}
|
||||
|
||||
@@ -289,6 +458,29 @@ func NewResolver(workspaceID pgtype.UUID) *Resolver {
|
||||
return &Resolver{workspaceID: workspaceID}
|
||||
}
|
||||
|
||||
// load fetches the catalog once, on the first call that needs it. A failed read
|
||||
// leaves the maps nil, which makes every lookup below fall back to its
|
||||
// key-unchanged branch — the same fail-safe the single-shot resolver applies.
|
||||
func (r *Resolver) load(ctx context.Context, q Querier) {
|
||||
if r.loaded {
|
||||
return
|
||||
}
|
||||
r.loaded = true
|
||||
entries, err := q.ListIssueStatusEntries(ctx, db.ListIssueStatusEntriesParams{
|
||||
WorkspaceID: r.workspaceID,
|
||||
IncludeArchived: true,
|
||||
})
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
r.categories = make(map[string]string, len(entries))
|
||||
r.names = make(map[string]string, len(entries))
|
||||
for _, e := range entries {
|
||||
r.categories[e.Key] = e.Category
|
||||
r.names[e.Key] = e.Name
|
||||
}
|
||||
}
|
||||
|
||||
// Effective mirrors the package-level Effective, but amortizes the catalog read
|
||||
// across every call. Same fail-safe direction: an unresolvable key is returned
|
||||
// unchanged rather than guessed at.
|
||||
@@ -296,22 +488,7 @@ func (r *Resolver) Effective(ctx context.Context, q Querier, status string) stri
|
||||
if IsBuiltIn(status) {
|
||||
return status
|
||||
}
|
||||
if !r.loaded {
|
||||
r.loaded = true
|
||||
entries, err := q.ListIssueStatusEntries(ctx, db.ListIssueStatusEntriesParams{
|
||||
WorkspaceID: r.workspaceID,
|
||||
IncludeArchived: true,
|
||||
})
|
||||
if err != nil {
|
||||
// Leave the map nil; every custom key then falls back to itself,
|
||||
// which is the same fail-safe the single-shot resolver applies.
|
||||
return status
|
||||
}
|
||||
r.categories = make(map[string]string, len(entries))
|
||||
for _, e := range entries {
|
||||
r.categories[e.Key] = e.Category
|
||||
}
|
||||
}
|
||||
r.load(ctx, q)
|
||||
category, ok := r.categories[status]
|
||||
if !ok || !IsCategory(category) {
|
||||
return status
|
||||
@@ -319,6 +496,26 @@ func (r *Resolver) Effective(ctx context.Context, q Querier, status string) stri
|
||||
return category
|
||||
}
|
||||
|
||||
// Name returns a CUSTOM status's display name, or "" for a built-in and for a
|
||||
// key the catalog does not hold.
|
||||
//
|
||||
// Built-ins return "" on purpose rather than their seeded English name: every
|
||||
// client renders those from the key through i18n, so handing back "In Progress"
|
||||
// would be the one string a Chinese workspace must ignore. Custom names are
|
||||
// user-authored and untranslatable, so they are the label everywhere — which is
|
||||
// also why a payload carrying a custom key should carry this beside it. A
|
||||
// generated key like `in_review_2` says nothing on its own. (MUL-6749)
|
||||
//
|
||||
// Shares the Resolver's single catalog read with Effective, so adding the name
|
||||
// to a response costs no extra query.
|
||||
func (r *Resolver) Name(ctx context.Context, q Querier, status string) string {
|
||||
if IsBuiltIn(status) {
|
||||
return ""
|
||||
}
|
||||
r.load(ctx, q)
|
||||
return r.names[status]
|
||||
}
|
||||
|
||||
// ExpandCategories turns a set of categories into the status keys that belong
|
||||
// to them, for use as an INDEXED `status = ANY(...)` predicate.
|
||||
//
|
||||
|
||||
@@ -3,6 +3,7 @@ package issuestatus
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -240,7 +241,20 @@ func TestValidateKeyRejectsReservedAndMalformed(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSlugifyKey(t *testing.T) {
|
||||
// takenSet builds the DeriveKey callback from a literal list of keys the
|
||||
// workspace already owns.
|
||||
func takenSet(keys ...string) map[string]bool {
|
||||
owned := make(map[string]bool, len(keys))
|
||||
for _, k := range keys {
|
||||
owned[k] = true
|
||||
}
|
||||
return owned
|
||||
}
|
||||
|
||||
// TestDeriveKeyKeepsTheSlugForSluggableNames pins the behavior an English
|
||||
// workspace already had: the key stays a readable slug of the name, and the
|
||||
// fallback introduced for MUL-6749 must not reach names that never needed it.
|
||||
func TestDeriveKeyKeepsTheSlugForSluggableNames(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"Human Review": "human_review",
|
||||
"Gate Approved!": "gate_approved",
|
||||
@@ -248,23 +262,189 @@ func TestSlugifyKey(t *testing.T) {
|
||||
"Waiting — 客户": "waiting",
|
||||
}
|
||||
for name, want := range cases {
|
||||
got, err := SlugifyKey(name)
|
||||
got, err := DeriveKey(name, InReview, takenSet())
|
||||
if err != nil {
|
||||
t.Errorf("SlugifyKey(%q) failed: %v", name, err)
|
||||
t.Errorf("DeriveKey(%q) failed: %v", name, err)
|
||||
continue
|
||||
}
|
||||
if got != want {
|
||||
t.Errorf("SlugifyKey(%q) = %q, want %q", name, got, want)
|
||||
t.Errorf("DeriveKey(%q) = %q, want %q", name, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyFallsBackToCategoryForNonLatinNames is the bug this package was
|
||||
// changed for (MUL-6749 / GitHub #7627): a display name written entirely in a
|
||||
// non-Latin script has nothing to slug, and used to be REFUSED outright — with
|
||||
// no field in the settings form to supply a key instead, that made the status
|
||||
// impossible to create at all.
|
||||
//
|
||||
// The fallback is the category plus an ordinal rather than a random suffix, so
|
||||
// the key still says which platform behavior the status inherits. It starts at
|
||||
// 2 because the category's own built-in already owns the bare key.
|
||||
func TestDeriveKeyFallsBackToCategoryForNonLatinNames(t *testing.T) {
|
||||
for _, name := range []string{"客户确认", "고객 확인", "確認待ち", "تأكيد العميل", "客戶確認"} {
|
||||
got, err := DeriveKey(name, InReview, takenSet())
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey(%q) failed: %v", name, err)
|
||||
}
|
||||
if got != "in_review_2" {
|
||||
t.Errorf("DeriveKey(%q) = %q, want %q", name, got, "in_review_2")
|
||||
}
|
||||
}
|
||||
|
||||
// A name that slugifies onto a built-in key must be refused, not silently
|
||||
// shadow the built-in.
|
||||
if _, err := SlugifyKey("In Progress"); err == nil {
|
||||
// A second non-Latin status in the same category takes the next ordinal
|
||||
// rather than colliding with the first.
|
||||
got, err := DeriveKey("供应商确认", InReview, takenSet("in_review_2"))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey on a second non-Latin name failed: %v", err)
|
||||
}
|
||||
if got != "in_review_3" {
|
||||
t.Errorf("second non-Latin in_review status = %q, want %q", got, "in_review_3")
|
||||
}
|
||||
|
||||
// The ordinal is per category, so a different category starts over.
|
||||
got, err = DeriveKey("待排期", Todo, takenSet("in_review_2", "in_review_3"))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey in another category failed: %v", err)
|
||||
}
|
||||
if got != "todo_2" {
|
||||
t.Errorf("first non-Latin todo status = %q, want %q", got, "todo_2")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyEvenWhenTheWorkspaceIsUnseeded covers the rolling-deploy case.
|
||||
// A workspace whose catalog rows have not landed reports NO keys as taken, so
|
||||
// the fallback would hand a custom status the bare category key — which is a
|
||||
// built-in — and let it shadow one. Built-ins count as occupied regardless of
|
||||
// what the workspace reports.
|
||||
func TestDeriveKeyEvenWhenTheWorkspaceIsUnseeded(t *testing.T) {
|
||||
got, err := DeriveKey("客户确认", InReview, takenSet())
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey on an unseeded workspace failed: %v", err)
|
||||
}
|
||||
if IsBuiltIn(got) {
|
||||
t.Fatalf("DeriveKey returned the built-in key %q on an unseeded workspace", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyDisambiguatesCollidingSlugs covers the sharper half of #7627:
|
||||
// the reported workaround was to mix ASCII into the display name, but the
|
||||
// non-ASCII part is dropped, so two DIFFERENT names collapse onto one slug. The
|
||||
// second create used to fail on the key's unique index with a message blaming
|
||||
// the display name, which was not taken at all.
|
||||
func TestDeriveKeyDisambiguatesCollidingSlugs(t *testing.T) {
|
||||
first, err := DeriveKey("待客户 Review", InReview, takenSet())
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey failed: %v", err)
|
||||
}
|
||||
if first != "review" {
|
||||
t.Fatalf("DeriveKey(\"待客户 Review\") = %q, want %q", first, "review")
|
||||
}
|
||||
second, err := DeriveKey("待供应商 Review", InReview, takenSet(first))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey on the colliding name failed: %v", err)
|
||||
}
|
||||
if second != "review_2" {
|
||||
t.Errorf("colliding slug resolved to %q, want %q", second, "review_2")
|
||||
}
|
||||
third, err := DeriveKey("待财务 Review", InReview, takenSet(first, second))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey on the third colliding name failed: %v", err)
|
||||
}
|
||||
if third != "review_3" {
|
||||
t.Errorf("third colliding slug resolved to %q, want %q", third, "review_3")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyCountsArchivedKeysAsTaken is a storage constraint, not a
|
||||
// preference: idx_issue_status_workspace_key is NOT partial, so an archived
|
||||
// status still owns its key. Handing it out again would fail on insert.
|
||||
func TestDeriveKeyCountsArchivedKeysAsTaken(t *testing.T) {
|
||||
got, err := DeriveKey("Human Review", InReview, takenSet("human_review"))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey failed: %v", err)
|
||||
}
|
||||
if got != "human_review_2" {
|
||||
t.Errorf("DeriveKey around an archived key = %q, want %q", got, "human_review_2")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyStillRefusesABuiltInSlug keeps the pre-existing guard: a name
|
||||
// that slugifies ONTO a built-in key is a collision the admin should see, not
|
||||
// something to silently renumber into a near-duplicate of a built-in status.
|
||||
func TestDeriveKeyStillRefusesABuiltInSlug(t *testing.T) {
|
||||
if _, err := DeriveKey("In Progress", InProgress, takenSet()); err == nil {
|
||||
t.Error("a name slugifying to a built-in key should be rejected")
|
||||
}
|
||||
if _, err := SlugifyKey("客户"); err == nil {
|
||||
t.Error("a name with no slug-able characters should be rejected")
|
||||
}
|
||||
|
||||
// TestDeriveKeyRejectsAnUnknownCategory guards the fallback's own input: it
|
||||
// builds the key OUT of the category, so an unvalidated one would mint a key
|
||||
// that the storage CHECK may not even accept.
|
||||
func TestDeriveKeyRejectsAnUnknownCategory(t *testing.T) {
|
||||
if _, err := DeriveKey("客户确认", "started", takenSet()); err == nil {
|
||||
t.Error("a non-Latin name in an unknown category should be rejected")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyKeepsSuffixedKeysWithinTheStorageLimit pins the truncation: the
|
||||
// key column caps at 32 characters, so the BASE gives way to the suffix rather
|
||||
// than the suffix being dropped — dropping it would return a key already taken.
|
||||
func TestDeriveKeyKeepsSuffixedKeysWithinTheStorageLimit(t *testing.T) {
|
||||
long := strings.Repeat("a", 40)
|
||||
base, err := DeriveKey(long, InReview, takenSet())
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey on a long name failed: %v", err)
|
||||
}
|
||||
if len(base) != 32 {
|
||||
t.Fatalf("a 40-character name produced a %d-character key %q, want 32", len(base), base)
|
||||
}
|
||||
suffixed, err := DeriveKey(long, InReview, takenSet(base))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey on a long colliding name failed: %v", err)
|
||||
}
|
||||
if len(suffixed) > 32 {
|
||||
t.Errorf("suffixed key %q is %d characters, over the 32 the column allows", suffixed, len(suffixed))
|
||||
}
|
||||
if suffixed == base {
|
||||
t.Error("the suffixed key collided with the key it was supposed to avoid")
|
||||
}
|
||||
if _, err := ValidateKey(suffixed); err != nil {
|
||||
t.Errorf("suffixed key %q fails the storage pattern: %v", suffixed, err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeriveKeyScanIsBoundedByTheCatalogNotAConstant pins that disambiguation
|
||||
// keeps going as long as the workspace has keys to collide with. An arbitrary
|
||||
// ceiling would fail with "provide one explicitly" — the exact error a UI with
|
||||
// no key field cannot act on, which is the whole reason this package changed.
|
||||
func TestDeriveKeyScanIsBoundedByTheCatalogNotAConstant(t *testing.T) {
|
||||
// Every candidate `zz`, `zz_2` … `zz_1200` is already taken, so a fixed
|
||||
// 1000-ish bound would give up here.
|
||||
keys := []string{"zz"}
|
||||
for n := 2; n <= 1200; n++ {
|
||||
keys = append(keys, "zz_"+strconv.Itoa(n))
|
||||
}
|
||||
got, err := DeriveKey("ZZ", Todo, takenSet(keys...))
|
||||
if err != nil {
|
||||
t.Fatalf("DeriveKey gave up on a large catalog: %v", err)
|
||||
}
|
||||
if got != "zz_1201" {
|
||||
t.Errorf("DeriveKey = %q, want %q", got, "zz_1201")
|
||||
}
|
||||
|
||||
// Same for the non-Latin fallback: the ordinal walks past any fixed cap.
|
||||
keys = nil
|
||||
for n := 2; n <= 1100; n++ {
|
||||
keys = append(keys, "todo_"+strconv.Itoa(n))
|
||||
}
|
||||
got, err = DeriveKey("待排期", Todo, takenSet(keys...))
|
||||
if err != nil {
|
||||
t.Fatalf("non-Latin fallback gave up on a large catalog: %v", err)
|
||||
}
|
||||
if got != "todo_1101" {
|
||||
t.Errorf("non-Latin fallback = %q, want %q", got, "todo_1101")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -771,7 +771,7 @@ func (s *AutopilotService) dispatchCreateIssue(ctx context.Context, ap db.Autopi
|
||||
ActorType: "agent",
|
||||
ActorID: util.UUIDToString(leader.ID),
|
||||
Payload: map[string]any{
|
||||
"issue": IssueToMapWithCategory(ctx, s.Queries, issue, prefix),
|
||||
"issue": IssueToMapResolved(ctx, s.Queries, issue, prefix),
|
||||
},
|
||||
})
|
||||
s.captureIssueCreatedFromAutopilot(ap, run, issue, leader.ID)
|
||||
|
||||
@@ -647,7 +647,7 @@ func (s *IssueService) PublishAttachmentsChanged(ctx context.Context, issue db.I
|
||||
ActorType: "member",
|
||||
ActorID: util.UUIDToString(actorID),
|
||||
Payload: map[string]any{
|
||||
"issue": IssueToMapWithCategory(ctx, s.Queries, current, workspace.IssuePrefix),
|
||||
"issue": IssueToMapResolved(ctx, s.Queries, current, workspace.IssuePrefix),
|
||||
"assignee_changed": false,
|
||||
"status_changed": false,
|
||||
"project_changed": false,
|
||||
|
||||
@@ -6751,7 +6751,7 @@ func (s *TaskService) broadcastIssueUpdated(ctx context.Context, issue db.Issue,
|
||||
ActorType: "system",
|
||||
ActorID: "",
|
||||
Payload: map[string]any{
|
||||
"issue": IssueToMapWithCategory(ctx, s.Queries, issue, prefix),
|
||||
"issue": IssueToMapResolved(ctx, s.Queries, issue, prefix),
|
||||
"status_changed": prevStatus != issue.Status,
|
||||
"prev_status": prevStatus,
|
||||
},
|
||||
@@ -6912,13 +6912,19 @@ func builtInStatusCategory(status string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// IssueToMapWithCategory is IssueToMap with an AUTHORITATIVE status_category,
|
||||
// resolved through the catalog so a custom status is not emitted with a blank
|
||||
// one. Background events go through here; clients treat this payload as a
|
||||
// complete issue and bucket it by category. (MUL-6243)
|
||||
func IssueToMapWithCategory(ctx context.Context, q issuestatus.Querier, issue db.Issue, issuePrefix string) map[string]any {
|
||||
// IssueToMapResolved is IssueToMap with an AUTHORITATIVE status_category and
|
||||
// status_name, both resolved through the catalog so a custom status is not
|
||||
// emitted with blanks. Background events go through here; clients treat this
|
||||
// payload as a complete issue and bucket it by category. (MUL-6243)
|
||||
//
|
||||
// Both fields come from ONE catalog read. Resolving them separately would
|
||||
// double the query on every event carrying a custom status, and the HTTP
|
||||
// rendering already shares a single read through its Resolver. (MUL-6749)
|
||||
func IssueToMapResolved(ctx context.Context, q issuestatus.Querier, issue db.Issue, issuePrefix string) map[string]any {
|
||||
m := IssueToMap(issue, issuePrefix)
|
||||
m["status_category"] = issuestatus.Effective(ctx, q, issue.WorkspaceID, issue.Status)
|
||||
category, name := issuestatus.EffectiveAndName(ctx, q, issue.WorkspaceID, issue.Status)
|
||||
m["status_category"] = category
|
||||
m["status_name"] = name
|
||||
return m
|
||||
}
|
||||
|
||||
@@ -6934,7 +6940,12 @@ func IssueToMap(issue db.Issue, issuePrefix string) map[string]any {
|
||||
// Mirrors handler.IssueResponse.StatusCategory: a built-in status IS
|
||||
// its own category, so this resolves with no catalog lookup. Empty for
|
||||
// a custom status, which consumers resolve via the catalog. (MUL-6243)
|
||||
"status_category": builtInStatusCategory(issue.Status),
|
||||
"status_category": builtInStatusCategory(issue.Status),
|
||||
// Mirrors handler.IssueResponse.StatusName. A built-in carries no name
|
||||
// — clients localize those from the key — and a CUSTOM one is filled in
|
||||
// by IssueToMapResolved, which has the catalog. Emitted unconditionally
|
||||
// so this rendering cannot lose a key the HTTP one carries. (MUL-6749)
|
||||
"status_name": "",
|
||||
"priority": issue.Priority,
|
||||
"assignee_type": util.TextToPtr(issue.AssigneeType),
|
||||
"assignee_id": util.UUIDToPtr(issue.AssigneeID),
|
||||
|
||||
Reference in New Issue
Block a user