oficina / sheets Public

Download this file

1	package sheets
2
3	import (
4	"testing"
5	"time"
6
7	"amadan.net/oficina/sheets/internal/sheets/store"
8	)
9
10	// These pin the findings a gpt-5.6-sol review raised against the first
11	// cut of import (2026-09-02). Each one failed before the fix.
12
13	// TestAnUnfinishedImportIsInvisible is the fix for the blocker: a sheet
14	// being filled from a workbook must not be visible to anybody, because
15	// the window between creating it and finishing it is one nothing can
16	// clean up if the process dies inside it.
17	//
18	// It asserts the flag directly rather than racing a real import,
19	// because the interesting state is the one that exists only while the
20	// job runs — and a test that tried to catch it mid-flight would be
21	// flaky in exactly the way that hides a regression.
22	func TestAnUnfinishedImportIsInvisible(t *testing.T) {
23	app := newTestApp(t)
24	member := testMember(t, app)
25
26	s, err := app.createFillingSheet("half a workbook", member)
27	if err != nil {
28	t.Fatalf("createFillingSheet: %v", err)
29	}
30
31	list, err := app.listSheets(member)
32	if err != nil {
33	t.Fatalf("listSheets: %v", err)
34	}
35	if len(list) != 0 {
36	t.Errorf("a sheet still being filled appears in the list (%d rows)", len(list))
37	}
38	if _, err := app.sheetSummary(s.ID, member); err == nil {
39	t.Error("a sheet still being filled can be opened by id")
40	}
41
42	// And it becomes ordinary the moment it is filled.
43	if err := app.markFilled(s.ID); err != nil {
44	t.Fatalf("markFilled: %v", err)
45	}
46	list, err = app.listSheets(member)
47	if err != nil {
48	t.Fatalf("listSheets after markFilled: %v", err)
49	}
50	if len(list) != 1 {
51	t.Fatalf("want the sheet visible once filled, got %d rows", len(list))
52	}
53	if _, err := app.sheetSummary(s.ID, member); err != nil {
54	t.Errorf("a filled sheet cannot be opened by id: %v", err)
55	}
56	}
57
58	// TestAnOrdinaryNewSheetIsVisibleImmediately guards the other
59	// direction: the flag must not leak into the plain Create path, or
60	// every new sheet vanishes.
61	func TestAnOrdinaryNewSheetIsVisibleImmediately(t *testing.T) {
62	app := newTestApp(t)
63	member := testMember(t, app)
64
65	if _, err := app.createSheet("an ordinary sheet", member); err != nil {
66	t.Fatalf("createSheet: %v", err)
67	}
68	list, err := app.listSheets(member)
69	if err != nil {
70	t.Fatalf("listSheets: %v", err)
71	}
72	if len(list) != 1 {
73	t.Fatalf("a normally created sheet is not visible (%d rows)", len(list))
74	}
75	}
76
77	// TestDiscardingAnImportRemovesItsGrants is the fix for the admission
78	// finding. Admits counts a member's grants to decide whether an address
79	// may sign in at all, so an owner grant left behind by a failed import
80	// would keep admitting somebody who has no live access to anything.
81	func TestDiscardingAnImportRemovesItsGrants(t *testing.T) {
82	// The allowlist has to be off, or Admits answers true from it and
83	// the assertion below proves nothing about grants at all — which is
84	// exactly what the first version of this test did.
85	app := newTestApp(t)
86	app.admit = func(string) bool { return false }
87	guest := testMember(t, app)
88
89	s, err := app.createFillingSheet("a workbook that will fail", guest)
90	if err != nil {
91	t.Fatalf("createFillingSheet: %v", err)
92	}
93	// Filled first, so the grant is one that genuinely admits. A
94	// still-filling sheet does not admit its own importer — that is
95	// TestAdmitsIgnoresGrantsToSheetsYouCannotOpen's subject — so
96	// starting there would make the assertion below pass for the wrong
97	// reason.
98	if err := app.markFilled(s.ID); err != nil {
99	t.Fatalf("markFilled: %v", err)
100	}
101	if !app.Admits(guest.Address) {
102	t.Fatal("a live grant does not admit; fixture is wrong")
103	}
104
105	app.discardSheet(s.ID)
106
107	var grants int64
108	if err := app.db.Model(&Grant{}).Where("sheet_id = ?", s.ID).Count(&grants).Error; err != nil {
109	t.Fatalf("count grants: %v", err)
110	}
111	if grants != 0 {
112	t.Errorf("a discarded import left %d grant(s) behind", grants)
113	}
114	if app.Admits(guest.Address) {
115	t.Error("a discarded import still admits the person who started it")
116	}
117	}
118
119	// TestAdmitsIgnoresGrantsToSheetsYouCannotOpen covers the wider half of
120	// the same finding: a grant to a deleted — or still-filling — sheet is
121	// not a reason to let an address sign in.
122	func TestAdmitsIgnoresGrantsToSheetsYouCannotOpen(t *testing.T) {
123	// admit refuses everybody, so Admits can only answer true on the
124	// strength of a grant. Otherwise the allowlist masks the bug.
125	app := newTestApp(t)
126	app.admit = func(string) bool { return false }
127	guest := testMember(t, app)
128
129	s, err := app.createSheet("shared with a guest", guest)
130	if err != nil {
131	t.Fatalf("createSheet: %v", err)
132	}
133	if !app.Admits(guest.Address) {
134	t.Fatal("a live grant does not admit; fixture is wrong")
135	}
136
137	now := time.Now().UTC()
138	if err := app.db.Model(&store.Sheet{}).Where("id = ?", s.ID).
139	Update("deleted_at", now).Error; err != nil {
140	t.Fatalf("soft delete: %v", err)
141	}
142	if app.Admits(guest.Address) {
143	t.Error("a grant to a deleted sheet still admits its holder")
144	}
145
146	// And the filling case on its own, which the deleted case above
147	// does not cover: without this, dropping filling_since from Admits
148	// would leave this test green.
149	if err := app.db.Model(&store.Sheet{}).Where("id = ?", s.ID).
150	Updates(map[string]any{"deleted_at": nil, "filling_since": now}).Error; err != nil {
151	t.Fatalf("mark filling: %v", err)
152	}
153	if app.Admits(guest.Address) {
154	t.Error("a grant to a sheet still being filled admits its holder")
155	}
156	}
157