Files
multica/server/internal/handler/issue_batch_test.go
Bohan Jiang a123dfc2df MUL-3508: stage sub-issues so the parent wakes per stage, not per child (#4410)
* feat(issues): stage sub-issues so the parent wakes per stage, not per child

Sub-issues under a parent can be grouped into ordered stages (issue.stage).
The child-done -> parent notification + assignee wake now fire only when a
stage barrier closes: every sub-issue in the lowest unfinished stage has
reached a terminal status (done/cancelled). An unstaged sibling set is one
implicit stage, so the parent is woken once when the last sub-issue finishes
instead of on every child — the default fix for the fire-on-every-child
cascade reported in discussion #4320 / MUL-3508.

Stage advancement stays agent-driven: the server only detects the closed
barrier and wakes the parent assignee, who decides whether to promote the
next stage.

- DB: nullable issue.stage (CHECK >= 1) + sqlc regen
- API: stage on issue create/update/response and batch update
- CLI: `issue create`/`issue update` --stage; new `issue children` command
  that lists sub-issues grouped by stage (table + json)
- stageBarrierClosed / stageProgressSummary in issue_child_done.go, with the
  wake comment now stage-aware, plus unit tests
- skill docs (multica-working-on-issues SKILL.md + source map)

Web UI (create-form stage picker, sidebar edit, group-by-stage display) is a
follow-up; the API already returns stage for it to consume.

MUL-3508

Co-authored-by: multica-agent <github@multica.ai>

* fix(issues): address review on stage barrier (cancel, batch, unstaged)

Resolves the three blockers from the PR review:

1. Cancel can close a stage. The child-done barrier now fires on any
   non-terminal -> terminal transition (done OR cancelled), not just done.
   isTerminalChildStatus already treats cancelled as terminal (a cancelled
   sibling never finishes, so it must not hold a stage open), so a cancelled
   last-open child now closes its stage and wakes the parent. Keying on the
   transition also makes a later cancelled -> done edit a no-op, avoiding a
   lagging duplicate wake.

2. Batch update of stage no longer no-ops. `hasMutation` now includes
   "stage", so `{"updates":{"stage":N}}` persists instead of returning
   {"updated": 0}.

3. Unstaged children no longer participate in the staged frontier. In a
   staged sibling set, NULL-stage children neither hold a stage open nor fire
   on their own completion, and the wake comment no longer renders "Stage 0".
   This matches migration 123 ("NULL does not participate in staged
   grouping") and the CLI's separate unstaged group, removing the footgun
   where an unstaged backlog child silently blocked Stage 1.

Tests: cancellation closes a stage (staged + unstaged), unstaged ignored in a
staged set, stage summary skips unstaged, and a stage-only batch update
persists.

MUL-3508

Co-authored-by: multica-agent <github@multica.ai>

* feat(web): stage UI — create picker, sidebar edit, group sub-issues by stage

Frontend for the sub-issue stage feature (web + desktop, shared via packages):

- core: `stage` on the Issue type + create/update request types; zod
  IssueSchema parses it (defaults to null for older backends) with schema
  tests for the numeric and omitted cases.
- StagePicker component (mirrors the other property pickers): "No stage" +
  Stage 1..N, offering one beyond the current/sibling max.
- Create-issue modal: a Stage pill, shown only when a parent is selected,
  threaded into the create payload.
- Issue detail sidebar: an editable Stage row + "add property" entry, gated to
  sub-issues (issues with a parent).
- Sub-issue list grouped by stage with per-stage headers (flat when unstaged).
- i18n: stage keys across en / zh-Hans / ja / ko (parity test passes).

Verified: full typecheck (6/6), core (591) + views (1433) vitest suites,
lint clean (no new findings). Backend/CLI shipped earlier in this PR.

MUL-3508

Co-authored-by: multica-agent <github@multica.ai>

* test(issues): add stage to Issue fixtures merged from main

The merge brought in new Issue fixtures that predate the required `stage`
field: core issues/batch.test.ts, views batch-action-toolbar.test.tsx, and
the mobile EMPTY_ISSUE_FALLBACK sentinel. Add `stage: null` so they satisfy
the Issue type (mobile reuses core's IssueSchema for parsing, so only the
sentinel needs it).

MUL-3508

Co-authored-by: multica-agent <github@multica.ai>

* fix(web): feed StagePicker the sibling max stage so higher stages stay selectable

The StagePicker accepts maxStage to extend its option list beyond the
floored Stage 1-3, but neither call site passed it, so a parent with an
existing Stage 4/5 child could not pick that stage when creating a new
sub-issue or editing one in the sidebar.

- Compute the sibling max stage at both call sites: the create modal now
  loads the parent's children (childIssuesOptions) and the detail sidebar
  reuses the already-loaded parentChildIssues.
- Extract maxSiblingStage + stageOptions as pure helpers on stage-picker
  and unit-test them (the regression: a Stage 5 sibling keeps Stage 5
  selectable and offers Stage 6).

MUL-3508

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-23 00:14:42 +08:00

185 lines
6.2 KiB
Go

package handler
import (
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
)
// TestBatchUpdateNoMutationReturnsZero — regression for #1660.
//
// When the request payload has valid issue_ids but the "updates" field
// is empty, missing, or doesn't decode any known mutation field, the
// handler used to walk every issue, run a no-op UPDATE, and increment
// `updated` for each one — returning {"updated": N} despite changing
// nothing. Reporters saw 200 + a positive count and assumed the call
// worked, then chased a phantom persistence bug.
//
// The fix is "tell the truth": when no mutation field is present, return
// {"updated": 0} immediately so the count matches reality.
func TestBatchUpdateNoMutationReturnsZero(t *testing.T) {
// Two fresh issues so we can also assert no fields actually changed.
a := createTestIssue(t, "BU-no-mut A", "todo", "low")
b := createTestIssue(t, "BU-no-mut B", "todo", "low")
t.Cleanup(func() { deleteTestIssue(t, a) })
t.Cleanup(func() { deleteTestIssue(t, b) })
cases := []struct {
desc string
body map[string]any
}{
{
desc: "updates_missing",
// Most common reporter pattern: status at top level.
body: map[string]any{"issue_ids": []string{a, b}, "status": "in_progress"},
},
{
desc: "updates_empty_object",
body: map[string]any{"issue_ids": []string{a, b}, "updates": map[string]any{}},
},
{
desc: "updates_misnamed",
// Singular "update" instead of plural "updates".
body: map[string]any{"issue_ids": []string{a, b}, "update": map[string]any{"status": "done"}},
},
{
desc: "updates_unknown_field_only",
// Payload IS nested correctly, but every key inside `updates` is
// unknown to the handler — same class of caller mistake as the
// shapes above. hasMutation must stay false; behavior is already
// correct, this case locks it in against future regressions.
body: map[string]any{"issue_ids": []string{a, b}, "updates": map[string]any{"foo": "bar"}},
},
}
for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues/batch-update", tc.body)
testHandler.BatchUpdateIssues(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp struct {
Updated int `json:"updated"`
}
json.NewDecoder(w.Body).Decode(&resp)
if resp.Updated != 0 {
t.Errorf("expected updated=0 when no mutation field present, got %d", resp.Updated)
}
// Belt and braces: confirm the issues weren't touched.
for _, id := range []string{a, b} {
gw := httptest.NewRecorder()
gr := newRequest("GET", "/api/issues/"+id, nil)
gr = withURLParam(gr, "id", id)
testHandler.GetIssue(gw, gr)
var got IssueResponse
json.NewDecoder(gw.Body).Decode(&got)
if got.Status != "todo" {
t.Errorf("issue %s: status changed to %q despite no-mutation request", id, got.Status)
}
}
})
}
}
// TestBatchUpdateValidUpdatesPersistAndCount — positive case to lock in
// happy-path behavior alongside the regression test above.
func TestBatchUpdateValidUpdatesPersistAndCount(t *testing.T) {
a := createTestIssue(t, "BU-ok A", "todo", "low")
b := createTestIssue(t, "BU-ok B", "todo", "low")
t.Cleanup(func() { deleteTestIssue(t, a) })
t.Cleanup(func() { deleteTestIssue(t, b) })
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues/batch-update", map[string]any{
"issue_ids": []string{a, b},
"updates": map[string]any{"status": "in_progress"},
})
testHandler.BatchUpdateIssues(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp struct {
Updated int `json:"updated"`
}
json.NewDecoder(w.Body).Decode(&resp)
if resp.Updated != 2 {
t.Errorf("expected updated=2, got %d", resp.Updated)
}
for _, id := range []string{a, b} {
gw := httptest.NewRecorder()
gr := newRequest("GET", "/api/issues/"+id, nil)
gr = withURLParam(gr, "id", id)
testHandler.GetIssue(gw, gr)
var got IssueResponse
json.NewDecoder(gw.Body).Decode(&got)
if got.Status != "in_progress" {
t.Errorf("issue %s: expected status=in_progress, got %q", id, got.Status)
}
}
}
// TestBatchUpdateStageOnly — regression for the stage barrier feature: a
// batch update whose only field is `stage` must count as a mutation (hasMutation
// includes "stage") and actually persist, not silently return {"updated": 0}.
func TestBatchUpdateStageOnly(t *testing.T) {
a := createTestIssue(t, "BU-stage A", "todo", "low")
t.Cleanup(func() { deleteTestIssue(t, a) })
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues/batch-update", map[string]any{
"issue_ids": []string{a},
"updates": map[string]any{"stage": 2},
})
testHandler.BatchUpdateIssues(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var resp struct {
Updated int `json:"updated"`
}
json.NewDecoder(w.Body).Decode(&resp)
if resp.Updated != 1 {
t.Fatalf("expected updated=1 for a stage-only batch update, got %d", resp.Updated)
}
gw := httptest.NewRecorder()
gr := newRequest("GET", "/api/issues/"+a, nil)
gr = withURLParam(gr, "id", a)
testHandler.GetIssue(gw, gr)
var got IssueResponse
json.NewDecoder(gw.Body).Decode(&got)
if got.Stage == nil || *got.Stage != 2 {
t.Errorf("expected stage=2 to persist, got %v", got.Stage)
}
}
// createTestIssue is a small helper to keep the table-driven cases clean.
// Returns the new issue's id; caller is responsible for cleanup.
func createTestIssue(t *testing.T, title, status, priority string) string {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": title,
"status": status,
"priority": priority,
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue %q: expected 201, got %d: %s", title, w.Code, w.Body.String())
}
var issue IssueResponse
json.NewDecoder(w.Body).Decode(&issue)
return issue.ID
}
func deleteTestIssue(t *testing.T, id string) {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("DELETE", "/api/issues/"+id, nil)
req = withURLParam(req, "id", id)
testHandler.DeleteIssue(w, req)
}