diff --git a/beads/build.go b/beads/build.go index 659e21c07f89f662bb82a67a21724a470b58dfc0..56a1ee6911144ca909c362d65b6c6694db49ff18 100644 --- a/beads/build.go +++ b/beads/build.go @@ -30,10 +30,27 @@ labels, labelsTotal, _ := readRowsOptional(ctx, sess, ref, "labels") 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 @@ }, Total: len(rolling) + len(linedUp) + len(stalled) + len(pastStand), Truncated: truncated, ShownOf: shownOf, + Clipped: clipped, Filter: filter, FilterOpts: opts, Layout: parseLayout(query.Get("layout")), @@ -155,6 +173,22 @@ if data.Layout == LayoutStream { data.Sections = streamSections(data.Lanes, created) } 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 diff --git a/beads/model.go b/beads/model.go index 49000b9768cd53ca3725412a9cd128f806685c37..2cb4acd5a94762b981d651e39bc5118252f846e1 100644 --- a/beads/model.go +++ b/beads/model.go @@ -45,6 +45,22 @@ // against the rows actually read. IssuesClipped is the comparison callers // 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 @@ -69,6 +85,23 @@ // epic mode: the issue's parent-child children and their rollup. Subtasks []Subtask SubtaskDone int // # of subtasks in the closed category 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 diff --git a/beads/truncation_test.go b/beads/truncation_test.go index f69ca75e25ebbab3d753137313266f5499b5463c..b66fc31a747a0f425ee742cdf8ed56ce3df45bdf 100644 --- a/beads/truncation_test.go +++ b/beads/truncation_test.go @@ -144,6 +144,28 @@ Total: n, } } +// 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 @@ } // --- 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 @@ assert.False(t, d.IssuesClipped(), "and Truncated is not the same fact as a clipped issue set") 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) }) } diff --git a/web/beads_test.go b/web/beads_test.go index c48631dcc9766fc5a57394f6fb4f4b8c0e9bb439..99425133242084d00ef263efcf2c88e58413878d 100644 --- a/web/beads_test.go +++ b/web/beads_test.go @@ -238,6 +238,52 @@ // beadsClippedTail is an id of beadsClippedFixture that exists in the tracker and // 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) { @@ -492,6 +538,111 @@ t.Errorf("the board banner changed; body=%s", body) } if strings.Contains(body, "This read was clipped") { t.Errorf("the detail pane's line is the detail pane's; body=%s", body) + } +} + +// 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, "labels 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, "custom_statuses 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, "dependencies 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) + } } } diff --git a/web/templates/beads.html b/web/templates/beads.html index 01b32340ec685aa34a645830f418916f09420b45..b2671418595bed43ea7c2f596c36e28eb5f01958 100644 --- a/web/templates/beads.html +++ b/web/templates/beads.html @@ -422,8 +422,23 @@ {{end}} {{else}} {{/* ---------------- board ---------------- */}} -{{if .Data.Truncated}} -
Showing the first {{.Data.Total}} of {{.Data.ShownOf}} issues.
+{{/* 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}} +
+ {{if .Data.IssuesClipped}}Showing the first {{.Data.Total}} of {{.Data.ShownOf}} issues.{{end}} + {{range .Data.Clipped}}{{if ne .Table "issues"}} +
The {{.Table}} table was clipped at {{.Shown}} of {{.Total}} rows: {{.Effect}}.
+ {{end}}{{end}} +
{{end}}