From 46e263f2d231776b719c72d8c2c9d7e1060a5071 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Tue, 8 Sep 2026 09:35:23 +0200 Subject: [PATCH] File a thread out of the box it is in, not the list it was found through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filing keys refused a thread opened from a bundle, a contact's threads or a search. That is the customer report behind the card, and the customer is right that all three file fine on the web. The web manages it because its toolbar reads the posting's own box — posting.box.kind, off the row rather than off the page you came from. HEY serves that box to every client: postings/_object.jbuilder writes box_id on every posting, and the SDK's generated type has carried BoxId all along. mail.Posting was dropping it on the way through, so the TUI had nothing to ask and fell back to asking the list. A list can draw rows from several boxes at once — a search, a label, a collection, a contact's threads — so the list's box was the wrong thing to ask. Filing measured "already in Paper Trail" against whatever box sat behind the results, and the fix for that was to refuse those lists outright rather than to ask the row. So: carry BoxID through, take the box from the posting, and the refusals go with it. Only a topic opened by its id has no row and nothing to file. The same lookup fixes filing from a label or a collection list, where the source carries no box kind at all and the already-in guard could never have fired. A filed row also leaves whichever list was drawn over the box list. Nothing re-reads a search's results or a bundle's threads — both are drawn once when they open — so a row left behind stays there offering to file a thread that has already moved. That is the second commit of #349, which I dropped while rebasing it as unreachable; this is what makes it reachable. --- docs/tui.md | 7 +- internal/mail/posting.go | 7 +- internal/mail/posting_test.go | 6 + internal/tui/mail.go | 50 ++++++- internal/tui/mail_test.go | 241 ++++++++++++++++++++++++++++------ 5 files changed, 265 insertions(+), 46 deletions(-) diff --git a/docs/tui.md b/docs/tui.md index 7893358b..d1ada41b 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -65,8 +65,11 @@ Most of those keep working while you are reading a thread, the way the web app's toolbar stays live: `r`, `f`, `v`, `b`, `u`, `i`, `l`, `a`, `d`, `p` and `t` all act on the thread on screen rather than on the list behind it. Filing a thread leaves it open in the box it landed in, so the next key files it on from there; `t` closes it, because a -trashed thread is not somewhere you file out of. A thread opened from search results, from -a bundle, or by its id has no row to file and says so instead. +trashed thread is not somewhere you file out of. Where you opened the thread from does +not come into it: a bundle, a contact's threads, a label and a search all work, because +every thread carries the box it is in and files out of that one rather than out of the +list you found it through. Filing takes the row out of the list you were reading. Only a +thread opened by its id has no row behind it, and says so instead. While writing a new message, reply or forward, Ctrl+T opens the searchable Snippets picker. HEY never chooses a default: Enter inserts the selected snippet at the body diff --git a/internal/mail/posting.go b/internal/mail/posting.go index c62c813c..6e31b949 100644 --- a/internal/mail/posting.go +++ b/internal/mail/posting.go @@ -15,7 +15,11 @@ import ( // formatting a time into a string and parsing it back is how a reader east of UTC ends // up looking at yesterday's date. type Posting struct { - ID int64 + ID int64 + // BoxID is the box this row is in, which HEY serves on every posting. A list can + // draw rows from several boxes at once — a search, a label, a contact's threads — + // so the box a thread files out of is the row's own rather than the list's. + BoxID int64 TopicID int64 CreatedAt time.Time Name string @@ -73,6 +77,7 @@ func Postings(postings []generated.Posting) []Posting { func NewPosting(posting generated.Posting) Posting { return Posting{ ID: posting.Id, + BoxID: posting.BoxId, TopicID: TopicIDOf(posting), CreatedAt: posting.CreatedAt, Name: terminal.SanitizeLine(posting.Name), diff --git a/internal/mail/posting_test.go b/internal/mail/posting_test.go index 4a013e5f..f1a3e4e6 100644 --- a/internal/mail/posting_test.go +++ b/internal/mail/posting_test.go @@ -12,6 +12,7 @@ func TestNewPostingKeepsWhatARowShows(t *testing.T) { posting := NewPosting(generated.Posting{ Id: 4471829, + BoxId: 91, AppUrl: "https://app.hey.com/topics/501", CreatedAt: created, Kind: "topic", @@ -35,6 +36,11 @@ func TestNewPostingKeepsWhatARowShows(t *testing.T) { if posting.ID != 4471829 || posting.TopicID != 501 || posting.Name != "Kitchen remodel quote" { t.Errorf("posting = %+v", posting) } + // The box is the row's own, not the list's: it is what a thread found through a + // search, a label or a contact files out of. + if posting.BoxID != 91 { + t.Errorf("box = %d, want the box HEY served the row in", posting.BoxID) + } // HEY's `bundled` means filed inside a bundle; only kind "bundle" makes a row one. if !posting.Seen || posting.IsBundle || !posting.BubbledUp || !posting.Muted || posting.VisibleEntryCount != 3 { t.Errorf("posting state = %+v", posting) diff --git a/internal/tui/mail.go b/internal/tui/mail.go index 65385d41..2a634ade 100644 --- a/internal/tui/mail.go +++ b/internal/tui/mail.go @@ -528,7 +528,7 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { if opened := v.openedPosting(msg.postingID); opened != nil { v.threadPosting = *opened } - v.threadBoxKind = v.actionBoxKind() + v.threadBoxKind = v.postingBoxKind(v.threadPosting) v.topicName = msg.title v.entries = msg.entries v.attachments = msg.attachments @@ -734,6 +734,9 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { v.postingList.postings[idx].Muted = false } } + if msg.effect == postingActionRemove { + v.removeFromOverlaidLists(msg.postingID) + } // The open thread can file back into the box on screen — out and back while // it stays open — and its row was removed when it first filed away, so the // list re-reads its head to hold what the server now does. @@ -1960,6 +1963,18 @@ func (v *mailView) removePostingAt(index int) { v.postingList.removeAt(index) } +// removeFromOverlaidLists takes a row out of the lists drawn over the box list. Nothing +// re-reads those — a search's results and a bundle's threads are drawn once when they +// open — so a row left behind stays on screen offering to file a thread that has +// already moved. +func (v *mailView) removeFromOverlaidLists(postingID int64) { + for _, list := range []*contentList{&v.searchList, &v.bundleList} { + if index := postingIndexIn(list.postings, postingID); index >= 0 { + list.removeAt(index) + } + } +} + func (v *mailView) moveAttachmentCursor(delta int) { if len(v.attachments) == 0 { return @@ -2400,8 +2415,13 @@ func (v *mailView) openThreadPicker(key string) tea.Cmd { // the thread opened, standing in for a row the list may no longer hold — the // automatic mark-seen resorts it under the cover and clamps the cursor away, and a // live refresh can drop it off the head page — while the thread stays on screen. +// +// Where the thread was opened from does not come into it. Every posting HEY serves +// carries its own box, so a thread found through a search, a bundle, a contact or a +// label files out of the box it is actually in rather than out of whatever list is +// behind it. Only a topic opened by its id has no row, and nothing to file. func (v *mailView) fileablePosting() *mail.Posting { - if v.searchActive || v.bundleActive || v.threadPosting.ID == 0 { + if v.threadPosting.ID == 0 { return nil } return &v.threadPosting @@ -2412,7 +2432,7 @@ func (v *mailView) handlePostingAction(key string) tea.Cmd { if selected == nil { return nil } - return v.postingAction(key, *selected, v.actionBoxKind()) + return v.postingAction(key, *selected, v.postingBoxKind(*selected)) } // actionBoxKind is the box kind a list row files out of, empty over a source that @@ -2424,6 +2444,30 @@ func (v *mailView) actionBoxKind() string { return "" } +// postingBoxKind is the box kind a posting files out of, taken from the posting's own +// box rather than the list showing it. A search, a label, a collection and a contact's +// threads all draw rows from several boxes at once, so the list's box says nothing +// about where any one row lives. Falls back to the list for a row HEY served without +// a box. +func (v *mailView) postingBoxKind(p mail.Posting) string { + if kind := v.boxKindOf(p.BoxID); kind != "" { + return kind + } + return v.actionBoxKind() +} + +func (v *mailView) boxKindOf(boxID int64) string { + if boxID == 0 { + return "" + } + for i := range v.boxes { + if v.boxes[i].Kind == mail.KindBox && v.boxes[i].ID == boxID { + return v.boxes[i].BoxKind + } + } + return "" +} + // postingAction runs key's action on p. fromBoxKind is the box kind the posting // files out of — the list's own box for a row, the box the open thread lives in // for a filing key pressed there — so a move to the box it is already in answers diff --git a/internal/tui/mail_test.go b/internal/tui/mail_test.go index 45a1fde5..9ff6a482 100644 --- a/internal/tui/mail_test.go +++ b/internal/tui/mail_test.go @@ -752,11 +752,8 @@ func TestMailViewRefusesThePickersWithoutAFileableThread(t *testing.T) { for _, key := range []string{"b", "v"} { t.Run(key, func(t *testing.T) { v := mailWithPostings() - v.searchActive = true - v.searchList.setPostings([]mail.Posting{{ID: 10, TopicID: 100, Name: "Hello world"}}) v.inThread = true - v.topicID = 100 - v.threadPosting = mail.Posting{ID: 10, TopicID: 100} + v.topicID = 555 // opened by URL, with no posting row behind it v.HandleContentKey(keyPress(key)) if v.modal != nil { @@ -769,6 +766,24 @@ func TestMailViewRefusesThePickersWithoutAFileableThread(t *testing.T) { } } +// A picker over a search result aims at the result's own row, which is the only thing +// that knows the labels it carries and the box it would move out of. +func TestMailViewOpensThePickersOverASearchResult(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + v.searchActive = true + v.searchList.setPostings([]mail.Posting{{ID: 10, BoxID: 3, TopicID: 100, Name: "Hello world"}}) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + + v.HandleContentKey(keyPress("b")) + picker, ok := v.modal.(*folderPicker) + if !ok { + t.Fatalf("modal = %#v, want the label picker", v.modal) + } + if picker.posting.ID != 10 { + t.Errorf("picker posting = %d, want the result's own 10", picker.posting.ID) + } +} + func TestMailViewFilesOpenThreadAfterMarkSeenCoversItsRow(t *testing.T) { v, recorded := mailWithTestServer(t, http.StatusNoContent) v.postingList.setCover(coverTopo) @@ -919,35 +934,64 @@ func TestMailViewFilesOpenThreadRecordsOnlyTheLatestFiling(t *testing.T) { } } -func TestMailViewFilesOpenThreadOnlyFromFilingLists(t *testing.T) { - t.Run("search result", func(t *testing.T) { - v := mailWithPostings() - v.searchActive = true - v.searchList.setPostings([]mail.Posting{{ID: 10, TopicID: 100, Name: "Hello world"}}) - v.inThread = true - v.topicID = 100 - v.threadPosting = mail.Posting{ID: 10, TopicID: 100} +// A search result carries its own box like every other posting, so a thread opened out +// of one files out of the box it is really in — which the results list, drawn from every +// box at once, could never have said. +func TestMailViewFilesAThreadOpenedFromSearchResults(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + v.searchActive = true + v.searchQuery = "quarterly planning" + // The result lives in Paper Trail while the box behind the search is the Imbox. + v.searchList.setPostings([]mail.Posting{{ID: 10, BoxID: 3, TopicID: 100, Name: "Hello world"}}) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + if !v.inThread { + t.Fatal("a search result should open") + } - if cmd := v.HandleContentKey(keyPress("a")); cmd != nil { - t.Errorf("a search-opened thread should not file: %#v", runCmd(cmd)) - } - if v.notice != "Can't file this thread from here" { - t.Errorf("notice = %q, want the filing explanation", v.notice) - } - }) + done, ok := runCmd(v.HandleContentKey(keyPress("a"))).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("filing a search result returned %#v", done) + } + if len(recorded.body.PostingIDs) != 1 || recorded.body.PostingIDs[0] != 10 { + t.Errorf("posting_ids = %v, want the result's own 10", recorded.body.PostingIDs) + } - t.Run("directly opened topic", func(t *testing.T) { - v := mailWithPostings() - v.inThread = true - v.topicID = 555 // opened by URL, with no posting row behind it + v.Update(done) + if len(v.searchList.postings) != 0 { + t.Errorf("results after filing = %+v, want the filed row gone", v.searchList.postings) + } +} - if cmd := v.HandleContentKey(keyPress("l")); cmd != nil { - t.Errorf("a directly opened thread should not file the selected row: %#v", runCmd(cmd)) - } - if v.notice != "Can't file this thread from here" { - t.Errorf("notice = %q, want the filing explanation", v.notice) - } - }) +// The guard measures against the thread's own box, not the list's, so a Paper Trail +// result found from the Imbox knows it is already in Paper Trail. +func TestMailViewMeasuresAlreadyInAgainstTheThreadsOwnBox(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + v.searchActive = true + v.searchList.setPostings([]mail.Posting{{ID: 10, BoxID: 3, TopicID: 100, Name: "Hello world"}}) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + + if cmd := v.HandleContentKey(keyPress("p")); cmd != nil { + t.Errorf("moving to the box it is in returned %#v, want nothing", runCmd(cmd)) + } + if v.notice != "Already in Paper Trail" { + t.Errorf("notice = %q, want the already-there explanation", v.notice) + } + if recorded.path == "/postings/moves.json" { + t.Error("a refused move still asked the server to move something") + } +} + +func TestMailViewWillNotFileATopicOpenedWithoutARow(t *testing.T) { + v := mailWithPostings() + v.inThread = true + v.topicID = 555 // opened by URL, with no posting row behind it + + if cmd := v.HandleContentKey(keyPress("l")); cmd != nil { + t.Errorf("a directly opened thread should not file the selected row: %#v", runCmd(cmd)) + } + if v.notice != "Can't file this thread from here" { + t.Errorf("notice = %q, want the filing explanation", v.notice) + } } func TestMailViewThreadHelpAdvertisesFilingKeys(t *testing.T) { @@ -2566,28 +2610,37 @@ func TestMailViewTrashesTheOpenThreadAndReturnsToTheList(t *testing.T) { // Trash keeps to the same rule as the other filing keys: a thread opened over // search results or a bundle has no row to file, so the key says so rather than // trashing whatever the box list's cursor happens to be sitting on. -func TestMailViewRefusesToTrashAThreadOpenedFromSearchResults(t *testing.T) { +// Trashing a search result trashes the result's own posting, and takes its row out of +// the results with it: nothing re-reads that list, so the row would otherwise stay +// there offering to reopen a thread that is now in the Trash. +func TestMailViewTrashesAThreadOpenedFromSearchResults(t *testing.T) { v, recorded := mailWithTestServer(t, http.StatusNoContent) v.searchActive = true v.searchQuery = "quarterly planning" - v.searchList.setPostings([]mail.Posting{{ID: 10, TopicID: 100, Name: "Hello world"}}) + v.searchList.setPostings([]mail.Posting{{ID: 10, BoxID: 3, TopicID: 100, Name: "Hello world"}}) v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) if !v.inThread { t.Fatal("the searched thread should have opened") } - if cmd := v.HandleContentKey(keyPress("t")); cmd != nil { - t.Errorf("trash over search results returned %#v, want nothing", runCmd(cmd)) + done, ok := runCmd(v.HandleContentKey(keyPress("t"))).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("trashing a search result returned %#v", done) } - if !v.inThread { - t.Error("a refused trash should leave the thread open") + if done.postingID != 10 { + t.Errorf("trashed posting %d, want the result's own 10", done.postingID) } - if v.notice == "" { - t.Error("a refused trash should say why") + if recorded.path != "/postings/trash.json" { + t.Errorf("request = %s %s, want POST /postings/trash.json", recorded.method, recorded.path) } - if recorded.path == "/postings/trash.json" { - t.Error("a refused trash still asked the server to trash something") + if v.inThread || !v.searchActive { + t.Errorf("trashing landed on open:%v search:%v, want the results", v.inThread, v.searchActive) + } + + v.Update(done) + if len(v.searchList.postings) != 0 { + t.Errorf("results after trashing = %+v, want the trashed row gone", v.searchList.postings) } } @@ -3847,6 +3900,114 @@ func TestMailViewOpensAReadBundleAsContactThreads(t *testing.T) { } } +// The reader's complaint was the help bar, not the keys: a thread opened in a bundle +// offered reply and forward and nothing else, so the filing keys read as missing even +// where they would have worked. +func TestMailViewBundleThreadHelpOffersTheFilingKeys(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + v.postingList.postings[0] = bundleRow() + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + if !v.inThread || !v.bundleActive { + t.Fatalf("thread state = open:%v bundle:%v", v.inThread, v.bundleActive) + } + + bindings := v.HelpBindings() + for _, key := range []string{"r", "f", "v", "b", "u", "i", "l", "a", "d", "p", "t"} { + if !hasHelpBinding(bindings, key) { + t.Errorf("bundle thread help misses %q: %+v", key, bindings) + } + } +} + +// A contact's threads are every thread with them, drawn from every box at once. Each +// row still carries the box it is in, so a thread opened from that list files out of +// its own box — the screen the customer reported, and the one the web app files from. +func TestMailViewFilesAThreadOpenedFromAContactsThreads(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + v.bundleActive = true + v.bundleContactID = 88 + v.bundleTitle = "All emails with GitHub" + v.bundleList.setPostings([]mail.Posting{{ID: 511, BoxID: 3, TopicID: 100, Name: "Deploy failed on main"}}) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + if !v.inThread { + t.Fatal("a contact's thread should open") + } + + bindings := v.HelpBindings() + for _, key := range []string{"r", "f", "v", "b", "u", "i", "l", "a", "d", "p", "t"} { + if !hasHelpBinding(bindings, key) { + t.Errorf("contact thread help misses %q: %+v", key, bindings) + } + } + + done, ok := runCmd(v.HandleContentKey(keyPress("a"))).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("filing a contact's thread returned %#v", done) + } + if len(recorded.body.PostingIDs) != 1 || recorded.body.PostingIDs[0] != 511 { + t.Errorf("posting_ids = %v, want the thread's own 511", recorded.body.PostingIDs) + } + + v.Update(done) + if len(v.bundleList.postings) != 0 { + t.Errorf("contact threads after filing = %+v, want the filed row gone", v.bundleList.postings) + } +} + +// A bundle's threads live in the box the bundle was opened from, so opening one and +// filing it is the same act as filing its row — which is what the web app does, where +// the topic's toolbar is the same however you reached it. +func TestMailViewFilesAThreadOpenedFromABundle(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + v.postingList.postings[0] = bundleRow() + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + if !v.inThread || !v.bundleActive { + t.Fatalf("thread state = open:%v bundle:%v", v.inThread, v.bundleActive) + } + + done, ok := runCmd(v.HandleContentKey(keyPress("a"))).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("filing from a bundle returned %#v", done) + } + if recorded.path != "/postings/moves.json" { + t.Errorf("request = %s %s, want POST /postings/moves.json", recorded.method, recorded.path) + } + if len(recorded.body.PostingIDs) != 1 || recorded.body.PostingIDs[0] != 511 { + t.Errorf("posting_ids = %v, want the bundle member's 511", recorded.body.PostingIDs) + } + if recorded.body.BoxID == nil || *recorded.body.BoxID != 3 { + t.Errorf("box_id = %v, want Set Aside's 3", recorded.body.BoxID) + } + + v.Update(done) + if len(v.bundleList.postings) != 0 { + t.Errorf("bundle postings after filing = %+v, want the filed row gone", v.bundleList.postings) + } +} + +// Nothing re-reads a bundle's list, so a row the reader has filed away has to be taken +// out here or it stays on screen offering to file a thread that has already moved. +func TestMailViewDropsATrashedBundleRow(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + v.postingList.postings[0] = bundleRow() + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + v.Update(runCmd(v.HandleContentKey(keyPress("enter")))) + + done, ok := runCmd(v.HandleContentKey(keyPress("t"))).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("trashing from a bundle returned %#v", done) + } + if v.inThread { + t.Error("trash should close the thread, leaving the bundle on screen") + } + v.Update(done) + if len(v.bundleList.postings) != 0 { + t.Errorf("bundle postings after trashing = %+v, want the trashed row gone", v.bundleList.postings) + } +} + func TestMailViewSaysWhyABundleListIsEmpty(t *testing.T) { v := mailWithPostings() v.bundleActive = true