diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/pgstore_integration_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/pgstore_integration_test.go index 4009d925..5e414749 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/pgstore_integration_test.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/pgstore_integration_test.go @@ -284,13 +284,31 @@ func TestPgStoreKeepsEachActWithItsLine(t *testing.T) { if err := s.Act(ctx, Handling{Entry: id, Item: "task 1", Handled: at}, ActionLine{What: ActHandled, Entry: id, Item: "task 1", Mode: "work", At: at}); err != nil { t.Fatal(err) } + // 1858/AC9 (review round 1 F2): a reminder's day, a date column written from midnight UTC, reads back the + // same day. + tomorrow := time.Date(2026, 10, 23, 0, 0, 0, 0, time.UTC) + if err := s.Act(ctx, Handling{Entry: id, Item: ItemMail, ShowsAgain: tomorrow}, ActionLine{What: ActReminded, Entry: id, Item: ItemMail, Mode: "work", At: at}); err != nil { + t.Fatal(err) + } + reminded := false + if hs, err := s.Handlings(ctx); err == nil { + for _, h := range hs { + reminded = reminded || (h.Item == ItemMail && h.ShowsAgain.Equal(tomorrow) && h.Handled.IsZero()) + } + } + if !reminded { + t.Fatal("the mail's reminder did not read back as 2026-10-23") + } + if err := s.Act(ctx, Handling{Entry: id, Item: ItemMail}, ActionLine{What: ActUndone, Entry: id, Item: ItemMail, Mode: "work", At: at}); err != nil { + t.Fatal(err) + } if err := s.Act(ctx, Handling{Entry: id + 99, Item: ItemMail, Handled: at}, ActionLine{What: ActHandled, Entry: id + 99, Item: ItemMail, Mode: "work", At: at}); err != ErrNotFound { t.Fatalf("an act on no entry: %v, want ErrNotFound", err) } hs, _ := s.Handlings(ctx) lines, _ := s.Actions(ctx, 10) - if len(hs) != 1 || !hs[0].Handled.Equal(at) || len(lines) != 1 || lines[0].Item != "task 1" { - t.Fatalf("handlings %+v, lines %+v; want the one act and its one line", hs, lines) + if len(hs) != 1 || !hs[0].Handled.Equal(at) || len(lines) != 3 || lines[2].Item != "task 1" { + t.Fatalf("handlings %+v, lines %+v; want task 1 handled, and a line for each of the three acts", hs, lines) } e, _, _ := s.Entry(ctx, id) e.Mark = MarkNeither @@ -301,8 +319,8 @@ func TestPgStoreKeepsEachActWithItsLine(t *testing.T) { if lessons, _ := s.Lessons(ctx); len(lessons) != 1 { t.Fatalf("lessons %+v, want the correction's one", lessons) } - if removed, err := s.RemoveActionsBefore(ctx, at.Add(30*time.Second)); err != nil || removed != 1 { - t.Fatalf("removed %d (%v), want the one line older than the bound", removed, err) + if removed, err := s.RemoveActionsBefore(ctx, at.Add(30*time.Second)); err != nil || removed != 3 { + t.Fatalf("removed %d (%v), want the three lines older than the bound", removed, err) } if _, err := s.pool.Exec(ctx, `delete from mail_entry where id = $1`, id); err != nil { t.Fatal(err) diff --git a/modules/mesh-mail-web/internal/web/acts.go b/modules/mesh-mail-web/internal/web/acts.go index 206c6754..4208fc3f 100644 --- a/modules/mesh-mail-web/internal/web/acts.go +++ b/modules/mesh-mail-web/internal/web/acts.go @@ -13,6 +13,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "time" ) @@ -38,13 +39,33 @@ func (b *Bundle) actFailed(w http.ResponseWriter, r *http.Request, mode Mode, ca renderNamed(w, http.StatusBadGateway, "list", data) } -// actDone answers an act that succeeded: done to the script, or back to the page at back. +// actDone answers an act that succeeded: done to the script, or back to the page at back, marked as the page +// after an act (ActedParam) so it records nothing. func actDone(w http.ResponseWriter, r *http.Request, back string) { if byScript(r) { answerJSON(w, http.StatusOK, map[string]any{"done": true}) return } - http.Redirect(w, r, back, http.StatusSeeOther) + http.Redirect(w, r, afterAct(back), http.StatusSeeOther) +} + +// ActedParam marks the page a form act goes back to (review round 1 F1; ADR 0517 §5: no act writes two +// lines): that page records neither page opened nor opened, since the act already wrote its one line. The +// script's own refresh says the same with RefreshHeader. +const ActedParam = "acted" + +// afterAct is back with ActedParam added. +func afterAct(back string) string { + sep := "?" + if strings.Contains(back, "?") { + sep = "&" + } + return back + sep + ActedParam + "=1" +} + +// afterAnAct says whether r asks the page shown after an act: by ActedParam, or the script's refresh. +func afterAnAct(r *http.Request) bool { + return r.URL.Query().Get(ActedParam) != "" || r.Header.Get(RefreshHeader) != "" } // listAddress is the list page's own address for mode. @@ -100,9 +121,10 @@ func (b *Bundle) modeHandler(w http.ResponseWriter, r *http.Request) { b.actFailed(w, r, mode, "The mode chosen could not be kept: "+err.Error()) return } + // The mode chosen is its own line: the list it goes to records nothing more (F1). if byScript(r) { - answerJSON(w, http.StatusOK, map[string]any{"done": true, "go": listAddress(mode)}) + answerJSON(w, http.StatusOK, map[string]any{"done": true, "go": afterAct(listAddress(mode))}) return } - http.Redirect(w, r, listAddress(mode), http.StatusSeeOther) + http.Redirect(w, r, afterAct(listAddress(mode)), http.StatusSeeOther) } diff --git a/modules/mesh-mail-web/internal/web/acts_test.go b/modules/mesh-mail-web/internal/web/acts_test.go index f89c380f..5bc67020 100644 --- a/modules/mesh-mail-web/internal/web/acts_test.go +++ b/modules/mesh-mail-web/internal/web/acts_test.go @@ -55,9 +55,9 @@ func TestTickingUndoingAndRemindingCallTheirVerbs(t *testing.T) { code, _, h := post(t, s, "/item", form, cookie, false) calls := reading.callsOf(act) - if code != http.StatusSeeOther || h.Get("Location") != "/?mode=work" || len(calls) != 1 || + if code != http.StatusSeeOther || h.Get("Location") != "/?mode=work&acted=1" || len(calls) != 1 || calls[0].Args["id"] != float64(3) || calls[0].Args["item"] != "task 1" || calls[0].Args["mode"] != "work" { - t.Fatalf("a post of %s: status %d to %q, calls %v; want 303 to /?mode=work and one call", act, code, h.Get("Location"), calls) + t.Fatalf("a post of %s: status %d to %q, calls %v; want 303 to /?mode=work&acted=1 and one call", act, code, h.Get("Location"), calls) } code, body, _ := post(t, s, "/item", form, cookie, true) if code != http.StatusOK || !strings.Contains(body, `"done":true`) || len(reading.callsOf(act)) != 2 { @@ -98,12 +98,12 @@ func TestChoosingAModeRecordsItAndShowsIt(t *testing.T) { s, cookie, token := signedIn(t, reading.bundle(time.Now())) code, _, h := post(t, s, "/mode", url.Values{"mode": {"personal"}, FormTokenField: {token}}, cookie, false) rec := reading.callsOf("record") - if code != http.StatusSeeOther || h.Get("Location") != "/?mode=personal" || len(rec) != 1 || + if code != http.StatusSeeOther || h.Get("Location") != "/?mode=personal&acted=1" || len(rec) != 1 || rec[0].Args["what"] != "mode" || rec[0].Args["mode"] != "personal" { t.Fatalf("choosing personal: status %d to %q, records %v", code, h.Get("Location"), rec) } code, body, _ := post(t, s, "/mode", url.Values{"mode": {"work"}, FormTokenField: {token}}, cookie, true) - if code != http.StatusOK || !strings.Contains(body, `"go":"/?mode=work"`) { + if code != http.StatusOK || !strings.Contains(body, `"go":"/?mode=work\u0026acted=1"`) { t.Fatalf("choosing work by the script: status %d %s", code, body) } } diff --git a/modules/mesh-mail-web/internal/web/list_integration_test.go b/modules/mesh-mail-web/internal/web/list_integration_test.go index c69f6b87..dcf1ad42 100644 --- a/modules/mesh-mail-web/internal/web/list_integration_test.go +++ b/modules/mesh-mail-web/internal/web/list_integration_test.go @@ -366,26 +366,46 @@ func sectionOfPage(t *testing.T, body, id string) string { return rest } -func postForm(t *testing.T, mw *startedMailWeb, path string, form url.Values, cookie *http.Cookie) { +// actOnPage posts one form act and follows its 303 as a browser does (review round 1 F1), answering the +// page it went back to — and checks that the act and that page together added exactly one line to the action +// log, of what: every line counted, whatever its kind. +func actOnPage(t *testing.T, mw *startedMailWeb, r *readerBundle, path string, form url.Values, cookie *http.Cookie, what string) string { t.Helper() + before := allLines(t, r) resp := doForm(t, mw, path, form, cookie) body, _ := io.ReadAll(resp.Body) _ = resp.Body.Close() if resp.StatusCode != http.StatusSeeOther { t.Fatalf("POST %s %v: status %d, want 303:\n%s", path, form, resp.StatusCode, body) } + page := pageBody(t, mw, resp.Header.Get("Location"), cookie) + after := allLines(t, r) + if len(after) != len(before)+1 || after[0]["what"] != what { + t.Fatalf("POST %s and the page it went back to left the lines %v after %v; want exactly one more, %s", path, after, before, what) + } + return page } -// linesOf answers the action lines of what, from the reader's own actions verb. -func linesOf(t *testing.T, r *readerBundle, what string) []map[string]any { +// allLines is the whole action log, newest first, from the reader's own actions verb. +func allLines(t *testing.T, r *readerBundle) []map[string]any { t.Helper() - out, err := r.call("actions", map[string]any{}) + out, err := r.call("actions", map[string]any{"limit": 1000}) if err != nil { t.Fatal(err) } var lines []map[string]any for _, l := range out.(map[string]any)["actions"].([]any) { - if line := l.(map[string]any); line["what"] == what { + lines = append(lines, l.(map[string]any)) + } + return lines +} + +// linesOf answers the action lines of what. +func linesOf(t *testing.T, r *readerBundle, what string) []map[string]any { + t.Helper() + var lines []map[string]any + for _, line := range allLines(t, r) { + if line["what"] == what { lines = append(lines, line) } } @@ -468,11 +488,10 @@ func TestTheOperatorsListEndToEnd(t *testing.T) { t.Fatal("the page carries no form token") } - // 1860/AC2: ticked with the session's token, the task is under handled today; actions holds one handled - // line, with no mail text; undo returns it. + // 1860/AC2: ticked with the session's token, the task is under handled today, and the act with the page + // it goes back to leaves one line in actions — handled, with no mail text; undo returns it, one line again. tick := url.Values{"id": {fmt.Sprint(int64(id))}, "item": {"task 1"}, "mode": {"everything"}, "act": {"handle"}, FormTokenField: {token}} - postForm(t, mw, "/item", tick, cookie) - body = pageBody(t, mw, "/", cookie) + body = actOnPage(t, mw, reader, "/item", tick, cookie, "handled") if !strings.Contains(sectionOfPage(t, body, "handled-today"), taskLine) || strings.Contains(sectionOfPage(t, body, "today"), taskLine) { t.Fatalf("the ticked task is not under handled today alone:\n%s", body) } @@ -487,23 +506,34 @@ func TestTheOperatorsListEndToEnd(t *testing.T) { } } tick.Set("act", "unhandle") - postForm(t, mw, "/item", tick, cookie) - body = pageBody(t, mw, "/", cookie) + body = actOnPage(t, mw, reader, "/item", tick, cookie, "undone") if !strings.Contains(sectionOfPage(t, body, "today"), taskLine) { t.Fatalf("the undone task is not back under today:\n%s", body) } - if undone := linesOf(t, reader, "undone"); len(undone) != 1 { - t.Fatalf("undone lines %v, want one", undone) + + // 1858/AC9 (review round 1 F2): reminded, the task is gone from the list for the rest of the day — the + // reminder's day, a date column, written and read back through postgres — with one reminded line. + tick.Set("act", "remind") + body = actOnPage(t, mw, reader, "/item", tick, cookie, "reminded") + if strings.Contains(body, taskLine) { + t.Fatalf("the reminded task still shows the same day:\n%s", body) + } + if reminded := linesOf(t, reader, "reminded"); len(reminded) != 1 || reminded[0]["item"] != "task 1" { + t.Fatalf("reminded lines %v, want one for task 1", reminded) } // 1860/AC3: a correction made on the page — read later, from the reading pane — is kept as a reading - // lesson and leaves one corrected line. + // lesson and leaves one corrected line; opening the pane leaves one opened line. + before := len(allLines(t, reader)) pane := pageBody(t, mw, "/read?id="+fmt.Sprint(int64(id))+"&mode=everything", cookie) if !strings.Contains(pane, "the offer for the Harbor demo is attached") { t.Fatalf("the reading pane does not show the mail's text:\n%s", pane) } - postForm(t, mw, "/correct", url.Values{"id": {fmt.Sprint(int64(id))}, "mode": {"everything"}, "read-later": {"on"}, - FormTokenField: {token}}, cookie) + if opened := allLines(t, reader); len(opened) != before+1 || opened[0]["what"] != "opened" { + t.Fatalf("opening the pane left %v, want one opened line more", opened) + } + actOnPage(t, mw, reader, "/correct", url.Values{"id": {fmt.Sprint(int64(id))}, "mode": {"everything"}, "read-later": {"on"}, + FormTokenField: {token}}, cookie, "corrected") lessons, err := reader.call("lessons", map[string]any{}) if err != nil { t.Fatal(err) @@ -514,9 +544,6 @@ func TestTheOperatorsListEndToEnd(t *testing.T) { if corrected := linesOf(t, reader, "corrected"); len(corrected) != 1 || corrected[0]["id"] != id { t.Fatalf("corrected lines %v, want one for mail %v", corrected, id) } - if opened := linesOf(t, reader, "opened"); len(opened) != 1 { - t.Fatalf("opened lines %v, want one: the pane opened once", opened) - } // 1860/AC4: the whole of it, setup included, within its bound. if took := time.Since(began); took > listTestBound { diff --git a/modules/mesh-mail-web/internal/web/listpage.go b/modules/mesh-mail-web/internal/web/listpage.go index c655a88f..1114f27e 100644 --- a/modules/mesh-mail-web/internal/web/listpage.go +++ b/modules/mesh-mail-web/internal/web/listpage.go @@ -370,7 +370,8 @@ func (b *Bundle) listPage(w http.ResponseWriter, r *http.Request) { } ctx, cancel := context.WithTimeout(r.Context(), 20*time.Second) defer cancel() - data := b.listData(ctx, r, modeNamed(r.URL.Query().Get("mode"))) + // Opening the page records it; the page shown after an act does not (review round 1 F1). + data := b.listDataOf(ctx, r, modeNamed(r.URL.Query().Get("mode")), !afterAnAct(r)) renderNamed(w, http.StatusOK, "list", data) } diff --git a/modules/mesh-mail-web/internal/web/quiet_test.go b/modules/mesh-mail-web/internal/web/quiet_test.go new file mode 100644 index 00000000..9d7b97d6 --- /dev/null +++ b/modules/mesh-mail-web/internal/web/quiet_test.go @@ -0,0 +1,64 @@ +package web + +// Review round 1 F1 (novox/hq ADR 0517 §3, §5: no act writes two lines): the page shown after an act — the +// redirect a form act answers, or the script's refresh — records nothing; opening the page or a mail does. + +import ( + "net/http" + "net/url" + "testing" + "time" +) + +// followed is GET of the address a 303 named, with cookie, as a browser follows it. +func followed(t *testing.T, s http.Handler, location string, cookie *http.Cookie, header map[string]string) int { + t.Helper() + r := newGet(location, cookie) + for k, v := range header { + r.Header.Set(k, v) + } + return serve(s, r).Code +} + +// 1859/AC5, 1858/AC4: a tick, undo, remind or correction, and the page it goes back to, record no line; +// the act's own line is its verb's (the reader's), so the page asks record nothing at all. +func TestThePageShownAfterAnActRecordsNothing(t *testing.T) { + for _, tc := range []struct { + path string + form url.Values + }{ + {"/item", url.Values{"id": {"3"}, "item": {"task 1"}, "mode": {"everything"}, "act": {"handle"}}}, + {"/item", url.Values{"id": {"3"}, "item": {"task 1"}, "mode": {"everything"}, "act": {"remind"}}}, + {"/correct", url.Values{"id": {"3"}, "mode": {"work"}, "read-later": {"on"}}}, + } { + reading, b := paneBundle(t) + s, cookie, token := signedIn(t, b) + tc.form.Set(FormTokenField, token) + code, _, h := post(t, s, tc.path, tc.form, cookie, false) + if code != http.StatusSeeOther { + t.Fatalf("%s: status %d", tc.path, code) + } + if code := followed(t, s, h.Get("Location"), cookie, nil); code != http.StatusOK { + t.Fatalf("%s: the page after it: status %d", tc.path, code) + } + if rec := reading.callsOf("record"); len(rec) != 0 { + t.Fatalf("%s and the page it went back to (%s) recorded %v, want nothing: the act's line is its own", tc.path, h.Get("Location"), rec) + } + } + // The script's own refresh after an act records nothing either. + reading, b := paneBundle(t) + s, cookie, _ := signedIn(t, b) + for _, path := range []string{"/?mode=everything", "/read?id=3&mode=everything"} { + followed(t, s, path, cookie, map[string]string{RefreshHeader: "1"}) + } + if rec := reading.callsOf("record"); len(rec) != 0 { + t.Fatalf("the script's refreshes recorded %v, want nothing", rec) + } + // Opening the page and a mail still record one line each. + followed(t, s, "/?mode=everything", cookie, nil) + followed(t, s, "/read?id=3&mode=work", cookie, nil) + if rec := reading.callsOf("record"); len(rec) != 2 || rec[0].Args["what"] != "page opened" || rec[1].Args["what"] != "opened" { + t.Fatalf("opening the page and a mail recorded %v, want page opened then opened", rec) + } + _ = time.Now +} diff --git a/modules/mesh-mail-web/internal/web/readpane.go b/modules/mesh-mail-web/internal/web/readpane.go index fea48c25..a0e121a7 100644 --- a/modules/mesh-mail-web/internal/web/readpane.go +++ b/modules/mesh-mail-web/internal/web/readpane.go @@ -187,7 +187,8 @@ func (b *Bundle) readPage(w http.ResponseWriter, r *http.Request) { http.NotFound(w, r) return } - b.renderRead(w, r, id, modeNamed(r.URL.Query().Get("mode")), http.StatusOK, "", true) + // Opening a mail records it; the pane shown again after a correction does not (review round 1 F1). + b.renderRead(w, r, id, modeNamed(r.URL.Query().Get("mode")), http.StatusOK, "", !afterAnAct(r)) } // correctHandler is POST /correct (1859/AC7): a correction of one mail entry — mark and mail group, not a diff --git a/modules/mesh-mail-web/internal/web/readpane_test.go b/modules/mesh-mail-web/internal/web/readpane_test.go index c7814c37..f03d02be 100644 --- a/modules/mesh-mail-web/internal/web/readpane_test.go +++ b/modules/mesh-mail-web/internal/web/readpane_test.go @@ -91,7 +91,7 @@ func TestEachCorrectionCallsCorrect(t *testing.T) { tc.form.Set(FormTokenField, token) code, _, h := post(t, s, "/correct", tc.form, cookie, false) calls := reading.callsOf("correct") - if code != http.StatusSeeOther || h.Get("Location") != "/read?id=3&mode=work" || len(calls) != 1 { + if code != http.StatusSeeOther || h.Get("Location") != "/read?id=3&mode=work&acted=1" || len(calls) != 1 { t.Fatalf("correct %v: status %d to %q, calls %v", tc.form, code, h.Get("Location"), calls) } want := map[string]any{"id": float64(3), "mode": "work"} diff --git a/modules/mesh-mail-web/internal/web/static.go b/modules/mesh-mail-web/internal/web/static.go index 3af9025c..650a0f50 100644 --- a/modules/mesh-mail-web/internal/web/static.go +++ b/modules/mesh-mail-web/internal/web/static.go @@ -35,3 +35,7 @@ func staticPage(w http.ResponseWriter, r *http.Request) { w.Header().Set("Cache-Control", "no-cache") _, _ = w.Write(body) } + +// RefreshHeader marks the script's own refresh of the page after an act (review round 1 F1): the page it +// answers records nothing, since the act already wrote its one line. +const RefreshHeader = "X-Mail-Refresh" diff --git a/modules/mesh-mail-web/internal/web/static/app.js b/modules/mesh-mail-web/internal/web/static/app.js index 80b6e8b6..4d04974e 100644 --- a/modules/mesh-mail-web/internal/web/static/app.js +++ b/modules/mesh-mail-web/internal/web/static/app.js @@ -17,9 +17,10 @@ list.insertBefore(p, list.firstChild); } - // Put back the parts of the page this address answers now. + // Put back the parts of the page this address answers now. The refresh says it is one, so the page + // records nothing: the act already wrote its one line. function refresh() { - return fetch(location.href, { credentials: "same-origin" }) + return fetch(location.href, { credentials: "same-origin", headers: { "X-Mail-Refresh": "1" } }) .then(function (r) { return r.text(); }) .then(function (html) { var fresh = new DOMParser().parseFromString(html, "text/html");