diff --git a/modules/messenger/README.md b/modules/messenger/README.md index 722c9cbe..bf6463df 100644 --- a/modules/messenger/README.md +++ b/modules/messenger/README.md @@ -183,7 +183,7 @@ ends takes them away everywhere; its expiry is said with its day when it is not | `messenger_status` | whether the operator can be reached and why not, per channel kind | | `messenger_recent` | the recent sends, edits, digests, answers and failures | | `messenger_test` | a test message now, to one kind or every one | -| `messenger_preview` | what a condition's message would say, tracking line and all, without sending it | +| `messenger_preview` | what a condition's message would say, tracking line and all, without sending it: the words a send would carry, reduced where the content rule refuses its own | | `messenger_check` | whether some words may leave the mesh | | `messenger_identities` | the accounts linked as the operator, and from when each approves | | `messenger_unlink` | unlink a kind's account, said on every channel | diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index 298698b2..755c224e 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -874,19 +874,9 @@ func (h *Holder) askLines(rec *AskRecord) []string { // until what remains may leave. The refusal is recorded and stated once per record. func (h *Holder) words(r *Record, what string) Message { now := h.Now() - if r.Refused != "" { - // Refused before: its words stay withheld in every later message too. - for w := 1; w <= 3; w++ { - m := h.composeTracked(r, what, now, w) - if _, ok := Check(m.Text()); ok { - return m - } - } - } - m := h.composeTracked(r, what, now, 0) - refusal, ok := Check(m.Text()) - if ok { - return m + said, _, refusal, ok := h.allowedWords(r, what, now) + if ok || r.Refused != "" { + return said } h.mu.Lock() r.Refused = refusal.What @@ -907,26 +897,56 @@ func (h *Holder) words(r *Record, what string) Message { h.Logf("[messenger] could not state the refusal on the bus: %v", err) } } - for w := 1; w <= 3; w++ { - m := h.composeTracked(r, what, now, w) - if _, ok := Check(m.Text()); ok { - return m - } - } - return withheld(refusal.What, r.Severity == Urgent) + return said } // composeTracked is compose with the line on the issues that track the condition (tracking.go), so the content // rule judges the message as it is sent. -func (h *Holder) composeTracked(r *Record, what string, now time.Time, withhold int) Message { +func (h *Holder) composeTracked(r *Record, what string, now time.Time, withhold int) (Message, string) { m := compose(r, what, now, h.zone(), withhold) if !h.tracksAsked() || withhold >= 2 { - return m + return m, "" } issues, known := h.trackedBy(r.Key) return withTracking(m, r, what, issues, known, now, h.zone()) } +// allowedWords is the message the content rule leaves of a record: its own words when they may leave the +// mesh, else the same message composed again with less of it until what remains may, else the withheld +// one. It answers the tracking line it carries, what its own words carried, and whether those words are +// what it says. **It changes nothing**: `words` is this walk with the refusal recorded and stated once, +// and `Preview` (preview.go) is this walk alone, so a read of a wording cannot alter what a send says. +func (h *Holder) allowedWords(r *Record, what string, now time.Time) (Message, string, Refusal, bool) { + if r.Refused != "" { + // Refused before: its words stay withheld in every later message too. + for w := 1; w <= 3; w++ { + if m, line := h.composeTracked(r, what, now, w); allowed(m) { + return m, line, Refusal{What: r.Refused}, false + } + } + } + m, line := h.composeTracked(r, what, now, 0) + refusal, ok := Check(m.Text()) + if ok { + return m, line, Refusal{}, true + } + // Composed from a copy carrying the reason, because a withheld message says what it carried. + reduced := *r + reduced.Refused = refusal.What + for w := 1; w <= 3; w++ { + if m, line := h.composeTracked(&reduced, what, now, w); allowed(m) { + return m, line, refusal, false + } + } + return withheld(refusal.What, r.Severity == Urgent), "", refusal, false +} + +// allowed says whether a message's words may leave the mesh. +func allowed(m Message) bool { + _, ok := Check(m.Text()) + return ok +} + // deliver sends a record's message where its severity routes it. func (h *Holder) channels() []Channel { diff --git a/modules/messenger/cmd/messenger/main.go b/modules/messenger/cmd/messenger/main.go index a4711ccb..e0f2499f 100644 --- a/modules/messenger/cmd/messenger/main.go +++ b/modules/messenger/cmd/messenger/main.go @@ -605,8 +605,10 @@ func tools(h *Holder, l *listening) []stdio.Tool { Run: func(a map[string]any) (any, error) { return h.Test(strArg(a, "channel"), strArg(a, "text")), nil }}, {Name: "messenger_preview", Description: "What a condition's message would say, without sending it: the title, the words, the " + - "line on the issues that track it, and whether the content rule would let them leave the mesh. " + - "Composed exactly as it would be sent, from a condition this router holds open; sends nothing.", + "line on the issues that track it, and whether the content rule would let them leave the mesh — " + + "where they may not, the reduced message a send would carry instead. Composed for a condition this " + + "router holds open, by the walk a send composes with; a clearing as if it cleared now, a silence " + + "only while there is one. It sends, queues and records nothing.", Input: map[string]any{ "key": str("the condition's key, as the controller names it, e.g. module.openrazer.laptop.relogin-needed"), "what": str("which message about it: raised (default), reminder, quiet-reminder, escalated, reopened, cleared, silenced"), diff --git a/modules/messenger/cmd/messenger/preview.go b/modules/messenger/cmd/messenger/preview.go index cf483bda..4ab44aa3 100644 --- a/modules/messenger/cmd/messenger/preview.go +++ b/modules/messenger/cmd/messenger/preview.go @@ -7,22 +7,27 @@ package main // they sent (`Sent` holds a key and an outcome, never words), and a notification still on the desk is // not in the notifier's history. So a wording could not be proven without nagging the operator. // -// It composes exactly what `composeTracked` composes for a send — the same words, the same tracking -// line, the same verdict — and sends nothing, queues nothing and records nothing. It also says whether -// the content rule would let those words leave the mesh, which is the other half of what a send decides. +// It is the sending path's own walk and nothing beside it: `allowedWords` (holder.go), which composes +// the message, carries the tracking line into it and reduces it where the content rule refuses its +// words — the same function `words` calls for a send, so a preview cannot drift from one. It records no +// refusal, states nothing on the bus, queues nothing, sends nothing and changes no record: the walk runs +// against a copy. // -// It reads the conditions this router holds open. A key it does not hold is refused by name: a -// condition that is not open has no message, and one invented here would prove nothing about the real -// one. +// What it does not say, because a channel adds it where a condition carries an open ask (`sayOn`): who +// asks, what each answer does, and the answers themselves. A preview of such a condition says that its +// message carries more, rather than letting the words read as the whole of it. +// +// Two messages are about a moment that has not come. A clearing is composed as if the condition cleared +// now, which the answer says, because a clearing composed from an open record would say "after a moment" +// for a condition open four hours. A silence is refused unless the condition is under one: a silence +// message says until when, and a condition nobody silenced has no such moment to say. import ( "sort" "strings" - "time" ) -// The messages a condition is said with, as `what` names them through the sending path (words.go). -// `raised` is the first message about it; `cleared` and `silenced` carry no tracking line (ADR 0285 §3). +// The messages a condition is said with, as `what` names them through the sending path (holder.go). var previewable = []string{"raised", "reminder", "quiet-reminder", "escalated", "reopened", "cleared", "silenced"} // Preview is the message for a condition this router holds, composed as it would be sent, and sent @@ -40,31 +45,51 @@ func (h *Holder) Preview(key, what string) (map[string]any, error) { return nil, errorf("what %q is not one of the messages a condition is said with: %s", what, strings.Join(previewable, ", ")) } - // Asked outside the work, as the router asks it on its own tick: a preview on a cold tracking word - // would say the tracking could not be checked, which is true of this moment and not of a send. + // Asked outside the work, as every tick that composes a message warms it: a preview on a cold + // tracking word would say the tracking could not be checked, which is true of this instant and not + // of the send it is read for. h.warmTracking() + now := h.Now() h.mu.Lock() - held, ok := h.open[key] + held, holding := h.open[key] var r Record - if ok { - r = *held // composed from a copy: a preview changes nothing this router holds + if holding { + r = *held // the walk runs against a copy: a preview changes no record this router holds } - open := make([]string, 0, len(h.open)) - for k := range h.open { - open = append(open, k) + var conditions []string + for k, rec := range h.open { + if rec.Origin == "condition" { + conditions = append(conditions, k) + } } h.mu.Unlock() - if !ok { - sort.Strings(open) - if len(open) == 0 { + + if !holding || r.Origin != "condition" { + sort.Strings(conditions) + switch { + case len(conditions) == 0: return nil, errorf("no condition %s is open here, and this router holds none: `mesh-controller.conditions` says what the controller holds open", key) + case holding: + return nil, errorf("%s is not a condition but this router's own %s record, which is said as part of an ask, never on its own; it holds the conditions %s", + key, r.Origin, strings.Join(conditions, ", ")) + default: + return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(conditions, ", ")) } - return nil, errorf("no condition %s is open here; it holds %s", key, strings.Join(open, ", ")) + } + if what == "silenced" && !r.silenced(now) { + return nil, errorf("%s is under no silence, and a silence message says until when it is quiet: there is no such message to read", key) + } + if what == "cleared" { + // As if it cleared now: the clearing says how long it was open, which an open record cannot say. + r.Cleared = now } - now := h.Now() - m := h.composeTracked(&r, what, now, 0) + asking := h.openAsk(&r) != nil // askOf takes the same lock, so this is asked outside it + m, line, refusal, ownWords := h.allowedWords(&r, what, now) + if r.Answered != "" { + m = h.answeredWords(&r, m) + } out := map[string]any{ "key": key, "what": what, @@ -78,31 +103,27 @@ func (h *Holder) Preview(key, what string) (map[string]any, error) { if m.Quiet { out["quiet"] = true } - if line, said := h.trackingLine(&r, what, now); said { + if line != "" { out["tracking"] = line } - if refusal, allowed := Check(m.Text()); allowed { - out["allowed"] = true - } else { - // A send would say less instead (compose with a withhold level), never these words. - out["allowed"] = false + if what == "cleared" { + out["as-if"] = "it cleared now: how long it was open is said from this moment" + } + if asking { + out["carries-more"] = "this condition has an open ask, so a channel's message also carries who asks, what each answer does, and the answers themselves" + } + out["allowed"] = ownWords + if !ownWords { + // What stands above is what would leave; its own words would not, and this says why. out["carried"] = refusal.What - out["class"] = refusal.Class + if refusal.Class != "" { + out["class"] = refusal.Class + } + out["instead"] = "its own words may not leave the mesh, so this is the message as reduced for sending" } return out, nil } -// trackingLine is the line on the issues that track the condition, and whether this message carries one: -// a clearing and a silence say none, and nor does a router that cannot ask the tracker. -func (h *Holder) trackingLine(r *Record, what string, now time.Time) (string, bool) { - if !h.tracksAsked() || r.Origin != "condition" || what == "cleared" || what == "silenced" { - return "", false - } - issues, known := h.trackedBy(r.Key) - line, _ := trackingWords(issues, known, now, h.zone()) - return line, line != "" -} - func previewableWhat(what string) bool { for _, w := range previewable { if w == what { diff --git a/modules/messenger/cmd/messenger/preview_test.go b/modules/messenger/cmd/messenger/preview_test.go index fc9ec02b..281b6e7a 100644 --- a/modules/messenger/cmd/messenger/preview_test.go +++ b/modules/messenger/cmd/messenger/preview_test.go @@ -1,43 +1,95 @@ package main import ( + "reflect" "strings" "testing" "time" ) -// A condition's message can be read before it is sent (novox/hq issue 422): the same words a send would -// carry, the tracking line among them, and nothing goes out while they are read. +// A condition's message can be read before it is sent (novox/hq issue 422): the words a send would +// carry, the tracking line among them, and nothing of this router moved while they were read. -// previewHolder is a router holding one raised condition that needs nothing, with the issues given -// tracking its key. -func previewHolder(t *testing.T, issues []TrackedIssue) (*Holder, *fakeChannel, *fakeChannel, *clock, Condition) { +// previewHolder is a router holding one urgent condition — one a send goes out for at once, so a preview +// that sends, queues or records anything is caught — with the issues given tracking its key. +func previewHolder(t *testing.T, issues []TrackedIssue) (*Holder, *fakeChannel, *fakeChannel, *clock, *memStore, Condition) { t.Helper() - h, tg, dt, c, _ := newHolder(t) + h, tg, dt, c, st := newHolder(t) h.Tracking = func() (map[string][]TrackedIssue, error) { return map[string][]TrackedIssue{"mesh.hand-acts.rolled-back.healer-wanted": issues}, nil } - k := cond("mesh.hand-acts.rolled-back.healer-wanted", Warning, "rolled-back was repaired by hand 4 times in 14 days") + k := cond("mesh.hand-acts.rolled-back.healer-wanted", Urgent, "rolled-back was repaired by hand 4 times in 14 days") k.Explanation = "Nothing for you to do. A person repaired rolled-back by hand 4 times in 14 days, so an automatic repair is wanted for it. Nothing is broken now." k.Needs = "" k.Raised = c.now() h.Condition(EventRaised, k) settle(h, c) - tg.sends, dt.sends = nil, nil - return h, tg, dt, c, k + if len(tg.sends) == 0 && len(dt.sends) == 0 { + t.Fatal("the condition this is read from was never sent, so a preview sending something would not show") + } + return h, tg, dt, c, st, k +} + +// unmoved fails when anything a send would move has moved: the sends, the queue, the history, what is +// stored, and the record itself. +type unmoved struct { + sends, queued, recent, refusals int + stored Record + held Record +} + +func mark(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, key string) unmoved { + t.Helper() + h.mu.Lock() + defer h.mu.Unlock() + queued := 0 // the outbox is held per channel, never per condition + for _, list := range h.outbox { + queued += len(list) + } + return unmoved{sends: len(tg.sends) + len(dt.sends), queued: queued, recent: len(h.recent), + refusals: len(h.refusals), stored: st.recs[key], held: *h.open[key]} +} + +func (was unmoved) check(t *testing.T, h *Holder, tg, dt *fakeChannel, st *memStore, c *clock, key string) { + t.Helper() + // A queued message waits for a tick, so the queue is read and then given one: either would catch it. + now := mark(t, h, tg, dt, st, key) + if now.sends != was.sends { + t.Errorf("a preview sent %d message(s)", now.sends-was.sends) + } + if now.queued != was.queued { + t.Errorf("a preview queued %d message(s)", now.queued-was.queued) + } + if now.recent != was.recent { + t.Errorf("a preview recorded %d send(s)", now.recent-was.recent) + } + if now.refusals != was.refusals { + t.Errorf("a preview recorded %d refusal(s): only a send does that", now.refusals-was.refusals) + } + if !reflect.DeepEqual(now.stored, was.stored) { + t.Errorf("a preview wrote the record: %+v", now.stored) + } + if !reflect.DeepEqual(now.held, was.held) { + t.Errorf("a preview changed the record held: %+v", now.held) + } + settle(h, c) + if after := mark(t, h, tg, dt, st, key); after.sends != was.sends { + t.Errorf("a preview queued a message, which the next tick sent: %d", after.sends-was.sends) + } } func TestAConditionsMessageIsReadWithoutBeingSent(t *testing.T) { - h, tg, dt, _, _ := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located", Needs: "decision"}}) + h, tg, dt, c, st, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located", Needs: "decision"}}) + was := mark(t, h, tg, dt, st, k.Key) - out, err := h.Preview("mesh.hand-acts.rolled-back.healer-wanted", "") + out, err := h.Preview(k.Key, "") if err != nil { t.Fatal(err) } if out["what"] != "raised" { t.Errorf("what %v; a preview with none asked is the first message", out["what"]) } - if out["title"] != "mesh hand-acts rolled-back healer-wanted" { + if out["title"] != "Urgent: mesh hand-acts rolled-back healer-wanted" { t.Errorf("title %q", out["title"]) } body, _ := out["body"].(string) @@ -54,54 +106,83 @@ func TestAConditionsMessageIsReadWithoutBeingSent(t *testing.T) { if strings.Contains(body, "Nothing for you to do.") { t.Errorf("the preview reassures where nothing is being done: %q", body) } - if len(tg.sends) != 0 || len(dt.sends) != 0 { - t.Fatalf("a preview sent something: telegram %d, desktop %d", len(tg.sends), len(dt.sends)) - } keyless(t, Message{Title: out["title"].(string), Body: body}) + was.check(t, h, tg, dt, st, c, k.Key) } -func TestAPreviewSaysTheSameWordsTheSendWouldCarry(t *testing.T) { +func TestAPreviewSaysTheWordsTheChannelWasGiven(t *testing.T) { issues := []TrackedIssue{{Number: 386, Status: "fixing", Worker: "fix-386@g14", Since: at(21, 10)}} - h, tg, dt, c, k := previewHolder(t, issues) + h, tg, dt, c, st, k := previewHolder(t, issues) + sent := dt.sends[len(dt.sends)-1] // the message the desk was actually handed + was := mark(t, h, tg, dt, st, k.Key) - out, err := h.Preview(k.Key, "reminder") + out, err := h.Preview(k.Key, "") if err != nil { t.Fatal(err) } - // The same record, the same moment, through the sending path. + if out["title"] != sent.Title || out["body"] != sent.Body { + t.Fatalf("preview %q / %q; the channel was given %q / %q", out["title"], out["body"], sent.Title, sent.Body) + } + if !strings.Contains(sent.Body, "Tracked as issue 386: fix-386 on g14 is fixing it since ") { + t.Fatalf("the message this is compared against carries no tracking line: %q", sent.Body) + } + was.check(t, h, tg, dt, st, c, k.Key) +} + +func TestAClearingIsPreviewedAsIfItClearedNow(t *testing.T) { + h, tg, dt, c, st, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) + c.pass(4 * time.Hour) + was := mark(t, h, tg, dt, st, k.Key) + + out, err := h.Preview(k.Key, "cleared") + if err != nil { + t.Fatal(err) + } + // Composed from the open record, this said "after a moment": the clearing's own time is zero, so the + // time it was open came out negative. + if out["title"] != "Resolved: mesh hand-acts rolled-back healer-wanted, after 4.0 h" { + t.Errorf("title %q; a clearing says how long it was open", out["title"]) + } + if out["as-if"] == nil { + t.Errorf("the answer does not say the clearing is composed as if it cleared now: %v", out) + } + if _, said := out["tracking"]; said { + t.Errorf("a clearing says a tracking line: %v", out["tracking"]) + } + was.check(t, h, tg, dt, st, c, k.Key) +} + +func TestASilenceIsPreviewedOnlyWhileThereIsOne(t *testing.T) { + h, tg, dt, c, st, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) + + // Under no silence, a silence message would say a moment that does not exist. + _, err := h.Preview(k.Key, "silenced") + if err == nil || !strings.Contains(err.Error(), "under no silence") { + t.Fatalf("a silence previewed without one: %v", err) + } + h.mu.Lock() - r := *h.open[k.Key] + h.open[k.Key].SilencedTill = c.now().Add(7 * 24 * time.Hour) + h.open[k.Key].SilencedWhy = "the operator knows" h.mu.Unlock() - want := h.composeTracked(&r, "reminder", c.now(), 0) - if out["title"] != want.Title || out["body"] != want.Body { - t.Fatalf("preview %q / %q; a send would say %q / %q", out["title"], out["body"], want.Title, want.Body) + was := mark(t, h, tg, dt, st, k.Key) + + out, err := h.Preview(k.Key, "silenced") + if err != nil { + t.Fatal(err) } - if !strings.Contains(want.Body, "Tracked as issue 386: fix-386 on g14 is fixing it since ") { - t.Fatalf("the message this is compared against carries no tracking line: %q", want.Body) + title, _ := out["title"].(string) + if !strings.HasPrefix(title, "Silenced until ") || strings.Contains(title, "Jan 1 ") || strings.Contains(title, " 1 Jan ") { + t.Errorf("title %q; a silence says until when it is quiet", title) } - if len(tg.sends) != 0 || len(dt.sends) != 0 { - t.Fatalf("a preview sent something: telegram %d, desktop %d", len(tg.sends), len(dt.sends)) + if _, said := out["tracking"]; said { + t.Errorf("a silence says a tracking line: %v", out["tracking"]) } + was.check(t, h, tg, dt, st, c, k.Key) } -func TestAClearingAndASilencePreviewWithoutATrackingLine(t *testing.T) { - h, _, _, _, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) - for _, what := range []string{"cleared", "silenced"} { - out, err := h.Preview(k.Key, what) - if err != nil { - t.Fatal(err) - } - if _, said := out["tracking"]; said { - t.Errorf("%s says a tracking line: %v", what, out["tracking"]) - } - if body, _ := out["body"].(string); strings.Contains(body, "issue") { - t.Errorf("%s names an issue: %q", what, body) - } - } -} - -func TestAPreviewOfAConditionTheRouterDoesNotHoldIsRefusedByName(t *testing.T) { - h, _, _, _, k := previewHolder(t, nil) +func TestAPreviewOfWhatThisRouterDoesNotHoldIsRefusedByName(t *testing.T) { + h, _, _, _, _, k := previewHolder(t, nil) _, err := h.Preview("machine.ace.silent", "") if err == nil { @@ -117,11 +198,20 @@ func TestAPreviewOfAConditionTheRouterDoesNotHoldIsRefusedByName(t *testing.T) { if _, err := h.Preview(k.Key, "shouted"); err == nil || !strings.Contains(err.Error(), "shouted") { t.Errorf("an unknown message: %v", err) } + + // An ask's own record is held here too, and is said as part of an ask, never as a condition. + h.mu.Lock() + h.open["ask.someone.1"] = &Record{Key: "ask.someone.1", Origin: "ask", Severity: Urgent, Summary: "a question"} + h.mu.Unlock() + _, err = h.Preview("ask.someone.1", "") + if err == nil || !strings.Contains(err.Error(), "not a condition") { + t.Errorf("an ask's record previewed as a condition: %v", err) + } } func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) { asked := 0 - h, _, _, c, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) + h, _, _, c, _, k := previewHolder(t, []TrackedIssue{{Number: 352, Status: "located"}}) h.Tracking = func() (map[string][]TrackedIssue, error) { asked++ return map[string][]TrackedIssue{k.Key: {{Number: 352, Status: "located"}}}, nil @@ -140,12 +230,18 @@ func TestAPreviewAsksTheTrackerWhenItsWordWentCold(t *testing.T) { } } -func TestAPreviewSaysWhenTheWordsWouldNotBeAllowedOut(t *testing.T) { - h, _, _, c, _ := previewHolder(t, nil) - k := cond("machine.ace.silent", Warning, "the home server has not been heard for 15 min") - k.Explanation = "Needs you: look at /home/someone/secrets.txt on it." +func TestAPreviewSaysTheReducedWordsAndWhyWhenItsOwnMayNotLeave(t *testing.T) { + h, tg, dt, c, st, _ := previewHolder(t, nil) + stated := 0 + h.Emit = func(string, any) error { stated++; return nil } + // A warning that needs nothing is held, not sent, so this refusal is one no send has met yet. + k := cond("machine.ace.unreadable", Warning, "a file on it could not be read") + k.Explanation = "Nothing for you to do. A file at /home/someone/secrets.txt could not be read." + k.Needs = "" k.Raised = c.now() h.Condition(EventRaised, k) + settle(h, c) + was := mark(t, h, tg, dt, st, k.Key) out, err := h.Preview(k.Key, "") if err != nil { @@ -154,4 +250,26 @@ func TestAPreviewSaysWhenTheWordsWouldNotBeAllowedOut(t *testing.T) { if out["allowed"] != false || out["carried"] == nil { t.Fatalf("allowed %v, carried %v; the content rule refuses a path", out["allowed"], out["carried"]) } + // The words said are the ones that would go out, not the refused ones. + body, _ := out["body"].(string) + if strings.Contains(body, "/home/") { + t.Errorf("the preview says words no send would carry: %q", body) + } + if !strings.Contains(body, "withheld") { + t.Errorf("body %q; a reduced message says that part of it was withheld", body) + } + if !strings.Contains(body, "a file path") { + t.Errorf("body %q; a reduced message says what it carried", body) + } + if out["instead"] == nil { + t.Errorf("the answer does not say these are the reduced words: %v", out) + } + // Reading it recorded no refusal, stated nothing on the bus, and left the record's own words alone. + if stated != 0 { + t.Errorf("a preview stated %d fact(s) on the bus", stated) + } + if was.held.Refused != "" { + t.Fatalf("the record was already refused before the preview: %q", was.held.Refused) + } + was.check(t, h, tg, dt, st, c, k.Key) } diff --git a/modules/messenger/cmd/messenger/tracking.go b/modules/messenger/cmd/messenger/tracking.go index eeea1f24..9e0a484e 100644 --- a/modules/messenger/cmd/messenger/tracking.go +++ b/modules/messenger/cmd/messenger/tracking.go @@ -159,9 +159,11 @@ const ( // withTracking is a condition's message with the line on the issues that track it, and its verdict changed where // an issue waits for the operator or nobody works on any of them. Cleared and silenced messages, and one withheld // whole, say nothing of it. -func withTracking(m Message, r *Record, what string, issues []TrackedIssue, known bool, now time.Time, loc *time.Location) Message { +// It answers the line it put in the message, empty when the message carries none, so whoever composed it +// never has to work out the rule a second time (novox/hq issue 422). +func withTracking(m Message, r *Record, what string, issues []TrackedIssue, known bool, now time.Time, loc *time.Location) (Message, string) { if r.Origin != "condition" || what == "cleared" || what == "silenced" || m.Title == "A mesh message was withheld" { - return m + return m, "" } line, verdict := trackingWords(issues, known, now, loc) // since is the last line, which never holds a line break; the explanation is everything before it. @@ -184,7 +186,7 @@ func withTracking(m Message, r *Record, what string, issues []TrackedIssue, know } } m.Body = strings.Join(parts, "\n") - return m + return m, line } // needsAnswer is the "Needs you:" sentence for the issues that wait for the operator's answer. diff --git a/modules/messenger/cmd/messenger/tracking_test.go b/modules/messenger/cmd/messenger/tracking_test.go index 47d2364a..fbf719c3 100644 --- a/modules/messenger/cmd/messenger/tracking_test.go +++ b/modules/messenger/cmd/messenger/tracking_test.go @@ -26,7 +26,8 @@ func trackedMessage(issues []TrackedIssue, known bool) Message { Explanation: "Nothing for you to do. A person repaired rolled-back by hand 4 times in 14 days, so an automatic repair is wanted for it. Nothing is broken now.", Raised: trackNow.Add(-26 * time.Hour)} m := compose(r, "quiet-reminder", trackNow, time.UTC, 0) - return withTracking(m, r, "quiet-reminder", issues, known, trackNow, time.UTC) + out, _ := withTracking(m, r, "quiet-reminder", issues, known, trackNow, time.UTC) + return out } func TestAConditionsMessageSaysWhereTheIssueThatTracksItStands(t *testing.T) { @@ -129,7 +130,7 @@ func TestNothingForYouToDoIsNeverSaidWhileTheIssueWaitsOnTheOperatorOrNobodyWork func TestAVerdictThatNeedsTheOperatorStands(t *testing.T) { r := &Record{Key: "module.mounts.shanks.unhealthy", Origin: "condition", Severity: Urgent, Headline: "mounts not working on shanks", Explanation: "Needs you: give the share's login. mounts on shanks is not healthy.", Raised: trackNow.Add(-time.Hour)} - m := withTracking(compose(r, "raised", trackNow, time.UTC, 0), r, "raised", + m, _ := withTracking(compose(r, "raised", trackNow, time.UTC, 0), r, "raised", []TrackedIssue{{Number: 386, Status: "open", Needs: "operator"}}, true, trackNow, time.UTC) want := "Needs you: give the share's login. mounts on shanks is not healthy.\nTracked as issue 386: it waits for your answer.\nSince 18:30." if m.Body != want { @@ -144,14 +145,16 @@ func TestAClearingASilenceAndANoticeSayNoIssue(t *testing.T) { Cleared: trackNow, SilencedTill: trackNow.Add(time.Hour)} for _, what := range []string{"cleared", "silenced"} { m := compose(r, what, trackNow, time.UTC, 0) - if got := withTracking(m, r, what, issues, true, trackNow, time.UTC); got.Body != m.Body { - t.Errorf("%s: %q", what, got.Body) + got, line := withTracking(m, r, what, issues, true, trackNow, time.UTC) + if got.Body != m.Body || line != "" { + t.Errorf("%s: %q, line %q", what, got.Body, line) } } n := &Record{Key: "n", Origin: "notify", Headline: "a notice", Raised: trackNow} m := compose(n, "raised", trackNow, time.UTC, 0) - if got := withTracking(m, n, "raised", issues, true, trackNow, time.UTC); got.Body != m.Body { - t.Errorf("notice: %q", got.Body) + got, line := withTracking(m, n, "raised", issues, true, trackNow, time.UTC) + if got.Body != m.Body || line != "" { + t.Errorf("notice: %q, line %q", got.Body, line) } }