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