diff --git a/docs/cli.md b/docs/cli.md index f860572e..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` 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, 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 3a33876f..d5455ecf 100644 --- a/internal/cmd/set_aside.go +++ b/internal/cmd/set_aside.go @@ -307,12 +307,26 @@ func groupPageTotal(page *hey.BoxGroupPage) int { return max(page.TotalCount, len(page.Group.Postings)) } +// 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) + } + 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,7 +343,6 @@ func newSetAsideGroupCreateCommand() *cobra.Command { if err != nil { return err } - group, err := sdk.Boxes().CreateGroup(cmd.Context(), boxID, ids) if err != nil { return apierr.FromSDK(err) @@ -337,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}) @@ -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`, @@ -389,6 +405,9 @@ func (c *setAsideGroupAddCommand) run(cmd *cobra.Command, args []string) error { 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 3a7604c6..fc244221 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 /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) } - 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]) + 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/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) } body := recorded.bodies[0] 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,127 @@ 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) + } +} + +// 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) { + 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)