From bf3b3a5e362bba4edb84c90cec7811df15ba0f6a Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 23:09:46 +0200 Subject: [PATCH] Review round 1: list's pending ignores an ask that has ended, and its before cursor never cuts a page between a decision and research effort of the same number (issue 1064) --- .../mesh-knowledge/cmd/mesh-knowledge/list.go | 11 ++- .../cmd/mesh-knowledge/list_review1_test.go | 77 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 modules/mesh-knowledge/cmd/mesh-knowledge/list_review1_test.go diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/list.go b/modules/mesh-knowledge/cmd/mesh-knowledge/list.go index e0a5ddd7..c1df234c 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/list.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/list.go @@ -53,7 +53,8 @@ type ListRow struct { Topic string `json:"topic,omitempty"` Date string `json:"date,omitempty"` Versions int `json:"versions"` - // Pending says a proposal waits for the knowledge base's own review or the operator's Approve. + // Pending says a version waits for the knowledge base's own review, or the operator still waits to Approve + // (an ask that ended, approved or declined, is not pending; novox/hq issue 1064, review round 1). Pending bool `json:"pending,omitempty"` } @@ -135,10 +136,14 @@ func (k *KB) List(a ListAsk) (*Listed, error) { } } out.Matched++ - if len(out.Rows) < limit { + // Never cut a page between two records of the same number: a decision and a research effort may share + // one, and a bare-number cursor (before) must stay correct for both (novox/hq issue 1064, review round + // 1). + if len(out.Rows) < limit || (len(out.Rows) > 0 && key.Number == lastNumber) { out.Rows = append(out.Rows, ListRow{ Number: rowNumber(r.Kind, r.Number), Kind: r.Kind, Title: r.Title, Status: r.Status, - Topic: r.Topic, Date: r.Date, Versions: e.Versions, Pending: e.Pending != nil || r.Ask != nil, + Topic: r.Topic, Date: r.Date, Versions: e.Versions, + Pending: e.Pending != nil || (r.Ask != nil && r.Ask.Said == AskWaiting), }) lastNumber = key.Number } diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/list_review1_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/list_review1_test.go new file mode 100644 index 00000000..bb712d79 --- /dev/null +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/list_review1_test.go @@ -0,0 +1,77 @@ +package main + +import ( + "context" + "testing" + "time" +) + +// list's pending counts only an ask still waiting, or a pending version: an ask that ended (approved or +// declined) does not make an accepted record show pending (review round 1, issue 1064). +func TestListPendingIgnoresAnAskThatHasEnded(t *testing.T) { + s := newMemStore() + seedRecord(t, s, &Record{Kind: KindDecision, Number: 500, Title: "approved, ask ended", Status: StatusAccepted, + Ask: &OperatorAsk{ID: "a1", Said: AskApproved}}, time.Now()) + seedRecord(t, s, &Record{Kind: KindDecision, Number: 501, Title: "still waiting on the operator", Status: StatusProposed, + Ask: &OperatorAsk{ID: "a2", Said: AskWaiting}}, time.Now()) + k := &KB{Store: s} + + got, err := k.List(ListAsk{}) + if err != nil { + t.Fatal(err) + } + byNumber := map[string]bool{} + for _, row := range got.Rows { + byNumber[row.Number] = row.Pending + } + if byNumber["0500"] { + t.Fatalf("an ended ask (approved) must not show pending: %+v", got.Rows) + } + if !byNumber["0501"] { + t.Fatalf("an ask still waiting must show pending: %+v", got.Rows) + } +} + +// list's before cursor never cuts a page between a decision and a research effort of the same bare number: the +// page extends to include the twin (review round 1, issue 1064). +func TestListNeverCutsAPageBetweenRecordsOfTheSameNumber(t *testing.T) { + s := newMemStore() + ctx := context.Background() + now := time.Now() + // Two records both numbered 100: a decision and a research effort. The decision sorts first (kind + // "decision" < "research"). + if err := s.Create(ctx, Version{By: "test", At: now, Why: "seed", + Record: &Record{Kind: KindDecision, Number: 100, Title: "decision 100", Status: StatusAccepted}}); err != nil { + t.Fatal(err) + } + if err := s.Create(ctx, Version{By: "test", At: now, Why: "seed", + Record: &Record{Kind: KindResearch, Number: 100, Title: "research 100", Status: StatusActive}}); err != nil { + t.Fatal(err) + } + if err := s.Create(ctx, Version{By: "test", At: now, Why: "seed", + Record: &Record{Kind: KindDecision, Number: 99, Title: "decision 99", Status: StatusAccepted}}); err != nil { + t.Fatal(err) + } + k := &KB{Store: s} + + // A limit of 1 would otherwise cut between decision 100 and research 100: both must be shown. + got, err := k.List(ListAsk{Limit: 1}) + if err != nil { + t.Fatal(err) + } + if len(got.Rows) != 2 || got.Rows[0].Number != "0100" || got.Rows[1].Number != "research 100" { + t.Fatalf("want both twins of 100 on the page, got %+v", got.Rows) + } + if got.Next != 100 { + t.Fatalf("want next below 100 (past both twins), got %d", got.Next) + } + + // The next page, asked with before the twins' number, must still answer decision 99. + got, err = k.List(ListAsk{Before: got.Next}) + if err != nil { + t.Fatal(err) + } + if len(got.Rows) != 1 || got.Rows[0].Number != "0099" { + t.Fatalf("want decision 99 on the next page, got %+v", got.Rows) + } +}