diff --git a/beads/build.go b/beads/build.go index 56a1ee6911144ca909c362d65b6c6694db49ff18..59edbcdcd6cb709020d3a0789d09935245f28c09 100644 --- a/beads/build.go +++ b/beads/build.go @@ -77,7 +77,7 @@ truncated: truncated || labelsTotal > Max || statusesTotal > Max, issuesTotal: issuesTotal, } return buildDetail(ctx, sess, ref, want, issues, issueCols, deps, depCols, - labelsByIssue, catByStatus, catByIssue, clip), nil + labels, labelsByIssue, catByStatus, catByIssue, clip), nil } // Board mode: parse the sticky filters and collect dropdown options from the @@ -209,7 +209,7 @@ func buildDetail( ctx context.Context, sess BrowseSession, ref, want string, issues *browse.RowPage, issueCols map[string]int, deps *browse.RowPage, depCols map[string]int, - labelsByIssue map[string][]string, + labels *browse.RowPage, labelsByIssue map[string][]string, catByStatus, catByIssue map[string]string, clip readClip, ) *Data { @@ -271,6 +271,19 @@ CloseReason: cell(issueCols, row, "close_reason"), Labels: labelsByIssue[want], } + // The stored rows behind everything above, in the order the tables were read. + // Only the tables that hold rows *of this issue* are here: custom_statuses is + // read for the lane, but its rows describe the tracker's statuses rather than + // this issue, so there is no row of it that belongs on this pane. comments and + // events are added below, where they are read. + data.addRaw(rawTableOf("issues", issues, matchColumn("id", want))) + data.addRaw(rawTableOf("labels", labels, matchColumn("issue_id", want))) + data.addRaw(rawTableOf("dependencies", deps, func(cols map[string]int, r rowCells) bool { + // Both directions: an edge is this issue's whether it points out of it or + // into it, and the pane draws both lists from exactly these rows. + return cell(cols, r, "issue_id") == want || cell(cols, r, "depends_on_issue_id") == want + })) + edge := func(id, typ string) Edge { st := statusByIssue[id] return Edge{ @@ -349,6 +362,7 @@ // comments table costs this issue's thread whatever sits past Max, so it // counts towards the flag like any other input. if comments, commentsTotal, err := readRowsOptional(ctx, sess, ref, "comments"); err == nil && comments != nil { data.Truncated = data.Truncated || commentsTotal > Max + data.addRaw(rawTableOf("comments", comments, matchColumn("issue_id", want))) ccols := indexCols(comments.Columns) for _, r := range rowsOf(comments) { if cell(ccols, r, "issue_id") != want { @@ -372,6 +386,7 @@ // The audit log (events) is optional too; when present it joins the comments // in the History tab as humanized, time-ordered entries. if events, eventsTotal, err := readRowsOptional(ctx, sess, ref, "events"); err == nil && events != nil { data.Truncated = data.Truncated || eventsTotal > Max + data.addRaw(rawTableOf("events", events, matchColumn("issue_id", want))) ecols := indexCols(events.Columns) for _, r := range rowsOf(events) { if cell(ecols, r, "issue_id") != want { diff --git a/beads/model.go b/beads/model.go index 2cb4acd5a94762b981d651e39bc5118252f846e1..430c86e77dd19c4e3a7fb18cc25974b4e94a1eb3 100644 --- a/beads/model.go +++ b/beads/model.go @@ -85,6 +85,17 @@ // 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 + + // Raw is the stored rows this detail was built from, in the order the tables + // were read: the issue's own row first, then the rows belonging to it in + // labels, dependencies, comments and events. Set in the detail modes only, + // and empty when the issue was not found — there is nothing stored to show. + // + // It is what makes everything above it checkable: every other field here is a + // reading of these rows, and a reading that dropped a column or humanized an + // event into the wrong sentence looks exactly like a correct one from the + // pane alone. See beads/raw.go for why the tables stop where they do. + Raw []RawTable } // ClippedTable is one table a projection read that exceeded Max: its name, how diff --git a/beads/raw.go b/beads/raw.go new file mode 100644 index 0000000000000000000000000000000000000000..de3ca991818351af8ea6c75722f646456e4c7133 --- /dev/null +++ b/beads/raw.go @@ -0,0 +1,133 @@ +package beads + +import "sourcecraft.dev/bigbes/sr-ht-dolt/browse" + +// --- the stored rows behind one issue ---------------------------------------- +// +// Everything else in this package is a reading: the Issue struct names the +// columns bd surfaces and drops the rest, humanizeEvent turns two JSON blobs +// into a sentence, an Edge keeps an id and a type out of a dependency row. Each +// of those is a choice, and a choice can be wrong — a column bd added that no +// field here models, a status this projection mapped to the wrong category, an +// event whose stored old_value says something the humanized line does not — and +// none of it is visible from the pane that made the choice. +// +// So the detail modes also carry the rows they were built from, as read. The +// point is to be checkable against the rendering above it, which is why nothing +// here is normalised, sorted or rewritten: the columns come in the order the +// table was read in, the values are the strings browse returned, and a cell that +// holds no value says so rather than rendering as one. + +// RawMax caps how many rows of one related table the raw section carries. The +// issues row is one row by construction; a busy issue's events or comments are +// not, and a section that dumps three hundred audit rows is not a check on the +// rendering, it is a second page nobody reads. RawTable.Matched says how many +// rows actually belong to the issue, so a capped table is visibly capped. +const RawMax = 50 + +// RawCell is one stored cell: the column it came from, the value browse rendered +// for it, and whether the cell holds a value at all. +// +// Null is the whole reason this type exists rather than a plain string pair. +// browse renders a cell that holds no value as the text "NULL", which is exactly +// what a cell storing those four characters renders as, and cell() resolves that +// collision by flattening a real NULL to "". That is right everywhere else here +// — a missing closed_at is rendered as nothing — and wrong in this one place: a +// view whose purpose is to be compared against the database may not report an +// absent cell and an empty string as the same thing. +// +// Value is "" for a null cell rather than the "NULL" browse printed. Carrying +// that text would put the collision straight back: a consumer that renders Value +// without reading Null would print "NULL" for both readings again, which is the +// state this type exists to leave behind. +type RawCell struct { + Column string + Value string + Null bool +} + +// RawRow is one stored row: its cells in the order the columns were read in. +// Column order is data here — it is the table's own order, and a raw view that +// sorted it would be hiding one of the things it is meant to show. +type RawRow struct { + Cells []RawCell +} + +// RawTable is the rows one table contributed to one issue. +// +// Matched is how many rows of the table belong to the issue; Rows holds at most +// RawMax of them, in table order. The two differ only when the cap bit, and that +// difference is what the pane says out loud. +type RawTable struct { + Table string + Rows []RawRow + Matched int +} + +// Clipped reports that the table had more rows for this issue than RawMax, so +// Rows is the first of them and not all of them. +func (t RawTable) Clipped() bool { return t.Matched > len(t.Rows) } + +// rawRow renders one row as stored, in the page's own column order. +// +// A cell's nullness is read exactly as cell() reads it — through rowCells.isNull, +// which is the one place in this package that decides — so the raw section and +// the fields above it can never disagree about which cells hold a value. +func rawRow(columns []string, r rowCells) RawRow { + out := RawRow{Cells: make([]RawCell, 0, len(columns))} + for i, name := range columns { + if i >= len(r.values) { + // A row shorter than its header is a malformed page, not a null cell: + // there is no cell here to report either way, so it is left out. + break + } + if r.isNull(i) { + out.Cells = append(out.Cells, RawCell{Column: name, Null: true}) + continue + } + out.Cells = append(out.Cells, RawCell{Column: name, Value: r.values[i]}) + } + return out +} + +// rawTableOf collects the rows of a page that belong to one issue, capped at +// RawMax and counted whole. A table with no rows for this issue yields nil — the +// pane lists the tables that had something to say, not every table it read. +func rawTableOf(name string, page *browse.RowPage, match func(map[string]int, rowCells) bool) *RawTable { + if page == nil { + return nil + } + cols := indexCols(page.Columns) + tbl := RawTable{Table: name} + for _, r := range rowsOf(page) { + if !match(cols, r) { + continue + } + tbl.Matched++ + if len(tbl.Rows) < RawMax { + tbl.Rows = append(tbl.Rows, rawRow(page.Columns, r)) + } + } + if tbl.Matched == 0 { + return nil + } + return &tbl +} + +// matchColumn matches the rows whose named column holds the wanted id — the +// shape of every per-issue table here except dependencies, which has two such +// columns and gets its own matcher at the call site. +func matchColumn(column, want string) func(map[string]int, rowCells) bool { + return func(cols map[string]int, r rowCells) bool { + return cell(cols, r, column) == want + } +} + +// addRaw appends a table's rows to the raw section, skipping the tables that had +// none. +func (d *Data) addRaw(t *RawTable) { + if t == nil { + return + } + d.Raw = append(d.Raw, *t) +} diff --git a/beads/raw_test.go b/beads/raw_test.go new file mode 100644 index 0000000000000000000000000000000000000000..f7bd43dfb0ccd5c193cea392ca8a137cc25cc8cd --- /dev/null +++ b/beads/raw_test.go @@ -0,0 +1,282 @@ +package beads + +import ( + "context" + "fmt" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" +) + +// The raw section is the pane's own check on itself: the rows the detail was +// built from, as read. What it has to get right is what every other projection +// here is allowed to get wrong — the column order it was read in, and the +// difference between a cell that holds no value and one that holds a string +// which happens to read like one. + +// rawTableByName finds one table's entry in a built detail's raw section. +func rawTableByName(d *Data, name string) *RawTable { + for i := range d.Raw { + if d.Raw[i].Table == name { + return &d.Raw[i] + } + } + return nil +} + +// rawCellByName finds one cell of a raw row by its column name. +func rawCellByName(r RawRow, column string) *RawCell { + for i := range r.Cells { + if r.Cells[i].Column == column { + return &r.Cells[i] + } + } + return nil +} + +// rawColumns lists a raw row's columns in the order they are carried. +func rawColumns(r RawRow) []string { + out := make([]string, 0, len(r.Cells)) + for _, c := range r.Cells { + out = append(out, c.Column) + } + return out +} + +// The issues row arrives in the table's own column order. beadsFixture orders +// its columns so nothing sits at a natural index, so an implementation that +// sorted or re-derived the order could not match this by accident. +func TestRawIssuesRowKeepsTheColumnOrderItWasReadIn(t *testing.T) { + d, err := Build(context.Background(), beadsFixture(), "main", url.Values{"issue": {"i-done"}}) + require.NoError(t, err) + + tbl := rawTableByName(d, "issues") + require.NotNil(t, tbl, "the issue's own row is the whole point of the section") + require.Len(t, tbl.Rows, 1) + assert.Equal(t, 1, tbl.Matched) + assert.False(t, tbl.Clipped()) + + assert.Equal(t, + []string{"id", "title", "status", "priority", "issue_type", "assignee", + "created_at", "closed_at", "close_reason", "is_blocked"}, + rawColumns(tbl.Rows[0]), + "column order is data: it is the order the table was read in") + + // Every column carries its own row value, addressed by name. + require.NotNil(t, rawCellByName(tbl.Rows[0], "close_reason")) + assert.Equal(t, "Fixed in commit abc123", rawCellByName(tbl.Rows[0], "close_reason").Value) +} + +// The three states the section exists for. A cell browse reported as NULL, a +// cell storing the four characters "NULL", and a cell storing the empty string +// must be three distinguishable things in the model — the first two render +// identically as strings, and the last two are identical as strings once a null +// has been flattened. +func TestRawCellsSeparateAbsentFromStoredNullFromEmpty(t *testing.T) { + issues := &browse.RowPage{ + Columns: []string{"id", "title", "assignee", "notes", "status"}, + Rows: [][]string{ + // id title assignee notes status + {"i-1", "NULL", "NULL", "", "open"}, + }, + Nulls: [][]bool{ + // title holds no value at all; assignee stores those four characters; + // notes stores the empty string. + {false, true, false, false, false}, + }, + Total: 1, + } + + sess := &fakeSession{rowsByTable: map[string]*browse.RowPage{ + "issues": issues, + "dependencies": { + Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"}, + Rows: [][]string{}, + Nulls: [][]bool{}, + }, + }} + + d, err := Build(context.Background(), sess, "main", url.Values{"issue": {"i-1"}}) + require.NoError(t, err) + tbl := rawTableByName(d, "issues") + require.NotNil(t, tbl) + require.Len(t, tbl.Rows, 1) + row := tbl.Rows[0] + + absent := rawCellByName(row, "title") + require.NotNil(t, absent) + assert.True(t, absent.Null, "a cell that holds no value says so") + assert.Equal(t, "", absent.Value, + "an absent cell carries no text: browse's \"NULL\" is a rendering of absence, not stored data") + + stored := rawCellByName(row, "assignee") + require.NotNil(t, stored) + assert.False(t, stored.Null, "those four characters are stored, so the cell holds a value") + assert.Equal(t, "NULL", stored.Value) + + empty := rawCellByName(row, "notes") + require.NotNil(t, empty) + assert.False(t, empty.Null, "an empty string is a value") + assert.Equal(t, "", empty.Value) + + // Pairwise distinguishable, which is the property the pane depends on: no two + // of the three agree on both (Value, Null). + assert.NotEqual(t, absent.Null, stored.Null, "absent vs. the stored text \"NULL\"") + assert.NotEqual(t, absent.Null, empty.Null, "absent vs. the empty string") + assert.NotEqual(t, stored.Value, empty.Value, "the stored text \"NULL\" vs. the empty string") +} + +// A page that carries no mask at all — a hand-built one; browse fills a mask for +// every page it returns — is read the way this package read every page before +// the mask existed, and the raw section reads it the same way the fields above +// it do. One reading, one decision site. +func TestRawFollowsTheSameNullReadingAsTheFields(t *testing.T) { + // beadsFixture's pages carry no Nulls, so "NULL" is how absence arrives. + d, err := Build(context.Background(), beadsFixture(), "main", url.Values{"issue": {"i-open"}}) + require.NoError(t, err) + require.NotNil(t, d.Issue) + assert.Equal(t, "", d.Issue.ClosedAt, "the field reads the unmasked \"NULL\" as absent") + + tbl := rawTableByName(d, "issues") + require.NotNil(t, tbl) + closed := rawCellByName(tbl.Rows[0], "closed_at") + require.NotNil(t, closed) + assert.True(t, closed.Null, "and so does the raw row: they cannot disagree") + assert.Equal(t, "", closed.Value) +} + +// Every table the pane draws rows of this issue from is carried, in read order, +// and only the rows that belong to the issue: another issue's comment is another +// issue's business. +func TestRawCarriesEveryPerIssueTableInReadOrder(t *testing.T) { + d, err := Build(context.Background(), beadsFixture(), "main", url.Values{"issue": {"i-open"}}) + require.NoError(t, err) + + var names []string + for _, tbl := range d.Raw { + names = append(names, tbl.Table) + } + assert.Equal(t, []string{"issues", "labels", "dependencies", "comments"}, names, + "read order, and only the tables that had rows for this issue") + + labels := rawTableByName(d, "labels") + require.NotNil(t, labels) + assert.Equal(t, 2, labels.Matched, "both of i-open's label rows") + + // i-open is the target of i-blocked's edge: an edge is this issue's in either + // direction, because the pane draws both lists from these rows. + deps := rawTableByName(d, "dependencies") + require.NotNil(t, deps) + require.Len(t, deps.Rows, 1) + assert.Equal(t, "i-blocked", rawCellByName(deps.Rows[0], "issue_id").Value) + + comments := rawTableByName(d, "comments") + require.NotNil(t, comments) + require.Len(t, comments.Rows, 1) + assert.Equal(t, "first!", rawCellByName(comments.Rows[0], "text").Value, + "i-prog's comment belongs to i-prog") + + // i-open has no audit events in this fixture, and a table with nothing to say + // is not listed at all. + assert.Nil(t, rawTableByName(d, "events")) + + // custom_statuses is read (the lane comes from it) and is deliberately absent: + // its rows describe the tracker's statuses, not this issue. + assert.Nil(t, rawTableByName(d, "custom_statuses")) +} + +// The events table is where stored and rendered are furthest apart — +// humanizeEvent turns two JSON blobs into one sentence — so the rows behind the +// History tab are carried with the strings intact. +func TestRawCarriesTheStoredEventStrings(t *testing.T) { + d, err := Build(context.Background(), beadsEpicFixture(), "main", url.Values{"issue": {"i-epic"}}) + require.NoError(t, err) + + events := rawTableByName(d, "events") + require.NotNil(t, events) + assert.Equal(t, 4, events.Matched, "i-c1's created event is not this issue's") + require.Len(t, events.Rows, 4) + assert.Equal(t, `{"status":"open"}`, rawCellByName(events.Rows[1], "old_value").Value, + "the exact stored string behind the humanized line") +} + +// The epic mode is the detail mode with a rollup on top, and it carries the raw +// rows for the same reason. +func TestRawIsCarriedInEpicMode(t *testing.T) { + d, err := Build(context.Background(), beadsEpicFixture(), "main", url.Values{"issue": {"i-epic"}}) + require.NoError(t, err) + require.Equal(t, "epic", d.Mode) + + tbl := rawTableByName(d, "issues") + require.NotNil(t, tbl) + require.Len(t, tbl.Rows, 1) + assert.Equal(t, "i-epic", rawCellByName(tbl.Rows[0], "id").Value) + assert.Equal(t, "epic", rawCellByName(tbl.Rows[0], "issue_type").Value) + + // The three parent-child edges are the issue's, in both directions. + deps := rawTableByName(d, "dependencies") + require.NotNil(t, deps) + assert.Equal(t, 3, deps.Matched) +} + +// An issue that is not there has no stored row, and the section has nothing to +// show rather than something empty to show. +func TestRawIsEmptyForAMissingIssue(t *testing.T) { + d, err := Build(context.Background(), beadsFixture(), "main", url.Values{"issue": {"nope"}}) + require.NoError(t, err) + require.True(t, d.Missing()) + assert.Empty(t, d.Raw, "nothing was found, so there is nothing stored to show") +} + +// The board is not a detail pane and carries none of this: it renders no stored +// row, and a board that carried every row of every card would be the table +// browser with lanes drawn on it. +func TestRawIsNotCarriedOnTheBoard(t *testing.T) { + d, err := Build(context.Background(), beadsFixture(), "main", url.Values{}) + require.NoError(t, err) + require.Equal(t, "board", d.Mode) + assert.Empty(t, d.Raw) +} + +// A table with more of this issue's rows than RawMax is cut, and says how many +// there were: a section that dumps three hundred audit rows is a second page +// nobody reads, and one that silently shows fifty of three hundred is a lie. +func TestRawCapsARelatedTableAndCountsItWhole(t *testing.T) { + sess := beadsFixture() + rows := make([][]string, 0, RawMax+7) + for i := range RawMax + 7 { + rows = append(rows, []string{"i-open", "alice", fmt.Sprintf("comment %d", i), "2024-01-05"}) + } + sess.rowsByTable["comments"] = &browse.RowPage{ + Columns: []string{"issue_id", "author", "text", "created_at"}, + Rows: rows, + Total: len(rows), + } + + d, err := Build(context.Background(), sess, "main", url.Values{"issue": {"i-open"}}) + require.NoError(t, err) + + tbl := rawTableByName(d, "comments") + require.NotNil(t, tbl) + assert.Len(t, tbl.Rows, RawMax) + assert.Equal(t, RawMax+7, tbl.Matched) + assert.True(t, tbl.Clipped()) + assert.Equal(t, "comment 0", rawCellByName(tbl.Rows[0], "text").Value, "the first of them, in table order") +} + +// A row shorter than its own header is a malformed page. There is no cell to +// report for the missing columns, so they are left out rather than invented as +// nulls — and the cells that do exist still line up with their column names. +func TestRawSkipsColumnsAShortRowDoesNotHave(t *testing.T) { + cols := []string{"id", "title", "status"} + row := rowCells{values: []string{"i-1", "Ahoy"}} + + got := rawRow(cols, row) + require.Len(t, got.Cells, 2) + assert.Equal(t, RawCell{Column: "id", Value: "i-1"}, got.Cells[0]) + assert.Equal(t, RawCell{Column: "title", Value: "Ahoy"}, got.Cells[1]) +} diff --git a/beads/rows.go b/beads/rows.go index b80c1335adc9e7bcb877922427058c6bda06c71b..8752021af9cd76f84f8043d9f95eab30b60895ef 100644 --- a/beads/rows.go +++ b/beads/rows.go @@ -102,17 +102,29 @@ i, ok := cols[name] if !ok || i < 0 || i >= len(row.values) { return "" } - if i < len(row.nulls) { - if row.nulls[i] { - return "" - } - return row.values[i] - } - v := row.values[i] - if v == "NULL" { + if row.isNull(i) { return "" } - return v + return row.values[i] +} + +// isNull reports whether the i-th cell of this row holds no value. +// +// It is the one place in this package that decides, so cell — which flattens an +// absent cell to "" — and the raw section — which must not — can never come to +// different answers about the same cell. The reading is the one cell documents: +// the mask decides when the row has one, and a row with none falls back to the +// pre-mask reading, where the text "NULL" is how absence arrived. +// +// A cell past the end of the row is not a value, so it reads as absent. +func (r rowCells) isNull(i int) bool { + if i < 0 || i >= len(r.values) { + return true + } + if i < len(r.nulls) { + return r.nulls[i] + } + return r.values[i] == "NULL" } // indexStatusCategories maps a status name (lowercased) to its category, from diff --git a/web/beads_test.go b/web/beads_test.go index 99425133242084d00ef263efcf2c88e58413878d..09b0475f7408cb449a8e14ee4a8df2028aaa01c8 100644 --- a/web/beads_test.go +++ b/web/beads_test.go @@ -1248,6 +1248,230 @@ } return rest[:j] } +// --- the stored rows section --------------------------------------------------- + +// beadsNullFixture is one issue carrying, in one row, the three states a raw +// view may never conflate: a cell that holds no value (closed_at), a cell that +// stores the four characters "NULL" (assignee), and a cell that stores the empty +// string (notes). The mask is what tells the first two apart, so it is set here +// exactly as browse fills it for a real read. +func beadsNullFixture() *fakeSession { + issues := &browse.RowPage{ + Columns: []string{"id", "title", "status", "assignee", "closed_at", "notes"}, + Rows: [][]string{ + {"i-null", "Three states", "open", "NULL", "NULL", ""}, + }, + Nulls: [][]bool{ + {false, false, false, false, true, false}, + }, + Total: 1, + } + return &fakeSession{ + branches: []browse.Branch{{Name: "main", Head: "abcdef1234567890"}}, + tables: beadsTables(), + rowsByTable: map[string]*browse.RowPage{ + "issues": issues, + "dependencies": { + Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"}, + Nulls: [][]bool{}, + Total: 0, + }, + }, + } +} + +// rawSection returns the markup of the collapsed stored-rows block. +func rawSection(t *testing.T, body string) string { + t.Helper() + i := strings.Index(body, `
`) + if i < 0 { + t.Fatalf("no stored-rows section in the page: %s", body) + } + rest := body[i:] + j := strings.Index(rest, "
") + if j < 0 { + t.Fatalf("unterminated stored-rows section") + } + return rest[:j] +} + +// rawTableBlock returns one table's markup inside the stored-rows section. +func rawTableBlock(t *testing.T, body, table string) string { + t.Helper() + sec := rawSection(t, body) + head := `
` + i := strings.Index(sec, head) + if i < 0 { + t.Fatalf("no %s rows in the stored-rows section: %s", table, sec) + } + rest := sec[i+len(head):] + if j := strings.Index(rest, `
= 0 { + rest = rest[:j] + } + return rest +} + +// rawCellValue returns the rendered value cell for one column of one table's +// first row in the stored-rows section. +func rawCellValue(t *testing.T, body, table, column string) string { + t.Helper() + block := rawTableBlock(t, body, table) + head := `` + column + `` + i := strings.Index(block, head) + if i < 0 { + t.Fatalf("no %s.%s cell in the stored-rows section: %s", table, column, block) + } + rest := block[i+len(head):] + open := `` + k := strings.Index(rest, open) + if k < 0 { + t.Fatalf("no value cell after %s.%s", table, column) + } + rest = rest[k+len(open):] + j := strings.Index(rest, "") + if j < 0 { + t.Fatalf("unterminated value cell for %s.%s", table, column) + } + return rest[:j] +} + +// The detail pane carries the rows it was built from, and carries them closed: +// this is a tool for checking the rendering, not the reason a reader opened the +// page, so it is a
with no open attribute — the stream layout's Past +// Stand idiom. +func TestBeadsDetailShowsTheStoredRowsCollapsed(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?issue=i-open", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + + if !strings.Contains(body, `
`) { + t.Fatalf("no stored-rows section on the detail pane; body=%s", body) + } + if strings.Contains(body, `
Ready to roll` { + t.Errorf("issues.title rendered as %q", got) + } + if got := rawCellValue(t, body, "comments", "text"); got != `first!` { + t.Errorf("comments.text rendered as %q", got) + } +} + +// The three states, on the page. A cell that holds no value renders as a NULL +// chip, a cell storing those four characters renders as the text it stores, and +// a cell storing the empty string says it is empty — the rendering used to make +// the first two identical and the last two indistinguishable. +func TestBeadsDetailStoredRowsSeparateNullFromEmpty(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 = beadsNullFixture() + setViews(t, h, &beadsView{}) + + rec := h.do("GET", "/~alice/db/view/beads?issue=i-null", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + + absent := rawCellValue(t, body, "issues", "closed_at") + stored := rawCellValue(t, body, "issues", "assignee") + empty := rawCellValue(t, body, "issues", "notes") + + if absent != `NULL` { + t.Errorf("a cell holding no value rendered as %q", absent) + } + if stored != `NULL` { + t.Errorf("a cell storing the text \"NULL\" rendered as %q", stored) + } + if empty != `` { + t.Errorf("a cell storing the empty string rendered as %q", empty) + } + if absent == stored || absent == empty || stored == empty { + t.Errorf("the three states are not three renderings: %q / %q / %q", absent, stored, empty) + } +} + +// Stored values are escaped, and they are not linkified. A script tag stored in +// a description reaches this section as text, and the ids in it stay the +// characters that are stored — the bodies above the section link them, but a raw +// view whose values have been rewritten is no longer showing what is stored. +func TestBeadsDetailStoredRowsEscapeAndDoNotLink(t *testing.T) { + h, _, _ := linkHarness(t) + + rec := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + + desc := rawCellValue(t, body, "issues", "description") + if strings.Contains(desc, "`) { + t.Errorf("the description above the section stopped linking; body=%s", body) + } + // Escaped and unlinked, the cell still decodes to exactly what is stored. + want := "blocked by beta-46c.2, and by alpha-2.\nnosuch-9z is nobody's.\n" + got := html.UnescapeString(strings.TrimSuffix(strings.TrimPrefix(desc, ``), "")) + if got != want { + t.Errorf("the stored description does not render as stored: %q", got) + } +} + +// The board renders no stored rows: it is not a detail pane, and a board that +// carried every row behind every card would be the table browser with lanes +// drawn on it. +func TestBeadsBoardHasNoStoredRowsSection(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 _, notWant := range []string{`class="bead-raw"`, "Stored rows", ``} { + if strings.Contains(body, notWant) { + t.Errorf("the board rendered the stored-rows section (%q); body=%s", notWant, body) + } + } +} + func TestBeadsEpicViewRender(t *testing.T) { h := newHarness(t) h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic}) diff --git a/web/templates/beads.html b/web/templates/beads.html index b2671418595bed43ea7c2f596c36e28eb5f01958..a7c779072d34d8f001fa2fd622100b4ca4412277 100644 --- a/web/templates/beads.html +++ b/web/templates/beads.html @@ -206,6 +206,50 @@ .bead-timeline .tl-dep::before { background: var(--lane-stalled); } .bead-timeline .tl-head { font-size: .85rem; color: var(--bd-fg); } .bead-timeline .tl-when { color: var(--bd-muted); } .bead-timeline .tl-body { white-space: pre-wrap; overflow-wrap: anywhere; color: var(--bd-fg); margin-top: .15rem; padding-left: .1rem; } + +/* stored rows: the pane's own check on itself, and not the reason a reader came + — so it opens collapsed, exactly as the stream's Past Stand section does. The + idiom is the rest of the page's: flat, square, hairline borders, monospace for + anything that came out of the database. */ +.bead-raw { margin: 1.5rem 0 0; border-top: 1px solid var(--bd-border); padding-top: .6rem; } +.bead-raw > summary { cursor: pointer; font-size: .8rem; color: var(--bd-muted); } +.bead-raw > summary .raw-note { font-style: italic; } +.bead-raw .raw-table { margin: .6rem 0 0; } +.bead-raw .raw-name { + display: flex; align-items: baseline; gap: .4rem; padding: .2rem .4rem; + background: var(--bd-panel); border: 1px solid var(--bd-border); + font-family: monospace; font-size: .75rem; color: var(--bd-fg); +} +.bead-raw .raw-name .raw-count { margin-left: auto; font-family: inherit; color: var(--bd-muted); } +.bead-raw table.raw-row { + width: 100%; border: 1px solid var(--bd-border); border-top: none; + border-collapse: collapse; margin: 0; +} +.bead-raw table.raw-row + table.raw-row { border-top: 2px solid var(--bd-border); } +.bead-raw table.raw-row td { padding: .1rem .4rem; vertical-align: top; border-top: 1px solid var(--bd-border); } +.bead-raw table.raw-row tr:first-child td { border-top: none; } +.bead-raw td.raw-col { + width: 11rem; white-space: nowrap; font-size: .72rem; color: var(--bd-muted); + border-right: 1px solid var(--bd-border); +} +/* A long value scrolls inside its own cell rather than stretching the page: this + is a section a reader glances into, and one 8kB description must not push + everything else out of reach. */ +.bead-raw .raw-text { + display: block; max-height: 9rem; overflow: auto; + font-family: monospace; font-size: .75rem; color: var(--bd-fg); + white-space: pre-wrap; overflow-wrap: anywhere; +} +/* The two absences a raw view may never conflate. A cell that holds no value is + the only place the word NULL is written by the page itself — a chip, muted and + italic, so it cannot be mistaken for the four characters a row might store — + and a cell holding the empty string says that it is empty instead of rendering + as nothing at all, which is what a null used to look like. */ +.bead-raw .raw-null { + font-size: .72rem; font-style: italic; color: var(--bd-muted); + border: 1px solid var(--bd-border); padding: 0 .25rem; +} +.bead-raw .raw-text:empty::after { content: "(empty)"; font-style: italic; color: var(--bd-muted); }
@@ -407,6 +451,48 @@ {{else}}

No activity yet.

{{end}}
+ +{{/* ------------- the stored rows behind all of the above ------------- + Everything on this pane is a reading of these rows — a field set that names + some columns and drops the rest, an event humanized into a sentence, an edge + reduced to an id and a type — and a reading that went wrong looks exactly + like one that did not. So the rows themselves are here, as they were read, + to be compared against the rendering above them. + + Collapsed, because this is a tool for checking the page and not the reason + anyone opened it — the same
the stream layout gives Past Stand. + + The values go through {{.Value}} and nothing else: they are stored text, so + html/template escapes them, and they are deliberately NOT run through + $.Links.Text the way the description and the comment bodies above are. A + linkifier rewrites what it is given — an id inside a value would come out as + an anchor — and a value that has been rewritten is no longer the value that + is stored, which is the only thing this section is for. */}} +{{if $.Data.Raw}} +
+ Stored rows — what this pane was built from, as read + {{range $.Data.Raw}} +
+
+ {{.Table}} + {{if .Clipped}}first {{len .Rows}} of {{.Matched}} rows{{else}}{{.Matched}} row{{if ne .Matched 1}}s{{end}}{{end}} +
+ {{range .Rows}} + + + {{range .Cells}} + + + + + {{end}} + +
{{.Column}}{{if .Null}}NULL{{else}}{{.Value}}{{end}}
+ {{end}} +
+ {{end}} +
+{{end}} {{else}} {{/* Two ways an issue can be missing, and the page may only claim the one it can see. A complete read that does not carry the id is an absence; a read