From 48ea50d33f59f0050a4db19722fdb36022e8acc1 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 7 Sep 2026 15:29:14 -0400 Subject: [PATCH 1/3] Move threads into Set Aside before grouping them hey set-aside group create and group add promised to move a thread into Set Aside when it was elsewhere, and relied on HEY's group routes to do it. Those routes relocate with a plain box write. "Bubbled up" is a value of a posting's seen state rather than a flag of its own, and only HEY's move marks a thread seen, so a bubbled-up thread gathered straight out of the Imbox arrived in Set Aside still bubbled up, which the web app draws as a Bubble Up row inside the Set Aside stack. Both commands now move the threads through HEY's move first, the way hey move --to set-aside does, and then group them. A thread already in Set Aside is unchanged by the move. The unit tests model both relocation paths and assert the bubble is cleared; the smoke test does the same against a live server. --- docs/cli.md | 2 +- internal/cmd/set_aside.go | 23 +++++- internal/cmd/set_aside_test.go | 127 ++++++++++++++++++++++++++++++--- tests/smoke/set_aside_test.go | 67 +++++++++++++++++ 4 files changed, 208 insertions(+), 11 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index f860572e..de78ff0c 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -279,7 +279,7 @@ Organization actions take the `id` values returned by `hey box view --json`, `he Collection IDs come from `hey collection list`. `hey collection view` returns both each posting `id` and its `topic_id`, plus `next_page` and `total_count`. Collection membership commands take `topic_id`; posting organization commands continue to take `id`. Creating a collection returns a confirmed mutation, and `hey collection list` provides its ID for subsequent commands. Collection updates accept a non-empty name, summary, or both. -Set Aside groups have no name in HEY; a group is its ID and its threads. `hey set-aside view` lists the same threads as `hey box view set-aside` and adds each thread's `box_group_id` (a `Group` column in the styled table). HEY's group index answers with IDs alone, so `hey set-aside group list` reads each group once for its thread count. `hey set-aside group view ` lists a group's threads with `next_page` and `total_count`, accepts `--page ` and `--all` like the other listings, and answers `not_found` for a group that is gone; HEY removes a group itself once its last thread leaves it. `group create`, `group add` and `group remove` take posting `id` values; `group view`, `group add --to` and `group delete` take a group ID from `group list`. `hey set-aside group create` moves threads into Set Aside if they are elsewhere. `hey set-aside group remove` leaves threads in Set Aside outside any group, while `hey set-aside group delete` sends the group's threads to Previously Seen, which is what HEY does when a group is dissolved in the web app. +Set Aside groups have no name in HEY; a group is its ID and its threads. `hey set-aside view` lists the same threads as `hey box view set-aside` and adds each thread's `box_group_id` (a `Group` column in the styled table). HEY's group index answers with IDs alone, so `hey set-aside group list` reads each group once for its thread count. `hey set-aside group view ` lists a group's threads with `next_page` and `total_count`, accepts `--page ` and `--all` like the other listings, and answers `not_found` for a group that is gone; HEY removes a group itself once its last thread leaves it. `group create`, `group add` and `group remove` take posting `id` values; `group view`, `group add --to` and `group delete` take a group ID from `group list`. `hey set-aside group create` and `hey set-aside group add` move threads into Set Aside if they are elsewhere, through the same move as `hey move --to set-aside`: the thread is marked seen, which also clears a bubble-up, since HEY keeps "bubbled up" as a seen state rather than a flag and a thread cannot be set aside and bubbled up at once. `hey set-aside group remove` leaves threads in Set Aside outside any group, while `hey set-aside group delete` sends the group's threads to Previously Seen, which is what HEY does when a group is dissolved in the web app. Workflow IDs come from `hey workflow list`, which includes the linked account ID for each workflow. `hey workflow view ` returns stages in position order; `--ids-only` and `--count` apply to those stages. Creating a workflow needs one linked mail account, selected with `--account` when more than one is available. HEY creates new stages as `Untitled`, so create the stage, read its ID with `hey workflow view `, then rename it. Workflow membership commands take `topic_id`. Adding a thread creates its workflow membership before selecting the requested stage; if stage selection fails, the thread remains in the workflow's first stage and the command reports the error. diff --git a/internal/cmd/set_aside.go b/internal/cmd/set_aside.go index 3a33876f..15a94341 100644 --- a/internal/cmd/set_aside.go +++ b/internal/cmd/set_aside.go @@ -307,12 +307,25 @@ func groupPageTotal(page *hey.BoxGroupPage) int { return max(page.TotalCount, len(page.Group.Postings)) } +// setAsideThreads moves the threads into Set Aside through HEY's move before they are +// grouped. The group routes relocate a thread themselves, but with a plain box write: +// "bubbled up" is a value of a posting's seen state, not a flag of its own, and only the +// move marks a thread seen. Grouping a bubbled-up thread straight out of the Imbox left it +// set aside and bubbled up at once, which the web app draws as a Bubble Up row inside the +// Set Aside stack. A thread already in Set Aside is unchanged by the move. +func setAsideThreads(ctx context.Context, boxID int64, ids []int64) error { + if err := sdk.Postings().Move(ctx, boxID, ids...); err != nil { + return apierr.FromSDK(err) + } + return nil +} + func newSetAsideGroupCreateCommand() *cobra.Command { return &cobra.Command{ Use: "create ...", Short: "Gather email threads into a new Set Aside group", Annotations: map[string]string{ - "agent_notes": "Accepts box item IDs from hey set-aside view. Threads not yet in Set Aside are moved there. Returns the new group's ID.", + "agent_notes": "Accepts box item IDs from hey set-aside view. Threads not yet in Set Aside are moved there and marked seen, which also clears a bubble-up. Returns the new group's ID.", }, Example: ` hey set-aside group create 12345 hey set-aside group create 12345 67890`, @@ -329,6 +342,9 @@ func newSetAsideGroupCreateCommand() *cobra.Command { if err != nil { return err } + if err = setAsideThreads(cmd.Context(), boxID, ids); err != nil { + return err + } group, err := sdk.Boxes().CreateGroup(cmd.Context(), boxID, ids) if err != nil { @@ -355,7 +371,7 @@ func newSetAsideGroupAddCommand() *setAsideGroupAddCommand { Use: "add ...", Short: "Add email threads to a Set Aside group", Annotations: map[string]string{ - "agent_notes": "Accepts box item IDs from hey set-aside view and a group ID from hey set-aside group list. A thread already in another group is moved to this one.", + "agent_notes": "Accepts box item IDs from hey set-aside view and a group ID from hey set-aside group list. Threads not yet in Set Aside are moved there and marked seen, which also clears a bubble-up. A thread already in another group is moved to this one.", }, Example: ` hey set-aside group add 12345 --to 42 hey set-aside group add 12345 67890 --to 42`, @@ -386,6 +402,9 @@ func (c *setAsideGroupAddCommand) run(cmd *cobra.Command, args []string) error { return err } + if err := setAsideThreads(cmd.Context(), boxID, ids); err != nil { + return err + } if err := sdk.Postings().AddToBoxGroup(cmd.Context(), boxID, groupID, ids...); err != nil { return apierr.FromSDK(err) } diff --git a/internal/cmd/set_aside_test.go b/internal/cmd/set_aside_test.go index 3a7604c6..468364c8 100644 --- a/internal/cmd/set_aside_test.go +++ b/internal/cmd/set_aside_test.go @@ -60,7 +60,7 @@ func setAsideServer(recorded *recordedSetAside) http.Handler { _, _ = io.WriteString(w, `{"id":44}`) case "DELETE /boxes/3/groups/42.json": w.WriteHeader(http.StatusNoContent) - case "POST /postings/box_groups.json", "DELETE /postings/box_groups.json": + case "POST /postings/moves.json", "POST /postings/box_groups.json", "DELETE /postings/box_groups.json": w.WriteHeader(http.StatusNoContent) default: http.NotFound(w, r) @@ -275,12 +275,17 @@ func TestSetAsideGroupCreate(t *testing.T) { if err != nil { t.Fatalf("execute group create: %v", err) } - want := []string{"GET /boxes.json", "POST /boxes/3/groups.json"} + want := []string{"GET /boxes.json", "POST /postings/moves.json", "POST /boxes/3/groups.json"} if strings.Join(recorded.requests, ",") != strings.Join(want, ",") { t.Errorf("requests = %v, want %v", recorded.requests, want) } - if ids := recorded.bodies[0]["posting_ids"].([]any); len(ids) != 2 || ids[0] != float64(101) || ids[1] != float64(102) { - t.Errorf("posting_ids = %#v", recorded.bodies[0]) + if move := recorded.bodies[0]; move["box_id"] != float64(3) { + t.Errorf("move body = %#v, want a move into Set Aside", move) + } + for i, body := range recorded.bodies[:2] { + if ids := body["posting_ids"].([]any); len(ids) != 2 || ids[0] != float64(101) || ids[1] != float64(102) { + t.Errorf("request %d posting_ids = %#v", i, body) + } } if response.Summary != "Group 44 created with 2 threads" { t.Errorf("summary = %q", response.Summary) @@ -301,16 +306,21 @@ func TestSetAsideGroupAdd(t *testing.T) { if err != nil { t.Fatalf("execute group add: %v", err) } - want := []string{"GET /boxes.json", "POST /postings/box_groups.json"} + want := []string{"GET /boxes.json", "POST /postings/moves.json", "POST /postings/box_groups.json"} if strings.Join(recorded.requests, ",") != strings.Join(want, ",") { t.Errorf("requests = %v, want %v", recorded.requests, want) } - body := recorded.bodies[0] + if move := recorded.bodies[0]; move["box_id"] != float64(3) { + t.Errorf("move body = %#v, want a move into Set Aside", move) + } + body := recorded.bodies[1] if body["box_id"] != float64(3) || body["box_group_id"] != float64(42) { t.Errorf("body = %#v", body) } - if ids := body["posting_ids"].([]any); len(ids) != 1 || ids[0] != float64(102) { - t.Errorf("posting_ids = %#v", body["posting_ids"]) + for i, body := range recorded.bodies[:2] { + if ids := body["posting_ids"].([]any); len(ids) != 1 || ids[0] != float64(102) { + t.Errorf("request %d posting_ids = %#v", i, body["posting_ids"]) + } } if response.Summary != "1 thread added to group 42" { t.Errorf("summary = %q", response.Summary) @@ -355,3 +365,104 @@ func TestSetAsideGroupDelete(t *testing.T) { t.Error("deleting group 0 succeeded, want a usage error") } } + +// fakeSetAsidePosting is what HEY keeps for a posting that the group commands touch. seen +// is haystack's enum as it is stored: 1 seen, 0 unseen, -1 bubbled up — "bubbled up" is a +// value of seen, not a flag of its own, which is why the two relocation routes below +// differ in what they leave behind. +type fakeSetAsidePosting struct { + box int64 + seen int + group int64 +} + +// setAsideStateServer models the two ways HEY relocates a posting into Set Aside. +// +// POST /postings/moves.json is Box#move_in: it writes the box and marks the posting seen, +// which is also what clears bubbled_up. +// +// The group routes — POST /boxes/3/groups.json and POST /postings/box_groups.json — are +// Posting#move_to_box_group: they write the box and the group and leave seen exactly as +// it was, so a bubbled-up thread arrives in Set Aside still bubbled up. +func setAsideStateServer(postings map[int64]*fakeSetAsidePosting) http.Handler { + postingIDs := func(body map[string]any) []int64 { + raw := body["posting_ids"].([]any) + ids := make([]int64, 0, len(raw)) + for _, id := range raw { + ids = append(ids, int64(id.(float64))) + } + return ids + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + if r.Body != nil { + _ = json.NewDecoder(r.Body).Decode(&body) + } + w.Header().Set("Content-Type", "application/json") + switch r.Method + " " + r.URL.Path { + case "GET /boxes.json": + _, _ = io.WriteString(w, `[{"id":1,"kind":"imbox","name":"Imbox"},{"id":3,"kind":"asidebox","name":"Set Aside"}]`) + case "POST /postings/moves.json": + for _, id := range postingIDs(body) { + postings[id].box = int64(body["box_id"].(float64)) + postings[id].seen = 1 + } + w.WriteHeader(http.StatusNoContent) + case "POST /boxes/3/groups.json": + for _, id := range postingIDs(body) { + postings[id].box = 3 + postings[id].group = 44 + } + _, _ = io.WriteString(w, `{"id":44}`) + case "POST /postings/box_groups.json": + for _, id := range postingIDs(body) { + postings[id].box = 3 + postings[id].group = int64(body["box_group_id"].(float64)) + } + w.WriteHeader(http.StatusCreated) + default: + http.NotFound(w, r) + } + }) +} + +// A thread bubbled up into the Imbox is stored as seen: -1. Gathering it into a Set Aside +// group must put it in Set Aside the way HEY's own move does — seen, with the bubble +// cleared — rather than leave it set aside and bubbled up at once (card 10279322895). +func TestSetAsideGroupCreateClearsTheBubbleOnAnImboxThread(t *testing.T) { + postings := map[int64]*fakeSetAsidePosting{201: {box: 1, seen: -1}} + response, err := runJSONCommand(t, setAsideStateServer(postings), "set-aside", "group", "create", "201") + if err != nil { + t.Fatalf("execute group create: %v", err) + } + if response.Summary != "Group 44 created with 1 thread" { + t.Errorf("summary = %q", response.Summary) + } + got := postings[201] + if got.box != 3 || got.group != 44 { + t.Errorf("posting = %+v, want box 3 in group 44", *got) + } + if got.seen == -1 { + t.Errorf("posting is still bubbled up in Set Aside: %+v", *got) + } +} + +// Adding an Imbox thread to a group moves it into Set Aside the same way create does, and +// must clear its bubble on the way for the same reason (cards 10279323648, 10279322895). +func TestSetAsideGroupAddClearsTheBubbleOnAnImboxThread(t *testing.T) { + postings := map[int64]*fakeSetAsidePosting{201: {box: 1, seen: -1}} + response, err := runJSONCommand(t, setAsideStateServer(postings), "set-aside", "group", "add", "201", "--to", "42") + if err != nil { + t.Fatalf("execute group add: %v", err) + } + if response.Summary != "1 thread added to group 42" { + t.Errorf("summary = %q", response.Summary) + } + got := postings[201] + if got.box != 3 || got.group != 42 { + t.Errorf("posting = %+v, want box 3 in group 42", *got) + } + if got.seen == -1 { + t.Errorf("posting is still bubbled up in Set Aside: %+v", *got) + } +} diff --git a/tests/smoke/set_aside_test.go b/tests/smoke/set_aside_test.go index 489aca1e..5fdac846 100644 --- a/tests/smoke/set_aside_test.go +++ b/tests/smoke/set_aside_test.go @@ -149,6 +149,73 @@ func TestSetAsideGroupMutations(t *testing.T) { } } +// A bubbled-up thread gathered into a group arrives in Set Aside seen, with the bubble +// cleared — the state HEY's own move leaves — rather than set aside and bubbled up at once. +func TestSetAsideGroupingClearsBubbleUp(t *testing.T) { + bubbledUpThread := func(label string) string { + uid := uniqueID() + subject := fmt.Sprintf("Disposable bubbled-up %s test %s", label, uid) + _, stderr, code := hey(t, "compose", + "--to", smokeEmail, + "--subject", subject, + "-m", "This disposable thread verifies that grouping clears a bubble-up.", + "--json", + ) + if code != 0 { + skipf(t, "could not create a disposable thread (exit %d): %s", code, stderr) + } + t.Cleanup(func() { cleanupThreadBySubject(t, subject) }) + postingID, _, _, err := waitForPostingAndTopicIDsBySubject(t, subject) + if err != nil || postingID == 0 { + t.Fatalf("could not find disposable thread: %v", err) + } + posting := strconv.FormatInt(postingID, 10) + if _, stderr, code := hey(t, "bubble", "up", posting, "--now", "--json"); code != 0 { + skipf(t, "could not bubble up thread %s (exit %d): %s", posting, code, stderr) + } + return posting + } + assertGroupedAndNotBubbledUp := func(group, posting string) { + t.Helper() + detail := dataAs[struct { + Postings []struct { + ID int64 `json:"id"` + BoxID int64 `json:"box_id"` + BubbledUp bool `json:"bubbled_up"` + Seen bool `json:"seen"` + } `json:"postings"` + }](t, heyJSON(t, "set-aside", "group", "view", group, "--all")) + for _, got := range detail.Postings { + if strconv.FormatInt(got.ID, 10) != posting { + continue + } + if got.BubbledUp || !got.Seen { + t.Errorf("posting %s in group %s = %+v, want seen and not bubbled up", posting, group, got) + } + return + } + t.Errorf("posting %s is not in group %s", posting, group) + } + + // Both threads are made before the group so that the group's cleanup runs first and + // returns them to the Imbox, where the thread cleanups look for them. + first := bubbledUpThread("create") + second := bubbledUpThread("add") + created := setAsideWriteJSON(t, "set-aside", "group", "create", first) + groupID := dataAs[struct { + ID int64 `json:"id"` + }](t, created).ID + if groupID == 0 { + t.Fatalf("group create answered no id: %+v", created) + } + group := strconv.FormatInt(groupID, 10) + t.Cleanup(func() { hey(t, "set-aside", "group", "delete", group, "--json") }) + assertGroupedAndNotBubbledUp(group, first) + + setAsideWriteJSON(t, "set-aside", "group", "add", second, "--to", group) + assertGroupedAndNotBubbledUp(group, second) +} + func TestSetAsideGroupMutationValidation(t *testing.T) { if _, stderr := heyFail(t, "set-aside", "group", "add", "1"); !strings.Contains(stderr, "--to") { t.Errorf("group add without --to should ask for it, got: %s", stderr) From 7c4bc9bce6462478ebeda49f4a7df6a360cd12af Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 7 Sep 2026 15:35:23 -0400 Subject: [PATCH 2/3] Group first, then move, so a refused group changes nothing Review: a positive but missing --to group was rejected by the group route only after the threads had already been moved into Set Aside and marked seen. The group route validates the group and relocates the thread itself, so it now runs first and the move follows it to mark the threads seen. A refused group fails before any thread is changed, and a thread already in Set Aside keeps its group because its box does not change. No extra request is needed. --- docs/cli.md | 2 +- internal/cmd/set_aside.go | 26 +++++++++++++------------- internal/cmd/set_aside_test.go | 33 ++++++++++++++++++++++++++++----- 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index de78ff0c..c8aa8266 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -279,7 +279,7 @@ Organization actions take the `id` values returned by `hey box view --json`, `he Collection IDs come from `hey collection list`. `hey collection view` returns both each posting `id` and its `topic_id`, plus `next_page` and `total_count`. Collection membership commands take `topic_id`; posting organization commands continue to take `id`. Creating a collection returns a confirmed mutation, and `hey collection list` provides its ID for subsequent commands. Collection updates accept a non-empty name, summary, or both. -Set Aside groups have no name in HEY; a group is its ID and its threads. `hey set-aside view` lists the same threads as `hey box view set-aside` and adds each thread's `box_group_id` (a `Group` column in the styled table). HEY's group index answers with IDs alone, so `hey set-aside group list` reads each group once for its thread count. `hey set-aside group view ` lists a group's threads with `next_page` and `total_count`, accepts `--page ` and `--all` like the other listings, and answers `not_found` for a group that is gone; HEY removes a group itself once its last thread leaves it. `group create`, `group add` and `group remove` take posting `id` values; `group view`, `group add --to` and `group delete` take a group ID from `group list`. `hey set-aside group create` and `hey set-aside group add` move threads into Set Aside if they are elsewhere, through the same move as `hey move --to set-aside`: the thread is marked seen, which also clears a bubble-up, since HEY keeps "bubbled up" as a seen state rather than a flag and a thread cannot be set aside and bubbled up at once. `hey set-aside group remove` leaves threads in Set Aside outside any group, while `hey set-aside group delete` sends the group's threads to Previously Seen, which is what HEY does when a group is dissolved in the web app. +Set Aside groups have no name in HEY; a group is its ID and its threads. `hey set-aside view` lists the same threads as `hey box view set-aside` and adds each thread's `box_group_id` (a `Group` column in the styled table). HEY's group index answers with IDs alone, so `hey set-aside group list` reads each group once for its thread count. `hey set-aside group view ` lists a group's threads with `next_page` and `total_count`, accepts `--page ` and `--all` like the other listings, and answers `not_found` for a group that is gone; HEY removes a group itself once its last thread leaves it. `group create`, `group add` and `group remove` take posting `id` values; `group view`, `group add --to` and `group delete` take a group ID from `group list`. `hey set-aside group create` and `hey set-aside group add` move threads into Set Aside if they are elsewhere, and then complete the move the way `hey move --to set-aside` does: the thread is marked seen, which also clears a bubble-up, since HEY keeps "bubbled up" as a seen state rather than a flag and a thread cannot be set aside and bubbled up at once. A group HEY refuses fails the command before any thread is changed. `hey set-aside group remove` leaves threads in Set Aside outside any group, while `hey set-aside group delete` sends the group's threads to Previously Seen, which is what HEY does when a group is dissolved in the web app. Workflow IDs come from `hey workflow list`, which includes the linked account ID for each workflow. `hey workflow view ` returns stages in position order; `--ids-only` and `--count` apply to those stages. Creating a workflow needs one linked mail account, selected with `--account` when more than one is available. HEY creates new stages as `Untitled`, so create the stage, read its ID with `hey workflow view `, then rename it. Workflow membership commands take `topic_id`. Adding a thread creates its workflow membership before selecting the requested stage; if stage selection fails, the thread remains in the workflow's first stage and the command reports the error. diff --git a/internal/cmd/set_aside.go b/internal/cmd/set_aside.go index 15a94341..d5455ecf 100644 --- a/internal/cmd/set_aside.go +++ b/internal/cmd/set_aside.go @@ -307,12 +307,13 @@ func groupPageTotal(page *hey.BoxGroupPage) int { return max(page.TotalCount, len(page.Group.Postings)) } -// setAsideThreads moves the threads into Set Aside through HEY's move before they are -// grouped. The group routes relocate a thread themselves, but with a plain box write: -// "bubbled up" is a value of a posting's seen state, not a flag of its own, and only the -// move marks a thread seen. Grouping a bubbled-up thread straight out of the Imbox left it -// set aside and bubbled up at once, which the web app draws as a Bubble Up row inside the -// Set Aside stack. A thread already in Set Aside is unchanged by the move. +// setAsideThreads completes the move the group routes began. Those routes relocate a +// thread into Set Aside themselves, but with a plain box write: "bubbled up" is a value of +// a posting's seen state, not a flag of its own, and only HEY's move marks a thread seen. +// Grouping a bubbled-up thread straight out of the Imbox left it set aside and bubbled up +// at once, which the web app draws as a Bubble Up row inside the Set Aside stack. The +// move runs after the group route so that a group HEY refuses fails before anything is +// changed; a thread already in Set Aside keeps its group, since its box does not change. func setAsideThreads(ctx context.Context, boxID int64, ids []int64) error { if err := sdk.Postings().Move(ctx, boxID, ids...); err != nil { return apierr.FromSDK(err) @@ -342,10 +343,6 @@ func newSetAsideGroupCreateCommand() *cobra.Command { if err != nil { return err } - if err = setAsideThreads(cmd.Context(), boxID, ids); err != nil { - return err - } - group, err := sdk.Boxes().CreateGroup(cmd.Context(), boxID, ids) if err != nil { return apierr.FromSDK(err) @@ -353,6 +350,9 @@ func newSetAsideGroupCreateCommand() *cobra.Command { if group == nil { return apierr.ErrAPI(200, "HEY did not answer with the new group") } + if err := setAsideThreads(cmd.Context(), boxID, ids); err != nil { + return err + } summary := fmt.Sprintf("Group %d created with %d %s", group.Id, len(ids), threadNoun(len(ids))) return writeMutation(cmd, summary, map[string]int64{"id": group.Id}) @@ -402,12 +402,12 @@ func (c *setAsideGroupAddCommand) run(cmd *cobra.Command, args []string) error { return err } - if err := setAsideThreads(cmd.Context(), boxID, ids); err != nil { - return err - } if err := sdk.Postings().AddToBoxGroup(cmd.Context(), boxID, groupID, ids...); err != nil { return apierr.FromSDK(err) } + if err := setAsideThreads(cmd.Context(), boxID, ids); err != nil { + return err + } return writeMutation(cmd, fmt.Sprintf("%d %s added to group %d", len(ids), threadNoun(len(ids)), groupID), nil) } diff --git a/internal/cmd/set_aside_test.go b/internal/cmd/set_aside_test.go index 468364c8..23d6cd27 100644 --- a/internal/cmd/set_aside_test.go +++ b/internal/cmd/set_aside_test.go @@ -275,11 +275,11 @@ func TestSetAsideGroupCreate(t *testing.T) { if err != nil { t.Fatalf("execute group create: %v", err) } - want := []string{"GET /boxes.json", "POST /postings/moves.json", "POST /boxes/3/groups.json"} + want := []string{"GET /boxes.json", "POST /boxes/3/groups.json", "POST /postings/moves.json"} if strings.Join(recorded.requests, ",") != strings.Join(want, ",") { t.Errorf("requests = %v, want %v", recorded.requests, want) } - if move := recorded.bodies[0]; move["box_id"] != float64(3) { + if move := recorded.bodies[1]; move["box_id"] != float64(3) { t.Errorf("move body = %#v, want a move into Set Aside", move) } for i, body := range recorded.bodies[:2] { @@ -306,14 +306,14 @@ func TestSetAsideGroupAdd(t *testing.T) { if err != nil { t.Fatalf("execute group add: %v", err) } - want := []string{"GET /boxes.json", "POST /postings/moves.json", "POST /postings/box_groups.json"} + want := []string{"GET /boxes.json", "POST /postings/box_groups.json", "POST /postings/moves.json"} if strings.Join(recorded.requests, ",") != strings.Join(want, ",") { t.Errorf("requests = %v, want %v", recorded.requests, want) } - if move := recorded.bodies[0]; move["box_id"] != float64(3) { + if move := recorded.bodies[1]; move["box_id"] != float64(3) { t.Errorf("move body = %#v, want a move into Set Aside", move) } - body := recorded.bodies[1] + body := recorded.bodies[0] if body["box_id"] != float64(3) || body["box_group_id"] != float64(42) { t.Errorf("body = %#v", body) } @@ -447,6 +447,29 @@ func TestSetAsideGroupCreateClearsTheBubbleOnAnImboxThread(t *testing.T) { } } +// A group HEY refuses fails the add before the threads are moved or marked seen. +func TestSetAsideGroupAddToARefusedGroupMovesNothing(t *testing.T) { + var requests []string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.Method+" "+r.URL.Path) + switch r.Method + " " + r.URL.Path { + case "GET /boxes.json": + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `[{"id":3,"kind":"asidebox","name":"Set Aside"}]`) + default: + http.NotFound(w, r) + } + }) + _, err := runJSONCommand(t, handler, "set-aside", "group", "add", "201", "--to", "99") + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("add to a missing group err = %v, want not found", err) + } + want := []string{"GET /boxes.json", "POST /postings/box_groups.json"} + if strings.Join(requests, ",") != strings.Join(want, ",") { + t.Errorf("requests = %v, want %v (no move)", requests, want) + } +} + // Adding an Imbox thread to a group moves it into Set Aside the same way create does, and // must clear its bubble on the way for the same reason (cards 10279323648, 10279322895). func TestSetAsideGroupAddClearsTheBubbleOnAnImboxThread(t *testing.T) { From 5ba89eb615d9f2281affda98fda3f9c3ceccc742 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 8 Sep 2026 13:13:18 -0700 Subject: [PATCH 3/3] Fail the group tests cleanly when the move request goes missing TestSetAsideGroupCreate and TestSetAsideGroupAdd index recorded.bodies[1] right after checking the request sequence. If the move ever regresses out, that indexes past the end and panics, aborting the rest of the package's tests instead of reporting one failure. Asserting the sequence with Fatalf stops the test at the legible message. --- internal/cmd/set_aside_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cmd/set_aside_test.go b/internal/cmd/set_aside_test.go index 23d6cd27..fc244221 100644 --- a/internal/cmd/set_aside_test.go +++ b/internal/cmd/set_aside_test.go @@ -277,7 +277,7 @@ func TestSetAsideGroupCreate(t *testing.T) { } want := []string{"GET /boxes.json", "POST /boxes/3/groups.json", "POST /postings/moves.json"} if strings.Join(recorded.requests, ",") != strings.Join(want, ",") { - t.Errorf("requests = %v, want %v", recorded.requests, want) + t.Fatalf("requests = %v, want %v", recorded.requests, want) } if move := recorded.bodies[1]; move["box_id"] != float64(3) { t.Errorf("move body = %#v, want a move into Set Aside", move) @@ -308,7 +308,7 @@ func TestSetAsideGroupAdd(t *testing.T) { } want := []string{"GET /boxes.json", "POST /postings/box_groups.json", "POST /postings/moves.json"} if strings.Join(recorded.requests, ",") != strings.Join(want, ",") { - t.Errorf("requests = %v, want %v", recorded.requests, want) + t.Fatalf("requests = %v, want %v", recorded.requests, want) } if move := recorded.bodies[1]; move["box_id"] != float64(3) { t.Errorf("move body = %#v, want a move into Set Aside", move)