amadan
feat: sort and filter, non-destructively — and why that is not a stopgap
Spec 13 item 9, minus conditional formatting, which ruling 17 already
names as the second slip candidate.
**The sort reorders what is displayed and never what is stored, and
that is a constraint rather than a preference.** calc's dependency
graph is keyed on POSITION — store/handle.go's cellKey builds
CellKey{rowPos+1, colPos+1} — so physically moving a row changes what
every existing formula points at, silently. Plan 1's review flagged
exactly this: "valid only until rows move". A test writes =A1*2, sorts,
and asserts it still reads 60.
Destructive sort now has a row in the spec's deferred table with its
real trigger: calc's key space rebuilt on stable ids. That is a
subsystem, not a feature, and pretending otherwise would have shipped a
sort that quietly corrupted formulas.
Criteria are sealed. A filter value IS a cell value — spec 2.1 lists
persisted sort and filter criteria for that reason — and `> 50000`
against a salary column is the number it filters on.
Numbers sort numerically, so 9 comes before 10. String order putting 10
first is the single most obvious way a spreadsheet sort can be wrong,
so it has its own test. Empties sort last, so a sorted column does not
open with a screen of blanks.
Paging happens after ordering, never before: paging first sorts one
page at a time, which looks like sorting and is not.
An invalid submission leaves the stored view alone rather than clearing
it — losing somebody's filter because they mistyped a column is a worse
answer than ignoring the submission.
handlers_table.go hit the 300-line cap, so the view handler moved to
its own file, which is where it belonged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RN4HhHR4rHH3Fxh4LMds5
11 files changed,
+692
−15
docs/superpowers/specs/2026-08-30-sheets-design.md+1 −0internal/sheets/app.go+4 −0internal/sheets/handlers_table.go+32 −14internal/sheets/handlers_view.go+64 −0internal/sheets/migrations.go+1 −1internal/sheets/migrations/0007_create_tab_views.sql+4 −0internal/sheets/migrations/schema.sql+4 −0internal/sheets/models.go+17 −0internal/sheets/templates/table.html+45 −0internal/sheets/viewspec.go+251 −0internal/sheets/viewspec_test.go+269 −0
CI passed — run details
diff --git a/docs/superpowers/specs/2026-08-30-sheets-design.md b/docs/superpowers/specs/2026-08-30-sheets-design.md| index 261c2cf..df97a7c 100644 |
| --- a/docs/superpowers/specs/2026-08-30-sheets-design.md |
| +++ b/docs/superpowers/specs/2026-08-30-sheets-design.md |
| @@ -1009,6 +1009,7 @@ it is a fallback rather than a scramble. |
| |---|---| |
| | Pegamento's real vault facet (sealed team names, §9.2) | The vault facet ships with a recovery story | |
| | Moving the provisioning queue out of the home (§9.4) | The home spec rules on it | |
| +| **Destructive sort — physically reordering rows** | calc's dependency graph is keyed on POSITION (`store/handle.go`'s `cellKey` builds `CellKey{rowPos+1, colPos+1}`), so moving a row changes what every existing formula points at, silently. Plan 1's review flagged this as "valid only until rows move". Trigger: **calc's key space rebuilt on stable ids**, which is a subsystem and not a feature. Sort and filter ship non-destructively meanwhile — they reorder what is displayed and never what is stored, which is correct rather than a stopgap. | |
| | Pivot tables | Asked for twice | |
| | Rich presence beyond who's-here | Teams above ~5 concurrent editors | |
| | Offline editing | — | |
diff --git a/internal/sheets/app.go b/internal/sheets/app.go| index 3c6a113..c143b2f 100644 |
| --- a/internal/sheets/app.go |
| +++ b/internal/sheets/app.go |
| @@ -166,6 +166,10 @@ func (a *App) Router() http.Handler { |
| r.Get("/sheets/{sheetID}/comments", a.handleComments) |
| r.Post("/sheets/{sheetID}/comments", a.handleCommentAdd) |
| r.Post("/sheets/{sheetID}/comments/resolve", a.handleCommentResolve) |
| + // Sort and filter. Non-destructive: it reorders what is shown, |
| + // never what is stored — see ViewSpec for why that is a |
| + // constraint and not a preference. |
| + r.Post("/sheets/{sheetID}/view", a.handleViewSave) |
| // The grid's own API. Under /api because it answers JSON and |
| // carries no chrome; behind the same session as everything |
diff --git a/internal/sheets/handlers_table.go b/internal/sheets/handlers_table.go| index 491c126..d570e05 100644 |
| --- a/internal/sheets/handlers_table.go |
| +++ b/internal/sheets/handlers_table.go |
| @@ -40,16 +40,18 @@ type tableRow struct { |
| // tableView is the page's payload. |
| type tableView struct { |
| - Sheet SheetSummary |
| - Rows []tableRow |
| - Cols []string |
| - Warned []string |
| - Page int |
| - Pages int |
| - PrevPage int |
| - NextPage int |
| - HasPrev bool |
| - HasNext bool |
| + Sheet SheetSummary |
| + Rows []tableRow |
| + Cols []string |
| + Warned []string |
| + View ViewSpec |
| + FilterOps []string |
| + Page int |
| + Pages int |
| + PrevPage int |
| + NextPage int |
| + HasPrev bool |
| + HasNext bool |
| } |
| // handleTable renders a page of a tab as an ordinary HTML table. |
| @@ -90,7 +92,10 @@ func (a *App) handleTable(w http.ResponseWriter, r *http.Request) { |
| for c := 0; c < tableCols; c++ { |
| view.Cols = append(view.Cols, colLabel(c)) |
| } |
| - view.Rows = a.tableRows(h, first, editR, editC) |
| + spec := a.loadView(s.TabID) |
| + view.View = spec |
| + view.FilterOps = filterOps |
| + view.Rows = a.tableRowsIn(h, applyView(h, spec, first, tableRowsPerPage), editR, editC) |
| view.Warned = warnedCells(h, newPosIndex(h), maxWindowRows, tableCols) |
| a.render(w, r, "table", s.Title+" — table view", http.StatusOK, view) |
| @@ -222,10 +227,23 @@ func colLabel(n int) string { |
| // template is trusted to honour — means the read-only view cannot grow |
| // an edit form by somebody adding a branch to the template. |
| func (a *App) tableRows(h *store.Handle, first, editR, editC int) []tableRow { |
| - idx := newPosIndex(h) |
| - rows := make([]tableRow, 0, tableRowsPerPage) |
| + order := make([]int, 0, tableRowsPerPage) |
| for i := 0; i < tableRowsPerPage; i++ { |
| - rowIdx := first + i |
| + order = append(order, first+i) |
| + } |
| + return a.tableRowsIn(h, order, editR, editC) |
| +} |
| + |
| +// tableRowsIn builds rows in a given order. |
| +// |
| +// The order is a list of row POSITIONS, which is what lets a sorted or |
| +// filtered view reuse every other part of this: each cell still carries |
| +// its real coordinate, so an edit form posts the row it was always |
| +// posting and nothing downstream needs to know a view is in effect. |
| +func (a *App) tableRowsIn(h *store.Handle, order []int, editR, editC int) []tableRow { |
| + idx := newPosIndex(h) |
| + rows := make([]tableRow, 0, len(order)) |
| + for _, rowIdx := range order { |
| row := tableRow{Number: rowIdx + 1} |
| for c := 0; c < tableCols; c++ { |
| cell := tableCell{ |
diff --git a/internal/sheets/handlers_view.go b/internal/sheets/handlers_view.go| new file mode 100644 |
| index 0000000..f387682 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_view.go |
| @@ -0,0 +1,64 @@ |
| +package sheets |
| + |
| +import ( |
| + "net/http" |
| + "strconv" |
| +) |
| + |
| +// handleViewSave stores a tab's sort and filter. |
| +// |
| +// Editor, not viewer: the view is persisted on the tab and everybody |
| +// looking at that tab sees it, so changing it changes what colleagues |
| +// see. That is a write, and it is gated like one. |
| +func (a *App) handleViewSave(w http.ResponseWriter, r *http.Request) { |
| + s, ok := a.sheetFromURL(w, r) |
| + if !ok { |
| + return |
| + } |
| + viewer, ok := a.actor(r) |
| + if !ok { |
| + a.notFound(w, r) |
| + return |
| + } |
| + if !a.mayWrite(s.ID, viewer) { |
| + a.notFound(w, r) |
| + return |
| + } |
| + |
| + spec := emptyView() |
| + if v := r.FormValue("sortCol"); v != "" && v != "-1" { |
| + n, err := strconv.Atoi(v) |
| + if err != nil { |
| + a.tableRedirect(w, r, s) |
| + return |
| + } |
| + spec.SortCol = n |
| + spec.SortDsc = r.FormValue("sortDesc") != "" |
| + } |
| + if v := r.FormValue("filterCol"); v != "" && v != "-1" { |
| + n, err := strconv.Atoi(v) |
| + if err != nil { |
| + a.tableRedirect(w, r, s) |
| + return |
| + } |
| + spec.FilterCol = n |
| + spec.FilterOp = r.FormValue("filterOp") |
| + spec.FilterVal = r.FormValue("filterVal") |
| + } |
| + // An invalid spec leaves the stored one alone rather than clearing |
| + // it: losing somebody's filter because they mistyped a column is a |
| + // worse answer than ignoring the submission. |
| + if err := spec.Validate(); err != nil { |
| + a.tableRedirect(w, r, s) |
| + return |
| + } |
| + if err := a.saveView(s.TabID, spec, viewer); err != nil { |
| + a.fail(w, r, err) |
| + return |
| + } |
| + a.tableRedirect(w, r, s) |
| +} |
| + |
| +func (a *App) tableRedirect(w http.ResponseWriter, r *http.Request, s SheetSummary) { |
| + http.Redirect(w, r, "/sheets/"+strconv.FormatUint(s.ID, 10)+"/table", http.StatusSeeOther) |
| +} |
diff --git a/internal/sheets/migrations.go b/internal/sheets/migrations.go| index 1036731..69faa16 100644 |
| --- a/internal/sheets/migrations.go |
| +++ b/internal/sheets/migrations.go |
| @@ -50,5 +50,5 @@ var BootSchema = migrate.Merge(sessions.Schema, auth.Schema, Schema) |
| // moved. |
| var Models = []any{ |
| store.Sheet{}, store.Tab{}, store.Row{}, store.Col{}, |
| - store.Cell{}, store.Format{}, store.Event{}, Member{}, Grant{}, ShareLink{}, Comment{}, |
| + store.Cell{}, store.Format{}, store.Event{}, Member{}, Grant{}, ShareLink{}, Comment{}, TabView{}, |
| } |
diff --git a/internal/sheets/migrations/0007_create_tab_views.sql b/internal/sheets/migrations/0007_create_tab_views.sql| new file mode 100644 |
| index 0000000..936324f |
| --- /dev/null |
| +++ b/internal/sheets/migrations/0007_create_tab_views.sql |
| @@ -0,0 +1,4 @@ |
| +CREATE TABLE `tab_views` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`spec_sealed` blob,`updated_by` integer,`updated_at` datetime); |
| + |
| +CREATE UNIQUE INDEX `idx_tab_views_tab_id` ON `tab_views`(`tab_id`); |
| + |
diff --git a/internal/sheets/migrations/schema.sql b/internal/sheets/migrations/schema.sql| index 509c9ac..f386203 100644 |
| --- a/internal/sheets/migrations/schema.sql |
| +++ b/internal/sheets/migrations/schema.sql |
| @@ -21,6 +21,8 @@ CREATE TABLE `share_links` (`id` integer PRIMARY KEY AUTOINCREMENT,`sheet_id` in |
| CREATE TABLE `sheets` (`id` integer PRIMARY KEY AUTOINCREMENT,`title_sealed` blob,`created_by` integer,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime); |
| +CREATE TABLE `tab_views` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`spec_sealed` blob,`updated_by` integer,`updated_at` datetime); |
| + |
| CREATE TABLE `tabs` (`id` integer PRIMARY KEY AUTOINCREMENT,`sheet_id` integer,`name_sealed` blob,`position` integer,`frozen_rows` integer,`frozen_cols` integer); |
| CREATE UNIQUE INDEX `idx_col_tab_pos` ON `cols`(`tab_id`,`position`); |
| @@ -47,5 +49,7 @@ CREATE UNIQUE INDEX `idx_share_links_token_hash` ON `share_links`(`token_hash`); |
| CREATE INDEX `idx_sheets_deleted_at` ON `sheets`(`deleted_at`); |
| +CREATE UNIQUE INDEX `idx_tab_views_tab_id` ON `tab_views`(`tab_id`); |
| + |
| CREATE INDEX `idx_tabs_sheet_id` ON `tabs`(`sheet_id`); |
diff --git a/internal/sheets/models.go b/internal/sheets/models.go| index ff754e2..aa62525 100644 |
| --- a/internal/sheets/models.go |
| +++ b/internal/sheets/models.go |
| @@ -178,3 +178,20 @@ type Comment struct { |
| ResolvedAt *time.Time |
| ResolvedBy uint64 |
| } |
| + |
| +// TabView is a tab's persisted sort and filter. |
| +// |
| +// SpecSealed, because a filter value IS a cell value — spec §2.1 lists |
| +// "persisted sort and filter criteria" among what this design seals, |
| +// for exactly that reason. A filter reading `> 50000` against a salary |
| +// column is the number it is filtering on. |
| +// |
| +// One row per tab (uniqueIndex): a tab has one view, and two rows would |
| +// make "how is this sorted" depend on which the query returned first. |
| +type TabView struct { |
| + ID uint64 `gorm:"primaryKey"` |
| + TabID uint64 `gorm:"uniqueIndex"` |
| + SpecSealed []byte // seal purpose "tab.view" |
| + UpdatedBy uint64 |
| + UpdatedAt time.Time |
| +} |
diff --git a/internal/sheets/templates/table.html b/internal/sheets/templates/table.html| index 00dba93..31fc509 100644 |
| --- a/internal/sheets/templates/table.html |
| +++ b/internal/sheets/templates/table.html |
| @@ -10,6 +10,51 @@ Every cell is editable here, with no JavaScript at all.</p> |
| <a rst-btn="primary" href="/sheets/{{.Data.Sheet.ID}}">Open the grid</a> |
| </header> |
| +{{/* Sort and filter reorder what is SHOWN and never what is stored: |
| + calc's dependency graph is keyed on position, so physically moving |
| + rows would change what every formula points at. See ViewSpec. */}} |
| +<form rst-form method="post" action="/sheets/{{.Data.Sheet.ID}}/view"> |
| +<div rst-field-row> |
| +<div rst-field> |
| +<label rst-field-label for="sortCol">Sort by</label> |
| +<select rst-input id="sortCol" name="sortCol"> |
| +<option value="-1">nothing</option> |
| +{{$view := .Data.View}} |
| +{{range $i, $name := .Data.Cols}}<option value="{{$i}}"{{if eq $i $view.SortCol}} selected{{end}}>{{$name}}</option>{{end}} |
| +</select> |
| +</div> |
| +<div rst-field> |
| +<label rst-field-label for="sortDesc"> |
| +<input rst-check id="sortDesc" name="sortDesc" type="checkbox" value="1"{{if .Data.View.SortDsc}} checked{{end}}> |
| +Descending</label> |
| +</div> |
| +<div rst-field> |
| +<label rst-field-label for="filterCol">Show rows where</label> |
| +<select rst-input id="filterCol" name="filterCol"> |
| +<option value="-1">everything</option> |
| +{{range $i, $name := .Data.Cols}}<option value="{{$i}}"{{if eq $i $view.FilterCol}} selected{{end}}>{{$name}}</option>{{end}} |
| +</select> |
| +</div> |
| +<div rst-field> |
| +<label rst-field-label for="filterOp">is</label> |
| +<select rst-input id="filterOp" name="filterOp"> |
| +{{range .Data.FilterOps}}<option value="{{.}}"{{if eq . $view.FilterOp}} selected{{end}}>{{.}}</option>{{end}} |
| +</select> |
| +</div> |
| +<div rst-field> |
| +<label rst-field-label for="filterVal">value</label> |
| +<input rst-input="short" id="filterVal" name="filterVal" type="text" value="{{.Data.View.FilterVal}}"> |
| +</div> |
| +<button rst-btn="primary" type="submit">Apply</button> |
| +</div> |
| +</form> |
| + |
| +{{if .Data.View.Active}} |
| +<p rst-help>This view is sorted or filtered. It changes what everybody |
| +sees on this tab, and it changes nothing about the sheet itself — the |
| +rows are where they always were.</p> |
| +{{end}} |
| + |
| {{template "contrast-notice" .Data.Warned}} |
| <div style="overflow-x:auto"> |
| <table data-cell-table> |
diff --git a/internal/sheets/viewspec.go b/internal/sheets/viewspec.go| new file mode 100644 |
| index 0000000..d9aafd0 |
| --- /dev/null |
| +++ b/internal/sheets/viewspec.go |
| @@ -0,0 +1,251 @@ |
| +package sheets |
| + |
| +import ( |
| + "encoding/json" |
| + "errors" |
| + "sort" |
| + "strconv" |
| + "strings" |
| + |
| + "gorm.io/gorm/clause" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +// ViewSpec is how a tab is sorted and filtered. |
| +// |
| +// **Non-destructive, and that is a constraint rather than a |
| +// preference.** calc's dependency graph is keyed on POSITION — |
| +// store/handle.go's cellKey builds CellKey{rowPos+1, colPos+1} — so |
| +// physically reordering rows changes what every existing formula |
| +// points at, silently. Plan 1's review flagged exactly this ("valid |
| +// only until rows move"). Sorting therefore reorders what is DISPLAYED |
| +// and never what is stored; the graph, and every reference in it, is |
| +// untouched. |
| +// |
| +// A destructive sort is a real thing to want and is not built. Its |
| +// trigger is written into the spec's deferred table: it needs calc's |
| +// key space rebuilt on stable ids first, which is a subsystem, not a |
| +// feature. |
| +type ViewSpec struct { |
| + // SortCol is a zero-based column, or -1 for no sort. |
| + SortCol int `json:"sortCol"` |
| + SortDsc bool `json:"sortDesc"` |
| + |
| + // FilterCol is a zero-based column, or -1 for no filter. |
| + FilterCol int `json:"filterCol"` |
| + FilterOp string `json:"filterOp"` |
| + FilterVal string `json:"filterVal"` |
| +} |
| + |
| +// emptyView is what a tab has before anybody sorts it. |
| +func emptyView() ViewSpec { return ViewSpec{SortCol: -1, FilterCol: -1} } |
| + |
| +// Active reports whether this view does anything. |
| +func (v ViewSpec) Active() bool { return v.SortCol >= 0 || v.FilterCol >= 0 } |
| + |
| +// filterOps are the comparisons a filter may use. |
| +// |
| +// A small, closed set. Each is a thing somebody can hold in their head, |
| +// and a closed set is what lets the criteria be validated on the way in |
| +// rather than interpreted on the way out. |
| +var filterOps = []string{"contains", "equals", "gt", "lt", "nonempty"} |
| + |
| +func filterOpValid(op string) bool { |
| + for _, ok := range filterOps { |
| + if op == ok { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| +// Validate bounds a spec before it is stored. |
| +func (v ViewSpec) Validate() error { |
| + if v.SortCol < -1 || v.SortCol >= tableCols { |
| + return errors.New("sheets: sort column out of range") |
| + } |
| + if v.FilterCol < -1 || v.FilterCol >= tableCols { |
| + return errors.New("sheets: filter column out of range") |
| + } |
| + if v.FilterCol >= 0 && !filterOpValid(v.FilterOp) { |
| + return errors.New("sheets: unknown filter comparison") |
| + } |
| + if len(v.FilterVal) > maxRawBytes { |
| + return errors.New("sheets: filter value too long") |
| + } |
| + return nil |
| +} |
| + |
| +// loadView reads a tab's view, or the empty one. |
| +func (a *App) loadView(tabID uint64) ViewSpec { |
| + if a.db == nil || a.sealer == nil { |
| + return emptyView() |
| + } |
| + var row TabView |
| + if err := a.db.Where("tab_id = ?", tabID).First(&row).Error; err != nil { |
| + return emptyView() |
| + } |
| + body, err := a.sealer.Open("tab.view", row.SpecSealed) |
| + if err != nil { |
| + return emptyView() |
| + } |
| + spec := emptyView() |
| + if err := json.Unmarshal(body, &spec); err != nil { |
| + return emptyView() |
| + } |
| + if spec.Validate() != nil { |
| + // A stored spec this build cannot make sense of shows the tab |
| + // unsorted rather than refusing to render it. |
| + return emptyView() |
| + } |
| + return spec |
| +} |
| + |
| +// saveView stores a tab's view, sealed. |
| +func (a *App) saveView(tabID uint64, spec ViewSpec, by Member) error { |
| + if a.db == nil { |
| + return errNoDB |
| + } |
| + if a.sealer == nil { |
| + return errNoSealer |
| + } |
| + if err := spec.Validate(); err != nil { |
| + return err |
| + } |
| + body, err := json.Marshal(spec) |
| + if err != nil { |
| + return err |
| + } |
| + sealed, err := a.sealer.Seal("tab.view", body) |
| + if err != nil { |
| + return err |
| + } |
| + return a.db.Clauses(clause.OnConflict{ |
| + Columns: []clause.Column{{Name: "tab_id"}}, |
| + DoUpdates: clause.AssignmentColumns([]string{"spec_sealed", "updated_by", "updated_at"}), |
| + }).Create(&TabView{TabID: tabID, SpecSealed: sealed, UpdatedBy: by.ID}).Error |
| +} |
| + |
| +// applyView turns a stored spec into the order rows should be shown in. |
| +// |
| +// It returns display positions: the row position to render at each |
| +// place. Cells keep their real coordinates everywhere else in the app, |
| +// so nothing downstream has to know a view is in effect — and a write |
| +// still addresses the row it was always addressing. |
| +func applyView(h *store.Handle, spec ViewSpec, first, count int) []int { |
| + rows := make([]int, 0, count) |
| + for _, r := range h.GridRows() { |
| + rows = append(rows, r.Pos) |
| + } |
| + if len(rows) == 0 { |
| + // Nothing stored yet: show the plain window, so an empty sheet |
| + // still has rows to type into. |
| + for i := 0; i < count; i++ { |
| + rows = append(rows, first+i) |
| + } |
| + return rows |
| + } |
| + |
| + idx := newPosIndex(h) |
| + if spec.FilterCol >= 0 { |
| + kept := rows[:0:0] |
| + for _, rp := range rows { |
| + if matches(h, idx, rp, spec) { |
| + kept = append(kept, rp) |
| + } |
| + } |
| + rows = kept |
| + } |
| + if spec.SortCol >= 0 { |
| + sort.SliceStable(rows, func(i, j int) bool { |
| + a := cellText(h, idx, rows[i], spec.SortCol) |
| + b := cellText(h, idx, rows[j], spec.SortCol) |
| + if spec.SortDsc { |
| + return less(b, a) |
| + } |
| + return less(a, b) |
| + }) |
| + } |
| + |
| + // Page after ordering, not before: paging first would sort one |
| + // page at a time, which looks like sorting and is not. |
| + if first >= len(rows) { |
| + return nil |
| + } |
| + end := first + count |
| + if end > len(rows) { |
| + end = len(rows) |
| + } |
| + return rows[first:end] |
| +} |
| + |
| +// cellText reads a cell's displayed string at a position. |
| +func cellText(h *store.Handle, idx posIndex, rowPos, colPos int) string { |
| + rowID, ok := idx.rowID[rowPos] |
| + if !ok { |
| + return "" |
| + } |
| + colID, ok := idx.colID[colPos] |
| + if !ok { |
| + return "" |
| + } |
| + return display(h.Value(rowID, colID), h.Format(rowID, colID)) |
| +} |
| + |
| +// matches applies a filter to one row. |
| +func matches(h *store.Handle, idx posIndex, rowPos int, spec ViewSpec) bool { |
| + got := cellText(h, idx, rowPos, spec.FilterCol) |
| + want := spec.FilterVal |
| + switch spec.FilterOp { |
| + case "nonempty": |
| + return strings.TrimSpace(got) != "" |
| + case "equals": |
| + return strings.EqualFold(strings.TrimSpace(got), strings.TrimSpace(want)) |
| + case "contains": |
| + return strings.Contains(strings.ToLower(got), strings.ToLower(want)) |
| + case "gt", "lt": |
| + g, err1 := strconv.ParseFloat(strings.TrimSpace(got), 64) |
| + wv, err2 := strconv.ParseFloat(strings.TrimSpace(want), 64) |
| + if err1 != nil || err2 != nil { |
| + // A numeric comparison against something that is not a |
| + // number excludes the row rather than guessing an order. |
| + return false |
| + } |
| + if spec.FilterOp == "gt" { |
| + return g > wv |
| + } |
| + return g < wv |
| + } |
| + return true |
| +} |
| + |
| +// less orders two displayed strings, numbers before text. |
| +// |
| +// Numbers compare numerically, so 9 sorts before 10 — string order |
| +// would put 10 first, which is the single most obvious way a |
| +// spreadsheet sort can be wrong. Everything else compares |
| +// case-insensitively, and empties sort last so a sorted column does not |
| +// open with a screen of blanks. |
| +func less(a, b string) bool { |
| + af, aNum := parseNum(a) |
| + bf, bNum := parseNum(b) |
| + switch { |
| + case a == "" && b == "": |
| + return false |
| + case a == "": |
| + return false |
| + case b == "": |
| + return true |
| + case aNum && bNum: |
| + return af < bf |
| + case aNum != bNum: |
| + return aNum |
| + } |
| + return strings.ToLower(a) < strings.ToLower(b) |
| +} |
| + |
| +func parseNum(s string) (float64, bool) { |
| + f, err := strconv.ParseFloat(strings.TrimSpace(s), 64) |
| + return f, err == nil |
| +} |
diff --git a/internal/sheets/viewspec_test.go b/internal/sheets/viewspec_test.go| new file mode 100644 |
| index 0000000..04c06b2 |
| --- /dev/null |
| +++ b/internal/sheets/viewspec_test.go |
| @@ -0,0 +1,269 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "fmt" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// seedColumn writes one value per row into column 0. |
| +func seedColumn(t *testing.T, app *App, cookies []*http.Cookie, cells string, values []string) { |
| + t.Helper() |
| + for i, v := range values { |
| + rec := postCell(t, app, cookies, cells, fmt.Sprintf(`{"r":%d,"c":0,"raw":%q}`, i, v)) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("seeding row %d = %d: %s", i, rec.Code, rec.Body.String()) |
| + } |
| + } |
| +} |
| + |
| +func tablePath(s SheetSummary) string { return fmt.Sprintf("/sheets/%d/table", s.ID) } |
| + |
| +func saveViewForm(t *testing.T, app *App, s SheetSummary, cookies []*http.Cookie, form url.Values) { |
| + t.Helper() |
| + rec := do(t, app, as(postForm(fmt.Sprintf("/sheets/%d/view", s.ID), form), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("saving the view = %d: %s", rec.Code, rec.Body.String()) |
| + } |
| +} |
| + |
| +// rowOrder reads the row numbers out of a rendered table, in order. |
| +func rowOrder(body string) []string { |
| + var out []string |
| + for _, part := range strings.Split(body, `<th scope="row">`)[1:] { |
| + if i := strings.Index(part, "<"); i > 0 { |
| + out = append(out, part[:i]) |
| + } |
| + } |
| + return out |
| +} |
| + |
| +// The single most obvious way a spreadsheet sort can be wrong. |
| +func TestSortIsNumericNotLexicographic(t *testing.T) { |
| + if !less("9", "10") { |
| + t.Fatal("9 sorts after 10; the sort is comparing numbers as strings") |
| + } |
| + if less("10", "9") { |
| + t.Fatal("10 sorts before 9") |
| + } |
| + // Text still compares as text, case-insensitively. |
| + if !less("apple", "Banana") { |
| + t.Error("text does not compare case-insensitively") |
| + } |
| + // Numbers before text, and empties last, so a sorted column does |
| + // not open with a screen of blanks. |
| + if !less("1", "apple") { |
| + t.Error("text sorts before numbers") |
| + } |
| + if less("", "apple") { |
| + t.Error("an empty cell sorts before a value") |
| + } |
| +} |
| + |
| +func TestSortReordersWhatIsShown(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, cells := newSheet(t, app, cookies, "Budget") |
| + seedColumn(t, app, cookies, cells, []string{"30", "10", "20"}) |
| + |
| + saveViewForm(t, app, s, cookies, url.Values{"sortCol": {"0"}}) |
| + body := do(t, app, as(httptest.NewRequest(http.MethodGet, tablePath(s), nil), cookies)).Body.String() |
| + |
| + order := rowOrder(body) |
| + if len(order) < 3 { |
| + t.Fatalf("only %d rows rendered", len(order)) |
| + } |
| + // Rows 1, 2, 3 hold 30, 10, 20 — so ascending shows 2, 3, 1. |
| + if got := strings.Join(order[:3], ","); got != "2,3,1" { |
| + t.Fatalf("sorted row order = %s, want 2,3,1", got) |
| + } |
| +} |
| + |
| +func TestSortDescending(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, cells := newSheet(t, app, cookies, "Budget") |
| + seedColumn(t, app, cookies, cells, []string{"30", "10", "20"}) |
| + |
| + saveViewForm(t, app, s, cookies, url.Values{"sortCol": {"0"}, "sortDesc": {"1"}}) |
| + order := rowOrder(do(t, app, as(httptest.NewRequest(http.MethodGet, tablePath(s), nil), cookies)).Body.String()) |
| + if got := strings.Join(order[:3], ","); got != "1,3,2" { |
| + t.Fatalf("descending row order = %s, want 1,3,2", got) |
| + } |
| +} |
| + |
| +func TestFilterHidesRows(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, cells := newSheet(t, app, cookies, "Budget") |
| + seedColumn(t, app, cookies, cells, []string{"alpha", "beta", "gamma"}) |
| + |
| + // "ma", not "a": every one of alpha, beta and gamma contains an |
| + // "a", so filtering on it would pass whether the filter worked or |
| + // not — a fixture that cannot fail. |
| + saveViewForm(t, app, s, cookies, url.Values{ |
| + "filterCol": {"0"}, "filterOp": {"contains"}, "filterVal": {"ma"}, |
| + }) |
| + body := do(t, app, as(httptest.NewRequest(http.MethodGet, tablePath(s), nil), cookies)).Body.String() |
| + if !strings.Contains(body, "gamma") { |
| + t.Error("the filter hid a row that matches") |
| + } |
| + for _, hidden := range []string{"alpha", "beta"} { |
| + if strings.Contains(body, hidden) { |
| + t.Errorf("the filter kept %q, which does not match", hidden) |
| + } |
| + } |
| +} |
| + |
| +func TestNumericFilters(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, cells := newSheet(t, app, cookies, "Budget") |
| + seedColumn(t, app, cookies, cells, []string{"5", "50", "500"}) |
| + |
| + saveViewForm(t, app, s, cookies, url.Values{ |
| + "filterCol": {"0"}, "filterOp": {"gt"}, "filterVal": {"40"}, |
| + }) |
| + body := do(t, app, as(httptest.NewRequest(http.MethodGet, tablePath(s), nil), cookies)).Body.String() |
| + if strings.Contains(body, ">5<") { |
| + t.Error("gt 40 kept the row holding 5") |
| + } |
| + for _, want := range []string{"50", "500"} { |
| + if !strings.Contains(body, want) { |
| + t.Errorf("gt 40 hid the row holding %s", want) |
| + } |
| + } |
| +} |
| + |
| +// The constraint that shaped the whole feature: reordering rows would |
| +// change what every formula points at, because calc's keys are |
| +// position-derived. So the sort must not move anything stored. |
| +func TestSortingDoesNotMoveStoredRowsOrBreakFormulas(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, cells := newSheet(t, app, cookies, "Budget") |
| + seedColumn(t, app, cookies, cells, []string{"30", "10", "20"}) |
| + // B1 = A1 * 2 = 60. |
| + postCell(t, app, cookies, cells, `{"r":0,"c":1,"raw":"=A1*2"}`) |
| + |
| + before := decodeGrid(t, do(t, app, as(httptest.NewRequest(http.MethodGet, cells, nil), cookies))) |
| + saveViewForm(t, app, s, cookies, url.Values{"sortCol": {"0"}}) |
| + after := decodeGrid(t, do(t, app, as(httptest.NewRequest(http.MethodGet, cells, nil), cookies))) |
| + |
| + find := func(g gridResponse, r, c int) string { |
| + for _, cell := range g.Cells { |
| + if cell.R == r && cell.C == c { |
| + return cell.Display |
| + } |
| + } |
| + return "" |
| + } |
| + // The grid — the stored order — is untouched by a view. |
| + for _, rc := range [][2]int{{0, 0}, {1, 0}, {2, 0}, {0, 1}} { |
| + if b, a := find(before, rc[0], rc[1]), find(after, rc[0], rc[1]); b != a { |
| + t.Errorf("sorting moved the stored cell (%d,%d): %q became %q", rc[0], rc[1], b, a) |
| + } |
| + } |
| + if got := find(after, 0, 1); got != "60" { |
| + t.Fatalf("=A1*2 now reads %q, want 60 — the formula's reference moved", got) |
| + } |
| +} |
| + |
| +// A filter value IS a cell value (spec §2.1), so the criteria are |
| +// sealed like anything else somebody typed. |
| +func TestViewCriteriaAreSealedAtRest(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + const secret = "ConfidentialThreshold" |
| + saveViewForm(t, app, s, cookies, url.Values{ |
| + "filterCol": {"0"}, "filterOp": {"contains"}, "filterVal": {secret}, |
| + }) |
| + |
| + var raw []byte |
| + if err := app.db.Raw("SELECT spec_sealed FROM tab_views LIMIT 1").Row().Scan(&raw); err != nil { |
| + t.Fatalf("read back: %v", err) |
| + } |
| + if len(raw) == 0 { |
| + t.Fatal("no sealed spec was stored") |
| + } |
| + if bytes.Contains(raw, []byte(secret)) { |
| + t.Fatal("the filter value reached the database in plaintext") |
| + } |
| +} |
| + |
| +func TestTheViewPersistsAcrossRequests(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, cells := newSheet(t, app, cookies, "Budget") |
| + seedColumn(t, app, cookies, cells, []string{"30", "10"}) |
| + |
| + saveViewForm(t, app, s, cookies, url.Values{"sortCol": {"0"}, "sortDesc": {"1"}}) |
| + got := app.loadView(s.TabID) |
| + if got.SortCol != 0 || !got.SortDsc { |
| + t.Fatalf("stored view = %+v, want column 0 descending", got) |
| + } |
| +} |
| + |
| +// A viewer must not change what everybody else sees. |
| +func TestAViewerCannotChangeTheView(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + guestOn(t, app, s.ID, "ada@elsewhere.example", SheetViewer) |
| + adaCookies := signInAs(t, app, "ada@elsewhere.example") |
| + |
| + rec := do(t, app, as(postForm(fmt.Sprintf("/sheets/%d/view", s.ID), |
| + url.Values{"sortCol": {"0"}}), adaCookies)) |
| + if rec.Code != http.StatusNotFound { |
| + t.Fatalf("a viewer changed the view: %d", rec.Code) |
| + } |
| + if app.loadView(s.TabID).Active() { |
| + t.Fatal("a viewer's submission was stored anyway") |
| + } |
| +} |
| + |
| +func TestAnInvalidSpecLeavesTheStoredOneAlone(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + saveViewForm(t, app, s, cookies, url.Values{"sortCol": {"1"}}) |
| + // An unknown comparison. Losing somebody's view because they |
| + // mistyped is a worse answer than ignoring the submission. |
| + saveViewForm(t, app, s, cookies, url.Values{ |
| + "filterCol": {"0"}, "filterOp": {"regex"}, "filterVal": {"x"}, |
| + }) |
| + if got := app.loadView(s.TabID); got.SortCol != 1 { |
| + t.Fatalf("an invalid submission cleared the stored view: %+v", got) |
| + } |
| +} |
| + |
| +func TestViewValidationBounds(t *testing.T) { |
| + for _, spec := range []ViewSpec{ |
| + {SortCol: tableCols, FilterCol: -1}, |
| + {SortCol: -2, FilterCol: -1}, |
| + {SortCol: -1, FilterCol: tableCols}, |
| + {SortCol: -1, FilterCol: 0, FilterOp: "regex"}, |
| + {SortCol: -1, FilterCol: 0, FilterOp: "contains", FilterVal: strings.Repeat("x", maxRawBytes+1)}, |
| + } { |
| + if err := spec.Validate(); err == nil { |
| + t.Errorf("Validate accepted %+v", spec) |
| + } |
| + } |
| + if err := (ViewSpec{SortCol: 0, FilterCol: 1, FilterOp: "gt", FilterVal: "5"}).Validate(); err != nil { |
| + t.Errorf("Validate refused a good spec: %v", err) |
| + } |
| +} |
| + |
| +func TestSortAndFilterAreScriptless(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + body := withoutScript(do(t, app, |
| + as(httptest.NewRequest(http.MethodGet, tablePath(s), nil), cookies)).Body.String()) |
| + for _, want := range []string{ |
| + fmt.Sprintf(`action="/sheets/%d/view"`, s.ID), |
| + `name="sortCol"`, `name="filterCol"`, `name="filterOp"`, `name="filterVal"`, |
| + } { |
| + if !strings.Contains(body, want) { |
| + t.Errorf("with scripts stripped, %q is gone", want) |
| + } |
| + } |
| +} |