issue 1859: the page shown after an act records nothing, so each act leaves one line
Review round 1 F1 (ADR 0517 §3, §5): a form act went back to the list or the pane, which recorded page opened or opened again, and the script's refresh did too, so last looked became the last act and opened counts grew. The redirect after an act carries acted=1 and the refresh an X-Mail-Refresh header; that page records nothing. The 1860 test now follows each redirect and counts every line, and reminds the task (F2, 1858/AC9), which the store's own integration test also reads back as a date.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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"}
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user