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)

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
 		}

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)
+	}
+}