M beads/build.go => beads/build.go +36 -2
@@ 30,11 30,28 @@ func Build(ctx context.Context, sess BrowseSession, ref string, query url.Values
statuses, statusesTotal, _ := readRowsOptional(ctx, sess, ref, "custom_statuses")
// The board's flag names the two required tables it buckets from, which is
- // what it has always reported; widening it to the optional ones is a separate
- // question from this one.
+ // what it has always reported. It stays that, because it is paired with a
+ // count line the other tables cannot change; the optional ones are reported
+ // beside it, one entry each.
truncated := issuesTotal > Max || depsTotal > Max
shownOf := issuesTotal
+ // Every table the board draws from, in read order, with what a clipped read
+ // of it costs the board. clipsOver keeps the ones that were actually clipped
+ // and drops the rest, so a board over complete reads carries an empty list.
+ // The totals are all in hand from the reads just made; naming a table here
+ // costs no second read.
+ clipped := clipsOver(
+ ClippedTable{Table: "issues", Total: issuesTotal,
+ Effect: "the lanes below hold only the rows that were read"},
+ ClippedTable{Table: "dependencies", Total: depsTotal,
+ Effect: "blocked and blocking counts are short, and a card standing in Lined Up may belong in Stalled"},
+ ClippedTable{Table: "labels", Total: labelsTotal,
+ Effect: "cards are missing label pills, and the label filter offers only the labels that were read"},
+ ClippedTable{Table: "custom_statuses", Total: statusesTotal,
+ Effect: "statuses defined past the cap fall back to name heuristics, so a card may be in the wrong lane"},
+ )
+
// status name → category, from custom_statuses (may be empty → heuristics).
catByStatus := indexStatusCategories(statuses)
@@ 143,6 160,7 @@ func Build(ctx context.Context, sess BrowseSession, ref string, query url.Values
Total: len(rolling) + len(linedUp) + len(stalled) + len(pastStand),
Truncated: truncated,
ShownOf: shownOf,
+ Clipped: clipped,
Filter: filter,
FilterOpts: opts,
Layout: parseLayout(query.Get("layout")),
@@ 157,6 175,22 @@ func Build(ctx context.Context, sess BrowseSession, ref string, query url.Values
return data, nil
}
+// clipsOver keeps the candidates whose table came back clipped — a reported
+// total past Max — in the order they were given, and fills in the rows that
+// were read. A table read whole is not on the list at all: the list is the
+// clips, not the tables.
+func clipsOver(cands ...ClippedTable) []ClippedTable {
+ var out []ClippedTable
+ for _, c := range cands {
+ if c.Total <= Max {
+ continue
+ }
+ c.Shown = Max
+ out = append(out, c)
+ }
+ return out
+}
+
// readClip is what the row reads reported about their own completeness: whether
// any table came back clipped at Max, and the issues table's true total. It is
// threaded from Build into buildDetail so the detail pane can say its read was
M beads/model.go => beads/model.go +33 -0
@@ 45,6 45,22 @@ type Data struct {
// usually want.
ShownOf int
+ // Clipped names every table the *board* draws from that came back clipped,
+ // in the order they were read, and is board mode's alone: it is empty in the
+ // detail modes, which read a different set of tables for one issue and say
+ // their one line from Truncated.
+ //
+ // It exists because Truncated could not grow to hold this. The flag is
+ // paired with a count line — "the first N of M issues" — so it can only mean
+ // the reads that decide N and M: issues and dependencies. The board also
+ // draws label pills and its whole filter vocabulary from labels, and every
+ // card's lane from custom_statuses, and a clip in either degrades the board
+ // without moving a single count. Folding those into the same bool would have
+ // made a flag that means four different things and a number that no longer
+ // follows from it, which is why they are reported here instead — named, with
+ // their own row counts, and with what the board lost by them.
+ Clipped []ClippedTable
+
// Query is the request's query as parsed, carried so the layout toggle can
// rebuild this exact URL with one key replaced (web's withQuery). The view
// envelope does not carry the query, and rebuilding it from Filter would
@@ 71,6 87,23 @@ type Data struct {
SubtaskTotal int // len(Subtasks); the progress denominator
}
+// ClippedTable is one table a projection read that exceeded Max: its name, how
+// many rows were read against how many exist, and the one line saying what this
+// surface lost by the rest.
+//
+// Effect is written at the read, because what a clipped table costs is a fact
+// about the projection that read it and not about the table: the same clipped
+// labels table costs the board every card's pills and the label filter's
+// options, and would cost a detail pane one issue's pills. Shown and Total are
+// what make the line checkable rather than a bare warning — a reader can tell
+// how much is missing.
+type ClippedTable struct {
+ Table string // the table as read, e.g. "labels"
+ Shown int // rows read: Max, by what a clip is
+ Total int // rows the store says the table holds
+ Effect string // what this surface loses by the rows it did not read
+}
+
// IssuesClipped reports that the issues table itself exceeded Max, so this
// projection saw only its first Max rows. Truncated is the wider fact (any
// input table was clipped); this is the one that decides whether the issue set
M beads/truncation_test.go => beads/truncation_test.go +165 -5
@@ 144,6 144,28 @@ func manyComments(n int) *browse.RowPage {
}
}
+// manyLabels builds n label rows, all on i-0000, named label-0000 upwards. A
+// read that stops at Max leaves the rest off every card and out of the board's
+// label filter.
+func manyLabels(n int) *browse.RowPage {
+ rows := make([][]string, 0, n)
+ for i := 0; i < n; i++ {
+ rows = append(rows, []string{"i-0000", fmt.Sprintf("label-%04d", i)})
+ }
+ return &browse.RowPage{Columns: []string{"issue_id", "label"}, Rows: rows, Total: n}
+}
+
+// manyStatuses builds n custom_statuses rows: the three real categories first,
+// then filler. The real ones lead so the lanes still bucket correctly and the
+// only thing under test is that the clip is reported.
+func manyStatuses(n int) *browse.RowPage {
+ rows := [][]string{{"open", "open"}, {"in_progress", "in_progress"}, {"closed", "closed"}}
+ for i := len(rows); i < n; i++ {
+ rows = append(rows, []string{fmt.Sprintf("status-%04d", i), "open"})
+ }
+ return &browse.RowPage{Columns: []string{"name", "category"}, Rows: rows, Total: len(rows)}
+}
+
func detail(t *testing.T, sess BrowseSession, id string) *Data {
t.Helper()
d, err := Build(context.Background(), sess, "main", url.Values{"issue": {id}})
@@ 221,7 243,13 @@ func TestDetailReportsAClippedLabelsRead(t *testing.T) {
// --- the board's flag is what it always was ----------------------------------
-func TestBoardTruncationIsUnchanged(t *testing.T) {
+// The flag and its number are what they were before the board learned to name
+// the tables it draws from: Truncated is still the issues/dependencies pair the
+// lanes are bucketed from, and ShownOf is still the issues table's total. The
+// per-table list is a second, wider fact carried beside them (Data.Clipped,
+// exercised below), never a new meaning for these two — mcpsrv's list_issues
+// reads them as they are.
+func TestBoardTruncationFlagIsUnchanged(t *testing.T) {
t.Run("clipped issues", func(t *testing.T) {
d, err := Build(context.Background(), bigTracker(), "main", url.Values{})
require.NoError(t, err)
@@ 251,16 279,148 @@ func TestBoardTruncationIsUnchanged(t *testing.T) {
assert.Equal(t, 3, d.ShownOf)
})
- t.Run("a clip in a table the board does not bucket from", func(t *testing.T) {
- // The board's flag has always named issues and dependencies. Widening it to
- // labels or comments would change what the board reports; the detail pane
- // covers them because it reads them for this one issue.
+ t.Run("a clip in a table the board does not read", func(t *testing.T) {
+ // The board's flag has always named issues and dependencies, and comments is
+ // not a table the board reads at all: the thread belongs to one issue's
+ // detail pane. Neither the flag nor the per-table list may mention it —
+ // the board reports the tables it draws from, and no others.
d, err := Build(context.Background(), smallTrackerWith(map[string]*browse.RowPage{
"comments": manyComments(Max + 3),
}), "main", url.Values{})
require.NoError(t, err)
assert.False(t, d.Truncated)
assert.False(t, d.IssuesClipped())
+ assert.Empty(t, clippedNames(d), "the board never read comments, so it has nothing to say about it")
+ })
+
+ t.Run("labels and custom_statuses do not flip the flag", func(t *testing.T) {
+ // They are reported — see TestBoardReportsEveryClippedTableItDrawsFrom —
+ // but through Clipped, not by widening Truncated. A flag that meant four
+ // different things would take the count line's meaning with it: neither of
+ // these tables changes how many issues were read.
+ d, err := Build(context.Background(), smallTrackerWith(map[string]*browse.RowPage{
+ "labels": manyLabels(Max + 2),
+ "custom_statuses": manyStatuses(Max + 1),
+ }), "main", url.Values{})
+ require.NoError(t, err)
+ assert.False(t, d.Truncated, "the issues and dependencies reads were both whole")
+ assert.False(t, d.IssuesClipped())
+ assert.Equal(t, 3, d.ShownOf)
+ })
+}
+
+// --- the board names every table it draws from -------------------------------
+
+// clippedNames is the board's clip list as table names, in the order it reports
+// them.
+func clippedNames(d *Data) []string {
+ out := make([]string, 0, len(d.Clipped))
+ for _, c := range d.Clipped {
+ out = append(out, c.Table)
+ }
+ return out
+}
+
+// clipFor returns the entry the board reported for a table, failing the test
+// when it reported none.
+func clipFor(t *testing.T, d *Data, table string) ClippedTable {
+ t.Helper()
+ for _, c := range d.Clipped {
+ if c.Table == table {
+ return c
+ }
+ }
+ require.FailNowf(t, "no entry", "the board says nothing about a clipped %s table; it reported %v", table, clippedNames(d))
+ return ClippedTable{}
+}
+
+// A clipped labels table degrades every card on the board — the pills go
+// missing and the label filter silently offers only the labels that were read —
+// without moving a single count, which is exactly why one flag could not carry
+// it. Each entry names its table, both numbers, and what the board lost.
+func TestBoardReportsEveryClippedTableItDrawsFrom(t *testing.T) {
+ t.Run("clipped labels", func(t *testing.T) {
+ d, err := Build(context.Background(), smallTrackerWith(map[string]*browse.RowPage{
+ "labels": manyLabels(Max + 2),
+ }), "main", url.Values{})
+ require.NoError(t, err)
+ require.Equal(t, []string{"labels"}, clippedNames(d))
+
+ c := clipFor(t, d, "labels")
+ assert.Equal(t, Max, c.Shown, "the rows that were read")
+ assert.Equal(t, Max+2, c.Total, "the rows the table holds")
+ assert.NotEmpty(t, c.Effect, "a table named without a cost is a bare warning")
+
+ // The degradation the entry is about, on the board itself: the labels past
+ // the cap are on neither a card nor the filter's list.
+ assert.NotContains(t, d.FilterOpts.Labels, fmt.Sprintf("label-%04d", Max+1),
+ "the filter offers only the labels that were read")
+ })
+
+ t.Run("clipped custom_statuses", func(t *testing.T) {
+ d, err := Build(context.Background(), smallTrackerWith(map[string]*browse.RowPage{
+ "custom_statuses": manyStatuses(Max + 1),
+ }), "main", url.Values{})
+ require.NoError(t, err)
+ require.Equal(t, []string{"custom_statuses"}, clippedNames(d))
+ assert.Equal(t, Max+1, clipFor(t, d, "custom_statuses").Total)
+ assert.False(t, d.Truncated, "and it is reported without touching the flag")
+ })
+
+ t.Run("clipped issues", func(t *testing.T) {
+ d, err := Build(context.Background(), bigTracker(), "main", url.Values{})
+ require.NoError(t, err)
+ require.Equal(t, []string{"issues"}, clippedNames(d))
+ c := clipFor(t, d, "issues")
+ assert.Equal(t, Max, c.Shown)
+ assert.Equal(t, Max+5, c.Total)
+ assert.Equal(t, d.ShownOf, c.Total, "the same number the count line has always shown")
+ })
+
+ t.Run("clipped dependencies", func(t *testing.T) {
+ deps := make([][]string, 0, Max+1)
+ for i := 0; i < Max+1; i++ {
+ deps = append(deps, []string{fmt.Sprintf("d%d", i), "i-0001", "i-0000", "blocks"})
+ }
+ d, err := Build(context.Background(), smallTrackerWith(map[string]*browse.RowPage{
+ "dependencies": {
+ Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"},
+ Rows: deps,
+ Total: len(deps),
+ },
+ }), "main", url.Values{})
+ require.NoError(t, err)
+ require.Equal(t, []string{"dependencies"}, clippedNames(d))
+ assert.Equal(t, Max+1, clipFor(t, d, "dependencies").Total)
+ })
+
+ t.Run("several at once, in read order", func(t *testing.T) {
+ sess := smallTrackerWith(map[string]*browse.RowPage{
+ "issues": manyIssues(Max + 5),
+ "labels": manyLabels(Max + 2),
+ "custom_statuses": manyStatuses(Max + 1),
+ })
+ d, err := Build(context.Background(), sess, "main", url.Values{})
+ require.NoError(t, err)
+ assert.Equal(t, []string{"issues", "labels", "custom_statuses"}, clippedNames(d),
+ "read order, so the list is the same on every request")
+ })
+
+ t.Run("a complete read", func(t *testing.T) {
+ d, err := Build(context.Background(), beadsFixture(), "main", url.Values{})
+ require.NoError(t, err)
+ assert.Empty(t, d.Clipped, "nothing was clipped, so the board has nothing to report")
+ assert.False(t, d.Truncated)
+ })
+
+ t.Run("the detail pane keeps its one line", func(t *testing.T) {
+ // Clipped is the board's answer. A detail pane reads a different set of
+ // tables for a single issue and says its one sentence from Truncated;
+ // giving it a board's list would be a second, board-shaped story about a
+ // page that never drew a lane.
+ d := detail(t, bigTracker(), "i-0007")
+ assert.True(t, d.Truncated)
+ assert.Empty(t, d.Clipped)
})
}
M web/beads_test.go => web/beads_test.go +151 -0
@@ 238,6 238,52 @@ func clipPage(p *browse.RowPage, limit int) *browse.RowPage {
// sits past the rows any read of it returns.
const beadsClippedTail = "i-2003"
+// beadsClippedLabelsFixture is beadsFixture with one table past the cap: labels.
+// Four issues, every one of them read, every lane and count exactly as the
+// complete fixture renders them — and beads.Max+3 label rows of which the read
+// returns the first beads.Max.
+//
+// That invariance is the case itself. A clipped labels table takes the pills off
+// the cards and the options out of the label filter while leaving "N issues"
+// true, so a count-shaped flag has nothing to say about it and the board is
+// degraded in silence.
+func beadsClippedLabelsFixture() *fakeSession {
+ sess := beadsFixture()
+ sess.rowsByTable["labels"] = clipPage(manyLabels(beads.Max+3), beads.Max)
+ return sess
+}
+
+// beadsClippedDepsFixture is beadsFixture with the dependencies table past the
+// cap and nothing else: the issue set is whole, so the board's count line has
+// nothing to report and the dependencies entry carries the whole message.
+func beadsClippedDepsFixture() *fakeSession {
+ sess := beadsFixture()
+ sess.rowsByTable["dependencies"] = clipPage(manyDeps(beads.Max+1), beads.Max)
+ return sess
+}
+
+// manyLabels builds n label rows on i-open, named label-0000 upwards.
+func manyLabels(n int) *browse.RowPage {
+ rows := make([][]string, 0, n)
+ for i := range n {
+ rows = append(rows, []string{"i-open", fmt.Sprintf("label-%04d", i)})
+ }
+ return &browse.RowPage{Columns: []string{"issue_id", "label"}, Rows: rows, Total: n}
+}
+
+// manyDeps builds n "blocks" edges from i-blocked to i-open.
+func manyDeps(n int) *browse.RowPage {
+ rows := make([][]string, 0, n)
+ for i := range n {
+ rows = append(rows, []string{fmt.Sprintf("d%d", i), "i-blocked", "i-open", "blocks"})
+ }
+ return &browse.RowPage{
+ Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"},
+ Rows: rows,
+ Total: n,
+ }
+}
+
// --- end to end --------------------------------------------------------------
func TestBeadsHandleViewBoard(t *testing.T) {
@@ 495,6 541,111 @@ func TestBeadsBoardTruncationBannerIsUnchanged(t *testing.T) {
}
}
+// The gap this closes: labels is a table the board draws from — every card's
+// pills and the whole label filter — and a clipped read of it moves no count, so
+// the board used to render a degraded page and say nothing at all.
+func TestBeadsBoardReportsAClippedLabelsRead(t *testing.T) {
+ h := newHarness(t)
+ h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})
+ h.browse.sess = beadsClippedLabelsFixture()
+ setViews(t, h, &beadsView{})
+
+ rec := h.do("GET", "/~alice/db/view/beads", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("board: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ // The table, both numbers, and what the board lost by them — the numbers are
+ // what make the line checkable rather than a bare warning.
+ if !strings.Contains(body, "<code>labels</code> table was clipped at 2000 of 2003 rows") {
+ t.Errorf("the board says nothing about a clipped labels read; body=%s", body)
+ }
+ if !strings.Contains(body, "the label filter offers only the labels that were read") {
+ t.Errorf("the board names the table without saying what it cost; body=%s", body)
+ }
+ // And it must not borrow the count line: every issue was read, so "the first
+ // N of M issues" would be false.
+ if strings.Contains(body, "Showing the first") {
+ t.Errorf("the issue set is whole; the count line has nothing to report; body=%s", body)
+ }
+ // The degradation itself, on the page: the labels past the cap reach neither
+ // a card nor the filter's options.
+ if strings.Contains(body, "label-2002") {
+ t.Errorf("a label past the cap cannot be on the page; body=%s", body)
+ }
+}
+
+// custom_statuses decides which lane every card stands in, so it is the board's
+// too.
+func TestBeadsBoardReportsAClippedStatusesRead(t *testing.T) {
+ h := newHarness(t)
+ h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})
+ sess := beadsFixture()
+ statuses := sess.rowsByTable["custom_statuses"]
+ rows := statuses.Rows
+ for i := len(rows); i < beads.Max+1; i++ {
+ rows = append(rows, []string{fmt.Sprintf("status-%04d", i), "open"})
+ }
+ sess.rowsByTable["custom_statuses"] = clipPage(
+ &browse.RowPage{Columns: statuses.Columns, Rows: rows, Total: len(rows)}, beads.Max)
+ h.browse.sess = sess
+ setViews(t, h, &beadsView{})
+
+ rec := h.do("GET", "/~alice/db/view/beads", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("board: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ if !strings.Contains(body, "<code>custom_statuses</code> table was clipped at 2000 of 2001 rows") {
+ t.Errorf("the board says nothing about a clipped custom_statuses read; body=%s", body)
+ }
+ if !strings.Contains(body, "a card may be in the wrong lane") {
+ t.Errorf("the board names the table without saying what it cost; body=%s", body)
+ }
+}
+
+// dependencies has always flipped the flag, and the flag has always printed the
+// count line — which says "the first 4 of 4 issues" when the issue set is whole.
+// The table now says the thing that is actually true of this read.
+func TestBeadsBoardReportsAClippedDependenciesRead(t *testing.T) {
+ h := newHarness(t)
+ h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})
+ h.browse.sess = beadsClippedDepsFixture()
+ setViews(t, h, &beadsView{})
+
+ rec := h.do("GET", "/~alice/db/view/beads", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("board: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ if !strings.Contains(body, "<code>dependencies</code> table was clipped at 2000 of 2001 rows") {
+ t.Errorf("the board says nothing about a clipped dependencies read; body=%s", body)
+ }
+ if strings.Contains(body, "Showing the first") {
+ t.Errorf("every issue was read, so the count line must stay silent; body=%s", body)
+ }
+}
+
+// And a board over complete reads stays quiet: the lines are facts about a read,
+// not decoration on every board.
+func TestBeadsBoardOnACompleteReadReportsNoClip(t *testing.T) {
+ h := newHarness(t)
+ h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})
+ h.browse.sess = beadsFixture()
+ setViews(t, h, &beadsView{})
+
+ rec := h.do("GET", "/~alice/db/view/beads", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("board: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ for _, unwanted := range []string{"was clipped at", "Showing the first"} {
+ if strings.Contains(body, unwanted) {
+ t.Errorf("nothing was clipped, so the board must not say %q; body=%s", unwanted, body)
+ }
+ }
+}
+
// rowsText is every cell of a page as one string, for asserting that an id is
// nowhere in the rows a fixture hands over.
func rowsText(p *browse.RowPage) string {
M web/templates/beads.html => web/templates/beads.html +17 -2
@@ 422,8 422,23 @@ pre.field-body {
{{else}}
{{/* ---------------- board ---------------- */}}
-{{if .Data.Truncated}}
-<div class="alert alert-warning">Showing the first {{.Data.Total}} of {{.Data.ShownOf}} issues.</div>
+{{/* The board reports the tables it draws from, and no others — and it reports
+ them one by one, because they fail differently. The count line is the issues
+ read and is printed only when the issues read was short: "the first 4 of 4"
+ is not a warning, it is a wrong sentence. Every other clipped table names
+ itself, both its row counts and what the board lost by the rows it never
+ read — a clipped labels table costs every card its pills and narrows the
+ label filter while leaving all four counts true, which is why one flag could
+ not have carried this. The issues entry is skipped in the list below because
+ the count line above is that entry, in the wording the board has always
+ used. */}}
+{{if .Data.Clipped}}
+<div class="alert alert-warning">
+ {{if .Data.IssuesClipped}}Showing the first {{.Data.Total}} of {{.Data.ShownOf}} issues.{{end}}
+ {{range .Data.Clipped}}{{if ne .Table "issues"}}
+ <div>The <code>{{.Table}}</code> table was clipped at {{.Shown}} of {{.Total}} rows: {{.Effect}}.</div>
+ {{end}}{{end}}
+</div>
{{end}}
<form class="beads-filter" method="get" action="/~{{.Repo.OwnerName}}/{{.Repo.Name}}/view/beads">