amadan

oficina / sheets Public

feat: create, list and read a sheet, with the title sealed

A sheet and its first tab are made in one transaction, because a sheet
with no tab lists and then 404s when clicked. Nothing else creates a
Sheet row, which is what makes "every sheet has a tab" an invariant
rather than a habit.

listSheets is unscoped by creator on purpose: sheets are team-shared,
and scope.Owned here would return only your own rows and read as a
short list rather than an error. The test pins two creators and two
rows so the wrong version fails loudly.

tabBelongsTo arrives now rather than with the grid: every grid route
takes a sheet id and a tab id from the URL, and without the pairing
check the sheet id means nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018RN4HhHR4rHH3Fxh4LMds5
Paul Campbell pushed by paul@keymail.dev 3ef248fbb46dff9e96128ec7cb84c8016773bb9a parent 56e72fb
2 files changed, +335 −0
  • internal/sheets/sheetsvc.go +182 −0
  • internal/sheets/sheetsvc_test.go +153 −0
diff --git a/internal/sheets/sheetsvc.go b/internal/sheets/sheetsvc.go
new file mode 100644
index 0000000..dc08f86
--- /dev/null
+++ b/internal/sheets/sheetsvc.go
@@ -0,0 +1,182 @@
+package sheets
+
+import (
+ "errors"
+ "strings"
+ "time"
+
+ "gorm.io/gorm"
+)
+
+// maxTitleRunes caps a sheet title. It is a UI-shaped limit, not a
+// storage one — the sealed column would take anything — and exists so a
+// pasted paragraph cannot become a sheet name that breaks every row of
+// the list.
+const maxTitleRunes = 200
+
+// firstTabName is what a new sheet's only tab is called. Sealed like
+// any other tab name: a tab name is sheet content (spec §2.1), and the
+// fact that this particular one is predictable does not make it exempt —
+// an exemption would be a plaintext write on the same column every
+// sealed name uses.
+const firstTabName = "Sheet 1"
+
+// errNoSealer is returned rather than panicking when an App built
+// without a sealer (page-rendering tests) reaches a path that needs one.
+var errNoSealer = errors.New("sheets: no sealer configured")
+
+// SheetSummary is one row of the sheet list: what a list row or a page
+// header needs, with the title already unsealed.
+//
+// TabID is the sheet's FIRST tab, which is the one a bare /sheets/{id}
+// opens. Carrying it here means the grid page never has to query for it,
+// and multi-tab navigation later adds a field rather than a round trip.
+type SheetSummary struct {
+ ID uint64
+ Title string
+ TabID uint64
+ UpdatedAt time.Time
+}
+
+// createSheet makes a sheet and its first tab in one transaction.
+//
+// One transaction because a sheet with no tab is not openable: it would
+// appear in the list and then 404 when clicked. Nothing outside this
+// function ever creates a Sheet row, which is what makes "every sheet
+// has at least one tab" a real invariant rather than a convention
+// everyone remembers.
+func (a *App) createSheet(title string, by Member) (SheetSummary, error) {
+ if a.db == nil {
+ return SheetSummary{}, errNoDB
+ }
+ if a.sealer == nil {
+ return SheetSummary{}, errNoSealer
+ }
+ clean := strings.TrimSpace(title)
+ if clean == "" {
+ return SheetSummary{}, errors.New("sheets: a sheet needs a title")
+ }
+ if r := []rune(clean); len(r) > maxTitleRunes {
+ clean = string(r[:maxTitleRunes])
+ }
+ sealedTitle, err := a.sealer.Seal("sheet.title", []byte(clean))
+ if err != nil {
+ return SheetSummary{}, err
+ }
+ sealedName, err := a.sealer.Seal("tab.name", []byte(firstTabName))
+ if err != nil {
+ return SheetSummary{}, err
+ }
+
+ sheet := Sheet{TitleSealed: sealedTitle, CreatedBy: by.ID}
+ tab := Tab{NameSealed: sealedName, Position: 0}
+ if err := a.db.Transaction(func(tx *gorm.DB) error {
+ if err := tx.Create(&sheet).Error; err != nil {
+ return err
+ }
+ tab.SheetID = sheet.ID
+ return tx.Create(&tab).Error
+ }); err != nil {
+ return SheetSummary{}, err
+ }
+ return SheetSummary{
+ ID: sheet.ID, Title: clean, TabID: tab.ID, UpdatedAt: sheet.UpdatedAt,
+ }, nil
+}
+
+// listSheets returns every live sheet on the instance, most recently
+// touched first.
+//
+// Deliberately unscoped by creator. Sheets are team-shared, and
+// scope.Owned here would return only the caller's own rows — failing as
+// a plausible short list rather than as an error, which is the worst
+// available failure shape and also the default thing to reach for
+// (AGENTS.md / Mechanical rules; TestListSheetsIsInstanceWideNotPerCreator
+// is the guard). When spec §8's grants land, this grows a hand-written
+// grants join. It does NOT grow a scope.Owned.
+func (a *App) listSheets() ([]SheetSummary, error) {
+ if a.db == nil {
+ return nil, errNoDB
+ }
+ if a.sealer == nil {
+ return nil, errNoSealer
+ }
+ var rows []Sheet
+ if err := a.db.Where("deleted_at IS NULL").
+ Order("updated_at DESC, id DESC").Find(&rows).Error; err != nil {
+ return nil, err
+ }
+ out := make([]SheetSummary, 0, len(rows))
+ for _, s := range rows {
+ sum, err := a.summarize(s)
+ if err != nil {
+ return nil, err
+ }
+ out = append(out, sum)
+ }
+ return out, nil
+}
+
+// sheetSummary reads one sheet. A sheet that is not there is
+// gorm.ErrRecordNotFound, which handlers turn into a 404 — never a 403
+// (AGENTS.md: a row that isn't yours 404s).
+func (a *App) sheetSummary(id uint64) (SheetSummary, error) {
+ if a.db == nil {
+ return SheetSummary{}, errNoDB
+ }
+ if a.sealer == nil {
+ return SheetSummary{}, errNoSealer
+ }
+ var s Sheet
+ if err := a.db.Where("id = ? AND deleted_at IS NULL", id).First(&s).Error; err != nil {
+ return SheetSummary{}, err
+ }
+ return a.summarize(s)
+}
+
+// tabBelongsTo reports whether a tab is part of a sheet.
+//
+// Every grid and stream route takes both ids from the URL, so without
+// this a caller could pair any sheet id with any tab id and read a tab
+// through a sheet it does not belong to. Once grants exist, the sheet is
+// where permission will be checked, which makes this pairing check the
+// thing that keeps the permission meaningful.
+func (a *App) tabBelongsTo(sheetID, tabID uint64) bool {
+ if a.db == nil {
+ return false
+ }
+ var n int64
+ if err := a.db.Model(&Tab{}).
+ Where("id = ? AND sheet_id = ?", tabID, sheetID).Count(&n).Error; err != nil {
+ return false
+ }
+ return n == 1
+}
+
+// touchSheet bumps a sheet's UpdatedAt so the list orders by real
+// activity rather than by creation. Called on every cell write; a
+// failure is not worth failing the write over, so callers ignore the
+// error and the list is merely stale.
+func (a *App) touchSheet(sheetID uint64) {
+ if a.db == nil {
+ return
+ }
+ a.db.Model(&Sheet{}).Where("id = ?", sheetID).
+ Update("updated_at", time.Now().UTC())
+}
+
+// summarize unseals a Sheet row's title and finds its first tab.
+func (a *App) summarize(s Sheet) (SheetSummary, error) {
+ title, err := a.sealer.Open("sheet.title", s.TitleSealed)
+ if err != nil {
+ return SheetSummary{}, err
+ }
+ var tab Tab
+ if err := a.db.Where("sheet_id = ?", s.ID).
+ Order("position ASC, id ASC").First(&tab).Error; err != nil {
+ return SheetSummary{}, err
+ }
+ return SheetSummary{
+ ID: s.ID, Title: string(title), TabID: tab.ID, UpdatedAt: s.UpdatedAt,
+ }, nil
+}
diff --git a/internal/sheets/sheetsvc_test.go b/internal/sheets/sheetsvc_test.go
new file mode 100644
index 0000000..f8b720f
--- /dev/null
+++ b/internal/sheets/sheetsvc_test.go
@@ -0,0 +1,153 @@
+package sheets
+
+import (
+ "bytes"
+ "errors"
+ "strings"
+ "testing"
+
+ "gorm.io/gorm"
+)
+
+func TestCreateSheetMakesAFirstTab(t *testing.T) {
+ app := newTestApp(t)
+ s, err := app.createSheet("Budget", testMember(t, app))
+ if err != nil {
+ t.Fatalf("createSheet: %v", err)
+ }
+ if s.ID == 0 {
+ t.Fatal("createSheet returned id 0")
+ }
+ if s.TabID == 0 {
+ t.Fatal("createSheet made a sheet with no tab; there would be nothing to open")
+ }
+ if s.Title != "Budget" {
+ t.Fatalf("title = %q, want %q", s.Title, "Budget")
+ }
+ if !app.tabBelongsTo(s.ID, s.TabID) {
+ t.Fatal("the tab createSheet returned does not belong to the sheet it returned")
+ }
+}
+
+// The one rule, at the only layer that could break it for a title.
+func TestSheetTitleIsSealedAtRest(t *testing.T) {
+ app := newTestApp(t)
+ const secret = "Salaries 2027"
+ if _, err := app.createSheet(secret, testMember(t, app)); err != nil {
+ t.Fatalf("createSheet: %v", err)
+ }
+ var raw []byte
+ // Row().Scan, not Raw().Scan: GORM reads a []byte destination on
+ // Scan as a slice of rows to fill, not as one blob column.
+ if err := app.db.Raw("SELECT title_sealed FROM sheets LIMIT 1").Row().Scan(&raw); err != nil {
+ t.Fatalf("read back: %v", err)
+ }
+ if len(raw) == 0 {
+ t.Fatal("no sealed title was stored")
+ }
+ if bytes.Contains(raw, []byte(secret)) {
+ t.Fatal("the sheet title reached the database in plaintext")
+ }
+}
+
+func TestTabNameIsSealedAtRest(t *testing.T) {
+ app := newTestApp(t)
+ if _, err := app.createSheet("Budget", testMember(t, app)); err != nil {
+ t.Fatalf("createSheet: %v", err)
+ }
+ var raw []byte
+ if err := app.db.Raw("SELECT name_sealed FROM tabs LIMIT 1").Row().Scan(&raw); err != nil {
+ t.Fatalf("read back: %v", err)
+ }
+ if bytes.Contains(raw, []byte(firstTabName)) {
+ t.Fatal("the tab name reached the database in plaintext; a predictable name is still content")
+ }
+}
+
+// This is the scope.Owned trap AGENTS.md names. The wrong
+// implementation returns one row and reads as a slightly short list
+// rather than as an error.
+func TestListSheetsIsInstanceWideNotPerCreator(t *testing.T) {
+ app := newTestApp(t)
+ ada, err := app.memberFor("ada@example.com")
+ if err != nil {
+ t.Fatalf("memberFor ada: %v", err)
+ }
+ bob, err := app.memberFor("bob@example.com")
+ if err != nil {
+ t.Fatalf("memberFor bob: %v", err)
+ }
+ if _, err := app.createSheet("Ada's", ada); err != nil {
+ t.Fatalf("createSheet ada: %v", err)
+ }
+ if _, err := app.createSheet("Bob's", bob); err != nil {
+ t.Fatalf("createSheet bob: %v", err)
+ }
+ got, err := app.listSheets()
+ if err != nil {
+ t.Fatalf("listSheets: %v", err)
+ }
+ if len(got) != 2 {
+ t.Fatalf("listSheets returned %d sheets, want 2 — sheets are team-shared", len(got))
+ }
+ titles := map[string]bool{}
+ for _, s := range got {
+ titles[s.Title] = true
+ }
+ if !titles["Ada's"] || !titles["Bob's"] {
+ t.Fatalf("listSheets returned %v, want both creators' sheets", titles)
+ }
+}
+
+func TestCreateSheetRefusesAnEmptyTitle(t *testing.T) {
+ app := newTestApp(t)
+ m := testMember(t, app)
+ if _, err := app.createSheet(" ", m); err == nil {
+ t.Fatal("createSheet with a blank title = nil error, want a refusal")
+ }
+ if _, err := app.createSheet("", m); err == nil {
+ t.Fatal(`createSheet("") = nil error, want a refusal`)
+ }
+}
+
+func TestCreateSheetTruncatesAnOverlongTitle(t *testing.T) {
+ app := newTestApp(t)
+ long := strings.Repeat("é", maxTitleRunes+50)
+ s, err := app.createSheet(long, testMember(t, app))
+ if err != nil {
+ t.Fatalf("createSheet: %v", err)
+ }
+ // Runes, not bytes: a multi-byte title truncated by byte count
+ // would slice a character in half.
+ if got := len([]rune(s.Title)); got != maxTitleRunes {
+ t.Fatalf("title is %d runes, want %d", got, maxTitleRunes)
+ }
+}
+
+func TestSheetSummaryForAMissingSheetIsNotFound(t *testing.T) {
+ app := newTestApp(t)
+ _, err := app.sheetSummary(999999)
+ if !errors.Is(err, gorm.ErrRecordNotFound) {
+ t.Fatalf("sheetSummary of a missing sheet = %v, want gorm.ErrRecordNotFound "+
+ "(handlers turn that into a 404, never a 403)", err)
+ }
+}
+
+func TestTabBelongsToRefusesACrossSheetPairing(t *testing.T) {
+ app := newTestApp(t)
+ m := testMember(t, app)
+ one, err := app.createSheet("One", m)
+ if err != nil {
+ t.Fatalf("createSheet one: %v", err)
+ }
+ two, err := app.createSheet("Two", m)
+ if err != nil {
+ t.Fatalf("createSheet two: %v", err)
+ }
+ if app.tabBelongsTo(one.ID, two.TabID) {
+ t.Fatal("a tab from another sheet was accepted; the sheet id in the URL would mean nothing")
+ }
+ if !app.tabBelongsTo(two.ID, two.TabID) {
+ t.Fatal("a tab was rejected from its own sheet")
+ }
+}