Merge branch 'feat/home-connect' into main
67 files changed,
+3822
−409
AGENTS.md+17 −19Makefile+3 −0cmd/sheets/main.go+4 −0docs/superpowers/specs/2026-09-07-atomic-cell-batch.md+22 −0docs/superpowers/specs/2026-09-07-canary-fixture-bootstrap.md+62 −0docs/superpowers/specs/2026-09-07-durable-tab-versions.md+56 −0docs/superpowers/specs/2026-09-07-reconcile-deployed-chrome.md+28 −0go.mod+5 −0go.sum+22 −2internal/guardstest/csstokens_test.go+11 −4internal/sheets/app.go+30 −19internal/sheets/auth.go+11 −1internal/sheets/auth_test.go+1 −0internal/sheets/handlers_cell_batch.go+65 −0internal/sheets/handlers_cell_batch_test.go+130 −0internal/sheets/handlers_import.go+2 −2internal/sheets/handlers_import_page.go+60 −0internal/sheets/handlers_import_refusals_test.go+13 −4internal/sheets/handlers_list_chrome_test.go+216 −0internal/sheets/handlers_rename.go+156 −0internal/sheets/handlers_rename_test.go+263 −0internal/sheets/handlers_sheets.go+17 −18internal/sheets/handlers_table_test.go+13 −2internal/sheets/home.go+168 −0internal/sheets/home_navigation_test.go+150 −0internal/sheets/home_routes.go+42 −0internal/sheets/home_test.go+211 −0internal/sheets/members.go+10 −0internal/sheets/migrations.go+5 −3internal/sheets/migrations/0010_alter_tabs.sql+2 −0internal/sheets/migrations/schema.sql+1 −1internal/sheets/page.go+16 −2internal/sheets/render.go+46 −11internal/sheets/static/cellqueue.js+20 −37internal/sheets/static/clipboard.js+5 −6internal/sheets/static/drop.js+56 −6internal/sheets/static/grid.js+3 −3internal/sheets/static/sheets.css+183 −3internal/sheets/static/theme.css+0 −187internal/sheets/store/handle_blank_cap_test.go+12 −1internal/sheets/store/handle_open.go+8 −1internal/sheets/store/handle_report.go+3 −0internal/sheets/store/handle_set.go+10 −1internal/sheets/store/handle_version.go+27 −0internal/sheets/store/handle_version_export_test.go+14 −0internal/sheets/store/handle_version_limits_test.go+184 −0internal/sheets/store/handle_version_migration_test.go+122 −0internal/sheets/store/handle_version_watermark_test.go+203 −0internal/sheets/store/handle_write_batch.go+96 −0internal/sheets/store/handle_write_batch_test.go+251 −0internal/sheets/store/models.go+7 −6internal/sheets/templates/home_link.html+1 −0internal/sheets/templates/home_oauth.html+2 −0internal/sheets/templates/import.html+53 −0internal/sheets/templates/layout.html+23 −3internal/sheets/templates/sheet.html+36 −2internal/sheets/templates/sheets.html+124 −53internal/sheets/theme.go+1 −1internal/sheets/theme_assets_test.go+65 −0tools/drive/batch-checks.mjs+52 −0tools/drive/canary-batch.mjs+75 −0tools/drive/chrome.mjs+17 −3tools/drive/clipboard.mjs+9 −2tools/drive/drive.mjs+22 −4tools/drive/drop.mjs+268 −0tools/drive/menus.mjs+7 −1tools/drive/tableformat.mjs+5 −1
CI passed — run details
diff --git a/AGENTS.md b/AGENTS.md| index 9d25ff7..69e01da 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -132,9 +132,15 @@ relying on remembering one, make it a test instead. |
| one-connection writer pool deadlocks. |
| - Never bind a request body onto a GORM model. Explicit `map[string]any` |
| plus a `.Select` allowlist. |
| -- Read the viewer with `auth.From(r)` or `sessions.Current(r)`. **Never |
| - `sessions.UserID`** — it returns `(0, false)` for an email Subject, and |
| - dropping that `ok` scopes every query to uid 0. |
| +- Resolve the viewer through the app's `identity` and `actor` helpers. Local |
| + sessions retain verified addresses; opt-in Home sessions contain opaque Home |
| + subjects and must resolve through the shared connection to existing members. |
| + Never create members from Home subjects or infer links from matching addresses. |
| + Never use `sessions.UserID`: it does not resolve either identity shape. |
| +- Home links require independent fresh local and Home proofs plus confirmation. |
| + Recheck existing admission and sheet grants on every Home request. Member IDs |
| + are AUTOINCREMENT and must never be reassigned; any future deletion path must |
| + remove the member's `home_identity_links` rows in the same transaction. |
| - Migrations are additive-only. New code over an old DB is always safe. |
| Never delete data to update. |
| - **Cell values are IEEE doubles end to end.** Currency is a number format, |
| @@ -201,22 +207,14 @@ relying on remembering one, make it a test instead. |
| `doctor` binary alongside the module** — a `doctor` one release behind |
| compares against its own library copy and calls the upgrade you just did |
| the drift. |
| -- **`doctor --fix` does not re-copy `static/theme.css`. Copy it by hand.** |
| - Measured on the v0.22.0 → v0.23.0 upgrade: `doctor --fix -theme day` |
| - printed `drift theme.css` with the diff, rewrote `tokens.css`, and left |
| - `theme.css` untouched without saying it had. Unnamed, the theme is not |
| - compared at all (`custom or drifted — not compared`), which reads like |
| - a clean bill. |
| - This is the worst shape the frozen-theme problem takes, because the two |
| - files are coupled: v0.23.0's `tokens.css` writes |
| - `border-bottom: 1px solid var(--rst-header-rule)` and v0.22.0's `day` |
| - declares no such token. An undefined custom property makes the whole |
| - shorthand invalid at computed-value time, so it resolves to `initial` — |
| - `border-style: none`. The page header's rule does not go grey or go |
| - wrong; **it disappears**, silently, on every screen. So after every |
| - upgrade: `cp "$(go env GOMODCACHE)/github.com/carlosframework/rastrillo@<ver>/ui/themes/day.css" |
| - internal/sheets/static/theme.css`, then re-run `doctor -theme day` and |
| - require `all matching the library`. |
| +- **Theme bytes come from the Estilo module pin.** There is no app-local |
| + `static/theme.css` to copy. On framework upgrades, update the Estilo pin |
| + to the same released `day` body and run the theme asset tests. Estilo's |
| + byte-for-byte test and this app's `theme_assets_test.go` detect drift; |
| + `rastrillo doctor` reports the absent theme as not compared. Its token |
| + checks still apply. Do not recreate an embedded theme file that the |
| + served filesystem would shadow. This matters at v0.23.0 because tokens |
| + require `--rst-header-rule`; an older theme silently loses the rule. |
| - Don't build on `.rst-page-header::after` — the rake line is retired in |
| favour of `--rst-header-rule` (rastrillo v0.23.0, design doc §6-v2.2). |
| It was retired because, built from the same parts in the same order, it |
diff --git a/Makefile b/Makefile| index a142a80..04e4959 100644 |
| --- a/Makefile |
| +++ b/Makefile |
| @@ -1,3 +1,6 @@ |
| +comma := , |
| +export GOPRIVATE := $(if $(GOPRIVATE),$(GOPRIVATE)$(comma))amadan.net/oficina/estilo.git |
| + |
| .PHONY: build release test test-vet test-fmt test-build test-migrations test-go test-race |
| # One binary, CGO_ENABLED=0, cgo-free SQLite (AGENTS.md / Deploys). Set |
diff --git a/cmd/sheets/main.go b/cmd/sheets/main.go| index ba72685..5401076 100644 |
| --- a/cmd/sheets/main.go |
| +++ b/cmd/sheets/main.go |
| @@ -102,6 +102,10 @@ func main() { |
| os.Exit(1) |
| } |
| app.UseAuth(signin) |
| + if err := app.ConfigureHome(authEnv.Home); err != nil { |
| + logger.Error("home configuration unavailable") |
| + os.Exit(1) |
| + } |
| // rastrillo.Options.Mux wants a *http.ServeMux; the app's own |
| // router is a chi.Router (an http.Handler), so it mounts under one. |
diff --git a/docs/superpowers/specs/2026-09-07-atomic-cell-batch.md b/docs/superpowers/specs/2026-09-07-atomic-cell-batch.md| new file mode 100644 |
| index 0000000..c70b2c0 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-09-07-atomic-cell-batch.md |
| @@ -0,0 +1,22 @@ |
| +# Atomic cell batch implementation contract |
| + |
| +Status: revision 3; Claude Fable 5.1 (claude-fable-5-1) approved revision 2 design at 1aca44c with these wording amendments, and requested code changes for committed browser queue proof and obsolete comments. Focused code re-review remains pending. Implements the overnight suite readiness request, under the existing Sheets design and next-three batch-route item. |
| + |
| +- POST /api/sheets/{sheetID}/tabs/{tabID}/cells/batch accepts {cells: [{r,c,raw,fromR?,fromC?}], client?}. Require an editor using openTab, preserving sheet/tab pairing and 404 isolation. |
| +- Limit to 500 distinct destinations and 1 MiB body. Validate every coordinate, copy origin, raw length, translated raw length, and formula before any committed writes. Duplicate destinations, empty batches, and trailing JSON are invalid. Reuse existing fixed response strings; add no UI copy. |
| +- Hold the live handle mutex while a transaction-scoped independent handle is built from the DB. It shares no mutable graph or maps with the live handle. Set its next version at least to the live handle version. Ensure rows/columns and perform existing SetReport operations through that transaction handle. Existing nested transactions become savepoints; only the outer transaction commits. Reject capped recalculation and over-limit resulting cell count atomically. |
| +- Preserve existing per-cell input history and recalculation semantics. Merge reports by coordinate, keeping final values and marking directly edited cells as direct. Return only after outer commit; swap graph/maps/version into the live handle after commit. On failure discard the transaction handle; no partial history, axes, data, or live publication. |
| +- One response and SSE publication contain all changed cells and the final version. Keep the current 500-cell paste cap. The existing cell queue submits one batch instead of one request per cell. Larger clear selections retain serial batches of 500; a failed batch must stop the queue. Existing post currently absorbs failures; make it return explicit success for queue control while preserving existing callers. |
| +- No format, row/column semantics, tab lifecycle, formula grammar, or permission changes. Clipboard provenance is a separate next slice; this changes only transmission, no behaviour change to clipboard.js; its batching comments are updated. |
| +- Cost: a second tab graph is built per batch. Measure a 500-cell batch and document this bounded first implementation. Avoid adding a graph clone API or redesigning calculation before evidence demands it. |
| +- Tests: dependent formulas and clears, overlapping formulas, encryption/history, invalid late formula, injected persistence failure after earlier writes, axis rollback, shared live handle unchanged on failure, viewer/stranger/cross-sheet isolation, request bounds, one publication, client queue one-request and stop-on-failure behavior in tools/drive/batch-checks.mjs, called by clipboard.mjs and canary-batch.mjs. These use real editor clipboard actions, intercept a forced first-chunk failure, and await the queue completion barrier rather than assuming a delay means success. Browser checks are a deployed-canary gate, not part of make test. Run make test and exact-head CI. Build with make release, deploy isolated canary, run browser checks, obtain Fable branch review, then coordinate squash merge and production deployment with root. |
| + |
| +## Revision 2 review answers |
| + |
| +1. The independent handle is built with `buildHandle(tx, ...)`; its `db` is exactly `tx`. Recovery, count, axis allocation, input/event writes, and nested transactions all use that handle. No Open/registry path runs inside the transaction. TestCellBatchRecoversStaleCacheInsideTransaction deliberately seeds a stale cell after opening the live handle, then requires recovery and batch completion within three seconds. Building from the outer DB deadlocks that test. |
| +2. Measured with CELL_BATCH_BENCH_N and the committed TestCellBatchLargeTabTiming fixture: 500 changed cells on a 50,000-cell tab took 660ms; on a 200,000-cell tab took 2.326s on this development machine. These are transaction duration including rebuild, and therefore instance-wide writer blocking durations. The fixture has one feeder and many dependent formulas; paste changes leaf cells. Production measurements remain required. This initial bounded implementation accepts this cost to preserve atomic rollback without inventing graph cloning. The trigger for moving the rebuild out of the writer transaction is measured unacceptable production/pilot latency; it must preserve recovery and handle consistency. Each SetReport uses nested savepoints, which GORM retains until outer commit (roughly 1,000 per 500-cell batch). |
| +3. The first non-error operation after outer Transaction returns is swapping graph and mutable maps into the live handle under its mutex. Report assembly occurs inside the transaction before commit; response formatting happens after the live state is updated. There is no fallible operation between commit and the swap. Failure to encode or deliver the HTTP response does not invalidate the now-current live handle; reconnect/snapshot recovers delivery. |
| +4. A merged report may contain at most calc.MaxDirtyPerPass (5,000) distinct cells. Exceeding either an individual recalc cap or the merged report cap rejects and rolls back the whole batch. This deliberately differs from the single-cell path: the batch success contract guarantees complete final values without depending on the pre-existing resync behavior. No new user copy is introduced; the existing write failure string is reused. Supporting larger dependency fanouts is a later slice with dependable resync and explicit recovery semantics. TestCellBatchRejectsMergedReportPastCalculationCap proves the aggregate limit is applied even when each individual edit is within its cap. |
| +5. Both the implementation and failure-injection tests exist. The live handle is preserved on rollback and updated immediately on success. Local make test and remote CI were green at initial implementation 50914fb; revision 2 requires fresh checks and review. |
| + |
| +Revision 3 records the aggregate request bound explicitly: the 1 MiB body limit may reject a set of individually valid 8 KiB cells; callers must stay within both limits. Batch copy origins must include both coordinates, stricter than the legacy single-cell route. For pilot measurement, a p95 batch latency above 1 second at 50,000 cells or 3 seconds at 200,000 cells triggers the next transaction-duration design review; these are investigation thresholds, not a readiness claim. |
diff --git a/docs/superpowers/specs/2026-09-07-canary-fixture-bootstrap.md b/docs/superpowers/specs/2026-09-07-canary-fixture-bootstrap.md| new file mode 100644 |
| index 0000000..335b5bc |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-09-07-canary-fixture-bootstrap.md |
| @@ -0,0 +1,62 @@ |
| +# Isolated Sheets canary session fixture |
| + |
| +Status: proposed fallback for Fable review; do not deploy yet. Normal SMTP-based sign-in remains preferred and root has asked the user about a controlled mailbox. This design is only for testing unchanged session validation and editor behavior if that mailbox is unavailable. |
| + |
| +## Target and boundary |
| + |
| +The only permitted target is app sheets, account bds, instance overnight-sheets.bds.oncarlos.com, channel canary/overnight-batch, with named environment overnight-batch, separately bound by root. Root has verified current platform CLI supports instance environment binding. Never change the app's default environment, production origin or production channel. The fixture receives fresh canary SHEETS_DEK and SHEETS_INSTANCE_KEY and an owner address oficina-fixture@example.test. It does not read or copy production secrets, sessions, databases or content. |
| + |
| +Use a temporary reviewed bootstrap binary on this isolated instance, then replace it with the exact normal candidate binary. The bootstrap binary contains no login endpoint, alternate auth middleware, universal cookie, content endpoint or API backdoor. It initializes existing schema/session rows during startup only, then serves only platform health/version responses while awaiting replacement. It never serves the editor, so no browser behavior can accidentally be certified against bootstrap code. |
| + |
| +## Host and environment guards |
| + |
| +- Compile the single expected HTTPS origin and canary hostname into the bootstrap. Require SHEETS_ORIGIN to equal that origin exactly (scheme/host, no wildcard or suffix match). |
| +- Require a one-use fixture run ID in a dedicated canary environment variable, and require it to equal the run ID compiled into this bootstrap build. It is a scope guard, not an authentication mechanism. |
| +- Require SHEETS_OWNERS to equal exactly the synthetic owner address, with no additional addresses. Validate the key formats normally and require root's fresh-key provisioning record; do not compare against or obtain production key values. |
| +- The supported target is the platform exec-child contract: ExecBackend.Start invokes the binary with --socket <route.Socket> --db <route.DBPath or DataDir/host.db>. Read both through rastrillo.Resolve, not by parsing HTTP Host. The observed canary socket is exactly /run/carlos/overnight-sheets.bds.oncarlos.com.sock. The expected default database is /data/overnight-sheets.bds.oncarlos.com.db (platform DataDir defaults to /data). Root must confirm that resolved DB path from the route/activation configuration before building. Compile the confirmed absolute DB and socket paths, and reject any different path even if its basename matches. |
| +- Validate both exact cleaned absolute paths and exact host-derived basenames. Reject parent traversal, a symlink database file, or a resolved parent directory outside the compiled expected root. For a fresh nonexistent DB, resolve/check the existing parent directory before opening it; after opening, confirm the path still identifies the expected file. Do not derive the expected paths from fixture-supplied environment values. If the deployment uses a non-default DataDir, review and compile that one actual absolute path rather than accepting arbitrary directories. |
| +- STATE_DIRECTORY is not required for this exec-child shape. If present, it must not contradict the confirmed runtime paths, but its absence alone is not a failure. Supporting a systemd unit-backed shape later requires a separate explicit guard variant, not a basename-only fallback. |
| +- Before deployment, root verifies the control-plane route has this exact host, named environment and canary-only channel. Runtime path/origin guards complement that check; neither a release name nor an HTTP Host header alone establishes isolation. |
| +- Refuse bootstrap if any app content or member/grant/session row already exists. This is a fresh-fixture tool, not a repair or reset tool. Existing migration ledger rows alone are harmless. Never clear tables to make the precondition pass. |
| + |
| +## Session material and initialization |
| + |
| +Generate a random 32-byte session token locally using the framework's token format (base64 raw URL encoding), and compute SHA-256 hex with sessions.HashToken. Keep the raw token only in a chmod-0600 local Playwright storage-state file. Send only the hash, synthetic subject and bounded expiry through the dedicated canary environment. No token, key, cookie or sign-in link goes into stdout, logs, discussions, screenshots, commits or release metadata. |
| + |
| +Run sheets.BootSchema through the normal migrate.Apply path. Seed only one row in the existing sessions table in one transaction after rechecking the empty-state precondition. The pinned sessions schema contains token_hash, subject, method, auth_time, created_at and expires_at. Use: |
| + |
| +- token_hash: supplied 64-character lowercase hex hash, validated strictly; |
| +- subject: exactly oficina-fixture@example.test; |
| +- method: fixture (do not claim the mailbox was verified); |
| +- auth_time: empty; |
| +- created_at: current UTC RFC3339; |
| +- expires_at: a supplied absolute UTC time no later than two hours after creation, validated before insertion. |
| + |
| +No custom table or migration is added. A repeated bootstrap start may accept exactly the same single hash/subject/expiry row as an idempotent success but must not extend its expiry or mint another session. Any other session or app data causes refusal. Once the candidate browser creates app content, bootstrap is intentionally unable to run again on that database. |
| + |
| +The normal candidate's actor() resolves the session subject and memberFor admits/promotes only the synthetic owner listed in SHEETS_OWNERS. Do not seed a broader role or grant independently. Local tests must prove the seeded row is accepted by the real sessions.New lookup/adoption path, while a random token and an expired fixture row are rejected. |
| + |
| +Playwright state sets the cookie __Host-rastrillo_session for the exact HTTPS canary origin: Secure=true, HttpOnly=true, SameSite=Lax, path=/, no Domain attribute. The raw value never reaches bootstrap. The cookie is a fixture for the existing session store, not a signature forged using an application key. |
| + |
| +## Deployment sequence and assertions |
| + |
| +1. Record production process version and route/channel/environment bindings without reading secrets. Verify isolated canary has fresh keys and the expected empty database. |
| +2. After Fable approves bootstrap design/code and local fixture tests, deploy its uniquely stamped binary only to the explicit canary host/channel with carlos deploy. Verify bootstrap /api/version and that every non-platform route refuses; no app assertions yet. |
| +3. Remove the fixture-specific hash/run/expiry settings from the named environment before replacing bootstrap. They are not used by the normal candidate. |
| +4. Deploy the exact clean candidate built with make release. Poll process-served /api/version until it equals the intended candidate SHA; verify the canary channel no longer names the bootstrap release. Verify production still names its previously recorded release. Never treat an edge version header or a successful promote as process proof. |
| +5. Only now open Playwright with the private cookie state. Verify /sheets accepts this fixture session, unsigned and random-cookie requests cannot write, and then run deployed clipboard/batch, clear/reopen version regression, reload, second-observer and scriptless-table checks. Record candidate SHA alongside outcomes. |
| +6. Do not claim these checks exercised email delivery, pending-cookie magic-link binding, SMTP credentials or mailbox access. They certify real persisted-session validation and candidate editor behavior only. Normal SMTP sign-in remains a separate readiness check. |
| + |
| +## Teardown |
| + |
| +Use the candidate's normal POST /signout while the fixture cookie is present; confirm it deletes/revokes the row and the old cookie no longer accesses /sheets. Close browser contexts and delete the private storage-state/token files. Fixture expiry is a second bound if signout fails. Remove remaining fixture-specific named-environment variables. Keep the canary channel on the normal candidate or remove only this disposable instance through supported CLI, according to root's overnight verification needs. Never leave bootstrap selected by any channel. Remove its channel reference first; an immutable release artifact may remain in storage if the platform has no safe deletion verb, but its origin/path/run guards and missing fixture env must make it inert if mistakenly selected. |
| + |
| +Do not delete production routes, releases, keys, backups or configuration during teardown. Any authentication or cleanup failure is recorded as a failed/limited canary check, not hidden by reseeding. |
| + |
| +## Evidence required before use |
| + |
| +Fable review, wrong-host/wrong-origin/nonempty-database/expired-session/local-session-acceptance tests, supported CLI route/environment inspection, bootstrap replacement proof, and exact candidate process SHA. No bootstrap deployment has occurred as part of writing this proposal. |
| + |
| +## Native seed path |
| + |
| +The existing platform supports a first-boot seed argv on an instance record; ExecBackend.Seed invokes that release binary with the configured argv followed by --db <path>, under the same merged environment, only when no local database or replica exists. This is an existing supported lifecycle, not an HTTP auth mechanism. It can be used only if root verifies the actual instance seed configuration through the supported CLI/API and Fable reviews the separate bootstrap command. The command must retain the same origin/run/owner/empty-database/exact-DB-path guards; a one-shot native seed has no socket argument and must verify the corresponding expected route socket in root's control-plane preflight. It writes ordinary session hashes into the existing schema, exits, and introduces no fixture command into the normal candidate. The candidate's deployed manifest/instance record must not keep a bootstrap seed argv referring to unsupported candidate commands. The initial fallback remains the simpler reviewed startup-only bootstrap; no native seed configuration has been changed. |
diff --git a/docs/superpowers/specs/2026-09-07-durable-tab-versions.md b/docs/superpowers/specs/2026-09-07-durable-tab-versions.md| new file mode 100644 |
| index 0000000..0c35dde |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-09-07-durable-tab-versions.md |
| @@ -0,0 +1,56 @@ |
| +# Durable tab version watermark |
| + |
| +Status: design approved for implementation at 9f6abe2 by Fable 5.1 (claude-fable-5-1), after the four amendments requested at 91e9991. Implementation starts at 6ccb609. Amadan discussion 19/task 3. Branch review, exact-head CI, reconciliation with deployed chrome and canary verification remain required before production deployment. |
| + |
| +## Reproduction |
| + |
| +A 500-cell paste returns version 503. Release/reopen the tab handle, clear those 500 cells, and the clear returns 1004. Release/reopen again and write one new cell: version 4. The browser correctly refuses version 4 as older than 1004, so the newly committed edit is absent from the display. The same defect affects a single-cell edit after clearing the last surviving cell. |
| + |
| +buildHandle derives maxVersion only from current cells' InputVersion and CacheVersion. Deleting the newest cell rows destroys that ordering evidence. The in-memory handle counter survives only while some caller retains the handle; SSE subscribers deliberately do not retain it. An event count or timestamp is not a reliable substitute: neither is the previously issued version. |
| + |
| +## Invariant and schema |
| + |
| +For each tab, every committed cell input or persisted calculation stamp is no greater than a durable tab watermark. Opening a handle starts above both that watermark and all surviving input/cache versions. Clearing cells, recovering stale caches, and releasing every handle must never make a future edit's version go backwards. |
| + |
| +Add Tab.ContentVersion as a non-null integer with default zero through an additive generated migration. It is metadata of the same class as the existing cell InputVersion/CacheVersion, not authored content. Existing tabs start at zero and retain all content, names, history, formats, rows and columns. At open, the maximum of watermark and existing cell versions preserves older populated databases without a destructive migration/backfill. Already-empty pre-upgrade databases have no reconstructable historic version; deployment must reconnect/refetch browsers, and all future edits gain the durable invariant. |
| + |
| +SQLite integers are signed 64-bit. Refuse an exhausted/corrupt watermark before allocating a wrapped stamp; do not silently return to zero. No user-facing strings are added; existing fixed storage failure responses remain. |
| + |
| +## Mutation paths |
| + |
| +| Path | Required handling | |
| +| --- | --- | |
| +| SetReport -> setContent | Advance watermark to at least v in the same first transaction as sealed input and AppendInput | |
| +| SetReport -> setBlank | Advance watermark to at least v in the same transaction as deletion, dirty dependent markers and AppendInput | |
| +| persistTouched | Advance watermark to at least v in the same transaction as sealed cache updates and AppendRecalc | |
| +| recoverStale | Uses persistTouched on the transaction-scoped handle; recovery version is also preserved even if its cells are later cleared | |
| +| SetBatch | Every nested update uses staged.db=tx; outer rollback undoes watermark changes too; successful live swap retains staged.nextVersion | |
| +| Table edit, XLSX import, restore, undo | Already call Set or SetReport; inherit the same durable handling without parallel logic | |
| +| SetFormat, widths/heights, saved views | Existing behavior does not allocate content versions; retain it. This fix does not pretend to solve concurrent ordering of distinct equal-version formatting responses | |
| +| EnsureRow/EnsureCol | Allocate stable axes only, not a content change; no new version | |
| +| Row/column insert/delete/move | Not implemented in this repository; future structural operations must allocate and persist a tab stamp in the same mutation transaction. Do not delete a watermark with cell/axis rows | |
| +| Tab add/rename/reorder | Future metadata events have separate ordering semantics; do not masquerade as cell history | |
| + |
| +Implement one small transaction-scoped helper for monotonic advancement using an allowlisted update of content_version on Tab by ID. It never lowers a stored value. Both input write paths and persistTouched call it. No outer db calls occur while a transaction holds the one writer connection. Use SET content_version = MAX(content_version, ?) WHERE id = ? so RowsAffected=0 means the tab is missing, not that a lower stamp was safely ignored. A missing tab row must fail rather than silently discarding the watermark. |
| + |
| +Read the tab watermark while building a handle, before allocating nextVersion; a missing tab row fails Open. Preserve the existing deterministic recovery-stamp reservation, even when there are no stale cells, so a snapshot version handed out before release is no greater than the first edit stamp after reopen. Check exhaustion at Open and SetReport allocation (including staged SetReport calls inside SetBatch). Move the in-memory nextVersion increment above the blank crash seam after transaction one commits. A failed input/recalculation transaction must not commit its watermark; the existing dirty-marker behavior after a committed input transaction remains valid. A failed atomic batch must leave both the live handle and persisted watermark unchanged. |
| + |
| +## Regression coverage |
| + |
| +1. Real release/reopen sequence for paste500, clear500, new edit; require strictly increasing report versions and the correct final value. Existing candidate fails 503 ->1004 ->4. |
| +2. Single-cell last-cell clear followed by new edit after reopen; require the new stamp to exceed the clear stamp. |
| +3. Blank crash seam after the first transaction: release/reopen and require the next edit to exceed the committed blank's allocated stamp. |
| +4. Overlapping formula/dependent recomputation, removal of the newest cached cells, and reopen preserve ordering and correct values. |
| +5. Existing failure injection into a late SetBatch event must leave Tab.ContentVersion unchanged alongside inputs, history and axes. A subsequent successful edit must remain usable. |
| +6. Migrate a database built with BootSchema through the pinned pre-watermark migration IDs (Sheets 0001 through 0009 plus framework schema); seed encrypted names/content and history, then apply the new migration. Verify byte-for-byte ciphertext retention, default watermark, schema agreement and monotonic first post-upgrade edits. |
| +7. A high watermark in an otherwise empty tab survives Open; the next write uses a higher stamp. A lower attempted advance cannot reduce it. Exhaustion refuses without a write. |
| +8. Snapshot Version() before release/restart must be no greater than the first edit stamp after reopening. Test both populated and cleared tabs. |
| +9. A capped blank with more than MaxDirtyPerPass dependents must advance the watermark in transaction one alone. Mutation-test by removing only that advancement; the regression must fail. |
| +10. A retained handle starts a batch with nextVersion above the persisted watermark, commits a batch, then rolls back a late-failing batch. After release/reopen the next edit must exceed the successful report, and the failed batch must leave its watermark unchanged. |
| +11. Run existing crash/recovery, plain SQLite/WAL leakage and race tests. The watermark contains no plaintext values and adds no logs. |
| + |
| +## Review and deploy |
| + |
| +Keep this prerequisite independently attributable in discussion 19. Fable reviews the design before implementation and the final code before merge. Integrate with atomic batch only after passing full make test, rastrillo migration check, exact-head CI and deployed canary browser regression (paste, clear, new paste, reload, second observer). A migration-only deploy must not erase or replace any user data. Root coordinates production promotion and /api/version proof. |
| + |
| +The current migration head is 0009_alter_formats_ink_intent. Confirm current main has not acquired 0010 before final integration; assign a fresh additive migration ID if it has. The old-database fixture must select known IDs, not filter SQL text, so future migrations mentioning the column cannot be silently omitted. |
diff --git a/docs/superpowers/specs/2026-09-07-reconcile-deployed-chrome.md b/docs/superpowers/specs/2026-09-07-reconcile-deployed-chrome.md| new file mode 100644 |
| index 0000000..1828c65 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-09-07-reconcile-deployed-chrome.md |
| @@ -0,0 +1,28 @@ |
| +# Reconcile deployed Sheets with main |
| + |
| +Status: assessment and review candidate only. Amadan discussion 20/task 4. No production deployment is authorized before the gates below pass. |
| + |
| +## Exact histories |
| + |
| +The control plane reports production release 7212864. That commit and origin/main 4aea8f4 share ancestor 8ed3926; neither tip contains the other. Production process confirmation remains root's deployment check. |
| + |
| +Main adds suite-doctrine amendments in AGENTS.md, PRODUCT.md and docs/superpowers/specs/2026-08-30-sheets-design.md. The seven deployed-side commits 9830466, 465160f, 51bc3e0, 4ea4031, 5f44bae, 51db7bc and 7212864 modify 21 disjoint code, template, test and browser-driver files. They add GET /sheets/import and POST /sheets/{sheetID}/rename, preserve scriptless import/rename, put creation/import in list chrome, and restore page-wide file drop. |
| + |
| +Candidate d1ecaaf merges both parents without textual conflicts. Its only differences from deployed 7212864 are the three main doctrine files. Its only differences from main are the exact 21 deployed files. No atomic or watermark implementation is included. Existing branches/worktrees remain untouched. |
| + |
| +## Resolution and integration |
| + |
| +No conflict resolution edits were needed: preserve main's doctrine verbatim and deployed implementation verbatim. This does not imply implementation satisfies all newly documented suite requirements; suite/lone identity, grants and link doctrine implementation remains tracked independently. |
| + |
| +After independent Fable review, exact-head checks, human copy review and deployed canary verification, squash this preservation branch to main. Then rebase or merge the independently reviewed atomic and durable-watermark work onto that approved main. Their app.go route addition must preserve the import-page/rename registrations and auth group. Re-run exact-head checks and branch review after integration. Never ship the old main-based atomic binary to production. |
| + |
| +Workbook-tabs design must treat the deployed route set as the baseline: selected-tab navigation must preserve workbook rename redirects and GET /sheets/import. Import remains workbook-scoped. Rename should return to the selected tab rather than silently reopening the first tab. Tabs implementation is still awaiting its own design review. |
| + |
| +## Required evidence |
| + |
| +- Fable must review the full deployed-side diff plus the doctrine delta; discussion 15 explicitly lacked this review and the rebased deployment has no separate discussion record. |
| +- Full make test and exact-head remote CI; existing rename permission/body-delay/query-body regressions and list/drop/scriptless assertions must remain green. |
| +- Human review of the carried strings. Existing deployment is not copy approval. Extract the changed/new list, rename, import and drop strings into a separate review index, preserve exact current text, and apply only submitted edits verbatim. Reused unchanged labels are identified in that index where they moved contexts. |
| +- Estilo review of existing deployed chrome, considering a shared component only from a real second usage. This preservation branch does not introduce a guessed abstraction. |
| +- Canary process SHA and browser checks: authenticated Create then Rename; query-only and rejected-body rename cannot change a title; viewer cannot rename; import page and refusal recovery; list drag/drop and Escape recovery; keyboard and scriptless paths; narrow/light/dark layout. Use synthetic fixture data and root's approved canary authentication mechanism. |
| +- Root records production process SHA before and after deployment. Until these gates pass, preserve production's current release and retain this reconciliation prerequisite in Sheets discussions 17/19 and suite discussion 5. |
diff --git a/go.mod b/go.mod| index 8c1af05..d7a564a 100644 |
| --- a/go.mod |
| +++ b/go.mod |
| @@ -3,6 +3,10 @@ module amadan.net/oficina/sheets |
| go 1.25.0 |
| require ( |
| + amadan.net/oficina/estilo.git v0.0.0-20260909234655-dec3908b237c |
| + amadan.net/oficina/home/connect v0.0.0-20260910000424-5b8d8410775f |
| + amadan.net/oficina/home/handoff v0.0.0-20260908102137-481d2d57f489 |
| + amadan.net/rastrillo/rastrillo v0.26.1-0.20260908075707-29e4491631bd |
| github.com/carlosframework/rastrillo v0.23.0 |
| github.com/go-chi/chi/v5 v5.3.2 |
| github.com/xuri/excelize/v2 v2.11.0 |
| @@ -12,6 +16,7 @@ require ( |
| require ( |
| github.com/dustin/go-humanize v1.0.1 // indirect |
| + github.com/go-json-experiment/json v0.0.0-20250725192818-e39067aee2d2 // indirect |
| github.com/google/uuid v1.6.0 // indirect |
| github.com/jinzhu/inflection v1.0.0 // indirect |
| github.com/jinzhu/now v1.1.5 // indirect |
diff --git a/go.sum b/go.sum| index a075856..6365c46 100644 |
| --- a/go.sum |
| +++ b/go.sum |
| @@ -1,15 +1,35 @@ |
| -github.com/carlosframework/rastrillo v0.22.0 h1:tG02WAiy2VSYqmLT6kRfYbUaiJd/pBhTIWxxjjsEoI8= |
| -github.com/carlosframework/rastrillo v0.22.0/go.mod h1:pZlrE5F5OhspvZFzef0DrfjHAjA8FXycb9J5/8vc8Tw= |
| +amadan.net/oficina/estilo.git v0.0.0-20260909234655-dec3908b237c h1:LgE7WLm6zMG/qZScoAHb4X2gguzorctB4XGjPYavXts= |
| +amadan.net/oficina/estilo.git v0.0.0-20260909234655-dec3908b237c/go.mod h1:0R45/vCUAT+kpS7S6K/ZxBdE8f7YxFN4HT3HyDJ3nPY= |
| +amadan.net/oficina/home/connect v0.0.0-20260910000424-5b8d8410775f h1:lkw3tjzHfZxe74pNfo+tGDz4mkZLg+D6R+FTkmsym6M= |
| +amadan.net/oficina/home/connect v0.0.0-20260910000424-5b8d8410775f/go.mod h1:YroavXe43Wietec+po8mtSk4a9Bir2h31Q3iAf4MWxI= |
| +amadan.net/oficina/home/handoff v0.0.0-20260908102137-481d2d57f489 h1:RPKtyry/71i06o8L+gjAaUyexbmG56kO1YPs601URAM= |
| +amadan.net/oficina/home/handoff v0.0.0-20260908102137-481d2d57f489/go.mod h1:YISpzFJ+iMgrM0rUL032OSzsAQbP4/nozvXLCCOcn1k= |
| +amadan.net/rastrillo/rastrillo v0.26.1-0.20260908075707-29e4491631bd h1:jlFSt95M1ontOg+VePU+kIJpChtWqDQAbnmlxqCnbgc= |
| +amadan.net/rastrillo/rastrillo v0.26.1-0.20260908075707-29e4491631bd/go.mod h1:RpyHVPD0udcSfHJroY0KZE/FUjgw6Tvfo8SJm2qeZGE= |
| github.com/carlosframework/rastrillo v0.23.0 h1:tKAAlwPt4Pg/rkqk7o6jvV12bbZ0r34oEVpuOlKfX7w= |
| github.com/carlosframework/rastrillo v0.23.0/go.mod h1:pZlrE5F5OhspvZFzef0DrfjHAjA8FXycb9J5/8vc8Tw= |
| +github.com/chromedp/cdproto v0.0.0-20250724212937-08a3db8b4327 h1:UQ4AU+BGti3Sy/aLU8KVseYKNALcX9UXY6DfpwQ6J8E= |
| +github.com/chromedp/cdproto v0.0.0-20250724212937-08a3db8b4327/go.mod h1:NItd7aLkcfOA/dcMXvl8p1u+lQqioRMq/SqDp71Pb/k= |
| +github.com/chromedp/chromedp v0.14.2 h1:r3b/WtwM50RsBZHMUm9fsNhhzRStTHrKdr2zmwbZSzM= |
| +github.com/chromedp/chromedp v0.14.2/go.mod h1:rHzAv60xDE7VNy/MYtTUrYreSc0ujt2O1/C3bzctYBo= |
| +github.com/chromedp/sysutil v1.1.0 h1:PUFNv5EcprjqXZD9nJb9b/c9ibAbxiYo4exNWZyipwM= |
| +github.com/chromedp/sysutil v1.1.0/go.mod h1:WiThHUdltqCNKGc4gaU50XgYjwjYIhKWoHGPTUfWTJ8= |
| github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= |
| github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= |
| github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= |
| github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= |
| github.com/go-chi/chi/v5 v5.3.2 h1:5YQkICvTCSZ25hoRsyJazN0scjzKGiu4VAUc7H1o1nY= |
| github.com/go-chi/chi/v5 v5.3.2/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto= |
| +github.com/go-json-experiment/json v0.0.0-20250725192818-e39067aee2d2 h1:iizUGZ9pEquQS5jTGkh4AqeeHCMbfbjeb0zMt0aEFzs= |
| +github.com/go-json-experiment/json v0.0.0-20250725192818-e39067aee2d2/go.mod h1:TiCD2a1pcmjd7YnhGH0f/zKNcCD06B029pHhzV23c2M= |
| github.com/go-sql-driver/mysql v1.7.0 h1:ueSltNNllEqE3qcWBTD0iQd3IpL/6U+mJxLkazJ7YPc= |
| github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= |
| +github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU= |
| +github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM= |
| +github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og= |
| +github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw= |
| +github.com/gobwas/ws v1.4.0 h1:CTaoG1tojrh4ucGPcoJFiAQUAsEWekEWvLy7GsVNqGs= |
| +github.com/gobwas/ws v1.4.0/go.mod h1:G3gNqMNtPppf5XUz7O4shetPpcZ1VJ7zt18dlUeakrc= |
| github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e h1:ijClszYn+mADRFY17kjQEVQ1XRhq2/JR1M3sGqeJoxs= |
| github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA= |
| github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= |
diff --git a/internal/guardstest/csstokens_test.go b/internal/guardstest/csstokens_test.go| index 043f231..dcbb4bf 100644 |
| --- a/internal/guardstest/csstokens_test.go |
| +++ b/internal/guardstest/csstokens_test.go |
| @@ -7,6 +7,8 @@ import ( |
| "sort" |
| "strings" |
| "testing" |
| + |
| + "amadan.net/oficina/estilo.git/theme" |
| ) |
| // ourStylesheets are the CSS files this app writes. The vendored ones |
| @@ -17,11 +19,10 @@ var ourStylesheets = []string{ |
| } |
| // tokenSources are the files that DEFINE rastrillo's tokens: the |
| -// structural ones and the theme's palette, both copied by |
| -// `rastrillo doctor` and verified by it. |
| +// structural ones copied by `rastrillo doctor` and the shared palette. |
| var tokenSources = []string{ |
| "internal/sheets/static/tokens.css", |
| - "internal/sheets/static/theme.css", |
| + "Estilo theme module", |
| } |
| var ( |
| @@ -48,7 +49,13 @@ func TestEveryTokenWeUseExists(t *testing.T) { |
| defined := map[string]bool{} |
| for _, rel := range tokenSources { |
| - body, err := os.ReadFile(filepath.Join(root, rel)) |
| + var body []byte |
| + var err error |
| + if rel == "Estilo theme module" { |
| + body = theme.CSS() |
| + } else { |
| + body, err = os.ReadFile(filepath.Join(root, rel)) |
| + } |
| if err != nil { |
| t.Fatalf("guards: read %s: %v — the token sources are what `rastrillo doctor` copies; "+ |
| "if one moved, this test needs updating, not deleting", rel, err) |
diff --git a/internal/sheets/app.go b/internal/sheets/app.go| index e7a000d..9a8052a 100644 |
| --- a/internal/sheets/app.go |
| +++ b/internal/sheets/app.go |
| @@ -5,6 +5,9 @@ package sheets |
| import ( |
| "net/http" |
| + "amadan.net/oficina/home/connect" |
| + "github.com/carlosframework/rastrillo/sessions" |
| + |
| "github.com/carlosframework/rastrillo/auth" |
| "github.com/carlosframework/rastrillo/csrf" |
| "github.com/carlosframework/rastrillo/jobs" |
| @@ -49,11 +52,14 @@ type Config struct { |
| // App holds the wiring. One per process. |
| type App struct { |
| - db *gorm.DB |
| - auth *auth.Auth |
| - sealer *seal.Sealer |
| - origin string |
| - admit func(string) bool |
| + home *connect.Bridge |
| + homeRoutes *connect.Routes |
| + homeSessions *sessions.Sessions |
| + db *gorm.DB |
| + auth *auth.Auth |
| + sealer *seal.Sealer |
| + origin string |
| + admit func(string) bool |
| // hub fans every committed edit out to every other open tab. One per |
| // process, because it holds the live subscriptions: a hub per |
| @@ -141,7 +147,9 @@ func (a *App) Router() http.Handler { |
| // a test App: a Protect("") would compare every Origin header |
| // against the empty string and refuse every POST, turning a missing |
| // test fixture into a wall of 403s that look like an auth bug. |
| - if a.origin != "" { |
| + if a.homeRoutes != nil { |
| + r.Use(a.homeRoutes.Middleware) |
| + } else if a.origin != "" { |
| r.Use(csrf.Protect(a.origin)) |
| } |
| @@ -159,7 +167,7 @@ func (a *App) Router() http.Handler { |
| // other path, which is what an operator hitting a wrong URL |
| // should see (TestIndexIsNotACatchAll pins this). |
| r.Get("/", a.handleIndex) |
| - r.Get("/signin", a.handleSignin) |
| + a.mountAuth(r) |
| // Fingerprinted and immutably cached, mounted at the FS's own path |
| // with no prefix stripped — rastrillo's Assets uses |
| // http.FileServerFS semantics, where the URL path is "/" plus the |
| @@ -175,21 +183,17 @@ func (a *App) Router() http.Handler { |
| // entry (spec §8.3). The token is the entire authorization. |
| r.Get("/l/{token}", a.handleLinkView) |
| - if a.auth != nil { |
| - // The four handlers rastrillo/auth expects an app to mount, at |
| - // the paths its own docs use. Verify reads ?token=; the path is |
| - // baked into every emailed link, so moving it later invalidates |
| - // links already in people's inboxes. |
| - r.Post("/signin", a.handleBegin) |
| - r.Get("/auth/callback", a.auth.Callback) |
| - r.Get("/auth/verify", a.auth.Verify) |
| - r.Post("/signout", a.auth.Signout) |
| - } |
| - |
| r.Group(func(r chi.Router) { |
| r.Use(a.requireSession) |
| r.Get("/sheets", a.handleSheetList) |
| r.Post("/sheets", a.handleSheetCreate) |
| + // Static before the {sheetID} pattern below. chi prefers an |
| + // exact segment over a wildcard whatever the order, so this is |
| + // belt and braces rather than load-bearing — but a route that |
| + // only works because of resolution order is a route one |
| + // refactor away from parsing "import" as a sheet id. |
| + // TestImportPathIsNotASheetID pins it. |
| + r.Get("/sheets/import", a.handleImportPage) |
| // Import. A whole workbook is too much work for a request, so |
| // it 303s to a job (spec §11) rather than holding the |
| // connection open — and the drop zone on the list posts to |
| @@ -197,6 +201,9 @@ func (a *App) Router() http.Handler { |
| // over the form, not a second way in. |
| r.Post("/sheets/import", a.handleSheetImport) |
| r.Get("/sheets/{sheetID}", a.handleSheetPage) |
| + // Rename. The other half of a Create button that no longer |
| + // asks for a name — see handlers_rename.go. |
| + r.Post("/sheets/{sheetID}/rename", a.handleSheetRename) |
| // The scriptless view. Spec §6 names the grid editor, drag-fill |
| // and live sync as the only script-required things; everything |
| // else, including editing any cell, works here. |
| @@ -231,6 +238,7 @@ func (a *App) Router() http.Handler { |
| // ask for it in. |
| r.Get("/api/sheets/{sheetID}/tabs/{tabID}/cells", a.handleCellsGet) |
| r.Post("/api/sheets/{sheetID}/tabs/{tabID}/cells", a.handleCellPost) |
| + r.Post("/api/sheets/{sheetID}/tabs/{tabID}/cells/batch", a.handleCellBatchPost) |
| r.Post("/api/sheets/{sheetID}/tabs/{tabID}/format", a.handleFormatPost) |
| r.Post("/api/sheets/{sheetID}/tabs/{tabID}/size", a.handleSizePost) |
| r.Get("/api/sheets/{sheetID}/tabs/{tabID}/stream", a.handleStream) |
| @@ -260,8 +268,11 @@ func (a *App) Router() http.Handler { |
| // refusal and a full magic-link round trip. Production cannot reach it |
| // — main exits before New if sign-in is not configured. |
| func (a *App) requireSession(next http.Handler) http.Handler { |
| + if a.home != nil { |
| + return a.homeRequire(next) |
| + } |
| if a.auth == nil { |
| return next |
| } |
| - return a.auth.RequireSession(next) |
| + return a.auth.RequireSession(a.rejectDisabledHome(next)) |
| } |
diff --git a/internal/sheets/auth.go b/internal/sheets/auth.go| index 4d4303c..806dd83 100644 |
| --- a/internal/sheets/auth.go |
| +++ b/internal/sheets/auth.go |
| @@ -1,6 +1,7 @@ |
| package sheets |
| import ( |
| + "amadan.net/oficina/home/connect" |
| "database/sql" |
| "errors" |
| "fmt" |
| @@ -16,6 +17,7 @@ import ( |
| // AuthEnv is everything sign-in needs from the environment, already |
| // validated. Building it is the only place these variable names appear. |
| type AuthEnv struct { |
| + Home *connect.Credentials |
| // Origin is the instance's external origin, scheme included. It is |
| // the base of every emailed link and the CSRF origin check, which |
| // makes a WRONG value the dangerous case rather than a missing one: |
| @@ -73,6 +75,7 @@ func loopback(origin string) bool { |
| // any address. Both serve pages and answer /healthz. |
| func LoadAuthEnv(logger *slog.Logger) (AuthEnv, error) { |
| env := AuthEnv{ |
| + Home: connect.CredentialsFromEnv("SHEETS"), |
| Origin: strings.TrimSpace(os.Getenv("SHEETS_ORIGIN")), |
| InstanceKey: strings.TrimSpace(os.Getenv("SHEETS_INSTANCE_KEY")), |
| Owners: splitOwners(os.Getenv("SHEETS_OWNERS")), |
| @@ -107,6 +110,9 @@ func LoadAuthEnv(logger *slog.Logger) (AuthEnv, error) { |
| logger.Warn("sign-in: no SMTP configured and origin is loopback — " + |
| "magic links will be LOGGED, NOT SENT") |
| } |
| + if _, err := connect.DecodeCredentials(env.Home, env.Origin); err != nil { |
| + return AuthEnv{}, errHome |
| + } |
| env.Mailer = mail.FromEnv("CARLOS", logger) |
| return env, nil |
| @@ -144,6 +150,10 @@ func NewAuth(db *sql.DB, env AuthEnv, logger *slog.Logger, authorize func(string |
| // is the version that says so. |
| return nil, errors.New("sign-in: refusing to wire auth with no admission gate") |
| } |
| + signedInPath := "/sheets" |
| + if env.Home != nil { |
| + signedInPath = connect.LocalCompletePath |
| + } |
| return auth.New(auth.Config{ |
| DB: db, |
| Origin: env.Origin, |
| @@ -155,7 +165,7 @@ func NewAuth(db *sql.DB, env AuthEnv, logger *slog.Logger, authorize func(string |
| // which is the anonymous entry page — so without this line |
| // every magic-link click arrives on a page that exists to |
| // offer sign-in, and has to be told it worked. |
| - SignedInPath: "/sheets", |
| + SignedInPath: signedInPath, |
| Subject: "Your Sheets sign-in link", |
| Logger: logger, |
| }) |
diff --git a/internal/sheets/auth_test.go b/internal/sheets/auth_test.go| index c3b31ba..775a114 100644 |
| --- a/internal/sheets/auth_test.go |
| +++ b/internal/sheets/auth_test.go |
| @@ -19,6 +19,7 @@ func setEnv(t *testing.T, kv map[string]string) { |
| t.Helper() |
| for _, k := range []string{ |
| "SHEETS_ORIGIN", "SHEETS_INSTANCE_KEY", "SHEETS_OWNERS", |
| + "SHEETS_HOME_ISSUER", "SHEETS_HOME_CONSUMER_ID", "SHEETS_HOME_KEYS_BASE64", "SHEETS_HOME_DEK_BASE64", |
| "CARLOS_SMTP_HOST", "CARLOS_SMTP_FROM", |
| } { |
| t.Setenv(k, "") |
diff --git a/internal/sheets/handlers_cell_batch.go b/internal/sheets/handlers_cell_batch.go| new file mode 100644 |
| index 0000000..1cb390a |
| --- /dev/null |
| +++ b/internal/sheets/handlers_cell_batch.go |
| @@ -0,0 +1,65 @@ |
| +package sheets |
| + |
| +import ( |
| + "encoding/json" |
| + "io" |
| + "net/http" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +func (a *App) handleCellBatchPost(w http.ResponseWriter, r *http.Request) { |
| + actor, ok := a.actor(r) |
| + if !ok { |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + return |
| + } |
| + h, ok := a.openTab(w, r, SheetEditor) |
| + if !ok { |
| + return |
| + } |
| + defer h.Release() |
| + |
| + var in struct { |
| + Cells []cellWrite `json:"cells"` |
| + Client string `json:"client"` |
| + } |
| + dec := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<20)) |
| + if err := dec.Decode(&in); err != nil { |
| + http.Error(w, "Malformed request.", http.StatusBadRequest) |
| + return |
| + } |
| + var extra any |
| + if err := dec.Decode(&extra); err != io.EOF || len(in.Cells) == 0 || len(in.Cells) > store.MaxCellBatch { |
| + http.Error(w, "Malformed request.", http.StatusBadRequest) |
| + return |
| + } |
| + writes := make([]store.CellWrite, len(in.Cells)) |
| + for i, cell := range in.Cells { |
| + dr, dc, valid := cell.origin() |
| + if !inCellRange(cell.R, cell.C) || !valid || (cell.FromR == nil) != (cell.FromC == nil) { |
| + http.Error(w, "Cell out of range.", http.StatusBadRequest) |
| + return |
| + } |
| + if len(cell.Raw) > maxRawBytes { |
| + http.Error(w, "Cell input too long.", http.StatusRequestEntityTooLarge) |
| + return |
| + } |
| + raw := translateCopied(cell.Raw, dr, dc) |
| + if len(raw) > maxRawBytes { |
| + http.Error(w, "Cell input too long.", http.StatusRequestEntityTooLarge) |
| + return |
| + } |
| + writes[i] = store.CellWrite{Row: cell.R, Col: cell.C, Raw: raw} |
| + } |
| + rep, err := h.SetBatch(writes, actor.ID) |
| + if err != nil { |
| + http.Error(w, "That cell could not be written.", http.StatusBadRequest) |
| + return |
| + } |
| + cells, styles := fromReport(rep, newPosIndex(h), h) |
| + resp := gridResponse{Version: rep.Version, Cells: cells, Formats: styles, Client: in.Client} |
| + a.touchSheet(sheetIDFrom(r)) |
| + a.publish(tabIDFrom(r), resp) |
| + writeJSON(w, http.StatusOK, resp) |
| +} |
diff --git a/internal/sheets/handlers_cell_batch_test.go b/internal/sheets/handlers_cell_batch_test.go| new file mode 100644 |
| index 0000000..6bf7103 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_cell_batch_test.go |
| @@ -0,0 +1,130 @@ |
| +package sheets |
| + |
| +import ( |
| + "encoding/json" |
| + "fmt" |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +func TestCellBatchPublishesOnlyFinalCommittedValues(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, path := newSheet(t, app, cookies, "Budget") |
| + sub := app.hub.Subscribe(s.TabID) |
| + defer sub.Close() |
| + rec := postCell(t, app, cookies, path+"/batch", `{"cells":[{"r":1,"c":0,"raw":"=A1*2"},{"r":0,"c":0,"raw":"21"}],"client":"batch-client"}`) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("batch %d: %s", rec.Code, rec.Body.String()) |
| + } |
| + got := decodeGrid(t, rec) |
| + if len(got.Cells) != 2 || got.Client != "batch-client" { |
| + t.Fatalf("response %+v", got) |
| + } |
| + for _, c := range got.Cells { |
| + if c.R == 1 && c.Display != "42" { |
| + t.Fatalf("stale formula %+v", c) |
| + } |
| + } |
| + select { |
| + case m := <-sub.C(): |
| + if string(m.Data) != strings.TrimSpace(rec.Body.String()) { |
| + t.Fatalf("publication differs: %s", m.Data) |
| + } |
| + default: |
| + t.Fatal("no publication") |
| + } |
| + select { |
| + case <-sub.C(): |
| + t.Fatal("intermediate publication") |
| + default: |
| + } |
| + var events int64 |
| + app.db.Model(&store.Event{}).Where("tab_id = ? AND kind = ?", s.TabID, store.KindInput).Count(&events) |
| + if events != 2 { |
| + t.Fatalf("input events = %d", events) |
| + } |
| +} |
| + |
| +func TestCellBatchRejectsInvalidRequestsWithoutWrites(t *testing.T) { |
| + for _, body := range []string{ |
| + `{}`, `{"cells":[]}`, `{"cells":[{"r":0,"c":0,"raw":"7"}]} {}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"7"},{"r":0,"c":0,"raw":"8"}]}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"7"},{"r":-1,"c":0,"raw":"8"}]}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"7"},{"r":1,"c":0,"raw":"=SUM(("}]}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"7","fromR":0}]}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"7","fromR":-1,"fromC":0}]}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"` + strings.Repeat("x", maxRawBytes+1) + `"}]}`, |
| + `{"cells":[{"r":0,"c":0,"raw":"` + strings.Repeat("x", 1<<20) + `"}]}`, |
| + } { |
| + app, cookies := signIn(t) |
| + s, path := newSheet(t, app, cookies, "Budget") |
| + sub := app.hub.Subscribe(s.TabID) |
| + rec := postCell(t, app, cookies, path+"/batch", body) |
| + if rec.Code != http.StatusBadRequest && rec.Code != http.StatusRequestEntityTooLarge { |
| + t.Fatalf("invalid status %d", rec.Code) |
| + } |
| + if strings.Contains(rec.Body.String(), "SUM") || strings.Contains(rec.Body.String(), "xxx") { |
| + t.Fatal("content in error") |
| + } |
| + snap := decodeGrid(t, do(t, app, as(httptest.NewRequest(http.MethodGet, path, nil), cookies))) |
| + if len(snap.Cells) != 0 { |
| + t.Fatalf("invalid batch wrote %+v", snap.Cells) |
| + } |
| + select { |
| + case <-sub.C(): |
| + t.Fatal("failed batch published") |
| + default: |
| + } |
| + sub.Close() |
| + } |
| + app, cookies := signIn(t) |
| + _, path := newSheet(t, app, cookies, "Budget") |
| + cells := make([]cellWrite, store.MaxCellBatch+1) |
| + for i := range cells { |
| + cells[i] = cellWrite{R: i, Raw: "1"} |
| + } |
| + body, _ := json.Marshal(map[string]any{"cells": cells}) |
| + if rec := postCell(t, app, cookies, path+"/batch", string(body)); rec.Code != http.StatusBadRequest { |
| + t.Fatalf("oversized batch = %d", rec.Code) |
| + } |
| +} |
| + |
| +func TestCellBatchUsesCopyTranslationAndRejectsWrongTab(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, path := newSheet(t, app, cookies, "Budget") |
| + rec := postCell(t, app, cookies, path+"/batch", `{"cells":[{"r":1,"c":0,"raw":"=A1+$B$1","fromR":0,"fromC":0}]}`) |
| + if rec.Code != 200 { |
| + t.Fatalf("copy %d", rec.Code) |
| + } |
| + if got := decodeGrid(t, rec).Cells[0].Raw; got != "=A2+$B$1" { |
| + t.Fatalf("translated = %q", got) |
| + } |
| + other, _ := newSheet(t, app, cookies, "Other") |
| + wrong := fmt.Sprintf("/api/sheets/%d/tabs/%d/cells/batch", s.ID, other.TabID) |
| + if rec := postCell(t, app, cookies, wrong, `{"cells":[{"r":0,"c":0,"raw":"1"}]}`); rec.Code != 404 { |
| + t.Fatalf("cross sheet = %d", rec.Code) |
| + } |
| + if rec := postCell(t, app, nil, path+"/batch", `{"cells":[{"r":0,"c":0,"raw":"1"}]}`); rec.Code == 200 { |
| + t.Fatal("unsigned write accepted") |
| + } |
| +} |
| + |
| +func TestCellBatchEnforcesViewerCommenterAndStrangerIsolation(t *testing.T) { |
| + for _, role := range []SheetRole{SheetViewer, SheetCommenter} { |
| + app, cookies := signIn(t) |
| + s, path := newSheet(t, app, cookies, "Budget") |
| + guestOn(t, app, s.ID, "ada@elsewhere.example", role) |
| + other, _ := newSheet(t, app, cookies, "Other") |
| + guestOn(t, app, other.ID, "stranger@elsewhere.example", SheetEditor) |
| + for _, email := range []string{"ada@elsewhere.example", "stranger@elsewhere.example"} { |
| + guest := signInAs(t, app, email) |
| + if rec := postCell(t, app, guest, path+"/batch", `{"cells":[{"r":0,"c":0,"raw":"1"}]}`); rec.Code != 404 { |
| + t.Fatalf("%s %s batch = %d", role, email, rec.Code) |
| + } |
| + } |
| + } |
| +} |
diff --git a/internal/sheets/handlers_import.go b/internal/sheets/handlers_import.go| index 09ba94f..5b7be53 100644 |
| --- a/internal/sheets/handlers_import.go |
| +++ b/internal/sheets/handlers_import.go |
| @@ -111,7 +111,7 @@ func (a *App) handleSheetImport(w http.ResponseWriter, r *http.Request) { |
| data, filename, refusal := readUpload(w, r) |
| if refusal != "" { |
| - a.renderSheetList(w, r, http.StatusBadRequest, refusal) |
| + a.renderImportPage(w, r, http.StatusBadRequest, refusal) |
| return |
| } |
| @@ -131,7 +131,7 @@ func (a *App) handleSheetImport(w http.ResponseWriter, r *http.Request) { |
| // nothing and nobody asked for it. |
| a.discardSheet(s.ID) |
| if errors.Is(err, jobs.ErrOwnerBusy) { |
| - a.renderSheetList(w, r, http.StatusTooManyRequests, refusalBusy) |
| + a.renderImportPage(w, r, http.StatusTooManyRequests, refusalBusy) |
| return |
| } |
| a.fail(w, r, err) |
diff --git a/internal/sheets/handlers_import_page.go b/internal/sheets/handlers_import_page.go| new file mode 100644 |
| index 0000000..a5e78d9 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_import_page.go |
| @@ -0,0 +1,60 @@ |
| +package sheets |
| + |
| +import "net/http" |
| + |
| +// The import page. |
| +// |
| +// Separate from handlers_import.go, which holds the POST that does the |
| +// work, for the dullest possible reason: together they were 305 lines |
| +// and the file cap is 300 (AGENTS.md / Mechanical rules, enforced by |
| +// TestFileLineCap). The split falls in a sensible place anyway — this |
| +// file is the screen, that one is the upload. |
| + |
| +// importPageView is the import page's payload. |
| +type importPageView struct { |
| + // Error is a refusal to show above the form — one of the refusal |
| + // constants in handlers_import.go, never a wrapped error, because |
| + // everything shown here is read by somebody deciding what to do |
| + // differently. |
| + Error string |
| +} |
| + |
| +// handleImportPage draws the import form on a page of its own. |
| +// |
| +// Import used to be a second form on the sheet list, below the form |
| +// that made a blank sheet. Both came off that page: a list of sheets |
| +// should be a list of sheets, and the verbs moved into the chrome |
| +// around it. |
| +// |
| +// A page rather than an item inside the File menu, which is where this |
| +// nearly went. Three reasons, and the third is the one that decided it: |
| +// a dropdown is a poor container for a file picker plus three limits; a |
| +// hidden input behind a menu-shaped label never submits itself, so that |
| +// menu item would have needed script to import anything at all, and |
| +// import is meant to work with scripting off; and the File menu exists |
| +// only on a sheet page, so somebody with no sheets yet — exactly the |
| +// person most likely to be importing one — would have had no way in. |
| +func (a *App) handleImportPage(w http.ResponseWriter, r *http.Request) { |
| + // Behind requireSession already; this is the same belt-and-braces |
| + // actor check the sibling handlers make, and it is what makes the |
| + // handler safe to call from a test that does not mount the group. |
| + if _, ok := a.actor(r); !ok { |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + return |
| + } |
| + a.renderImportPage(w, r, http.StatusOK, "") |
| +} |
| + |
| +// renderImportPage draws that form, optionally carrying a refusal and |
| +// the status that goes with it. |
| +// |
| +// A refused import re-renders this page rather than redirecting to it, |
| +// so the refusal survives without a flash: the person lands back on the |
| +// form they posted from, with the reason above it. No session storage, |
| +// no cookie, no redirect, and it works with scripting off by |
| +// construction. That reasoning arrived here from renderSheetList along |
| +// with the form itself. |
| +func (a *App) renderImportPage(w http.ResponseWriter, r *http.Request, status int, refusal string) { |
| + a.render(w, r, "import", "Import a spreadsheet — Oficina Sheets", status, |
| + importPageView{Error: refusal}) |
| +} |
diff --git a/internal/sheets/handlers_import_refusals_test.go b/internal/sheets/handlers_import_refusals_test.go| index ac77d1b..1d448e6 100644 |
| --- a/internal/sheets/handlers_import_refusals_test.go |
| +++ b/internal/sheets/handlers_import_refusals_test.go |
| @@ -15,8 +15,12 @@ import ( |
| ) |
| // TestImportRefusesWhatItCannotRead covers every refusal a person can |
| -// hit before a job is ever started — each one answering on the list |
| -// page they posted from, with the reason beside the form. |
| +// hit before a job is ever started — each one answering on the import |
| +// page they posted from, with the reason above the form. |
| +// |
| +// That page used to be the sheet list, which was where the import form |
| +// lived. The form moved; the principle did not, and it is the reason a |
| +// refusal still needs no flash, no cookie and no redirect. |
| func TestImportRefusesWhatItCannotRead(t *testing.T) { |
| for _, tc := range []struct { |
| name string |
| @@ -175,11 +179,16 @@ func TestTitleFromFilename(t *testing.T) { |
| // product rather than the fallback, and drag-and-drop is the easiest |
| // place in this app to break it — a fetch-based uploader would look |
| // identical in a browser and leave nothing behind for anyone else. |
| +// Asked of /sheets/import, not of /sheets. The form moved to a page of |
| +// its own when the list page's two inline forms came off it; the |
| +// question this test asks is unchanged and is the reason the form |
| +// became a page rather than an item in the File menu, where a hidden |
| +// input behind a label would have needed script to submit at all. |
| func TestImportIsScriptless(t *testing.T) { |
| app, cookies := signIn(t) |
| - rec := do(t, app, as(httptest.NewRequest(http.MethodGet, "/sheets", nil), cookies)) |
| + rec := do(t, app, as(httptest.NewRequest(http.MethodGet, "/sheets/import", nil), cookies)) |
| if rec.Code != http.StatusOK { |
| - t.Fatalf("sheet list answered %d, want 200", rec.Code) |
| + t.Fatalf("import page answered %d, want 200", rec.Code) |
| } |
| body := rec.Body.String() |
diff --git a/internal/sheets/handlers_list_chrome_test.go b/internal/sheets/handlers_list_chrome_test.go| new file mode 100644 |
| index 0000000..5369f48 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_list_chrome_test.go |
| @@ -0,0 +1,216 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "mime/multipart" |
| + "net/http" |
| + "net/http/httptest" |
| + "os" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// The sheet list is a list of sheets. |
| +// |
| +// It used to open with two stacked forms — a title field with a Create |
| +// button, then a dashed drop zone with a file input and four lines of |
| +// limits — so the sheets themselves started below both. These tests pin |
| +// the removal, because the easiest way to undo it is for somebody to |
| +// add "just one small form" back to the top of the page. |
| + |
| +// signedInListBody renders the list for a fresh signed-in member. |
| +// preview_test.go's listBody takes an app and cookies because the |
| +// thumbnail tests need sheets in it; these tests want the chrome around |
| +// an empty one, so this wraps the sign-in rather than duplicating it. |
| +func signedInListBody(t *testing.T) string { |
| + t.Helper() |
| + app, cookies := signIn(t) |
| + return listBody(t, app, cookies) |
| +} |
| + |
| +// dropOverlay returns the sheet list's body up to the drop overlay, and |
| +// the overlay's own start tag. Everything before the overlay is what |
| +// the page actually renders; the overlay itself is hidden until a file |
| +// is dragged over the window. |
| +func dropOverlay(t *testing.T, body string) (inFlow, tag string) { |
| + t.Helper() |
| + start := strings.Index(body, "<form data-drop") |
| + if start < 0 { |
| + t.Fatal("the sheet list has no drop overlay — dropping a file on this page is the affordance it traded its drop box for") |
| + } |
| + end := strings.Index(body[start:], ">") |
| + if end < 0 { |
| + t.Fatal("the drop overlay's start tag never closes") |
| + } |
| + return body[:start], body[start : start+end+1] |
| +} |
| + |
| +func TestSheetListCarriesNoInlineForms(t *testing.T) { |
| + body := signedInListBody(t) |
| + |
| + // The title field is simply gone. A sheet is named by renaming it, |
| + // on the sheet itself, so there is nothing left for this to be. |
| + if strings.Contains(body, `name="title"`) { |
| + t.Error(`the sheet list is carrying name="title" again — naming a sheet is a rename, in the top bar of the sheet`) |
| + } |
| + |
| + // The file input is back, but only inside the overlay. Anything of |
| + // it appearing BEFORE the overlay's start tag is a box in the page |
| + // again, which is the thing this change removed. |
| + inFlow, _ := dropOverlay(t, body) |
| + for _, gone := range []string{ |
| + `type="file"`, |
| + `enctype="multipart/form-data"`, |
| + "data-drop", |
| + } { |
| + if strings.Contains(inFlow, gone) { |
| + t.Errorf("%s renders in the flow of the sheet list — the drop target is an overlay, not a box above the sheets", gone) |
| + } |
| + } |
| +} |
| + |
| +// The overlay is a real import form that happens to be invisible until |
| +// a drag brings it up. Both halves of that matter: hidden at rest, so |
| +// it costs the list no layout and no tab stop, and a genuine multipart |
| +// POST to /sheets/import, so a drop is the ordinary upload and there is |
| +// no second code path on the server. |
| +func TestTheDropOverlayIsHiddenAtRest(t *testing.T) { |
| + _, tag := dropOverlay(t, signedInListBody(t)) |
| + |
| + for _, want := range []string{ |
| + "hidden", |
| + `method="post"`, |
| + `action="/sheets/import"`, |
| + `enctype="multipart/form-data"`, |
| + } { |
| + if !strings.Contains(tag, want) { |
| + t.Errorf("the drop overlay's start tag is missing %s: %s", want, tag) |
| + } |
| + } |
| +} |
| + |
| +// The overlay is nothing without drop.js: it is `hidden`, and only that |
| +// script ever takes the attribute off. A page that renders the markup |
| +// and does not load the script has an import affordance that can never |
| +// appear. |
| +func TestTheSheetListLoadsDropJS(t *testing.T) { |
| + // By prefix, not by filename: assets are served fingerprinted, so |
| + // the tag reads /static/drop.<hash>.js and matching the literal |
| + // name passes only until the file next changes. |
| + body := signedInListBody(t) |
| + if !strings.Contains(body, `src="/static/drop.`) { |
| + t.Error("the sheet list renders the drop overlay but never loads drop.js, so nothing will ever reveal it") |
| + } |
| +} |
| + |
| +// The paint that keeps the overlay hidden. |
| +// |
| +// [rst-modal-overlay] sets display:flex, and an author declaration beats |
| +// the user agent's [hidden] { display: none } however weak its selector |
| +// looks. Drop the reinstating rule and the overlay covers the sheet list |
| +// permanently — worst with scripting off, where nothing would ever come |
| +// along to hide it again. The markup test above passes either way, so |
| +// this reads the stylesheet. |
| +func TestTheDropOverlayStaysHiddenInCSS(t *testing.T) { |
| + css, err := os.ReadFile(filepath.Join("static", "sheets.css")) |
| + if err != nil { |
| + t.Fatalf("read sheets.css: %v", err) |
| + } |
| + if !strings.Contains(string(css), "[data-drop-page][hidden]") { |
| + t.Error("sheets.css no longer reinstates display:none for the hidden drop overlay — it will cover the sheet list") |
| + } |
| + |
| + // The drop BOX's paint must exclude the overlay, which carries |
| + // data-drop too. [data-drop][data-drop-over] is 0-2-0 against |
| + // [rst-modal-overlay]'s 0-1-0, so without the exclusion the scrim is |
| + // painted accent-soft and the sheet list disappears behind a flat |
| + // blue screen mid-drag. A browser found that; nothing here could. |
| + if !strings.Contains(string(css), "[data-drop]:not([data-drop-page])") { |
| + t.Error("the drop box's paint no longer excludes the overlay — it will out-specify the modal scrim and cover the list") |
| + } |
| +} |
| + |
| +// Create is a real form posting to /sheets, in the top bar. A link |
| +// would be wrong twice over: it makes a sheet, so it must not be a GET, |
| +// and it has to work with scripting off. |
| +func TestCreateLivesInTheTopBar(t *testing.T) { |
| + body := signedInListBody(t) |
| + |
| + bar := body[:strings.Index(body, `<main`)] |
| + if !strings.Contains(bar, `action="/sheets"`) || !strings.Contains(bar, `type="submit"`) { |
| + t.Error("the top bar has no Create form") |
| + } |
| + if !strings.Contains(bar, `href="/sheets/import"`) { |
| + t.Error("the top bar has no Import link") |
| + } |
| + // Beside the account menu, not adrift somewhere else in the shell. |
| + if strings.Index(bar, `action="/sheets"`) > strings.Index(bar, `action="/signout"`) { |
| + t.Error("Create renders after the account menu, not beside it") |
| + } |
| +} |
| + |
| +// The empty state is the one screen with nothing to do but say what to |
| +// do next, and it used to say "make one above" — which pointed at the |
| +// form this change removed. |
| +func TestEmptyStateDoesNotPointAtAFormThatIsGone(t *testing.T) { |
| + body := signedInListBody(t) |
| + if !strings.Contains(body, "No sheets yet") { |
| + t.Fatal("no empty state on a list with no sheets") |
| + } |
| + if strings.Contains(body, "above and it opens") || strings.Contains(body, "Make one above") { |
| + t.Error("the empty state still points at the removed form") |
| + } |
| + if !strings.Contains(body, `href="/sheets/import"`) { |
| + t.Error("the empty state offers no way to import, which is the one route a person with no sheets wants") |
| + } |
| +} |
| + |
| +// /sheets/import must reach the import page, not be parsed as a sheet |
| +// called "import". chi prefers a static segment over {sheetID} whatever |
| +// the registration order, so this passes today — it is pinned because a |
| +// route that only works by resolution order is one refactor from a 404 |
| +// that looks like a missing sheet. |
| +func TestImportPathIsNotASheetID(t *testing.T) { |
| + app, cookies := signIn(t) |
| + rec := do(t, app, as(httptest.NewRequest(http.MethodGet, "/sheets/import", nil), cookies)) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("GET /sheets/import = %d, want 200", rec.Code) |
| + } |
| + if !strings.Contains(rec.Body.String(), `action="/sheets/import"`) { |
| + t.Error("/sheets/import did not render the import form") |
| + } |
| +} |
| + |
| +// A refused upload re-renders the import page, where the form is — |
| +// never the sheet list, which no longer has one to try again with. |
| +func TestRefusalRendersOnTheImportPage(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + // An empty multipart body: the shape a submit with nothing chosen |
| + // actually arrives in. A urlencoded post would refuse too, but with |
| + // "could not be read" rather than "choose a spreadsheet", which is |
| + // a different sentence about a different mistake. |
| + var buf bytes.Buffer |
| + w := multipart.NewWriter(&buf) |
| + if err := w.Close(); err != nil { |
| + t.Fatalf("close writer: %v", err) |
| + } |
| + req := httptest.NewRequest(http.MethodPost, "/sheets/import", &buf) |
| + req.Header.Set("Content-Type", w.FormDataContentType()) |
| + |
| + rec := do(t, app, as(req, cookies)) |
| + if rec.Code != http.StatusBadRequest { |
| + t.Fatalf("import with no file = %d, want 400", rec.Code) |
| + } |
| + body := rec.Body.String() |
| + if !strings.Contains(body, refusalNoFile) { |
| + t.Error("the refusal is not on the page") |
| + } |
| + if !strings.Contains(body, `action="/sheets/import"`) { |
| + t.Error("the refusal rendered somewhere without the form to retry from") |
| + } |
| + if strings.Contains(body, "All sheets</h1>") { |
| + t.Error("the refusal rendered on the sheet list") |
| + } |
| +} |
diff --git a/internal/sheets/handlers_rename.go b/internal/sheets/handlers_rename.go| new file mode 100644 |
| index 0000000..f0ddfb1 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_rename.go |
| @@ -0,0 +1,156 @@ |
| +package sheets |
| + |
| +import ( |
| + "errors" |
| + "net/http" |
| + "strconv" |
| + "strings" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +// Rename exists because Create stopped asking for a name. |
| +// |
| +// The sheet list used to carry a form with a title field, so a sheet |
| +// was named once, at the moment it was made, and never again — there |
| +// was no rename route and layout.html said so in a comment. That was |
| +// survivable only because the title field existed. Removing it in |
| +// favour of a one-click Create, which lands every new sheet as |
| +// "Untitled sheet", would have made the missing route the whole |
| +// product: a list of sheets all called the same thing, distinguished |
| +// only by their preview thumbnails, with no way to fix any of them. |
| +// |
| +// So this is not a feature that arrived beside the topbar Create |
| +// button. It is the other half of it. |
| + |
| +// errBlankTitle is returned rather than silently keeping the old title. |
| +// A person who cleared the field and submitted meant something by it, |
| +// and the honest answer is that a sheet needs a name — the same rule |
| +// createSheet applies. |
| +var errBlankTitle = errors.New("sheets: a sheet needs a title") |
| + |
| +// handleSheetRename renames a sheet from the title in the top bar. |
| +// |
| +// Editors, not just owners: renaming is an edit to the document, of a |
| +// piece with editing a cell, and mayWrite is what says who may make |
| +// one. A viewer or commenter gets the 404 that AGENTS.md requires for |
| +// a row that is not theirs to change — never a 403, which would |
| +// confirm the sheet exists to somebody who may not see it. |
| +func (a *App) handleSheetRename(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 |
| + } |
| + // Bounded, then parsed, and only THEN authorised. The order is the |
| + // point and it is deliberately not the order the rest of the app |
| + // uses. |
| + // |
| + // Every other write route here authorises first and reads the body |
| + // after (openTab before the decode, in handlers_format and |
| + // handlers_size). That leaves the body read — the one step whose |
| + // duration the client chooses — sitting between the permission |
| + // check and the write, so an editor can begin a rename, hold the |
| + // body open while their grant is revoked, then finish it and have |
| + // the write go through. Reading first collapses that: the delay a |
| + // client controls now happens BEFORE anything is checked, and the |
| + // gap between mayWrite and the UPDATE is straight-line code. |
| + // |
| + // It is not free — a request that turns out to be forbidden has |
| + // paid to parse its own body — but the cap on the line below is |
| + // what makes that cheap enough to be the obvious trade. The wider |
| + // question, whether every other write route should be reordered |
| + // the same way, is a real one and belongs in the branch discussion |
| + // rather than in this handler. |
| + // |
| + // 8 KiB matches handlers_format's cap on the same shape of form |
| + // post. Go's own ParseForm ceiling for a urlencoded body is 10 MiB, |
| + // so this is not the difference between bounded and unbounded — it |
| + // is three orders of magnitude off a limit no rename needs, for a |
| + // value truncated to maxTitleRunes moments later. |
| + r.Body = http.MaxBytesReader(w, r.Body, 8<<10) |
| + if err := r.ParseForm(); err != nil { |
| + // Refused rather than carried on with. FormValue would swallow |
| + // this error and fall through to the QUERY, which is how |
| + // ?title=x on an oversized body silently became the new name — |
| + // caught by review, not by the suite. |
| + http.Error(w, "Malformed request.", http.StatusBadRequest) |
| + return |
| + } |
| + // PostForm, never Form: the title comes from the body a person |
| + // submitted, never from the URL. r.FormValue merges the query in, |
| + // so with it a GET-shaped ?title= could rename a sheet — a |
| + // state-changing value arriving somewhere it can be linked to. |
| + // |
| + // Read field by field into a local, never bound onto a GORM model |
| + // (AGENTS.md / Mechanical rules): a form that could reach the model |
| + // could set CreatedBy, or DeletedAt, or an id. |
| + title := r.PostForm.Get("title") |
| + |
| + // Last thing before the write, with nothing client-paced in |
| + // between. See the note above on why this moved down here. |
| + if !a.mayWrite(s.ID, viewer) { |
| + a.notFound(w, r) |
| + return |
| + } |
| + |
| + if err := a.renameSheet(s.ID, title); err != nil { |
| + if errors.Is(err, errBlankTitle) { |
| + // Straight back to the sheet, which still shows the name it |
| + // had. Nothing was changed and there is nothing to explain |
| + // that the unchanged title does not already say. |
| + a.sheetRedirect(w, r, s.ID) |
| + return |
| + } |
| + a.fail(w, r, err) |
| + return |
| + } |
| + // 303 rather than a render: a POST that renders is a POST that |
| + // resubmits on reload. |
| + a.sheetRedirect(w, r, s.ID) |
| +} |
| + |
| +// sheetRedirect sends the browser back to a sheet. |
| +func (a *App) sheetRedirect(w http.ResponseWriter, r *http.Request, id uint64) { |
| + http.Redirect(w, r, "/sheets/"+strconv.FormatUint(id, 10), http.StatusSeeOther) |
| +} |
| + |
| +// renameSheet writes a new sealed title. |
| +// |
| +// The title is sealed under the same "sheet.title" purpose createSheet |
| +// uses. A rename that wrote plaintext would put a sheet's name in the |
| +// database file, and the one rule does not have an exception for the |
| +// second time a name is set. |
| +// |
| +// Update names its column explicitly rather than saving a struct, so |
| +// this statement cannot touch a field nobody asked it to. GORM bumps |
| +// UpdatedAt as part of it, which is what makes a renamed sheet rise to |
| +// the top of the list — correct, since renaming it is changing it. |
| +func (a *App) renameSheet(sheetID uint64, title string) error { |
| + if a.db == nil { |
| + return errNoDB |
| + } |
| + if a.sealer == nil { |
| + return errNoSealer |
| + } |
| + clean := strings.TrimSpace(title) |
| + if clean == "" { |
| + return errBlankTitle |
| + } |
| + // Truncated to the same limit createSheet applies, by runes rather |
| + // than bytes so a multi-byte title is cut where a character ends. |
| + if r := []rune(clean); len(r) > maxTitleRunes { |
| + clean = string(r[:maxTitleRunes]) |
| + } |
| + sealed, err := a.sealer.Seal("sheet.title", []byte(clean)) |
| + if err != nil { |
| + return err |
| + } |
| + return a.db.Model(&store.Sheet{}). |
| + Where("id = ?", sheetID). |
| + Update("title_sealed", sealed).Error |
| +} |
diff --git a/internal/sheets/handlers_rename_test.go b/internal/sheets/handlers_rename_test.go| new file mode 100644 |
| index 0000000..fa05307 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_rename_test.go |
| @@ -0,0 +1,263 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "net/http" |
| + "net/url" |
| + "strconv" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// Rename is the other half of a Create button that stopped asking for a |
| +// name. These tests are written against that pairing rather than |
| +// against the route on its own: the failure they exist to catch is a |
| +// list of sheets all called "Untitled sheet" with no way to fix any of |
| +// them. |
| + |
| +func renamePath(id uint64) string { |
| + return "/sheets/" + strconv.FormatUint(id, 10) + "/rename" |
| +} |
| + |
| +func TestRenameChangesTheTitle(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Untitled sheet") |
| + |
| + rec := do(t, app, as(postForm(renamePath(s.ID), |
| + url.Values{"title": {"Q3 budget"}}), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename = %d, want 303: %s", rec.Code, rec.Body.String()) |
| + } |
| + if loc := rec.Header().Get("Location"); loc != "/sheets/"+strconv.FormatUint(s.ID, 10) { |
| + t.Errorf("rename redirected to %q, want the sheet", loc) |
| + } |
| + |
| + got, err := app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "Q3 budget" { |
| + t.Errorf("title is %q after rename, want %q", got.Title, "Q3 budget") |
| + } |
| +} |
| + |
| +// The one rule reaches the second time a name is set, not just the |
| +// first. A rename that wrote the new title straight into the column |
| +// would put a sheet's name in the database file in plaintext, and |
| +// nothing about createSheet's sealing would have caught it. |
| +func TestRenamedTitleIsSealedAtRest(t *testing.T) { |
| + const secret = "Redundancies Q4" |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Untitled sheet") |
| + |
| + if rec := do(t, app, as(postForm(renamePath(s.ID), |
| + url.Values{"title": {secret}}), cookies)); rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename = %d", rec.Code) |
| + } |
| + |
| + var raw []byte |
| + if err := app.db.Raw("SELECT title_sealed FROM sheets WHERE id = ?", s.ID). |
| + 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 renamed title reached the database in plaintext") |
| + } |
| +} |
| + |
| +// A blank title keeps the name the sheet had. The alternative is a |
| +// sheet called nothing at all, which the list would render as a row you |
| +// cannot click by name. |
| +func TestRenameRefusesABlankTitle(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + for _, blank := range []string{"", " "} { |
| + rec := do(t, app, as(postForm(renamePath(s.ID), |
| + url.Values{"title": {blank}}), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename to %q = %d, want 303", blank, rec.Code) |
| + } |
| + got, err := app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "Budget" { |
| + t.Errorf("a blank rename changed the title to %q", got.Title) |
| + } |
| + } |
| +} |
| + |
| +// Truncation matches createSheet's, and by runes: cutting a multi-byte |
| +// title at a byte boundary stores half a character. |
| +func TestRenameTruncatesToTheSameCapAsCreate(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + long := strings.Repeat("é", maxTitleRunes+50) |
| + if rec := do(t, app, as(postForm(renamePath(s.ID), |
| + url.Values{"title": {long}}), cookies)); rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename = %d", rec.Code) |
| + } |
| + got, err := app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if n := len([]rune(got.Title)); n != maxTitleRunes { |
| + t.Errorf("title is %d runes after rename, want %d", n, maxTitleRunes) |
| + } |
| +} |
| + |
| +// The body is capped before it is parsed. |
| +// |
| +// An independent review pass found rename reading its form with nothing |
| +// bounding the request — the only write route in the app doing so. |
| +// FormValue buffers the WHOLE body to pull one field out of it, so |
| +// without the cap a megabyte of "title=" is a megabyte held in memory |
| +// on the way to a value that gets truncated to maxTitleRunes anyway. |
| +// |
| +// An oversized body is REFUSED, not quietly ignored. The parse error is |
| +// checked rather than swallowed, which is the difference between this |
| +// and the first attempt at the fix — see the query test below for what |
| +// swallowing it cost. |
| +func TestRenameCapsTheBodyItWillParse(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + // Well past the 8 KiB cap, as one enormous field value. |
| + huge := url.Values{"title": {strings.Repeat("x", 64<<10)}} |
| + rec := do(t, app, as(postForm(renamePath(s.ID), huge), cookies)) |
| + if rec.Code != http.StatusBadRequest { |
| + t.Fatalf("oversized rename = %d, want 400: %s", rec.Code, rec.Body.String()) |
| + } |
| + got, err := app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "Budget" { |
| + t.Errorf("an oversized body renamed the sheet to %q; the cap is not there", got.Title) |
| + } |
| +} |
| + |
| +// The title comes from the body, never from the URL. |
| +// |
| +// This is the defect the second review pass found in the first fix. Go's |
| +// r.FormValue merges the query string into the form, and it swallows a |
| +// parse failure rather than reporting it — so capping the body without |
| +// checking the error created a NEW way in: POST ?title=Wrong with an |
| +// oversized body, the cap rejects the body, FormValue falls through to |
| +// the query, and the sheet is renamed to whatever the URL said. A |
| +// state-changing value that can travel in a link is exactly what a POST |
| +// route must not accept. |
| +// |
| +// Both halves are asserted, because either alone would pass a broken |
| +// handler: the query is ignored when the body is fine, and an oversized |
| +// body is refused outright rather than falling back to it. |
| +func TestRenameTakesTheTitleFromTheBodyNotTheQuery(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + |
| + // A good body and a hostile query: the body must win. |
| + req := postForm(renamePath(s.ID)+"?title=FromTheQuery", |
| + url.Values{"title": {"FromTheBody"}}) |
| + if rec := do(t, app, as(req, cookies)); rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename = %d, want 303: %s", rec.Code, rec.Body.String()) |
| + } |
| + got, err := app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "FromTheBody" { |
| + t.Errorf("title is %q, want the body's value — the query reached the write", got.Title) |
| + } |
| + |
| + // A body carrying no title at all, and a query that does. This is |
| + // the case that actually distinguishes PostForm from FormValue, and |
| + // it exists because the first version of this test did not: with |
| + // ParseForm's error checked, a well-formed body wins under either |
| + // spelling, so swapping PostForm back for FormValue passed. Here |
| + // r.Form merges the query in and FormValue would answer |
| + // "FromTheQuery"; PostForm answers "", which is blank, which keeps |
| + // the name the sheet had. |
| + req = postForm(renamePath(s.ID)+"?title=FromTheQuery", |
| + url.Values{"unrelated": {"x"}}) |
| + if rec := do(t, app, as(req, cookies)); rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename with no body title = %d, want 303", rec.Code) |
| + } |
| + got, err = app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "FromTheBody" { |
| + t.Errorf("title is %q; the query renamed the sheet without a body field", got.Title) |
| + } |
| + |
| + // An oversized body with a query title: refused, and the query must |
| + // not stand in for the body it could not read. |
| + huge := url.Values{"title": {strings.Repeat("x", 64<<10)}} |
| + req = postForm(renamePath(s.ID)+"?title=FromTheQuery", huge) |
| + if rec := do(t, app, as(req, cookies)); rec.Code != http.StatusBadRequest { |
| + t.Fatalf("oversized rename = %d, want 400", rec.Code) |
| + } |
| + got, err = app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "FromTheBody" { |
| + t.Errorf("title is %q; a rejected body let the query rename the sheet", got.Title) |
| + } |
| +} |
| + |
| +// A viewer may not rename, and is told 404 rather than 403 — AGENTS.md |
| +// requires it, because a 403 confirms the sheet exists to somebody who |
| +// was never shown it. |
| +func TestRenameIsNotForViewers(t *testing.T) { |
| + app, cookies := signIn(t) |
| + s, _ := newSheet(t, app, cookies, "Budget") |
| + guestOn(t, app, s.ID, "ada@elsewhere.example", SheetViewer) |
| + guest := signInAs(t, app, "ada@elsewhere.example") |
| + |
| + rec := do(t, app, as(postForm(renamePath(s.ID), |
| + url.Values{"title": {"Mine now"}}), guest)) |
| + if rec.Code != http.StatusNotFound { |
| + t.Errorf("a viewer renaming = %d, want 404", rec.Code) |
| + } |
| + |
| + got, err := app.sheetSummary(s.ID, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "Budget" { |
| + t.Errorf("a viewer renamed the sheet to %q", got.Title) |
| + } |
| +} |
| + |
| +// TestCreateFromTheTopBarLandsAnUntitledSheet drives the button the |
| +// list page now carries: an empty POST, no title field anywhere in it. |
| +// |
| +// This is the whole reason rename exists, so it is pinned here rather |
| +// than left to the create handler's own tests: if an empty create ever |
| +// starts refusing instead of defaulting, the top bar's only action |
| +// breaks and the failure should name the button. |
| +func TestCreateFromTheTopBarLandsAnUntitledSheet(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + rec := do(t, app, as(postForm("/sheets", url.Values{}), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("create with no title = %d, want 303: %s", rec.Code, rec.Body.String()) |
| + } |
| + id, err := strconv.ParseUint(strings.TrimPrefix(rec.Header().Get("Location"), "/sheets/"), 10, 64) |
| + if err != nil { |
| + t.Fatalf("Location %q is not a sheet URL: %v", rec.Header().Get("Location"), err) |
| + } |
| + got, err := app.sheetSummary(id, signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("sheetSummary: %v", err) |
| + } |
| + if got.Title != "Untitled sheet" { |
| + t.Errorf("a titleless create landed as %q, want %q", got.Title, "Untitled sheet") |
| + } |
| +} |
diff --git a/internal/sheets/handlers_sheets.go b/internal/sheets/handlers_sheets.go| index 88f122e..2c79762 100644 |
| --- a/internal/sheets/handlers_sheets.go |
| +++ b/internal/sheets/handlers_sheets.go |
| @@ -19,15 +19,14 @@ type sheetRow struct { |
| } |
| // sheetListView is the sheet list's payload. |
| +// |
| +// It carried an ImportError until the import form moved off this page |
| +// to /sheets/import. The reasoning for keeping a refusal on the same |
| +// page as its form was right and still is — it needs no flash, no |
| +// cookie and no redirect — which is why the field moved with the form |
| +// rather than being dropped: it is importPageView.Error now. |
| type sheetListView struct { |
| Sheets []sheetRow |
| - // ImportError is a refused upload's explanation, empty on an |
| - // ordinary render. It lives on the list rather than in a flash |
| - // because the list is where the import form is: a refusal shown |
| - // beside the form that caused it needs no session storage, no |
| - // redirect and no cookie, and it works with scripting off by |
| - // construction. |
| - ImportError string |
| } |
| // sheetPageView is the grid page's payload. |
| @@ -79,18 +78,18 @@ type sheetPageView struct { |
| // handleSheetList renders the sheets the viewer may see. |
| func (a *App) handleSheetList(w http.ResponseWriter, r *http.Request) { |
| - a.renderSheetList(w, r, http.StatusOK, "") |
| + a.renderSheetList(w, r, http.StatusOK) |
| } |
| -// renderSheetList draws the list, optionally carrying a refused |
| -// upload's explanation and the status that goes with it. |
| +// renderSheetList draws the list. |
| // |
| -// A refused import re-renders this page rather than redirecting to it, |
| -// so the refusal survives without a flash: the person lands back on the |
| -// list they posted from, with the reason beside the form. The status is |
| -// the caller's because only the caller knows whether this is an |
| -// ordinary 200 or a refusal. |
| -func (a *App) renderSheetList(w http.ResponseWriter, r *http.Request, status int, importErr string) { |
| +// It used to take a refused upload's explanation and a status to go |
| +// with it, because the import form lived on this page and a refusal |
| +// belongs beside the form that caused it. The form moved to |
| +// /sheets/import and the refusal went with it — renderImportPage keeps |
| +// both the parameter and the reasoning. What is left here has one |
| +// caller and one status, so it no longer takes either. |
| +func (a *App) renderSheetList(w http.ResponseWriter, r *http.Request, status int) { |
| viewer, ok := a.actor(r) |
| if !ok { |
| http.Error(w, "Sign in first.", http.StatusForbidden) |
| @@ -105,8 +104,8 @@ func (a *App) renderSheetList(w http.ResponseWriter, r *http.Request, status int |
| for _, s := range list { |
| rows = append(rows, sheetRow{SheetSummary: s, Preview: a.previewFor(s)}) |
| } |
| - a.render(w, r, "sheets", "Your sheets — Oficina Sheets", status, |
| - sheetListView{Sheets: rows, ImportError: importErr}) |
| + a.render(w, r, "sheets", "All sheets — Oficina Sheets", status, |
| + sheetListView{Sheets: rows}) |
| } |
| // handleSheetCreate makes a sheet and sends you to it. |
diff --git a/internal/sheets/handlers_table_test.go b/internal/sheets/handlers_table_test.go| index c6d4dad..a9a28dc 100644 |
| --- a/internal/sheets/handlers_table_test.go |
| +++ b/internal/sheets/handlers_table_test.go |
| @@ -129,8 +129,19 @@ func TestEveryPageWorksWithoutScript(t *testing.T) { |
| // Sign-out is checked on /sheets rather than on "/": it is |
| // layout chrome, present on every signed-in page, and "/" |
| // stopped being one when it started redirecting onward. |
| - {"/sheets", []string{`action="/sheets"`, `name="title"`, `action="/signout"`}}, |
| - {fmt.Sprintf("/sheets/%d", s.ID), []string{fmt.Sprintf("/sheets/%d/table", s.ID)}}, |
| + // |
| + // The list no longer carries name="title": Create is a button |
| + // in the top bar that posts an empty form and lands the sheet |
| + // as "Untitled sheet". The naming moved to the rename field on |
| + // the sheet page, two rows down, which is where the title is |
| + // now asserted. |
| + {"/sheets", []string{`action="/sheets"`, `type="submit"`, `action="/signout"`}}, |
| + {"/sheets/import", []string{`action="/sheets/import"`, `type="file"`, `type="submit"`}}, |
| + {fmt.Sprintf("/sheets/%d", s.ID), []string{ |
| + fmt.Sprintf("/sheets/%d/table", s.ID), |
| + // Rename: the only place a sheet is named now, so it is |
| + // the one that has to survive with scripting off. |
| + fmt.Sprintf(`action="/sheets/%d/rename"`, s.ID), `name="title"`}}, |
| {fmt.Sprintf("/sheets/%d/table", s.ID), []string{"?page=1&edit=0,0", "Next rows"}}, |
| {fmt.Sprintf("/sheets/%d/table?page=1&edit=0,0", s.ID), |
| []string{`method="post"`, `name="raw"`, "Save"}}, |
diff --git a/internal/sheets/home.go b/internal/sheets/home.go| new file mode 100644 |
| index 0000000..b9928a2 |
| --- /dev/null |
| +++ b/internal/sheets/home.go |
| @@ -0,0 +1,168 @@ |
| +package sheets |
| + |
| +import ( |
| + "context" |
| + "database/sql" |
| + "errors" |
| + "net/http" |
| + "net/url" |
| + "strings" |
| + "time" |
| + |
| + "amadan.net/oficina/home/connect" |
| + "amadan.net/oficina/home/connect/linkstore" |
| + "github.com/carlosframework/rastrillo/csrf" |
| + "github.com/carlosframework/rastrillo/sessions" |
| +) |
| + |
| +var errHome = errors.New("sheets: Home connection unavailable") |
| + |
| +// ConfigureHome runs after local auth is attached and before serving requests. |
| +func (a *App) ConfigureHome(raw *connect.Credentials) error { |
| + settings, err := connect.DecodeCredentials(raw, a.origin) |
| + if err != nil { |
| + return errHome |
| + } |
| + if settings == nil { |
| + return nil |
| + } |
| + if a.db == nil || a.auth == nil { |
| + return errHome |
| + } |
| + d, err := a.db.DB() |
| + if err != nil { |
| + return errHome |
| + } |
| + sess, err := sessions.New(sessions.Config{DB: d, Origin: a.origin, SigninPath: "/signin"}) |
| + if err != nil || sess.CookieName() != a.auth.SessionCookie() { |
| + return errHome |
| + } |
| + read := func(r *http.Request) (*connect.Session, error) { |
| + s, ok := sess.From(r) |
| + if !ok { |
| + return nil, nil |
| + } |
| + return &connect.Session{Subject: s.Subject, Method: s.Method, AuthTime: s.AuthTime, At: s.At}, nil |
| + } |
| + render := func(w http.ResponseWriter, r *http.Request, state, label string, status int) { |
| + w.Header().Set("Cache-Control", "no-store") |
| + p := connect.Page(state, label) |
| + a.render(w, r, "home_link", p.Title, status, p) |
| + } |
| + bridge, err := connect.New(connect.Config{DB: d, Origin: a.origin, Issuer: settings.Issuer, ConsumerID: settings.ConsumerID, Namespace: "oficina/sheets", Keys: settings.Keys, DEK: settings.DEK, SessionCookie: sess.CookieName(), ReadSession: read, |
| + MintHomeSession: func(w http.ResponseWriter, r *http.Request, s connect.Session) error { |
| + return sess.SignIn(w, r, sessions.Session{Subject: s.Subject, Method: s.Method, AuthTime: s.AuthTime}) |
| + }, |
| + LookupPerson: a.homePerson, |
| + LookupLocalSession: func(ctx context.Context, s connect.Session) (connect.Person, error) { |
| + var id int64 |
| + address := strings.ToLower(strings.TrimSpace(s.Subject)) |
| + if err := d.QueryRowContext(ctx, "SELECT id FROM members WHERE address=?", address).Scan(&id); err != nil { |
| + return connect.Person{}, err |
| + } |
| + label, err := a.homePerson(ctx, d, id) |
| + return connect.Person{ID: id, Label: label}, err |
| + }, Render: render}) |
| + if err != nil { |
| + return errHome |
| + } |
| + routes, err := connect.NewRoutes(bridge, connect.RoutesConfig{LocalLandingPath: "/sheets", Namespace: "oficina/sheets", Origin: a.origin, Issuer: settings.Issuer, SessionCookie: sess.CookieName(), DEK: settings.DEK, ReadSession: read, SignOut: sess.SignOut, LocalSignin: a.handleSignin, Protect: csrf.Protect(a.origin), Render: render, FormPolicy: connect.FormPolicy(settings.Issuer)}) |
| + if err != nil { |
| + return errHome |
| + } |
| + a.home, a.homeRoutes, a.homeSessions = bridge, routes, sess |
| + return nil |
| +} |
| + |
| +// The same current admission rule as Admits, using the link transaction's q. |
| +func (a *App) homePerson(ctx context.Context, q linkstore.Queryer, id int64) (string, error) { |
| + var address string |
| + if id <= 0 { |
| + return "", sql.ErrNoRows |
| + } |
| + if err := q.QueryRowContext(ctx, "SELECT address FROM members WHERE id=?", id).Scan(&address); err != nil { |
| + return "", err |
| + } |
| + if address == "" { |
| + return "", sql.ErrNoRows |
| + } |
| + if a.admit != nil && a.admit(address) { |
| + return address, nil |
| + } |
| + var count int |
| + err := q.QueryRowContext(ctx, `SELECT COUNT(*) FROM grants JOIN sheets ON sheets.id=grants.sheet_id WHERE grants.member_id=? AND sheets.deleted_at IS NULL AND sheets.filling_since IS NULL`, id).Scan(&count) |
| + if err != nil { |
| + return "", err |
| + } |
| + if count == 0 { |
| + return "", sql.ErrNoRows |
| + } |
| + return address, nil |
| +} |
| + |
| +func (a *App) homeRequire(next http.Handler) http.Handler { |
| + return a.homeSessions.Middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + session, ok := a.homeSessions.From(r) |
| + if ok && session.Method == connect.HomeMethod { |
| + if _, err := a.home.ResolveSession(r); errors.Is(err, linkstore.ErrStorage) { |
| + w.Header().Set("Cache-Control", "no-store") |
| + http.Error(w, "Sign-in unavailable.", http.StatusServiceUnavailable) |
| + return |
| + } |
| + } |
| + address, admitted := a.identity(r) |
| + if !ok || !admitted || !a.Admits(address) { |
| + if ok { |
| + a.homeSessions.SignOut(w, r) |
| + } |
| + w.Header().Set("Cache-Control", "no-store") |
| + if r.Method == http.MethodGet || r.Method == http.MethodHead { |
| + http.Redirect(w, r, "/signin?return_to="+url.QueryEscape(r.URL.RequestURI()), http.StatusSeeOther) |
| + } else { |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + } |
| + return |
| + } |
| + // Jobs use an already resolved local address so local and Home sessions have |
| + // the same owner. The bridge always rereads the actual cookie-backed session. |
| + session.Subject = address |
| + next.ServeHTTP(w, sessions.WithSession(r, session)) |
| + })) |
| +} |
| + |
| +func (a *App) homeLocalComplete(w http.ResponseWriter, r *http.Request) { |
| + s, ok := a.homeSessions.From(r) |
| + if !ok { |
| + a.homeRoutes.LocalComplete(w, r) |
| + return |
| + } |
| + _, fresh := connect.FreshLocal(connect.Session{Subject: s.Subject, Method: s.Method, AuthTime: s.AuthTime, At: s.At}, time.Now()) |
| + if !fresh || !a.Admits(s.Subject) { |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + return |
| + } |
| + // Only an independently verified local sign-in may create its local member. |
| + // Home assertions and lookup hooks never pass through this completion handler. |
| + if _, err := a.memberFor(s.Subject); err != nil { |
| + http.Error(w, "Sign-in unavailable.", http.StatusServiceUnavailable) |
| + return |
| + } |
| + a.homeRoutes.LocalComplete(w, r) |
| +} |
| +func (a *App) homeLocalBegin(w http.ResponseWriter, r *http.Request) { |
| + connect.OAuthContinuation(a.origin, a.handleBegin, func(w http.ResponseWriter, r *http.Request, destination string) { |
| + p := connect.OAuthPage(destination) |
| + a.render(w, r, "home_oauth", p.Title, http.StatusOK, p) |
| + })(w, r) |
| +} |
| + |
| +func (a *App) rejectDisabledHome(next http.Handler) http.Handler { |
| + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + if s, ok := sessions.Current(r); ok && s.Method == connect.HomeMethod { |
| + w.Header().Set("Cache-Control", "no-store") |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + return |
| + } |
| + next.ServeHTTP(w, r) |
| + }) |
| +} |
diff --git a/internal/sheets/home_navigation_test.go b/internal/sheets/home_navigation_test.go| new file mode 100644 |
| index 0000000..6998448 |
| --- /dev/null |
| +++ b/internal/sheets/home_navigation_test.go |
| @@ -0,0 +1,150 @@ |
| +package sheets |
| + |
| +import ( |
| + "amadan.net/oficina/home/connect" |
| + "context" |
| + "github.com/carlosframework/rastrillo" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +func TestHomeLocalPluginRoundTripAndSignout(t *testing.T) { |
| + f := newHomeFixture(t) |
| + if w := f.request(t, "GET", "/account/home", nil, false); w.Code != 303 { |
| + t.Fatal(w.Code) |
| + } |
| + if w := f.request(t, "POST", "/signin", url.Values{"address": {signedInAddress}, "force": {"1"}}, false); w.Code != 303 { |
| + t.Fatal(w.Code) |
| + } |
| + u, err := url.Parse(f.app.testMailer(t).lastLink(t)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + verify := f.request(t, "GET", u.RequestURI(), nil, false) |
| + if verify.Code != 303 || verify.Header().Get("Location") != connect.LocalCompletePath { |
| + t.Fatal("local callback", verify.Code, verify.Header().Get("Location")) |
| + } |
| + completed := f.request(t, "GET", connect.LocalCompletePath, nil, false) |
| + if completed.Code != 303 || completed.Header().Get("Location") != "/account/home" { |
| + t.Fatal("local completion", completed.Code, completed.Header().Get("Location")) |
| + } |
| + if page := f.request(t, "GET", "/account/home", nil, false); page.Code != 200 { |
| + t.Fatal("local account unavailable", page.Code) |
| + } |
| + var count int64 |
| + f.app.db.Model(&Member{}).Count(&count) |
| + if count != 1 { |
| + t.Fatal("local member missing", count) |
| + } |
| + f.app.db.Table("sheets").Count(&count) |
| + if count != 0 { |
| + t.Fatal("sign-in seeded content", count) |
| + } |
| + f.link(t, "round-trip") |
| + out := f.request(t, "POST", "/signout", nil, false) |
| + if out.Code != 303 || out.Header().Get("Location") != "/signed-out" { |
| + t.Fatal("signout", out.Code) |
| + } |
| + if page := f.request(t, "GET", "/signed-out", nil, false); page.Code != 200 || page.Header().Get("Location") != "" { |
| + t.Fatal("signout restarted Home") |
| + } |
| +} |
| +func TestHomeDeletedMemberIsNeverReassigned(t *testing.T) { |
| + f := newHomeFixture(t) |
| + old := f.local(t, signedInAddress) |
| + f.link(t, "deleted-person") |
| + if err := f.app.db.Exec("DELETE FROM members WHERE id=?", old.ID).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + replacement, err := f.app.memberFor(signedInAddress) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if replacement.ID <= old.ID { |
| + t.Fatal("member IDs reused") |
| + } |
| + if _, err := f.app.home.Resolve(context.Background(), "deleted-person"); err == nil { |
| + t.Fatal("deleted account resolved to replacement") |
| + } |
| +} |
| +func TestHomeFrameworkPolicyAndDuplicateCookies(t *testing.T) { |
| + f := newHomeFixture(t) |
| + f.local(t, signedInAddress) |
| + mux := http.NewServeMux() |
| + mux.Handle("/", f.app.Router()) |
| + handler, close, err := rastrillo.Handler(rastrillo.Options{Mux: mux}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer close() |
| + r := httptest.NewRequest("GET", testOrigin+"/signin/local", nil) |
| + w := httptest.NewRecorder() |
| + handler.ServeHTTP(w, r) |
| + got := w.Header().Get("Content-Security-Policy") |
| + if !strings.Contains(got, "form-action 'self' "+homeTestIssuer) || !strings.Contains(got, "frame-ancestors 'none'") { |
| + t.Fatal("Home policy lost", got) |
| + } |
| + r = httptest.NewRequest("GET", testOrigin+"/account/home", nil) |
| + for _, c := range f.jar { |
| + r.AddCookie(c) |
| + r.AddCookie(c) |
| + } |
| + w = httptest.NewRecorder() |
| + handler.ServeHTTP(w, r) |
| + if w.Code != 400 { |
| + t.Fatal("duplicate session accepted", w.Code) |
| + } |
| +} |
| + |
| +func TestHomeDisabledRefusesRetainedOpaqueSession(t *testing.T) { |
| + f := newHomeFixture(t) |
| + f.local(t, signedInAddress) |
| + f.link(t, "retained-home") |
| + f.jar = map[string]*http.Cookie{} |
| + start := f.request(t, "GET", "/auth/start", nil, false) |
| + if w := f.exchange(t, start, "retained-home"); w.Code != 303 { |
| + t.Fatal(w.Code) |
| + } |
| + f.app.home = nil |
| + f.app.homeRoutes = nil |
| + f.app.homeSessions = nil |
| + var before, after int64 |
| + f.app.db.Model(&Member{}).Count(&before) |
| + w := f.request(t, "GET", "/sheets", nil, false) |
| + if w.Code != 403 { |
| + t.Fatal("retained Home session accepted", w.Code) |
| + } |
| + f.app.db.Model(&Member{}).Count(&after) |
| + if after != before { |
| + t.Fatal("opaque member created") |
| + } |
| +} |
| +func TestHomeStorageFailureDoesNotRevokeSession(t *testing.T) { |
| + f := newHomeFixture(t) |
| + f.local(t, signedInAddress) |
| + f.link(t, "storage-home") |
| + f.jar = map[string]*http.Cookie{} |
| + if w := f.exchange(t, f.request(t, "GET", "/auth/start", nil, false), "storage-home"); w.Code != 303 { |
| + t.Fatal(w.Code) |
| + } |
| + cookie := f.jar[f.app.homeSessions.CookieName()] |
| + if err := f.app.db.Exec("ALTER TABLE home_identity_links RENAME TO unavailable_links").Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + w := f.request(t, "GET", "/sheets", nil, false) |
| + if w.Code != 503 { |
| + t.Fatal("storage response", w.Code) |
| + } |
| + if f.jar[cookie.Name] == nil || f.jar[cookie.Name].Value != cookie.Value { |
| + t.Fatal("storage failure revoked session") |
| + } |
| + if err := f.app.db.Exec("ALTER TABLE unavailable_links RENAME TO home_identity_links").Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if w := f.request(t, "GET", "/sheets", nil, false); w.Code != 200 { |
| + t.Fatal("session did not recover", w.Code) |
| + } |
| +} |
diff --git a/internal/sheets/home_routes.go b/internal/sheets/home_routes.go| new file mode 100644 |
| index 0000000..880650f |
| --- /dev/null |
| +++ b/internal/sheets/home_routes.go |
| @@ -0,0 +1,42 @@ |
| +package sheets |
| + |
| +import ( |
| + "amadan.net/oficina/home/connect" |
| + "github.com/go-chi/chi/v5" |
| +) |
| + |
| +func (a *App) mountAuth(r chi.Router) { |
| + if a.homeRoutes != nil { |
| + r.Get("/signin", a.homeRoutes.Signin) |
| + r.Get("/signin/local", a.homeRoutes.LocalGuard(a.handleSignin)) |
| + r.Get("/auth/start", a.home.Start) |
| + r.Post("/auth/assertion", a.home.Callback) |
| + r.Get("/auth/finish", a.home.Finish) |
| + r.Get(connect.LocalCompletePath, a.homeRoutes.LocalGuard(a.homeLocalComplete)) |
| + r.Get("/account/home", a.homeRoutes.Account) |
| + r.Post("/account/home", a.homeRoutes.LinkStart) |
| + r.Get("/account/home/confirm", a.home.ConfirmPage) |
| + r.Post("/account/home/confirm", a.homeRoutes.ConfirmPost) |
| + r.Get("/signed-out", a.homeRoutes.SignedOut) |
| + } else { |
| + r.Get("/signin", a.handleSignin) |
| + } |
| + if a.auth != nil { |
| + // The four handlers rastrillo/auth expects an app to mount, at |
| + // the paths its own docs use. Verify reads ?token=; the path is |
| + // baked into every emailed link, so moving it later invalidates |
| + // links already in people's inboxes. |
| + if a.homeRoutes != nil { |
| + r.Post("/signin", a.homeRoutes.LocalGuard(a.homeLocalBegin)) |
| + r.Get("/auth/callback", a.homeRoutes.LocalGuard(a.auth.Callback)) |
| + r.Get("/auth/verify", a.homeRoutes.LocalGuard(a.auth.Verify)) |
| + r.Post("/auth/verify", a.homeRoutes.LocalGuard(a.auth.Verify)) |
| + r.Post("/signout", a.homeRoutes.Signout) |
| + } else { |
| + r.Post("/signin", a.handleBegin) |
| + r.Get("/auth/callback", a.auth.Callback) |
| + r.Get("/auth/verify", a.auth.Verify) |
| + r.Post("/signout", a.auth.Signout) |
| + } |
| + } |
| +} |
diff --git a/internal/sheets/home_test.go b/internal/sheets/home_test.go| new file mode 100644 |
| index 0000000..7e4b6e5 |
| --- /dev/null |
| +++ b/internal/sheets/home_test.go |
| @@ -0,0 +1,211 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "encoding/base64" |
| + "encoding/json" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "amadan.net/oficina/home/connect" |
| + "amadan.net/rastrillo/rastrillo/assertion" |
| + assertioncrypto "amadan.net/rastrillo/rastrillo/crypto" |
| + "github.com/carlosframework/rastrillo/sessions" |
| +) |
| + |
| +const homeTestIssuer = "https://id.example.com" |
| + |
| +type homeFixture struct { |
| + app *App |
| + key *assertioncrypto.Keypair |
| + jar map[string]*http.Cookie |
| +} |
| + |
| +func newHomeFixture(t *testing.T) *homeFixture { |
| + t.Helper() |
| + app, _ := appWithOrigin(t, testOrigin) |
| + key, err := assertioncrypto.Generate() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + keys, _ := json.Marshal(map[string]string{"key": base64.StdEncoding.EncodeToString(key.SignPub())}) |
| + cfg := connect.Credentials{Issuer: homeTestIssuer, ConsumerID: "sheets-preview", KeysBase64: base64.StdEncoding.EncodeToString(keys), DEKBase64: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{81}, 32))} |
| + d, err := app.db.DB() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + localAuth, err := NewAuth(d, AuthEnv{Origin: testOrigin, InstanceKey: "test-instance-key", Mailer: app.testMailer(t), Home: &cfg}, quietLogger(), app.Admits) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + app.UseAuth(localAuth) |
| + if err := app.ConfigureHome(&cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + return &homeFixture{app: app, key: key, jar: map[string]*http.Cookie{}} |
| +} |
| +func (f *homeFixture) request(t *testing.T, method, path string, form url.Values, issuer bool) *httptest.ResponseRecorder { |
| + t.Helper() |
| + body := "" |
| + if form != nil { |
| + body = form.Encode() |
| + } |
| + r := httptest.NewRequest(method, testOrigin+path, strings.NewReader(body)) |
| + if method == "POST" { |
| + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + origin := testOrigin |
| + if issuer { |
| + origin = homeTestIssuer |
| + } |
| + r.Header.Set("Origin", origin) |
| + } |
| + if !issuer { |
| + for _, cookie := range f.jar { |
| + r.AddCookie(cookie) |
| + } |
| + } |
| + w := httptest.NewRecorder() |
| + f.app.Router().ServeHTTP(w, r) |
| + for _, cookie := range w.Result().Cookies() { |
| + if cookie.MaxAge < 0 { |
| + delete(f.jar, cookie.Name) |
| + } else { |
| + f.jar[cookie.Name] = cookie |
| + } |
| + } |
| + return w |
| +} |
| +func (f *homeFixture) local(t *testing.T, address string) Member { |
| + t.Helper() |
| + m, err := f.app.memberFor(address) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + token, err := f.app.homeSessions.Mint(sessions.Session{Subject: address, Method: "magiclink"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + f.jar[f.app.homeSessions.CookieName()] = &http.Cookie{Name: f.app.homeSessions.CookieName(), Value: token} |
| + return m |
| +} |
| +func (f *homeFixture) exchange(t *testing.T, start *httptest.ResponseRecorder, subject string) *httptest.ResponseRecorder { |
| + t.Helper() |
| + if start.Code != 303 { |
| + t.Fatal("start", start.Code, start.Body.String()) |
| + } |
| + target, err := url.Parse(start.Header().Get("Location")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if target.Host != "id.example.com" || target.Query().Get("consumer") != "sheets-preview" { |
| + t.Fatal("wrong Home destination", target.Host) |
| + } |
| + now := time.Now().Unix() |
| + id, _ := assertion.RandomID() |
| + token, err := assertion.Sign(f.key, assertion.Claims{Version: 1, Issuer: homeTestIssuer, Audience: testOrigin, Subject: subject, KeyID: "key", ID: id, Nonce: target.Query().Get("nonce"), IssuedAt: now, ExpiresAt: now + 120, AuthTime: now - 10}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + callback := f.request(t, "POST", "/auth/assertion", url.Values{"assertion": {token}}, true) |
| + if callback.Code != 303 { |
| + t.Fatal("callback", callback.Code, callback.Body.String()) |
| + } |
| + return f.request(t, "GET", callback.Header().Get("Location"), nil, false) |
| +} |
| +func (f *homeFixture) link(t *testing.T, subject string) { |
| + t.Helper() |
| + start := f.request(t, "POST", "/account/home", nil, false) |
| + if start.Code != 303 { |
| + t.Fatal("link", start.Code, start.Body.String()) |
| + } |
| + authStart := f.request(t, "GET", start.Header().Get("Location"), nil, false) |
| + target, _ := url.Parse(authStart.Header().Get("Location")) |
| + if target.Query().Get("max_age") != "300" { |
| + t.Fatal("link missing fresh Home proof") |
| + } |
| + finish := f.exchange(t, authStart, subject) |
| + if finish.Code != 303 || finish.Header().Get("Location") != "/account/home/confirm" { |
| + t.Fatal("finish link", finish.Code, finish.Body.String()) |
| + } |
| + page := f.request(t, "GET", "/account/home/confirm", nil, false) |
| + if page.Code != 200 || !strings.Contains(page.Body.String(), "Confirm account connection") { |
| + t.Fatal("confirm page", page.Code, page.Body.String()) |
| + } |
| + confirmed := f.request(t, "POST", "/account/home/confirm", nil, false) |
| + if confirmed.Code != 303 { |
| + t.Fatal("confirm", confirmed.Code, confirmed.Body.String()) |
| + } |
| +} |
| +func TestHomeConnectionPreservesSheetsAndCurrentAdmission(t *testing.T) { |
| + f := newHomeFixture(t) |
| + member := f.local(t, signedInAddress) |
| + sheet, err := f.app.createSheet("Existing linked workbook", member) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + f.link(t, "opaque-home-subject") |
| + f.jar = map[string]*http.Cookie{} |
| + signin := f.request(t, "GET", "/signin?return_to=%2Fsheets", nil, false) |
| + start := f.request(t, "GET", signin.Header().Get("Location"), nil, false) |
| + finish := f.exchange(t, start, "opaque-home-subject") |
| + if finish.Code != 303 || finish.Header().Get("Location") != "/sheets" { |
| + t.Fatal("ordinary sign-in", finish.Code, finish.Body.String()) |
| + } |
| + page := f.request(t, "GET", "/sheets", nil, false) |
| + if page.Code != 200 || !strings.Contains(page.Body.String(), "Existing linked workbook") || strings.Contains(page.Body.String(), "opaque-home-subject") { |
| + t.Fatal("existing sheet missing or subject exposed", page.Code) |
| + } |
| + var members int64 |
| + if err := f.app.db.Model(&Member{}).Count(&members).Error; err != nil || members != 1 { |
| + t.Fatal("Home created members", members, err) |
| + } |
| + r := httptest.NewRequest("GET", testOrigin+"/sheets", nil) |
| + for _, c := range f.jar { |
| + r.AddCookie(c) |
| + } |
| + actor, ok := f.app.actor(r) |
| + if !ok || actor.ID != member.ID { |
| + t.Fatal("owner changed") |
| + } |
| + if !f.app.mayWrite(sheet.ID, actor) { |
| + t.Fatal("existing grant changed") |
| + } |
| + f.app.admit = func(string) bool { return false } |
| + if err := f.app.db.Where("member_id = ?", member.ID).Delete(&Grant{}).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + denied := f.request(t, "GET", "/sheets", nil, false) |
| + if denied.Code != 303 || strings.Contains(denied.Body.String(), "Existing linked workbook") { |
| + t.Fatal("revoked member admitted", denied.Code) |
| + } |
| + if _, ok := f.jar[f.app.homeSessions.CookieName()]; ok { |
| + t.Fatal("revoked session retained") |
| + } |
| +} |
| +func TestHomeUnknownAndMalformedFlowsNeverCreateMembers(t *testing.T) { |
| + f := newHomeFixture(t) |
| + start := f.request(t, "GET", "/auth/start?return_to=%2Fsheets", nil, false) |
| + finish := f.exchange(t, start, "unknown-subject") |
| + if finish.Code != 403 || !strings.Contains(finish.Body.String(), "/signin/local") { |
| + t.Fatal("unknown identity accepted", finish.Code) |
| + } |
| + var members int64 |
| + f.app.db.Model(&Member{}).Count(&members) |
| + if members != 0 { |
| + t.Fatal("Home created local identity", members) |
| + } |
| + if _, ok := f.jar[f.app.homeSessions.CookieName()]; ok { |
| + t.Fatal("unknown identity minted session") |
| + } |
| + req := httptest.NewRequest("POST", testOrigin+"/account/home", nil) |
| + req.Header.Set("Origin", "https://evil.example") |
| + rec := httptest.NewRecorder() |
| + f.app.Router().ServeHTTP(rec, req) |
| + if rec.Code != 403 { |
| + t.Fatal("cross-origin link allowed", rec.Code) |
| + } |
| +} |
diff --git a/internal/sheets/members.go b/internal/sheets/members.go| index 13c2ac1..8ca6376 100644 |
| --- a/internal/sheets/members.go |
| +++ b/internal/sheets/members.go |
| @@ -82,6 +82,16 @@ func (a *App) memberFor(address string) (Member, error) { |
| // through requireSession, so a false here in production means the |
| // session went away between the middleware and the handler. |
| func (a *App) actor(r *http.Request) (Member, bool) { |
| + if a.home != nil { |
| + s, ok := a.homeSessions.From(r) |
| + if ok && s.Method == "home-assertion" { |
| + p, err := a.home.ResolveSession(r) |
| + if err != nil { |
| + return Member{}, false |
| + } |
| + return a.memberByID(uint64(p.ID)) |
| + } |
| + } |
| address, ok := a.identity(r) |
| if !ok { |
| return Member{}, false |
diff --git a/internal/sheets/migrations.go b/internal/sheets/migrations.go| index 69faa16..da34efc 100644 |
| --- a/internal/sheets/migrations.go |
| +++ b/internal/sheets/migrations.go |
| @@ -1,6 +1,8 @@ |
| package sheets |
| import ( |
| + "amadan.net/oficina/home/connect/linkstore" |
| + "amadan.net/oficina/home/handoff" |
| "embed" |
| "github.com/carlosframework/rastrillo/auth" |
| @@ -34,9 +36,9 @@ var Schema = migrate.MustFromFS(migrationsFS, "sheets") |
| // |
| // The order is not a preference. auth's backfill migration reads the |
| // sessions table, so sessions.Schema must apply first; migrate.Merge's |
| -// argument order IS apply order. Sheets' own Schema comes last because |
| -// it depends on neither. |
| -var BootSchema = migrate.Merge(sessions.Schema, auth.Schema, Schema) |
| +// argument order IS apply order. Sheets and optional Home protocol schemas |
| +// follow the local authentication schemas. |
| +var BootSchema = migrate.Merge(sessions.Schema, auth.Schema, Schema, migrate.MustFromFS(handoff.Migrations, "oficina_handoff"), migrate.MustFromFS(linkstore.Migrations, linkstore.SchemaNamespace)) |
| // Models lists every GORM model `rastrillo migration generate`/`check` |
| // diff Schema against — Sheet, Tab, Row, Col, Cell, Format and Member |
diff --git a/internal/sheets/migrations/0010_alter_tabs.sql b/internal/sheets/migrations/0010_alter_tabs.sql| new file mode 100644 |
| index 0000000..661e081 |
| --- /dev/null |
| +++ b/internal/sheets/migrations/0010_alter_tabs.sql |
| @@ -0,0 +1,2 @@ |
| +ALTER TABLE `tabs` ADD `content_version` integer NOT NULL DEFAULT 0; |
| + |
diff --git a/internal/sheets/migrations/schema.sql b/internal/sheets/migrations/schema.sql| index a15b30a..6d8675c 100644 |
| --- a/internal/sheets/migrations/schema.sql |
| +++ b/internal/sheets/migrations/schema.sql |
| @@ -23,7 +23,7 @@ CREATE TABLE `sheets` (`id` integer PRIMARY KEY AUTOINCREMENT,`title_sealed` blo |
| 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 TABLE `tabs` (`id` integer PRIMARY KEY AUTOINCREMENT,`sheet_id` integer,`name_sealed` blob,`position` integer,`frozen_rows` integer,`frozen_cols` integer, `content_version` integer NOT NULL DEFAULT 0); |
| CREATE UNIQUE INDEX `idx_col_tab_pos` ON `cols`(`tab_id`,`position`); |
diff --git a/internal/sheets/page.go b/internal/sheets/page.go| index 3687ca8..f546fc9 100644 |
| --- a/internal/sheets/page.go |
| +++ b/internal/sheets/page.go |
| @@ -1,6 +1,10 @@ |
| package sheets |
| -import "net/http" |
| +import ( |
| + "net/http" |
| + |
| + "amadan.net/oficina/home/connect" |
| +) |
| // The root page is the anonymous entry page, and nothing else. |
| // |
| @@ -35,11 +39,21 @@ func (a *App) handleIndex(w http.ResponseWriter, r *http.Request) { |
| // identity returns the signed-in address, if any. A nil auth (tests) |
| // simply has no session. |
| func (a *App) identity(r *http.Request) (string, bool) { |
| + if a.home != nil { |
| + s, ok := a.homeSessions.From(r) |
| + if !ok { |
| + return "", false |
| + } |
| + if s.Method == connect.HomeMethod { |
| + p, err := a.home.ResolveSession(r) |
| + return p.Label, err == nil |
| + } |
| + } |
| if a.auth == nil { |
| return "", false |
| } |
| id, ok := a.auth.SessionFrom(r) |
| - if !ok { |
| + if !ok || string(id.Method) == connect.HomeMethod { |
| return "", false |
| } |
| return id.Address, true |
diff --git a/internal/sheets/render.go b/internal/sheets/render.go| index eaf9f10..1afa1bf 100644 |
| --- a/internal/sheets/render.go |
| +++ b/internal/sheets/render.go |
| @@ -1,6 +1,7 @@ |
| package sheets |
| import ( |
| + "amadan.net/oficina/home/connect" |
| "bytes" |
| "embed" |
| "html/template" |
| @@ -8,6 +9,9 @@ import ( |
| "net/http" |
| "path" |
| "strings" |
| + "testing/fstest" |
| + |
| + "amadan.net/oficina/estilo.git/theme" |
| "github.com/carlosframework/rastrillo" |
| "github.com/carlosframework/rastrillo/ui" |
| @@ -20,7 +24,7 @@ import ( |
| var templatesFS embed.FS |
| // staticFS holds everything served under /static: rastrillo's vendored |
| -// tokens.css, theme.css and rastrillo.js (copied by the framework's own |
| +// tokens.css and rastrillo.js (copied by the framework's own |
| // scaffold and verified by `rastrillo doctor`), our own CSS and JS, and |
| // the vendored grid. |
| // |
| @@ -34,7 +38,31 @@ var staticFS embed.FS |
| // assets fingerprints static files so they can be cached forever and |
| // still update on an ordinary reload: the URL carries the content hash, |
| // so the browser has never cached the new one stale. |
| -var assets = rastrillo.NewAssets(staticFS) |
| +var servedStaticFS = sharedStaticFiles() |
| +var assets = rastrillo.NewAssets(servedStaticFS) |
| + |
| +func sharedStaticFiles() fs.FS { |
| + files := fstest.MapFS{} |
| + err := fs.WalkDir(staticFS, "static", func(name string, entry fs.DirEntry, err error) error { |
| + if err != nil { |
| + return err |
| + } |
| + if entry.IsDir() { |
| + return nil |
| + } |
| + data, err := staticFS.ReadFile(name) |
| + if err != nil { |
| + return err |
| + } |
| + files[name] = &fstest.MapFile{Data: data} |
| + return nil |
| + }) |
| + if err != nil { |
| + panic(err) |
| + } |
| + files["static/theme.css"] = &fstest.MapFile{Data: theme.CSS()} |
| + return files |
| +} |
| // pages is one parsed template tree per page. |
| // |
| @@ -73,6 +101,7 @@ func parsePages() map[string]*template.Template { |
| } |
| t := template.Must(template.New(name).Funcs(funcs). |
| ParseFS(ui.Templates(), "*.html")) |
| + t = template.Must(t.ParseFS(connect.Templates(), "templates/*.html")) |
| t = template.Must(t.ParseFS(templatesFS, "templates/"+layoutFile, f)) |
| out[name] = t |
| } |
| @@ -87,7 +116,8 @@ func parsePages() map[string]*template.Template { |
| // every coloured cell. A page carrying it in its first byte cannot |
| // flash; one that asks for it can. |
| type PageData struct { |
| - Title string |
| + HomeLinkLabel string |
| + Title string |
| // Mode is the viewer's light/dark choice — light, dark or auto. |
| // ThemeAttr is what <html data-theme> gets, and is EMPTY for auto: |
| // rastrillo's toggle is two rules keyed on that attribute, and no |
| @@ -107,14 +137,19 @@ type PageData struct { |
| func (a *App) pageData(r *http.Request, title string, data any) PageData { |
| address, signedIn := a.identity(r) |
| mode := modeFrom(r) |
| + homeLabel := "" |
| + if a.home != nil { |
| + homeLabel = connect.English["menu_action"] |
| + } |
| return PageData{ |
| - Title: title, |
| - Mode: mode, |
| - ThemeAttr: themeAttr(mode), |
| - Address: address, |
| - SignedIn: signedIn, |
| - Path: r.URL.Path, |
| - Data: data, |
| + HomeLinkLabel: homeLabel, |
| + Title: title, |
| + Mode: mode, |
| + ThemeAttr: themeAttr(mode), |
| + Address: address, |
| + SignedIn: signedIn, |
| + Path: r.URL.Path, |
| + Data: data, |
| } |
| } |
| @@ -153,4 +188,4 @@ func staticHandler() http.Handler { return assets.Handler() } |
| // staticFiles is the embedded tree, exposed for the guard test that |
| // walks our JavaScript for the line cap. |
| -func staticFiles() fs.FS { return staticFS } |
| +func staticFiles() fs.FS { return servedStaticFS } |
diff --git a/internal/sheets/static/cellqueue.js b/internal/sheets/static/cellqueue.js| index 3e4124c..04e3bdb 100644 |
| --- a/internal/sheets/static/cellqueue.js |
| +++ b/internal/sheets/static/cellqueue.js |
| @@ -1,43 +1,26 @@ |
| -/* Send a batch of cell writes, safely. |
| - * |
| - * Its own module because "how do we send N cell writes without losing |
| - * one" is a different question from "what did the person copy", and |
| - * because the answer is subtle enough to be worth stating once in a |
| - * place both callers can read. |
| - * |
| - * **The writes go out ONE AT A TIME, and that is the whole point.** |
| - * |
| - * The first version sent six at once, on the reasonable ground that a |
| - * 200-cell paste in series is 200 sequential round trips. Responses to |
| - * concurrent writes come back in whatever order the server finishes |
| - * them, and grid.js correctly discards a paint whose version is older |
| - * than one already applied — so a six-cell paste landed five on screen. |
| - * Every cell was stored; one simply never painted, which looks exactly |
| - * like a cell that was never sent. |
| - * |
| - * The second version kept the six lanes and added a refetch afterwards |
| - * to repair them. That was worse: refetch() zeroes grid.js's version |
| - * before applying, so a snapshot still in flight when a live edit |
| - * arrives from another tab paints over that newer edit. A repair that |
| - * introduces a race is not a repair. |
| - * |
| - * Serial writes have neither problem. Versions arrive monotonically, |
| - * nothing is discarded, and no snapshot is needed to fix anything. It is |
| - * slower per cell, which is what the caller's own size cap is sized |
| - * against — the honest trade until a batch cell route exists, which is |
| - * the real answer and is named in the slice spec's deferred table. |
| - */ |
| +/* Send one atomic batch per 500 cells, with chunks and invocations in order. |
| + * Concurrent writes lost paints when older replies were discarded. |
| + * A final refetch still raced with live edits. Serial chunks avoid both; |
| + * stop on the first failed chunk. */ |
| (function () { |
| 'use strict'; |
| - /* jobs is an array of cell-write bodies; post is grid.js's own post, |
| - * passed in rather than reached for, so this module never has to know |
| - * whether the grid booted. */ |
| + var tail = Promise.resolve(); |
| window.SheetsCellQueue = function (jobs, post, done) { |
| - var i = 0; |
| - (function step() { |
| - if (i >= jobs.length) { if (done) done(); return; } |
| - post('/cells', jobs[i++]).then(step, step); |
| - }()); |
| + var run = function () { |
| + var i = 0; |
| + function step() { |
| + if (i >= jobs.length) { if (done) done(); return true; } |
| + var cells = jobs.slice(i, i + 500); |
| + i += cells.length; |
| + return post('/cells/batch', { cells: cells }).then(function (ok) { |
| + if (ok === true) return step(); |
| + return false; |
| + }, function () { return false; }); |
| + } |
| + return step(); |
| + }; |
| + tail = tail.then(run, run); |
| + return tail; |
| }; |
| }()); |
diff --git a/internal/sheets/static/clipboard.js b/internal/sheets/static/clipboard.js| index 1efb886..0623c09 100644 |
| --- a/internal/sheets/static/clipboard.js |
| +++ b/internal/sheets/static/clipboard.js |
| @@ -13,8 +13,8 @@ |
| * anyway, which is silent data loss that looks fine until a reload. |
| * |
| * So this module takes the keydown AND the three clipboard events at |
| - * the capture phase, and every cell it writes goes through the same |
| - * POST a keystroke uses. tsv.js is the wire format and cellqueue.js |
| + * the capture phase, and every cell it writes goes through the atomic batch |
| + * route. tsv.js is the wire format and cellqueue.js |
| * sends the writes. |
| * |
| * It also carries spec §4.2's translation pass. A pasted formula names |
| @@ -28,8 +28,8 @@ |
| var surface = document.querySelector('[data-sheet-surface]'); |
| if (!surface) return; |
| - /* Sized against SERIAL writes (cellqueue.js), not against the store. |
| - * Raise it when a batch cell route lands, and not before. */ |
| + /* Keep a paste within one atomic batch. Raising this cap needs a |
| + * separate decision about failures across multiple chunks. */ |
| var MAX_CELLS = 500; |
| function ready() { |
| @@ -84,8 +84,7 @@ |
| var TSV = window.SheetsTSV; |
| if (!TSV) return; |
| - /* cellqueue.js sends the writes. See its own comment for why they |
| - * go one at a time. */ |
| + /* cellqueue.js sends atomic batches in order and stops on failure. */ |
| var writeAll = window.SheetsCellQueue; |
| if (!writeAll) return; |
diff --git a/internal/sheets/static/drop.js b/internal/sheets/static/drop.js| index 30baa12..3a89703 100644 |
| --- a/internal/sheets/static/drop.js |
| +++ b/internal/sheets/static/drop.js |
| @@ -1,4 +1,11 @@ |
| -/* Drag and drop for the import form. |
| +/* Drag and drop for the import form, in both places it appears. |
| + * |
| + * On /sheets/import the zone is a box in the page. On the sheet list it |
| + * is an overlay that stays `hidden` until a drag carrying files is |
| + * actually over the window — the list carries no visible drop box, |
| + * because a list of sheets should be a list of sheets. Which shape it |
| + * is comes from data-drop-page on the zone, and it changes only where |
| + * the events are listened for and whether the zone is revealed. |
| * |
| * This file adds no capability. The form it decorates already imports a |
| * spreadsheet with scripting off, and every drop here ends as that same |
| @@ -32,8 +39,32 @@ |
| if (typeof window.DataTransfer !== 'function') return; |
| var hint = zone.querySelector('[data-drop-hint]'); |
| + var hintText = hint ? hint.textContent : ''; |
| var depth = 0; |
| + /* Two shapes, one behaviour. |
| + * |
| + * On /sheets/import the zone is a box in the page and the events |
| + * belong to it. On the sheet list there is no box — a list of sheets |
| + * should be a list of sheets — so the zone is an overlay that is |
| + * `hidden` until a drag carrying files enters the window, and the |
| + * events belong to the document because a hidden element receives |
| + * none. Everything after this line is the same either way. */ |
| + var page = zone.hasAttribute('data-drop-page'); |
| + var source = page ? document : zone; |
| + |
| + function reveal() { if (page) zone.hidden = false; } |
| + |
| + /* dismiss puts the hint back as well. Otherwise a refusal ("drop one |
| + * file at a time") is still sitting there the next time the overlay |
| + * comes up, answering a drag that has not happened yet. */ |
| + function dismiss() { |
| + if (!page) return; |
| + zone.hidden = true; |
| + zone.removeAttribute('data-drop-over'); |
| + say(hintText); |
| + } |
| + |
| /* dragenter/dragleave fire for every child element the pointer |
| * crosses, so a plain enter/leave pair flickers the whole time a |
| * file is moved across the box. Counting depth is what makes the |
| @@ -42,6 +73,7 @@ |
| if (!hasFiles(e)) return; |
| e.preventDefault(); |
| depth++; |
| + reveal(); |
| zone.setAttribute('data-drop-over', ''); |
| } |
| @@ -49,7 +81,10 @@ |
| if (!hasFiles(e)) return; |
| e.preventDefault(); |
| depth = Math.max(0, depth - 1); |
| - if (depth === 0) zone.removeAttribute('data-drop-over'); |
| + if (depth === 0) { |
| + zone.removeAttribute('data-drop-over'); |
| + dismiss(); |
| + } |
| } |
| /* dragover must preventDefault on every tick or the browser refuses |
| @@ -117,8 +152,23 @@ |
| if (hint) hint.textContent = text; |
| } |
| - zone.addEventListener('dragenter', enter); |
| - zone.addEventListener('dragleave', leave); |
| - zone.addEventListener('dragover', over); |
| - zone.addEventListener('drop', drop); |
| + source.addEventListener('dragenter', enter); |
| + source.addEventListener('dragleave', leave); |
| + source.addEventListener('dragover', over); |
| + source.addEventListener('drop', drop); |
| + |
| + /* A refused drop leaves the overlay up on purpose — the message is |
| + * the whole point of refusing in the client's own words, and the |
| + * chooser beside it is how the person recovers. So the overlay needs |
| + * a way out that is not the back button: Escape, or a click on the |
| + * backdrop outside the panel. Both are what every other dismissible |
| + * layer in this app answers to, so neither has to be taught. */ |
| + if (page) { |
| + document.addEventListener('keydown', function (e) { |
| + if (e.key === 'Escape' && !zone.hidden) dismiss(); |
| + }); |
| + zone.addEventListener('click', function (e) { |
| + if (e.target === zone) dismiss(); |
| + }); |
| + } |
| })(); |
diff --git a/internal/sheets/static/grid.js b/internal/sheets/static/grid.js| index 6ab046d..b6c2f0b 100644 |
| --- a/internal/sheets/static/grid.js |
| +++ b/internal/sheets/static/grid.js |
| @@ -150,9 +150,9 @@ |
| headers: { 'Content-Type': 'application/json' }, |
| body: JSON.stringify(body) |
| }).then(function (resp) { |
| - if (!resp.ok) { refetch(); return; } |
| - return resp.json().then(applyBatch); |
| - }).catch(refetch); |
| + if (!resp.ok) { refetch(); return false; } |
| + return resp.json().then(function (payload) { applyBatch(payload); return true; }); |
| + }).catch(function () { refetch(); return false; }); |
| } |
| function write(r, c, text) { |
diff --git a/internal/sheets/static/sheets.css b/internal/sheets/static/sheets.css| index 81a8c4c..be9c11d 100644 |
| --- a/internal/sheets/static/sheets.css |
| +++ b/internal/sheets/static/sheets.css |
| @@ -65,18 +65,120 @@ |
| color: var(--rst-text); |
| } |
| +/* A page's own actions in the top bar, against the account menu. |
| + * |
| + * The auto margin has to live HERE and be taken off the account, not |
| + * added alongside it. rastrillo gives [rst-shell-account] a |
| + * margin-inline-start: auto, which absorbs all the bar's free space and |
| + * would leave Create stranded back beside the nav — measured, not |
| + * guessed: that is exactly how it first rendered. A second auto margin |
| + * does not fix it either, because flexbox splits free space equally |
| + * between auto margins and the buttons end up mid-bar. |
| + * |
| + * Only above 800px. Below it rastrillo stacks the tail into a column |
| + * behind the Menu disclosure and zeroes that same margin itself, and an |
| + * auto inline margin in a column flex box shoves one item to the |
| + * trailing edge — the bug the framework's own comment at that rule |
| + * describes. Same breakpoint as the framework's, deliberately: two |
| + * shells with two breakpoints is two systems. */ |
| +[data-topbar-actions] { |
| + display: flex; |
| + align-items: center; |
| + gap: var(--rst-sp-2); |
| +} |
| + |
| +/* Both selectors are written against the framework's own shape, and |
| + * both have to out-specify it. rastrillo re-applies the account's auto |
| + * margin INSIDE this same media query at |
| + * [rst-shell-topbar] [rst-shell-tail] > [rst-shell-account], which is |
| + * 0-3-0; a plain [data-topbar-actions] ~ [rst-shell-account] is 0-2-0 |
| + * and loses, which is not a subtle failure — the buttons come to rest |
| + * dead centre of the bar, because the two auto margins then split the |
| + * free space between them. Measured, twice. |
| + * |
| + * The tail is display: contents up here, so these are siblings in the |
| + * bar's own flex line even though they are nested in the markup. */ |
| +@media (min-width: 800px) { |
| + [rst-shell-topbar] [rst-shell-tail] > [data-topbar-actions] { |
| + margin-inline-start: auto; |
| + } |
| + [rst-shell-topbar] [rst-shell-tail] > [data-topbar-actions] ~ [rst-shell-account] { |
| + margin-inline-start: 0; |
| + } |
| +} |
| + |
| /* The document title, in the top bar beside the brand. It truncates |
| * rather than wrapping: the bar is a fixed height, and a two-line title |
| - * would push the account menu out of it. */ |
| + * would push the account menu out of it. |
| + * |
| + * The clipping lives on the WRAPPER, not on the text or the input. It |
| + * used to sit directly on [data-doc-title] back when that was a span, |
| + * and left there it would clip a rename input's own caret: type past |
| + * 22rem and the cursor walks out of a box with overflow: hidden, so you |
| + * are editing text you cannot see. */ |
| [data-doc-title] { |
| color: var(--rst-text); |
| font-weight: 600; |
| max-inline-size: 22rem; |
| +} |
| + |
| +[data-doc-title-text] { |
| + display: block; |
| overflow: hidden; |
| text-overflow: ellipsis; |
| white-space: nowrap; |
| } |
| +/* The rename form. It reads as a title until somebody means to edit it: |
| + * no border, no field background, the same weight and colour as the |
| + * static version above. This is the one control on the page that has to |
| + * NOT look like a control at rest — a text input sitting permanently in |
| + * the top bar of a spreadsheet competes with the grid for "the thing |
| + * you type in", and the grid must win by default. */ |
| +form[data-doc-title] { |
| + display: flex; |
| + align-items: center; |
| + gap: var(--rst-sp-1); |
| +} |
| + |
| +[data-doc-title-input] { |
| + min-inline-size: 0; |
| + padding: var(--rst-sp-1); |
| + border: 1px solid transparent; |
| + border-radius: var(--rst-radius); |
| + background: none; |
| + color: inherit; |
| + font: inherit; |
| + text-overflow: ellipsis; |
| +} |
| + |
| +[data-doc-title-input]:hover { |
| + border-color: var(--rst-line); |
| +} |
| + |
| +/* Focus turns it into an obvious field. :focus-visible is not enough |
| + * here — a click into the box must show the same affordance a Tab into |
| + * it does, because at rest the two are indistinguishable. */ |
| +[data-doc-title-input]:focus { |
| + border-color: var(--rst-accent); |
| + background: var(--rst-surface); |
| + outline: none; |
| + text-overflow: clip; |
| +} |
| + |
| +/* The submit is hidden until the field is engaged, and hidden with |
| + * clip rather than display: none so it stays focusable — Tab from the |
| + * input must reach it. Scripting off, this button is the only way to |
| + * commit a rename that was not submitted with Enter. */ |
| +form[data-doc-title]:not(:focus-within) button { |
| + position: absolute; |
| + inline-size: 1px; |
| + block-size: 1px; |
| + overflow: hidden; |
| + clip-path: inset(50%); |
| + white-space: nowrap; |
| +} |
| + |
| [data-menubar] { |
| display: flex; |
| align-items: center; |
| @@ -925,14 +1027,21 @@ |
| * page crosses this box repeatedly, and animating the outline is the |
| * difference between a box that acknowledges the drag and one that |
| * flashes. */ |
| -[data-drop] { |
| +/* :not([data-drop-page]) on both, because there are two zones now and |
| + only one of them is a box. The overlay on the sheet list carries |
| + data-drop too — it has to, it is what drop.js looks for — and these |
| + two selectors are 0-2-0 and 0-3-0, so without the exclusion they beat |
| + [rst-modal-overlay]'s 0-1-0 and paint the sheet of glass over the |
| + whole page in accent-soft. It renders as a flat blue screen with the |
| + list gone: a defect a browser found and no test could. */ |
| +[data-drop]:not([data-drop-page]) { |
| border: 1px dashed var(--rst-line); |
| border-radius: var(--rst-radius); |
| padding: var(--rst-sp-3); |
| transition: border-color 120ms ease, background-color 120ms ease; |
| } |
| -[data-drop][data-drop-over] { |
| +[data-drop][data-drop-over]:not([data-drop-page]) { |
| border-color: var(--rst-accent); |
| background: var(--rst-accent-soft); |
| } |
| @@ -945,6 +1054,77 @@ |
| } |
| } |
| +/* ── The sheet list's drop overlay ─────────────────────────────────── |
| + * |
| + * The list has no drop box. It has a full-window target that does not |
| + * exist until a file is over the window, and drop.js is what reveals |
| + * it — see the comment on [data-drop-page] in sheets.html. |
| + * |
| + * It borrows rastrillo's modal paint rather than growing its own, so |
| + * the layer that appears over the list looks like every other layer in |
| + * the suite. Only the four rules below are ours. |
| + * |
| + * THIS FIRST RULE IS LOAD-BEARING. [rst-modal-overlay] sets |
| + * display: flex, and an author declaration beats the user agent's |
| + * [hidden] { display: none } no matter how weak its selector looks. |
| + * Without it the overlay covers the sheet list permanently and the page |
| + * is unusable — with scripting off, where nothing would ever come along |
| + * to hide it again. TestTheDropOverlayIsHiddenAtRest pins the markup; |
| + * this pins the paint. */ |
| +[data-drop-page][hidden] { |
| + display: none; |
| +} |
| + |
| +[data-drop-page] [rst-modal-panel] { |
| + max-width: 24rem; |
| + transition: border-color 120ms ease; |
| +} |
| + |
| +/* The panel answers the drag, the same way the box on /sheets/import |
| + does and with the same token, so the two read as one gesture. */ |
| +[data-drop-page][data-drop-over] [rst-modal-panel] { |
| + border-color: var(--rst-accent); |
| +} |
| + |
| +[data-drop-page-body] { |
| + display: grid; |
| + gap: var(--rst-sp-3); |
| + justify-items: center; |
| + padding: var(--rst-sp-5); |
| + text-align: center; |
| + width: 100%; |
| +} |
| + |
| +[data-drop-page-title] { |
| + font-size: var(--rst-fs-base); |
| + font-weight: 600; |
| + margin: 0; |
| +} |
| + |
| +/* The chooser is for afterwards. |
| + * |
| + * While a file is over the window the panel has one thing to say, and a |
| + * "Choose File" button beside "Drop it to import" offers a second way to |
| + * do the thing the person is already doing. It appears when the overlay |
| + * is up WITHOUT a drag over it, which is exactly the state a refused |
| + * drop leaves behind — and then it is the way out of that refusal. */ |
| +[data-drop-page-recover] { |
| + display: grid; |
| + gap: var(--rst-sp-3); |
| + justify-items: center; |
| + width: 100%; |
| +} |
| + |
| +[data-drop-page][data-drop-over] [data-drop-page-recover] { |
| + display: none; |
| +} |
| + |
| +@media (prefers-reduced-motion: reduce) { |
| + [data-drop-page] [rst-modal-panel] { |
| + transition: none; |
| + } |
| +} |
| + |
| /* ── Cell formatting ───────────────────────────────────────────────── |
| * |
| * The paint for a cell's stored Format: bold, italic, alignment, fill |
diff --git a/internal/sheets/static/theme.css b/internal/sheets/static/theme.css| deleted file mode 100644 |
| index 5da4384..0000000 |
| --- a/internal/sheets/static/theme.css |
| +++ /dev/null |
| @@ -1,187 +0,0 @@ |
| -/* day — rastrillo's default theme: an everyday blue on white and grey. |
| - Structure lives in tokens.css; this file is colour, type and shape. |
| - rastrillo new writes it once as static/theme.css. |
| - |
| - The look: the ordinary, personable interface. A white page, a plain |
| - grey text scale, one friendly blue that means "this does something", |
| - and the conventional green/amber/red for status. Nothing here is |
| - trying to be memorable — day is the theme an app ships on while it |
| - works out what it wants to look like, and the one that never gets in |
| - the way of the content. Semantic status colours are functional and |
| - never the accent, so a pill can never be mistaken for a link. |
| - |
| - ── One block, both schemes ─────────────────────────────────────────── |
| - Every token is declared once, as light-dark(<light>, <dark>), under a |
| - single :root that sets color-scheme: light dark. The browser picks the |
| - half that matches the resolved scheme, so there is no second copy of |
| - the palette to keep in sync and no way to author dark half-done. The |
| - two rules at the bottom are the whole toggle: setting color-scheme on |
| - :root[data-theme=…] re-resolves every light-dark() in the file at |
| - once, in both directions, which is why an explicit choice beats the OS |
| - without restating a single colour. |
| - |
| - Tokens with one value in both schemes — the font stack, the radii — |
| - are written plain, because light-dark(x, x) says nothing twice. |
| - |
| - light-dark() is a colour function: it may only stand where a <color> |
| - is expected. So a shadow token wraps its colour in it and writes the |
| - geometry once, outside the call — `0 8px 24px light-dark(a, b)`, never |
| - `light-dark(0 8px 24px a, …)`, which no browser would accept as a |
| - box-shadow. |
| - |
| - ── The shape axis ──────────────────────────────────────────────────── |
| - Radius and shadow are the theme's now, not tokens.css's: a theme that |
| - can only change colour cannot change how an app feels. day is soft and |
| - unremarkable — 8px cards, 6px controls, shadows with a wide blur and |
| - little alpha, the depth of paper resting on paper. |
| - |
| - ── WCAG 2.2 AA, measured ───────────────────────────────────────────── |
| - Normal text needs 4.5:1; large text and UI component boundaries need |
| - 3:1. Every pair below was computed from these exact hex values, in |
| - both schemes. If you change a token, recompute its rows. |
| - |
| - LIGHT ratio requirement |
| - --rst-text on --rst-surface 16.91 4.5:1 text |
| - --rst-text on --rst-bg 16.91 4.5:1 text |
| - --rst-text on --rst-surface-2 15.77 4.5:1 text |
| - --rst-text on --rst-accent-soft 15.21 4.5:1 text |
| - --rst-text-muted on --rst-surface 7.56 4.5:1 text |
| - --rst-text-muted on --rst-bg 7.56 4.5:1 text |
| - --rst-text-muted on --rst-surface-2 7.05 4.5:1 text |
| - --rst-text-muted on --rst-accent-soft 6.80 4.5:1 text |
| - --rst-text-faint on --rst-surface 5.29 4.5:1 text |
| - --rst-text-faint on --rst-bg 5.29 4.5:1 text |
| - --rst-text-faint on --rst-surface-2 4.93 4.5:1 text |
| - --rst-text-faint on --rst-accent-soft 4.76 4.5:1 text |
| - --rst-accent on --rst-surface 5.28 4.5:1 text + focus ring |
| - --rst-accent on --rst-bg 5.28 4.5:1 text + focus ring |
| - --rst-accent on --rst-surface-2 4.93 4.5:1 text |
| - --rst-accent on --rst-accent-soft 4.75 4.5:1 text |
| - --rst-on-accent on --rst-accent 5.28 4.5:1 button label |
| - --rst-on-accent on --rst-accent-strong 7.18 4.5:1 button label, hover |
| - --rst-line-strong on --rst-surface 3.63 3:1 control border |
| - --rst-line-strong on --rst-bg 3.63 3:1 control border |
| - --rst-line-strong on --rst-surface-2 3.39 3:1 control border |
| - pill neutral fg on its own tint 9.02 4.5:1 text |
| - pill positive fg on its own tint 6.49 4.5:1 text |
| - pill warning fg on its own tint 6.15 4.5:1 text |
| - pill negative fg on its own tint 5.30 4.5:1 text |
| - --rst-on-accent on negative fg (fill) 6.47 4.5:1 danger button label |
| - |
| - DARK ratio requirement |
| - --rst-text on --rst-surface 13.74 4.5:1 text |
| - --rst-text on --rst-bg 15.32 4.5:1 text |
| - --rst-text on --rst-surface-2 14.63 4.5:1 text |
| - --rst-text on --rst-accent-soft 13.02 4.5:1 text |
| - --rst-text-muted on --rst-surface 7.56 4.5:1 text |
| - --rst-text-muted on --rst-bg 8.43 4.5:1 text |
| - --rst-text-muted on --rst-surface-2 8.05 4.5:1 text |
| - --rst-text-muted on --rst-accent-soft 7.16 4.5:1 text |
| - --rst-text-faint on --rst-surface 6.33 4.5:1 text |
| - --rst-text-faint on --rst-bg 7.06 4.5:1 text |
| - --rst-text-faint on --rst-surface-2 6.74 4.5:1 text |
| - --rst-text-faint on --rst-accent-soft 6.00 4.5:1 text |
| - --rst-accent on --rst-surface 6.95 4.5:1 text + focus ring |
| - --rst-accent on --rst-bg 7.75 4.5:1 text + focus ring |
| - --rst-accent on --rst-surface-2 7.40 4.5:1 text |
| - --rst-accent on --rst-accent-soft 6.59 4.5:1 text |
| - --rst-on-accent on --rst-accent 7.86 4.5:1 button label |
| - --rst-on-accent on --rst-accent-strong 10.29 4.5:1 button label, hover |
| - --rst-line-strong on --rst-surface 4.26 3:1 control border |
| - --rst-line-strong on --rst-bg 4.75 3:1 control border |
| - --rst-line-strong on --rst-surface-2 4.53 3:1 control border |
| - pill neutral fg on its own tint 8.44 4.5:1 text |
| - pill positive fg on its own tint 10.18 4.5:1 text |
| - pill warning fg on its own tint 9.62 4.5:1 text |
| - pill negative fg on its own tint 7.85 4.5:1 text |
| - --rst-on-accent on negative fg (fill) 8.99 4.5:1 danger button label |
| - |
| - The tightest pair in the set is --rst-accent on --rst-accent-soft in |
| - light, at 4.75:1 against a 4.5:1 floor. Put a token on a surface not |
| - listed above and you are guessing; recompute first. |
| - |
| - --rst-line is a hairline divider between rows, not a control border or |
| - a meaningful graphic, so 1.4.11's 3:1 does not apply to it. Control |
| - borders use --rst-line-strong, which does clear 3:1. |
| - |
| - --rst-header-rule is not in the table either, and deliberately so |
| - (design doc §6-v2.2): the page header's rule is decoration, not |
| - structure — the heading's size, weight and spacing carry the |
| - structure — so no contrast floor applies to it and it is not in |
| - ui/contrast_test.go's pair table. Do not argue it up to 3:1. |
| - |
| - Pagination chips are the one deliberate exception: they are bordered |
| - with --rst-line rather than --rst-line-strong, because a page strip |
| - ringed in hard outlines shouts louder than the list it belongs to. |
| - 1.4.11 is satisfied without that border — each chip's meaning is |
| - carried by its text label at 4.5:1 or better, and the current page |
| - adds a filled background, a heavier weight and aria-current on top. |
| - |
| - --rst-shadow-pop, --rst-shadow-knob, --rst-shadow-lift and |
| - --rst-overlay are not in the table: they paint a translucent shadow or |
| - scrim over whatever sits underneath, not text over a fixed |
| - background, so a contrast ratio does not apply. The scrim deepens in |
| - dark, matching --rst-shadow-pop's own jump: black reads lighter |
| - against a dark backdrop, so the overlay needs more alpha to still |
| - read as a dimming layer. |
| - ──────────────────────────────────────────────────────────────────── */ |
| - |
| -:root { |
| - color-scheme: light dark; |
| - |
| - /* Type: the family only. Sizes and spacing are structural and stay in |
| - tokens.css. */ |
| - --rst-font: system-ui, -apple-system, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif; |
| - |
| - /* Shape: soft, ordinary, unremarkable. */ |
| - --rst-radius: 8px; |
| - --rst-radius-sm: 6px; |
| - --rst-radius-pill: 999px; |
| - |
| - /* Surfaces and lines */ |
| - --rst-bg: light-dark(#ffffff, #111418); |
| - --rst-surface: light-dark(#ffffff, #1a1f26); |
| - --rst-surface-2: light-dark(#f6f7f8, #15191f); |
| - --rst-line: light-dark(#e5e7eb, #2a3038); |
| - --rst-line-strong: light-dark(#7f8792, #79828e); |
| - |
| - /* The page header's rule, derived rather than authored: the accent |
| - folded into the line at 18%, so a theme that changes its accent |
| - gets a matching rule with no second value to keep in step. day's |
| - is barely there — a hairline you would call grey until you put it |
| - beside plain's. Decorative: it carries no contrast floor. */ |
| - --rst-header-rule: color-mix(in oklab, var(--rst-accent) 18%, var(--rst-line)); |
| - |
| - /* Text */ |
| - --rst-text: light-dark(#1a1d21, #e8eaed); |
| - --rst-text-muted: light-dark(#4b5563, #a8b0ba); |
| - --rst-text-faint: light-dark(#656c78, #98a1ac); |
| - |
| - /* Accent */ |
| - --rst-accent: light-dark(#2464e0, #7ba7ff); |
| - --rst-accent-strong: light-dark(#1a4fc0, #a3c0ff); |
| - --rst-accent-soft: light-dark(#eef3fe, #17233a); |
| - --rst-on-accent: light-dark(#ffffff, #0b1220); |
| - |
| - /* Status tones */ |
| - --rst-tone-neutral-fg: light-dark(#374151, #c3c9d1); |
| - --rst-tone-neutral-bg: light-dark(#eef0f2, #262c34); |
| - --rst-tone-positive-fg: light-dark(#166534, #6ee7a8); |
| - --rst-tone-positive-bg: light-dark(#dcfce7, #10281c); |
| - --rst-tone-warning-fg: light-dark(#854d0e, #f0c46a); |
| - --rst-tone-warning-bg: light-dark(#fef3c7, #2c2110); |
| - --rst-tone-negative-fg: light-dark(#b91c1c, #f79aa0); |
| - --rst-tone-negative-bg: light-dark(#fee2e2, #331819); |
| - |
| - /* Depth */ |
| - --rst-shadow-pop: 0 8px 24px light-dark(rgba(0, 0, 0, 0.12), rgba(0, 0, 0, 0.5)); |
| - --rst-shadow-knob: 0 1px 2px rgba(0, 0, 0, 0.3); |
| - --rst-shadow-lift: 0 1px 2px rgba(0, 0, 0, 0.08); |
| - --rst-overlay: light-dark(rgba(0, 0, 0, 0.45), rgba(0, 0, 0, 0.6)); |
| -} |
| - |
| -/* The whole toggle. Setting color-scheme re-resolves every light-dark() |
| - above, so an explicit choice beats the OS in both directions without |
| - restating a colour. */ |
| -:root[data-theme="light"] { color-scheme: light; } |
| -:root[data-theme="dark"] { color-scheme: dark; } |
diff --git a/internal/sheets/store/handle_blank_cap_test.go b/internal/sheets/store/handle_blank_cap_test.go| index 3074408..f06ce08 100644 |
| --- a/internal/sheets/store/handle_blank_cap_test.go |
| +++ b/internal/sheets/store/handle_blank_cap_test.go |
| @@ -137,9 +137,20 @@ func TestBlankFeederOverCapDependentsAreRecoverable(t *testing.T) { |
| t.Fatalf("test setup: only %d dependents wired, want more than MaxDirtyPerPass (%d)", len(touched), calc.MaxDirtyPerPass) |
| } |
| - if err := h.Set(feederRow, colA, "", 7); err != nil { |
| + rep, err := h.SetReport(feederRow, colA, "", 7) |
| + if err != nil { |
| t.Fatalf("blank feeder: %v", err) |
| } |
| + if !rep.Capped { |
| + t.Fatal("blank unexpectedly recalculated") |
| + } |
| + var watermark uint64 |
| + if err := db.Raw("SELECT content_version FROM tabs WHERE id = ?", localTab).Scan(&watermark).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if watermark < rep.Version { |
| + t.Fatalf("capped blank stamp %d not retained by watermark %d", rep.Version, watermark) |
| + } |
| h.Release() |
| // The real proof: reopen and read the dependents' actual VALUE, not |
diff --git a/internal/sheets/store/handle_open.go b/internal/sheets/store/handle_open.go| index 9e76354..8f3498b 100644 |
| --- a/internal/sheets/store/handle_open.go |
| +++ b/internal/sheets/store/handle_open.go |
| @@ -43,6 +43,10 @@ import ( |
| // calc's evaluator to real ids, which is the more invasive but more |
| // durable fix. This handle does neither because it needs neither yet. |
| func buildHandle(db *gorm.DB, sealer *seal.Sealer, tabID uint64) (*Handle, error) { |
| + var tab Tab |
| + if err := db.Select("id", "content_version").First(&tab, tabID).Error; err != nil { |
| + return nil, fmt.Errorf("store: load tab version: %w", err) |
| + } |
| var rows []Row |
| if err := db.Where("tab_id = ?", tabID).Find(&rows).Error; err != nil { |
| return nil, fmt.Errorf("store: load rows: %w", err) |
| @@ -78,7 +82,7 @@ func buildHandle(db *gorm.DB, sealer *seal.Sealer, tabID uint64) (*Handle, error |
| } |
| var stale []calc.CellKey |
| - var maxVersion uint64 |
| + maxVersion := tab.ContentVersion |
| for _, c := range cells { |
| key, ok := h.cellKey(c.RowID, c.ColID) |
| @@ -125,6 +129,9 @@ func buildHandle(db *gorm.DB, sealer *seal.Sealer, tabID uint64) (*Handle, error |
| stale = append(stale, key) |
| } |
| } |
| + if maxVersion >= maxContentVersion-1 { |
| + return nil, errVersionExhausted |
| + } |
| h.nextVersion = maxVersion + 1 |
| // Crash recovery: any cell whose CacheVersion trails its InputVersion |
diff --git a/internal/sheets/store/handle_report.go b/internal/sheets/store/handle_report.go| index 15111d8..835298f 100644 |
| --- a/internal/sheets/store/handle_report.go |
| +++ b/internal/sheets/store/handle_report.go |
| @@ -105,6 +105,9 @@ func (h *Handle) SetReport(rowID, colID uint64, raw string, actor uint64) (Repor |
| return Report{}, err |
| } |
| + if h.nextVersion >= maxContentVersion { |
| + return Report{}, errVersionExhausted |
| + } |
| v := h.nextVersion |
| prevRaw := h.rawText[key] |
diff --git a/internal/sheets/store/handle_set.go b/internal/sheets/store/handle_set.go| index f4a8039..c64abbc 100644 |
| --- a/internal/sheets/store/handle_set.go |
| +++ b/internal/sheets/store/handle_set.go |
| @@ -52,6 +52,9 @@ func (h *Handle) setBlank(rowID, colID uint64, key calc.CellKey, v uint64, actor |
| withinCap := len(touched) > 0 && len(touched) <= calc.MaxDirtyPerPass |
| if err := h.db.Transaction(func(tx *gorm.DB) error { |
| + if err := h.advanceContentVersion(tx, v); err != nil { |
| + return err |
| + } |
| if err := tx.Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, rowID, colID). |
| Delete(&Cell{}).Error; err != nil { |
| return err |
| @@ -78,6 +81,7 @@ func (h *Handle) setBlank(rowID, colID uint64, key calc.CellKey, v uint64, actor |
| }); err != nil { |
| return nil, false, err |
| } |
| + h.nextVersion++ |
| // Test seam: a crash landing exactly here — transaction 1 durably |
| // committed, nothing after it has run yet — is what |
| // TestBlankCrashLeavesDependentsRecoverable simulates. Always nil in |
| @@ -85,7 +89,6 @@ func (h *Handle) setBlank(rowID, colID uint64, key calc.CellKey, v uint64, actor |
| if err := afterBlankTransactionOneCommits(); err != nil { |
| return nil, false, err |
| } |
| - h.nextVersion++ |
| delete(h.rawText, key) |
| delete(h.keyToIDs, key) |
| h.graph.Clear(key) |
| @@ -124,6 +127,9 @@ func (h *Handle) setContent(rowID, colID uint64, key calc.CellKey, node calc.Nod |
| return nil, false, err |
| } |
| if err := h.db.Transaction(func(tx *gorm.DB) error { |
| + if err := h.advanceContentVersion(tx, v); err != nil { |
| + return err |
| + } |
| res := tx.Model(&Cell{}). |
| Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, rowID, colID). |
| Updates(map[string]any{ |
| @@ -228,6 +234,9 @@ func (h *Handle) recalcTouched(origin calc.CellKey, includeOrigin bool) (map[cal |
| // caused it — there was never a single-trigger claim to violate. |
| func (h *Handle) persistTouched(touched map[calc.CellKey]struct{}, v uint64) error { |
| return h.db.Transaction(func(tx *gorm.DB) error { |
| + if err := h.advanceContentVersion(tx, v); err != nil { |
| + return err |
| + } |
| for key := range touched { |
| ids, ok := h.keyToIDs[key] |
| if !ok { |
diff --git a/internal/sheets/store/handle_version.go b/internal/sheets/store/handle_version.go| new file mode 100644 |
| index 0000000..765d787 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_version.go |
| @@ -0,0 +1,27 @@ |
| +package store |
| + |
| +import ( |
| + "errors" |
| + "math" |
| + |
| + "gorm.io/gorm" |
| +) |
| + |
| +const maxContentVersion uint64 = math.MaxInt64 |
| + |
| +var errVersionExhausted = errors.New("store: content version exhausted") |
| + |
| +func (h *Handle) advanceContentVersion(tx *gorm.DB, version uint64) error { |
| + if version >= maxContentVersion { |
| + return errVersionExhausted |
| + } |
| + result := tx.Model(&Tab{}).Where("id = ?", h.tabID). |
| + Select("content_version").Update("content_version", gorm.Expr("MAX(content_version, ?)", version)) |
| + if result.Error != nil { |
| + return result.Error |
| + } |
| + if result.RowsAffected != 1 { |
| + return gorm.ErrRecordNotFound |
| + } |
| + return nil |
| +} |
diff --git a/internal/sheets/store/handle_version_export_test.go b/internal/sheets/store/handle_version_export_test.go| new file mode 100644 |
| index 0000000..4ea4ba1 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_version_export_test.go |
| @@ -0,0 +1,14 @@ |
| +package store |
| + |
| +import "gorm.io/gorm" |
| + |
| +func AdvanceContentVersionForTest(db *gorm.DB, tabID, version uint64) error { |
| + h := &Handle{tabID: tabID} |
| + return db.Transaction(func(tx *gorm.DB) error { return h.advanceContentVersion(tx, version) }) |
| +} |
| + |
| +func SetNextVersionForTest(h *Handle, version uint64) { |
| + h.mu.Lock() |
| + defer h.mu.Unlock() |
| + h.nextVersion = version |
| +} |
diff --git a/internal/sheets/store/handle_version_limits_test.go b/internal/sheets/store/handle_version_limits_test.go| new file mode 100644 |
| index 0000000..b5f006f |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_version_limits_test.go |
| @@ -0,0 +1,184 @@ |
| +package store_test |
| + |
| +import ( |
| + "math" |
| + "strconv" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +func TestVersionWatermarkNeverLowersAndRequiresTab(t *testing.T) { |
| + db, _, id := seeded(t) |
| + if err := store.AdvanceContentVersionForTest(db, id, 100); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := store.AdvanceContentVersionForTest(db, id, 4); err != nil { |
| + t.Fatal(err) |
| + } |
| + var mark uint64 |
| + if err := db.Raw("SELECT content_version FROM tabs WHERE id = ?", id).Scan(&mark).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if mark != 100 { |
| + t.Fatalf("watermark lowered to %d", mark) |
| + } |
| + if err := store.AdvanceContentVersionForTest(db, id+1000, 4); err == nil { |
| + t.Fatal("missing tab accepted") |
| + } |
| +} |
| + |
| +func TestOpenRequiresTabVersionRow(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id+1000) |
| + if err == nil { |
| + h.Release() |
| + t.Fatal("missing tab opened") |
| + } |
| +} |
| + |
| +func TestMissingTabRefusesRetainedHandleWrite(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + if err = db.Delete(&store.Tab{}, id).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err = h.SetReport(rowA, colA, "9", actor); err == nil { |
| + t.Fatal("write accepted without version row") |
| + } |
| + if h.Value(rowA, colA).Num != 5 { |
| + t.Fatal("failed write changed graph") |
| + } |
| +} |
| + |
| +func TestHighWatermarkSurvivesEmptyTabOpen(t *testing.T) { |
| + db, s, id := seeded(t) |
| + if err := db.Where("tab_id = ?", id).Delete(&store.Cell{}).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + const high = math.MaxInt64 - 10 |
| + if err := store.AdvanceContentVersionForTest(db, id, high); err != nil { |
| + t.Fatal(err) |
| + } |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + rep, err := h.SetReport(rowA, colA, "9", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if rep.Version <= high { |
| + t.Fatalf("high watermark lost: %d", rep.Version) |
| + } |
| +} |
| + |
| +func TestVersionExhaustionRefusesWithoutMutation(t *testing.T) { |
| + for _, mark := range []int64{math.MaxInt64, math.MaxInt64 - 1, -1} { |
| + t.Run(strconv.FormatInt(mark, 10), func(t *testing.T) { |
| + db, s, id := seeded(t) |
| + if err := db.Model(&store.Tab{}).Where("id = ?", id).Update("content_version", mark).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + h, err := store.Open(db, s, id) |
| + if err == nil { |
| + h.Release() |
| + t.Fatal("invalid watermark opened") |
| + } |
| + }) |
| + } |
| + for _, batch := range []bool{false, true} { |
| + t.Run(map[bool]string{false: "single", true: "batch"}[batch], func(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + var before store.Tab |
| + if err = db.First(&before, id).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + store.SetNextVersionForTest(h, math.MaxInt64) |
| + if batch { |
| + _, err = h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: "9"}}, actor) |
| + } else { |
| + _, err = h.SetReport(rowA, colA, "9", actor) |
| + } |
| + if err == nil { |
| + t.Fatal("exhausted allocation accepted") |
| + } |
| + var after store.Tab |
| + if err = db.First(&after, id).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if after.ContentVersion != before.ContentVersion || h.Value(rowA, colA).Num != 5 { |
| + t.Fatal("exhausted write mutated state") |
| + } |
| + }) |
| + } |
| +} |
| + |
| +func TestFormulaRecoveryVersionSurvivesClearingNewestCaches(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err = h.Set(rowB, colA, "=A1*2", actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err = h.Set(rowA, colA, "8", actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + if h.Value(rowB, colA).Num != 16 { |
| + t.Fatal("formula did not recalculate") |
| + } |
| + h.Release() |
| + if err = db.Model(&store.Cell{}).Where("tab_id = ? AND row_id = ?", id, rowB).Update("cache_version", 0).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var tab store.Tab |
| + if err = db.First(&tab, id).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + var cell store.Cell |
| + if err = db.Where("tab_id = ? AND row_id = ?", id, rowB).First(&cell).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if tab.ContentVersion < cell.CacheVersion { |
| + t.Fatal("recovery stamp not retained") |
| + } |
| + if err = h.Set(rowA, colA, "", actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + if h.Value(rowB, colA).Num != 0 { |
| + t.Fatal("blank feeder did not recalculate") |
| + } |
| + last, err := h.SetReport(rowB, colA, "", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + h.Release() |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + next, err := h.SetReport(rowA, colA, "9", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if next.Version <= last.Version { |
| + t.Fatal("deleting recovered cells lost watermark") |
| + } |
| +} |
diff --git a/internal/sheets/store/handle_version_migration_test.go b/internal/sheets/store/handle_version_migration_test.go| new file mode 100644 |
| index 0000000..9887a6e |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_version_migration_test.go |
| @@ -0,0 +1,122 @@ |
| +package store_test |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| + rdb "github.com/carlosframework/rastrillo/db" |
| + "github.com/carlosframework/rastrillo/migrate" |
| +) |
| + |
| +func TestVersionWatermarkMigrationPreservesExistingContent(t *testing.T) { |
| + d, err := rdb.Open(filepath.Join(t.TempDir(), "old.db"), nil) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer d.Close() |
| + old := &migrate.Set{} |
| + for _, m := range sheets.BootSchema.All() { |
| + if !strings.HasPrefix(m.ID, "sheets/") || m.ID <= "sheets/0009_alter_formats_ink_intent" { |
| + old.Add(m) |
| + } |
| + } |
| + if _, err := migrate.Apply(context.Background(), d, old); err != nil { |
| + t.Fatal(err) |
| + } |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + name, err := s.Seal("tab.name", []byte("Original tab")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + raw, err := s.Seal("cell.input", []byte("5")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cache, err := s.Seal("cell.cached", store.EncodeValueForTest(calc.Value{Kind: calc.KindNum, Num: 5})) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + payload, err := s.Seal("event.payload", []byte(`{"r":100,"c":200,"o":"","n":"5"}`)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, q := range []struct { |
| + sql string |
| + args []any |
| + }{ |
| + {"INSERT INTO events (id,tab_id,kind,actor_id,payload_sealed) VALUES (?,?,?,?,?)", []any{1, tabID, "input", actor, payload}}, |
| + {"INSERT INTO tabs (id,sheet_id,name_sealed,position) VALUES (?,?,?,?)", []any{tabID, 1, name, 0}}, |
| + {"INSERT INTO rows (id,tab_id,position) VALUES (?,?,?)", []any{rowA, tabID, 0}}, |
| + {"INSERT INTO cols (id,tab_id,position) VALUES (?,?,?)", []any{colA, tabID, 0}}, |
| + {"INSERT INTO cells (tab_id,row_id,col_id,input_sealed,cached_sealed,input_version,cache_version) VALUES (?,?,?,?,?,?,?)", []any{tabID, rowA, colA, raw, cache, 700, 700}}, |
| + } { |
| + if err := d.G.Exec(q.sql, q.args...).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil { |
| + t.Fatal(err) |
| + } |
| + var event store.Event |
| + if err := d.G.First(&event, 1).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if !bytes.Equal(event.PayloadSealed, payload) || event.ActorID != actor { |
| + t.Fatal("migration altered history") |
| + } |
| + var mark uint64 |
| + if err := d.G.Raw("SELECT content_version FROM tabs WHERE id = ?", tabID).Scan(&mark).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if mark != 0 { |
| + t.Fatalf("new watermark default=%d", mark) |
| + } |
| + var cell store.Cell |
| + if err := d.G.First(&cell).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if !bytes.Equal(cell.InputSealed, raw) || !bytes.Equal(cell.CachedSealed, cache) { |
| + t.Fatal("migration altered encrypted cell") |
| + } |
| + var tab store.Tab |
| + if err := d.G.First(&tab, tabID).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if !bytes.Equal(tab.NameSealed, name) { |
| + t.Fatal("migration altered encrypted name") |
| + } |
| + h, err := store.Open(d.G, s, tabID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + clear, err := h.SetReport(rowA, colA, "", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + h.Release() |
| + if clear.Version <= 700 { |
| + t.Fatalf("ignored old cell stamp: %d", clear.Version) |
| + } |
| + h, err = store.Open(d.G, s, tabID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + next, err := h.SetReport(rowA, colA, "6", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if next.Version <= clear.Version { |
| + t.Fatalf("post-upgrade versions %d -> %d", clear.Version, next.Version) |
| + } |
| +} |
diff --git a/internal/sheets/store/handle_version_watermark_test.go b/internal/sheets/store/handle_version_watermark_test.go| new file mode 100644 |
| index 0000000..012b082 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_version_watermark_test.go |
| @@ -0,0 +1,203 @@ |
| +package store_test |
| + |
| +import ( |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| + "errors" |
| + "gorm.io/gorm" |
| + "testing" |
| +) |
| + |
| +func TestBatchVersionsSurviveClearingAndReopen(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, e := store.Open(db, s, id) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + jobs := make([]store.CellWrite, 500) |
| + for i := range jobs { |
| + jobs[i] = store.CellWrite{Row: i, Col: 1, Raw: "1"} |
| + } |
| + a, e := h.SetBatch(jobs, actor) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + h.Release() |
| + h, e = store.Open(db, s, id) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + for i := range jobs { |
| + jobs[i].Raw = "" |
| + } |
| + b, e := h.SetBatch(jobs, actor) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + h.Release() |
| + h, e = store.Open(db, s, id) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + defer h.Release() |
| + c, e := h.SetBatch([]store.CellWrite{{Row: 0, Col: 1, Raw: "2"}}, actor) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + if c.Version <= b.Version { |
| + t.Fatalf("version regression: paste=%d clear=%d next edit=%d; browser discards latest edit", a.Version, b.Version, c.Version) |
| + } |
| +} |
| + |
| +func TestSingleCellVersionSurvivesLastCellClear(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + first, err := h.SetReport(rowA, colA, "8", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + h.Release() |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cleared, err := h.SetReport(rowA, colA, "", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + h.Release() |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + next, err := h.SetReport(rowA, colA, "9", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if cleared.Version <= first.Version || next.Version <= cleared.Version { |
| + t.Fatalf("versions regressed: first=%d cleared=%d next=%d", first.Version, cleared.Version, next.Version) |
| + } |
| +} |
| + |
| +func TestBlankCrashRetainsCommittedVersion(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + allocated := h.Version() |
| + reset := store.InterruptBlankAfterFirstTransaction(func() error { return errors.New("simulated crash") }) |
| + _, err = h.SetReport(rowA, colA, "", actor) |
| + reset() |
| + h.Release() |
| + if err == nil { |
| + t.Fatal("crash injection failed") |
| + } |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + next, err := h.SetReport(rowA, colA, "9", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if next.Version <= allocated { |
| + t.Fatalf("committed blank stamp=%d next=%d", allocated, next.Version) |
| + } |
| +} |
| + |
| +func TestSnapshotVersionSurvivesReopen(t *testing.T) { |
| + for _, clear := range []bool{false, true} { |
| + t.Run(map[bool]string{false: "populated", true: "cleared"}[clear], func(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if clear { |
| + if err = h.Set(rowA, colA, "", actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + snapshot := h.Version() |
| + h.Release() |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + rep, err := h.SetReport(rowA, colA, "9", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if rep.Version < snapshot { |
| + t.Fatalf("snapshot %d exceeds new edit %d", snapshot, rep.Version) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +func TestRetainedHandleBatchRollbackKeepsDurableVersion(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err = h.Set(rowA, colA, "", actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + var before uint64 |
| + if err = db.Raw("SELECT content_version FROM tabs WHERE id = ?", id).Scan(&before).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if h.Version() <= before { |
| + t.Fatal("fixture did not retain an unused stamp") |
| + } |
| + rep, err := h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: "8"}}, actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err = db.Raw("SELECT content_version FROM tabs WHERE id = ?", id).Scan(&before).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + calls := 0 |
| + if err = db.Callback().Create().Before("gorm:create").Register("watermark-failure", func(tx *gorm.DB) { |
| + if tx.Statement.Table == "events" { |
| + calls++ |
| + if calls == 3 { |
| + tx.AddError(errors.New("injected failure")) |
| + } |
| + } |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, err = h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: ""}, {Row: 0, Col: 1, Raw: "9"}}, actor) |
| + db.Callback().Create().Remove("watermark-failure") |
| + if err == nil { |
| + t.Fatal("batch failure not injected") |
| + } |
| + var after uint64 |
| + if err = db.Raw("SELECT content_version FROM tabs WHERE id = ?", id).Scan(&after).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + if after != before { |
| + t.Fatalf("watermark survived rollback: %d -> %d", before, after) |
| + } |
| + h.Release() |
| + h, err = store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + next, err := h.SetReport(rowA, colA, "10", actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if next.Version <= rep.Version { |
| + t.Fatalf("last successful %d >= reopened edit %d", rep.Version, next.Version) |
| + } |
| +} |
diff --git a/internal/sheets/store/handle_write_batch.go b/internal/sheets/store/handle_write_batch.go| new file mode 100644 |
| index 0000000..f3a2378 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_write_batch.go |
| @@ -0,0 +1,96 @@ |
| +package store |
| + |
| +import ( |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| + "errors" |
| + |
| + "gorm.io/gorm" |
| +) |
| + |
| +const MaxCellBatch = 500 |
| + |
| +type CellWrite struct { |
| + Row, Col int |
| + Raw string |
| +} |
| + |
| +var errCellBatch = errors.New("store: invalid cell batch") |
| + |
| +func (h *Handle) SetBatch(writes []CellWrite, actor uint64) (Report, error) { |
| + if len(writes) == 0 || len(writes) > MaxCellBatch { |
| + return Report{}, errCellBatch |
| + } |
| + seen := make(map[[2]int]bool, len(writes)) |
| + for _, w := range writes { |
| + pos := [2]int{w.Row, w.Col} |
| + if w.Row < 0 || w.Col < 0 || seen[pos] { |
| + return Report{}, errCellBatch |
| + } |
| + seen[pos] = true |
| + if _, _, err := contentNode(w.Raw); err != nil { |
| + return Report{}, errCellBatch |
| + } |
| + } |
| + h.mu.Lock() |
| + defer h.mu.Unlock() |
| + |
| + var staged *Handle |
| + var result Report |
| + err := h.db.Transaction(func(tx *gorm.DB) error { |
| + var err error |
| + staged, err = buildHandle(tx, h.sealer, h.tabID) |
| + if err != nil { |
| + return err |
| + } |
| + if staged.nextVersion < h.nextVersion { |
| + staged.nextVersion = h.nextVersion |
| + } |
| + changed := make(map[CellCoord]ChangedCell) |
| + direct := make(map[CellCoord]bool) |
| + for _, w := range writes { |
| + r, err := staged.EnsureRow(w.Row) |
| + if err != nil { |
| + return err |
| + } |
| + c, err := staged.EnsureCol(w.Col) |
| + if err != nil { |
| + return err |
| + } |
| + rep, err := staged.SetReport(r, c, w.Raw, actor) |
| + if err != nil { |
| + return err |
| + } |
| + if rep.Capped { |
| + return errCellBatch |
| + } |
| + direct[CellCoord{r, c}] = true |
| + result.Version = rep.Version |
| + for _, cell := range rep.Cells { |
| + changed[CellCoord{cell.RowID, cell.ColID}] = cell |
| + } |
| + if len(changed) > calc.MaxDirtyPerPass { |
| + return errCellBatch |
| + } |
| + } |
| + var count int64 |
| + if err := tx.Model(&Cell{}).Where("tab_id = ?", h.tabID).Count(&count).Error; err != nil { |
| + return err |
| + } |
| + if count > MaxCellsPerTab { |
| + return ErrTabTooLarge |
| + } |
| + for coord, cell := range changed { |
| + cell.Derived = !direct[coord] |
| + result.Cells = append(result.Cells, cell) |
| + } |
| + return nil |
| + }) |
| + if err != nil { |
| + return Report{}, err |
| + } |
| + h.graph, h.rawText, h.keyToIDs = staged.graph, staged.rawText, staged.keyToIDs |
| + h.rowPos, h.colPos = staged.rowPos, staged.colPos |
| + h.rowByPos, h.colByPos = staged.rowByPos, staged.colByPos |
| + h.nextVersion = staged.nextVersion |
| + return result, nil |
| +} |
diff --git a/internal/sheets/store/handle_write_batch_test.go b/internal/sheets/store/handle_write_batch_test.go| new file mode 100644 |
| index 0000000..59fec8f |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_write_batch_test.go |
| @@ -0,0 +1,251 @@ |
| +package store_test |
| + |
| +import ( |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| + "amadan.net/oficina/sheets/internal/sheets/sealtest" |
| + "errors" |
| + "os" |
| + "path/filepath" |
| + "strconv" |
| + "testing" |
| + "time" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| + "gorm.io/gorm" |
| +) |
| + |
| +func TestCellBatchFinalValuesAndDirectOrigins(t *testing.T) { |
| + h := openSeeded(t) |
| + before := h.Version() |
| + rep, err := h.SetBatch([]store.CellWrite{{Row: 1, Col: 0, Raw: "=A1*2"}, {Row: 0, Col: 0, Raw: "21"}}, actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if rep.Version <= before || h.Version() <= rep.Version { |
| + t.Fatalf("versions %d %d %d", before, rep.Version, h.Version()) |
| + } |
| + for _, c := range rep.Cells { |
| + if c.Derived { |
| + t.Fatalf("edited origin marked derived: %+v", c) |
| + } |
| + } |
| + if got := findCell(t, rep, rowB, colA); got.Value.Num != 42 { |
| + t.Fatalf("final formula = %+v", got) |
| + } |
| + rep, err = h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: ""}}, actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if c := findCell(t, rep, rowB, colA); c.Value.Num != 0 || !c.Derived { |
| + t.Fatalf("clear dependent = %+v", c) |
| + } |
| +} |
| + |
| +func TestCellBatchRollsBackStorageHistoryAxesAndLiveHandle(t *testing.T) { |
| + db, sealer, id := seeded(t) |
| + h, err := store.Open(db, sealer, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + other, err := store.Open(db, sealer, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer other.Release() |
| + before := h.Version() |
| + var history int64 |
| + db.Model(&store.Event{}).Count(&history) |
| + calls := 0 |
| + if err := db.Callback().Create().Before("gorm:create").Register("batch-failure", func(tx *gorm.DB) { |
| + if tx.Statement.Table == "events" { |
| + calls++ |
| + if calls == 3 { |
| + tx.AddError(errors.New("injected failure")) |
| + } |
| + } |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + defer db.Callback().Create().Remove("batch-failure") |
| + _, err = h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: "21"}, {Row: 30, Col: 40, Raw: "99"}}, actor) |
| + if err == nil { |
| + t.Fatal("injected persistence failure accepted") |
| + } |
| + if got := other.Value(rowA, colA).Num; got != 5 { |
| + t.Fatalf("live graph changed: %v", got) |
| + } |
| + if h.Version() != before { |
| + t.Fatal("failed batch advanced version") |
| + } |
| + var now int64 |
| + db.Model(&store.Event{}).Count(&now) |
| + if now != history { |
| + t.Fatalf("history %d -> %d", history, now) |
| + } |
| + db.Model(&store.Row{}).Where("tab_id = ? AND position = ?", id, 30).Count(&now) |
| + if now != 0 { |
| + t.Fatal("row survived rollback") |
| + } |
| + db.Model(&store.Col{}).Where("tab_id = ? AND position = ?", id, 40).Count(&now) |
| + if now != 0 { |
| + t.Fatal("column survived rollback") |
| + } |
| + db.Callback().Create().Remove("batch-failure") |
| + rep, err := h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: "8"}}, actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if findCell(t, rep, rowA, colA).Value.Num != 8 || other.Value(rowA, colA).Num != 8 { |
| + t.Fatal("live handle failed to recover") |
| + } |
| +} |
| + |
| +func TestCellBatchValidatesAllInputsBeforeWriting(t *testing.T) { |
| + for _, writes := range [][]store.CellWrite{ |
| + nil, |
| + {{Row: 0, Col: 0, Raw: "7"}, {Row: 30, Col: 20, Raw: "=SUM(("}}, |
| + {{Row: 0, Col: 0, Raw: "7"}, {Row: 0, Col: 0, Raw: "8"}}, |
| + {{Row: -1, Col: 0, Raw: "7"}}, |
| + make([]store.CellWrite, store.MaxCellBatch+1), |
| + } { |
| + h := openSeeded(t) |
| + v := h.Version() |
| + if _, err := h.SetBatch(writes, actor); err == nil { |
| + t.Fatal("invalid batch accepted") |
| + } |
| + if h.Value(rowA, colA).Num != 5 || h.Version() != v { |
| + t.Fatal("invalid batch changed cell") |
| + } |
| + } |
| +} |
| + |
| +func TestCellBatchFiveHundredCells(t *testing.T) { |
| + h := openSeeded(t) |
| + writes := make([]store.CellWrite, store.MaxCellBatch) |
| + for i := range writes { |
| + writes[i] = store.CellWrite{Row: i, Col: 1, Raw: "=A1*2"} |
| + } |
| + start := time.Now() |
| + rep, err := h.SetBatch(writes, actor) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(rep.Cells) != store.MaxCellBatch { |
| + t.Fatalf("got %d cells", len(rep.Cells)) |
| + } |
| + for _, c := range rep.Cells { |
| + if c.Value.Num != 10 { |
| + t.Fatalf("value = %+v", c) |
| + } |
| + } |
| + t.Logf("500-cell atomic batch: %s", time.Since(start)) |
| +} |
| + |
| +func TestCellBatchKeepsCellsAndHistorySealed(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "test.db") |
| + db := openTestDB(t, path) |
| + sealer, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := db.Create(&store.Tab{ID: tabID, SheetID: 1}).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + h, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + if _, err := h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: string(sealtest.Canaries[0])}, {Row: 1, Col: 0, Raw: "=A1"}}, actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + sealtest.AssertNoPlaintext(t, path) |
| +} |
| + |
| +func TestCellBatchRecoversStaleCacheInsideTransaction(t *testing.T) { |
| + db, s, id := seeded(t) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + if err := h.Set(rowB, colA, "=A1*2", actor); err != nil { |
| + t.Fatal(err) |
| + } |
| + sealed, err := s.Seal("cell.input", []byte("9")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := db.Model(&store.Cell{}).Where("tab_id = ? AND row_id = ? AND col_id = ?", id, rowA, colA).Updates(map[string]any{"input_sealed": sealed, "input_version": h.Version() + 1}).Error; err != nil { |
| + t.Fatal(err) |
| + } |
| + done := make(chan error, 1) |
| + go func() { _, err := h.SetBatch([]store.CellWrite{{Row: 0, Col: 1, Raw: "=A2"}}, actor); done <- err }() |
| + select { |
| + case err := <-done: |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + case <-time.After(3 * time.Second): |
| + panic("batch recovery deadlocked") |
| + } |
| + if h.Value(rowB, colA).Num != 18 { |
| + t.Fatal("stale dependent not recovered") |
| + } |
| +} |
| + |
| +func TestCellBatchLargeTabTiming(t *testing.T) { |
| + raw := os.Getenv("CELL_BATCH_BENCH_N") |
| + if raw == "" { |
| + t.Skip("set CELL_BATCH_BENCH_N to measure full-tab rebuild") |
| + } |
| + n, err := strconv.Atoi(raw) |
| + if err != nil || n < 501 || n > store.MaxCellsPerTab { |
| + t.Fatal("invalid CELL_BATCH_BENCH_N") |
| + } |
| + db := openTestDB(t, filepath.Join(t.TempDir(), "test.db")) |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + id := tabID |
| + seedBlankRecoveryFixture(t, db, s, id, rowA, colA, n-1) |
| + h, err := store.Open(db, s, id) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + writes := make([]store.CellWrite, 500) |
| + for i := range writes { |
| + writes[i] = store.CellWrite{Row: i + 1, Col: 0, Raw: "8"} |
| + } |
| + start := time.Now() |
| + _, err = h.SetBatch(writes, actor) |
| + elapsed := time.Since(start) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Logf("%d-cell tab, 500 writes with transaction rebuild: %s", n, elapsed) |
| +} |
| + |
| +func TestCellBatchRejectsMergedReportPastCalculationCap(t *testing.T) { |
| + db := openTestDB(t, filepath.Join(t.TempDir(), "test.db")) |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + seedBlankRecoveryFixture(t, db, s, tabID, rowA, colA, 4999) |
| + h, err := store.Open(db, s, tabID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + _, err = h.SetBatch([]store.CellWrite{{Row: 0, Col: 0, Raw: "7"}, {Row: 0, Col: 1, Raw: "8"}}, actor) |
| + if err == nil { |
| + t.Fatal("oversized merged report accepted") |
| + } |
| + if h.Value(rowA, colA).Num != 5 { |
| + t.Fatal("oversized report committed partial write") |
| + } |
| +} |
diff --git a/internal/sheets/store/models.go b/internal/sheets/store/models.go| index 0faed88..9bd5195 100644 |
| --- a/internal/sheets/store/models.go |
| +++ b/internal/sheets/store/models.go |
| @@ -52,12 +52,13 @@ type Sheet struct { |
| // Tab is one sheet within a document — the tabs along the bottom. |
| type Tab struct { |
| - ID uint64 `gorm:"primaryKey"` |
| - SheetID uint64 `gorm:"index"` |
| - NameSealed []byte // seal purpose "tab.name" |
| - Position int |
| - FrozenRows int |
| - FrozenCols int |
| + ID uint64 `gorm:"primaryKey"` |
| + SheetID uint64 `gorm:"index"` |
| + NameSealed []byte // seal purpose "tab.name" |
| + Position int |
| + FrozenRows int |
| + FrozenCols int |
| + ContentVersion uint64 `gorm:"not null;default:0"` |
| } |
| // Row and Col carry stable identity. Position is a property of the record, |
diff --git a/internal/sheets/templates/home_link.html b/internal/sheets/templates/home_link.html| new file mode 100644 |
| index 0000000..db66ed9 |
| --- /dev/null |
| +++ b/internal/sheets/templates/home_link.html |
| @@ -0,0 +1 @@ |
| +{{define "content"}}{{template "home-connect" .Data}}{{end}} |
diff --git a/internal/sheets/templates/home_oauth.html b/internal/sheets/templates/home_oauth.html| new file mode 100644 |
| index 0000000..ad744a9 |
| --- /dev/null |
| +++ b/internal/sheets/templates/home_oauth.html |
| @@ -0,0 +1,2 @@ |
| +{{define "head"}}{{template "home-oauth-head" .Data}}{{end}} |
| +{{define "content"}}{{template "home-oauth" .Data}}{{end}} |
diff --git a/internal/sheets/templates/import.html b/internal/sheets/templates/import.html| new file mode 100644 |
| index 0000000..c3ec8da |
| --- /dev/null |
| +++ b/internal/sheets/templates/import.html |
| @@ -0,0 +1,53 @@ |
| +{{define "head"}} |
| +{{/* Drag and drop only. Everything the drop zone does, the form below |
| + it already does with scripting off — drop.js fills that same file |
| + input and submits that same form, so there is one route, one CSRF |
| + check and one code path either way. |
| + |
| + This moved here with the form it decorates. It used to sit on the |
| + sheet list, which is where the import form used to be. */}} |
| +<script defer src="{{asset "static/drop.js"}}"></script> |
| +{{end}} |
| + |
| +{{define "content"}} |
| +<header rst-page-header> |
| +<div rst-page-header-titles> |
| +<h1>Import a spreadsheet</h1> |
| +{{/* The limits are stated BEFORE the import, not discovered after it. |
| + Import reads one worksheet (xlsx.Import), and a workbook with |
| + three of them would otherwise arrive looking whole — the quiet |
| + kind of wrong PRODUCT.md's first principle forbids. |
| + |
| + They are a subhead here rather than a hint under the field |
| + because this page has the room the dropdown they nearly went in |
| + did not, and because drop.js overwrites the hint to speak about |
| + a drop — which would take the file limits down with it. */}} |
| +<p rst-page-header-sub>Excel .xlsx or .xlsm, up to 20 MB. The first |
| +worksheet is imported.</p> |
| +</div> |
| +</header> |
| + |
| +{{if .Data.Error}} |
| +{{/* A refusal renders HERE, on the page holding the form, rather than |
| + back on the sheet list. The list is not where the person is and |
| + no longer carries a form to try again with. */}} |
| +<div rst-callout rst-tone="negative"> |
| +<div rst-callout-body> |
| +<p>{{.Data.Error}}</p> |
| +</div> |
| +</div> |
| +{{end}} |
| + |
| +<form rst-form method="post" action="/sheets/import" enctype="multipart/form-data"> |
| +<div data-drop> |
| +<div rst-field-row> |
| +<div rst-field> |
| +<label rst-field-label for="file">Choose a file</label> |
| +<input rst-input id="file" name="file" type="file" accept=".xlsx,.xlsm"> |
| +<p rst-field-hint data-drop-hint>You can also drag a file onto this box.</p> |
| +</div> |
| +<button rst-btn="primary" type="submit">Import</button> |
| +</div> |
| +</div> |
| +</form> |
| +{{end}} |
diff --git a/internal/sheets/templates/layout.html b/internal/sheets/templates/layout.html| index 6920a26..de3b604 100644 |
| --- a/internal/sheets/templates/layout.html |
| +++ b/internal/sheets/templates/layout.html |
| @@ -75,9 +75,28 @@ |
| </details> |
| {{end}} |
| <div rst-shell-tail> |
| -<nav rst-shell-nav> |
| -{{if .SignedIn}}<a href="/sheets">Your sheets</a>{{end}} |
| -</nav> |
| +{{/* There is no nav here, and the empty landmark went with the link. |
| + |
| + It held one item, "Your sheets", pointing at /sheets — which is |
| + where the brand two elements to the left already goes: handleIndex |
| + 303s a signed-in visitor from / to /sheets. So the bar read |
| + "Sheets ... Your sheets" and both were the same destination under |
| + two names, which is a menu of one choice offered twice. |
| + |
| + The brand is the way back to the list. That is what a wordmark in |
| + the top-left is for, and it is one fewer thing to read. */}} |
| +{{/* A page's own top-bar actions, beside the account menu. |
| + |
| + Deliberately a block rather than a field on PageData, and |
| + deliberately empty by default: the sheet list fills it with Create |
| + and Import, and the grid page fills it with nothing, because the |
| + grid already carries a File menu holding both and a second Create |
| + in the bar above it would be the same verb twice on one screen. |
| + |
| + Inside the tail, so it collapses behind the narrow-viewport Menu |
| + disclosure along with the nav and the account rather than being |
| + the one control still floating in the bar at 360px. */}} |
| +{{block "topbaractions" .}}{{end}} |
| {{if .SignedIn}} |
| <details rst-dropdown rst-shell-account name="rst-menus"> |
| <summary>{{.Address}}<span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary> |
| @@ -92,6 +111,7 @@ |
| <button rst-btn="ghost" type="submit" name="mode" value="dark"{{if eq .Mode "dark"}} aria-current="true"{{end}}>Dark</button> |
| <button rst-btn="ghost" type="submit" name="mode" value="auto"{{if eq .Mode "auto"}} aria-current="true"{{end}}>System</button> |
| </form> |
| +{{if .HomeLinkLabel}}<a href="/account/home">{{.HomeLinkLabel}}</a>{{end}} |
| <form method="post" action="/signout"> |
| <button rst-btn="ghost" type="submit">Sign out</button> |
| </form> |
diff --git a/internal/sheets/templates/sheet.html b/internal/sheets/templates/sheet.html| index e29d0d0..88091f4 100644 |
| --- a/internal/sheets/templates/sheet.html |
| +++ b/internal/sheets/templates/sheet.html |
| @@ -23,7 +23,37 @@ |
| {{end}} |
| {{define "doctitle"}} |
| -<span data-doc-title title="{{.Data.Sheet.Title}}">{{.Data.Sheet.Title}}</span> |
| +{{/* The open document's title, and — for anyone who may edit it — the |
| + rename. |
| + |
| + This was static text, under a comment saying a rename route did |
| + not exist and that anything looking editable would be a promise |
| + the app could not keep. The route exists now, and it had to: the |
| + sheet list's Create button stopped asking for a title, so every |
| + sheet is born "Untitled sheet" and this is the only place the |
| + name is ever fixed. See handlers_rename.go. |
| + |
| + An inline form, not a popover. A <details> here would join |
| + rastrillo's "rst-menus" exclusive group along with the File menu |
| + and the account dropdown — opening one would close the others, |
| + and a rename that shuts the menu you opened it from is the |
| + nested-accordion trap TestNoNestedExclusiveDetails exists to |
| + catch. An input has no such problem. |
| + |
| + Scriptless by construction: it is a real form with a real submit. |
| + The button is visually hidden until the input has focus, so the |
| + bar stays a title bar until somebody means to edit it, and Enter |
| + submits from the input either way. */}} |
| +{{if .Data.MayWrite}} |
| +<form data-doc-title method="post" action="/sheets/{{.Data.Sheet.ID}}/rename"> |
| +<input data-doc-title-input name="title" value="{{.Data.Sheet.Title}}" |
| + aria-label="Sheet title" autocomplete="off" spellcheck="false"> |
| +<button rst-btn="ghost" type="submit">Rename</button> |
| +</form> |
| +{{else}} |
| +<span data-doc-title><span data-doc-title-text |
| + title="{{.Data.Sheet.Title}}">{{.Data.Sheet.Title}}</span></span> |
| +{{end}} |
| {{end}} |
| {{define "main"}} |
| @@ -92,7 +122,11 @@ |
| <summary>File</summary> |
| <div rst-dropdown-menu> |
| <form method="post" action="/sheets"><button type="submit">New sheet</button></form> |
| -<a href="/sheets">Import a spreadsheet…</a> |
| +{{/* A page, not a file input in this dropdown. A hidden input behind a |
| + menu-shaped label never submits itself, so the menu item would |
| + have needed script to import anything at all — see |
| + handleImportPage. */}} |
| +<a href="/sheets/import">Import a spreadsheet…</a> |
| <a href="/sheets/{{.Data.Sheet.ID}}/export.xlsx">Download as .xlsx</a> |
| <hr> |
| <a href="/sheets/{{.Data.Sheet.ID}}/table">Open as table</a> |
diff --git a/internal/sheets/templates/sheets.html b/internal/sheets/templates/sheets.html| index 2dbef11..4844b61 100644 |
| --- a/internal/sheets/templates/sheets.html |
| +++ b/internal/sheets/templates/sheets.html |
| @@ -1,64 +1,66 @@ |
| +{{/* The verbs live in the top bar; the page is the list. |
| + |
| + This screen used to open with two stacked forms — a title field |
| + with a Create button, then a dashed drop zone with a file input and |
| + four lines of limits — and the sheets themselves began below the |
| + fold of both. A list of sheets should be a list of sheets. |
| + |
| + Create is a button beside the account menu now, and makes a sheet |
| + called "Untitled sheet" in one click; the name is fixed afterwards |
| + by renaming it in the top bar of the sheet itself, which is what |
| + handlers_rename.go is for. Import moved to a page of its own at |
| + /sheets/import — see handleImportPage for why a page and not an |
| + item in the File menu. */}} |
| {{define "head"}} |
| -{{/* Drag and drop only. Everything the drop zone does, the form below |
| - it already does with scripting off — drop.js fills that same file |
| - input and submits that same form, so there is one route, one CSRF |
| - check and one code path either way. */}} |
| +{{/* Drag and drop, and only that. The overlay at the foot of this file |
| + is hidden until a file is dragged over the window, and every drop |
| + ends as a submit of the same form /sheets/import serves — same |
| + route, same CSRF check, same job. |
| + |
| + With scripting off it never appears and the Import link in the top |
| + bar is the entire feature. That is what AGENTS.md's "every |
| + drag-only affordance needs a menu equivalent" asks for: the link |
| + is the floor, and the drag is a shortcut to it. */}} |
| <script defer src="{{asset "static/drop.js"}}"></script> |
| {{end}} |
| +{{define "topbaractions"}} |
| +{{if .SignedIn}} |
| +{{/* The wrapper is what carries the auto margin that puts these |
| + against the account menu — see sheets.css. It lives here rather |
| + than in layout.html so that a page which fills this block with |
| + nothing contributes no element at all, and the grid's bar is |
| + exactly what it was. */}} |
| +<div data-topbar-actions> |
| +{{/* Import first, Create last: Create is the primary action and sits |
| + closest to the account menu, which is the corner the eye returns |
| + to. A link rather than a form, because it opens a page rather |
| + than changing anything. */}} |
| +<a rst-btn href="/sheets/import">Import</a> |
| +{{/* No title field, and so no dialog and no second step: the button |
| + IS the create. handleSheetCreate defaults an empty title to |
| + "Untitled sheet" and 303s into the new sheet, so a click lands |
| + you in a grid you can type into. */}} |
| +<form method="post" action="/sheets"> |
| +<button rst-btn="primary" type="submit">Create</button> |
| +</form> |
| +</div> |
| +{{end}} |
| +{{end}} |
| + |
| {{define "content"}} |
| <header rst-page-header> |
| <div rst-page-header-titles> |
| -<h1>Your sheets</h1> |
| +{{/* "All sheets", not "Your sheets". The old heading was wrong twice: |
| + it repeated the brand in the bar directly above it, and it claimed |
| + ownership the line underneath immediately contradicts — everyone on |
| + the instance opens every sheet here, so almost none of these are |
| + yours. A heading its own subtitle corrects is worse than none. */}} |
| +<h1>All sheets</h1> |
| <p rst-page-header-sub>Everyone on this instance can open every sheet here.</p> |
| </div> |
| </header> |
| -{{if .Data.ImportError}} |
| -<div rst-callout rst-tone="negative"> |
| -<div rst-callout-body> |
| -<p>{{.Data.ImportError}}</p> |
| -</div> |
| -</div> |
| -{{end}} |
| - |
| -<form rst-form method="post" action="/sheets"> |
| -<div rst-field-row> |
| -<div rst-field> |
| -<label rst-field-label for="title">New sheet</label> |
| -<input rst-input id="title" name="title" type="text" placeholder="Untitled sheet" autocomplete="off"> |
| -</div> |
| -<button rst-btn="primary" type="submit">Create</button> |
| -</div> |
| -</form> |
| - |
| -{{/* Import. The file input and its button are the whole feature — they |
| - work with scripting off, and dropping a file onto the zone below |
| - does nothing this form does not already do. |
| - |
| - data-drop is drop.js's hook and is deliberately a data- attribute |
| - rather than an rst- one: the rst- namespace is rastrillo's, and an |
| - invented rst- attribute is a name tokens.css does not define, which |
| - is the mistake this app has already made once in CSS. */}} |
| -<form rst-form method="post" action="/sheets/import" enctype="multipart/form-data"> |
| -<div data-drop> |
| -<div rst-field-row> |
| -<div rst-field> |
| -<label rst-field-label for="file">Import a spreadsheet</label> |
| -<input rst-input id="file" name="file" type="file" accept=".xlsx,.xlsm"> |
| -{{/* The first-worksheet limit is stated BEFORE the import, not |
| - discovered after it. Import reads one worksheet (xlsx.Import), and |
| - a workbook with three of them would otherwise arrive looking whole |
| - — which is the quiet kind of wrong PRODUCT.md's first principle |
| - forbids. */}} |
| -<p rst-field-hint data-drop-hint>Excel .xlsx or .xlsm, up to 20 MB. The |
| -first worksheet is imported. You can also drag a file onto this box.</p> |
| -</div> |
| -<button rst-btn type="submit">Import</button> |
| -</div> |
| -</div> |
| -</form> |
| - |
| {{if .Data.Sheets}} |
| <ul rst-list data-sheet-list> |
| {{range .Data.Sheets}} |
| @@ -80,11 +82,80 @@ first worksheet is imported. You can also drag a file onto this box.</p> |
| {{end}} |
| </ul> |
| {{else}} |
| +{{/* The empty state points at the two controls in the top bar. It |
| + used to say "make one above", which was true when the form was |
| + directly above it and would now be a sentence pointing at a |
| + header. An empty screen is the one place in a product that has |
| + nothing to do but explain what to do next. */}} |
| <div rst-empty> |
| <p rst-empty-title>No sheets yet</p> |
| -<p rst-empty-body>Make one above and it opens with a blank grid you can type |
| -into straight away, or import an Excel file to start from what you already |
| -have.</p> |
| +<p rst-empty-body>Create one from the top bar and it opens with a blank |
| +grid you can type into straight away, or |
| +<a href="/sheets/import">import an Excel file</a> to start from what you |
| +already have.</p> |
| +</div> |
| +{{end}} |
| + |
| +{{if .SignedIn}} |
| +{{/* The drop target for the whole page. |
| + |
| + This screen used to carry a dashed box you could drop a file on, |
| + and taking the box away took the drop with it — so the drop comes |
| + back without the box. It is `hidden` at rest, contributes nothing |
| + to the layout and nothing to the tab order, and drop.js reveals it |
| + only while a drag carrying files is actually over the window. An |
| + affordance that appears exactly when it applies beats a permanent |
| + rectangle explaining that it exists. |
| + |
| + It is the same form as the one on /sheets/import, deliberately: |
| + drop.js fills this file input and submits this form, so a drop |
| + from here is the ordinary multipart POST and there is no second |
| + code path on the server. The file field is visible inside the |
| + panel rather than hidden, because a refused drop ("drop one file |
| + at a time") needs something to recover with that is not the back |
| + button. |
| + |
| + `hidden` is load-bearing and is asserted in a test: rst-modal-overlay |
| + sets display:flex, an author rule beats the UA's [hidden], and |
| + without the reinstating rule in sheets.css this would cover the |
| + list permanently. */}} |
| +<form data-drop data-drop-page hidden rst-modal-overlay |
| + method="post" action="/sheets/import" enctype="multipart/form-data"> |
| +<div rst-modal-panel> |
| +<div data-drop-page-body> |
| +<p data-drop-page-title>Drop it to import</p> |
| +{{/* The same limits /sheets/import states, and stated before the |
| + import for the same reason: one worksheet is read, and a |
| + three-sheet workbook would otherwise arrive looking whole. This |
| + is also the line drop.js rewrites to refuse a drop, which is |
| + why it is a sentence and not a list. */}} |
| +<p rst-field-hint data-drop-hint>Excel .xlsx or .xlsm, up to 20 MB. The |
| +first worksheet is imported.</p> |
| +{{/* Chooser and submit, so a refused drop has something to recover |
| + with, and so the panel is a working form rather than a picture |
| + of one. They are hidden while a drag is actually over the window |
| + — see [data-drop-page-recover] in sheets.css — because offering |
| + a file picker to somebody mid-drag is a second way to do the |
| + thing they are already doing. |
| + |
| + They carry no tabindex, and its absence is a fix rather than an |
| + omission. Both used to carry tabindex="-1", to "keep them out of |
| + the tab order while the overlay is up" — which is exactly backwards. |
| + The overlay stays up after a REFUSED drop precisely so these two |
| + controls can be used, and a negative tabindex meant a keyboard user |
| + tabbed straight past the recovery form into the list behind it. |
| + The other two states never needed the help: `hidden` on the form |
| + takes the whole subtree out of the tab order at rest, and |
| + [data-drop-page-recover] is display:none under [data-drop-over], |
| + which does the same mid-drag. So it was redundant where it was |
| + harmless and harmful where it was not. */}} |
| +<div data-drop-page-recover> |
| +<input rst-input name="file" type="file" accept=".xlsx,.xlsm" |
| + aria-label="Spreadsheet to import"> |
| +<button rst-btn="primary" type="submit">Import</button> |
| +</div> |
| </div> |
| +</div> |
| +</form> |
| {{end}} |
| {{end}} |
diff --git a/internal/sheets/theme.go b/internal/sheets/theme.go| index 3d855d4..b7b6030 100644 |
| --- a/internal/sheets/theme.go |
| +++ b/internal/sheets/theme.go |
| @@ -60,7 +60,7 @@ func modeFrom(r *http.Request) string { |
| // This is also the bug this function exists to prevent recurring. The |
| // attribute used to be handed rastrillo's THEME NAME — day, plain, |
| // signal — which are the palette files an app picks once at scaffold |
| -// (`rastrillo doctor` copies one to static/theme.css), not a per-person |
| +// (this app serves day through Estilo), not a per-person |
| // setting. Neither toggle rule matched, so an explicit choice never |
| // beat the OS and the switcher did nothing at all. Theme and scheme are |
| // orthogonal: this app ships `day` (ruling 7) and every person picks |
diff --git a/internal/sheets/theme_assets_test.go b/internal/sheets/theme_assets_test.go| new file mode 100644 |
| index 0000000..fd26cbb |
| --- /dev/null |
| +++ b/internal/sheets/theme_assets_test.go |
| @@ -0,0 +1,65 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "crypto/sha256" |
| + "fmt" |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "testing" |
| + |
| + "amadan.net/oficina/estilo.git/theme" |
| + "github.com/carlosframework/rastrillo/ui" |
| +) |
| + |
| +func TestSharedThemeAssetFingerprintAndCache(t *testing.T) { |
| + app := New(Config{}) |
| + page := do(t, app, httptest.NewRequest(http.MethodGet, "/signin", nil)) |
| + if page.Code != http.StatusOK { |
| + t.Fatalf("signin status %d", page.Code) |
| + } |
| + day, ok := ui.ThemeCSS("day") |
| + if !ok { |
| + t.Fatal("selected day missing") |
| + } |
| + body := func(css []byte) []byte { |
| + if bytes.HasPrefix(css, []byte("/*")) { |
| + if end := bytes.Index(css, []byte("*/")); end >= 0 { |
| + return css[end+2:] |
| + } |
| + } |
| + return css |
| + } |
| + if !bytes.Equal(body(theme.CSS()), body(day)) { |
| + t.Fatal("shared theme differs from selected day") |
| + } |
| + for _, item := range []struct { |
| + name string |
| + data []byte |
| + }{ |
| + {"theme", theme.CSS()}, {"tokens", ui.TokensCSS()}, |
| + } { |
| + sum := sha256.Sum256(item.data) |
| + hashed := fmt.Sprintf("/static/%s.%x.css", item.name, sum[:8]) |
| + if !strings.Contains(page.Body.String(), `href="`+hashed+`"`) { |
| + t.Fatalf("missing selected %s fingerprint", item.name) |
| + } |
| + for _, path := range []string{hashed, "/static/" + item.name + ".css", "/static/" + item.name + ".0000000000000000.css"} { |
| + res := do(t, app, httptest.NewRequest(http.MethodGet, path, nil)) |
| + if res.Code != http.StatusOK || !bytes.Equal(res.Body.Bytes(), item.data) { |
| + t.Fatalf("%s asset response mismatch status%d", item.name, res.Code) |
| + } |
| + if !strings.HasPrefix(res.Header().Get("Content-Type"), "text/css") { |
| + t.Fatal("asset MIME mismatch") |
| + } |
| + want := "no-cache" |
| + if path == hashed { |
| + want = "public, max-age=31536000, immutable" |
| + } |
| + if res.Header().Get("Cache-Control") != want { |
| + t.Fatal("asset cache policy mismatch") |
| + } |
| + } |
| + } |
| +} |
diff --git a/tools/drive/batch-checks.mjs b/tools/drive/batch-checks.mjs| new file mode 100644 |
| index 0000000..90bfe58 |
| --- /dev/null |
| +++ b/tools/drive/batch-checks.mjs |
| @@ -0,0 +1,52 @@ |
| +import assert from 'node:assert/strict'; |
| + |
| +export async function checkCellQueue(page) { |
| + const isBatch = request => request.method() === 'POST' && new URL(request.url()).pathname.endsWith('/cells/batch'); |
| + const requests = []; |
| + const record = request => { if (isBatch(request)) requests.push(request); }; |
| + const text = Array.from({length: 25}, (_, r) => Array.from({length: 20}, (_, c) => String(r * 20 + c + 1)).join('\t')).join('\n'); |
| + await page.bringToFront(); |
| + await page.evaluate(async text => navigator.clipboard.writeText(text), text); |
| + await page.locator('[data-sheet-grid] tbody tr:first-child td:nth-child(2)').click(); |
| + await page.evaluate(() => { window.SheetsGrid.worksheet().updateSelectionFromCoords(0, 0, 0, 0); window.SheetsGrid.focus(); }); |
| + page.on('request', record); |
| + try { |
| + const pasted = page.waitForRequest(isBatch); |
| + await page.keyboard.press('Control+v'); |
| + await pasted; |
| + await page.evaluate(() => window.SheetsCellQueue([], window.SheetsGrid.post)); |
| + assert.equal(requests.length, 1, 'paste must send one atomic request'); |
| + assert.equal(requests[0].postDataJSON().cells.length, 500); |
| + await page.waitForFunction(() => window.SheetsGrid.rawAt(24, 19) === '500'); |
| + } finally { |
| + page.off('request', record); |
| + } |
| + |
| + assert.equal(await page.evaluate(() => window.SheetsGrid.write(24, 20, '501')), true); |
| + await page.evaluate(() => { window.SheetsGrid.worksheet().updateSelectionFromCoords(0, 0, 20, 24); window.SheetsGrid.focus(); }); |
| + const count = await page.evaluate(() => { |
| + let n = 0; |
| + for (let r = 0; r < 25; r++) for (let c = 0; c < 21; c++) if (window.SheetsGrid.rawAt(r, c) !== '') n++; |
| + return n; |
| + }); |
| + assert.ok(count > 500, 'cut must need at least two chunks'); |
| + const cuts = []; |
| + const refuse = async route => { |
| + cuts.push(route.request().postDataJSON()); |
| + await route.fulfill({status: 500, contentType: 'text/plain', body: ''}); |
| + }; |
| + await page.route('**/cells/batch', refuse); |
| + try { |
| + const attempted = page.waitForRequest(isBatch); |
| + await page.keyboard.press('Control+x'); |
| + await attempted; |
| + await page.evaluate(() => window.SheetsCellQueue([], window.SheetsGrid.post)); |
| + assert.equal(cuts.length, 1, 'failed first chunk must stop the cut queue'); |
| + assert.equal(cuts[0].cells.length, 500); |
| + assert.ok(cuts[0].cells.every(cell => cell.raw === '')); |
| + await page.waitForFunction(() => window.SheetsGrid.rawAt(0, 0) === '1' && window.SheetsGrid.rawAt(24, 20) === '501'); |
| + } finally { |
| + await page.unroute('**/cells/batch', refuse); |
| + } |
| + return {pasteRequests: 1, pasteCells: 500, failedCutRequests: 1, failedCutStopped: true}; |
| +} |
diff --git a/tools/drive/canary-batch.mjs b/tools/drive/canary-batch.mjs| new file mode 100644 |
| index 0000000..e37b64f |
| --- /dev/null |
| +++ b/tools/drive/canary-batch.mjs |
| @@ -0,0 +1,75 @@ |
| +import assert from 'node:assert/strict'; |
| +import fs from 'node:fs'; |
| +import { checkCellQueue } from './batch-checks.mjs'; |
| +const pw = await import(process.env.PLAYWRIGHT || 'playwright'); |
| +const base = process.env.SHEETS_CANARY_ORIGIN; |
| +const state = process.env.SHEETS_CANARY_AUTH_STATE; |
| +if (base !== 'https://overnight-sheets.bds.oncarlos.com' || !state) throw new Error('Missing HTTPS canary origin or authentication state'); |
| +if ((fs.statSync(state).mode & 0o077) !== 0) throw new Error('Authentication state must be private'); |
| +const expected = process.env.SHEETS_CANARY_VERSION; |
| +if (!/^[a-f0-9]{7,40}$/.test(expected || '')) throw new Error('Expected candidate version is required'); |
| +const browser = await pw.chromium.launch({headless:true,executablePath:process.env.CHROMIUM_EXECUTABLE_PATH}); |
| +try { |
| + const context = await browser.newContext({storageState:state, permissions:['clipboard-read','clipboard-write']}); |
| + let signinPosted = false; |
| + await context.route('**/signin', route => { |
| + if (route.request().method() === 'POST') { signinPosted = true; return route.abort(); } |
| + return route.continue(); |
| + }); |
| + const version = await context.request.get(base+'/api/version'); |
| + assert.equal((await version.text()).trim(), expected); |
| + const page = await context.newPage(); |
| + const errors=[];page.on('pageerror',e=>errors.push(e.message)); |
| + await page.goto(base+'/sheets'); |
| + assert.equal(new URL(page.url()).pathname,'/sheets'); |
| + const location = await page.evaluate(async()=>{ |
| + const r=await fetch('/sheets',{method:'POST',body:new URLSearchParams({title:'Atomic batch canary '+Date.now()})}); |
| + if(!r.ok)throw new Error('Fixture creation failed');return r.url; |
| + }); |
| + await page.goto(location); |
| + await page.waitForFunction(()=>window.SheetsGrid?.worksheet()); |
| + const ids=await page.locator('[data-sheet-surface]').evaluate(el=>({sheet:el.dataset.sheetId,tab:el.dataset.tabId})); |
| + const api=`/api/sheets/${ids.sheet}/tabs/${ids.tab}/cells`; |
| + const second=await context.newPage();second.on('pageerror',e=>errors.push(e.message)); |
| + await second.goto(location);await second.waitForFunction(()=>window.SheetsGrid?.worksheet()); |
| + const requests=[];page.on('request',r=>{if(r.method()==='POST'&&r.url().includes('/cells'))requests.push(r.url());}); |
| + const text=Array.from({length:25},(_,r)=>Array.from({length:20},(_,c)=>String(r*20+c+1)).join('\t')).join('\n'); |
| + await page.bringToFront(); |
| + await page.evaluate(async text=>{await navigator.clipboard.writeText(text);},text); |
| + await page.locator('[data-sheet-grid] tbody tr:first-child td:nth-child(2)').click(); |
| + const start=Date.now(); |
| + await page.keyboard.press('Control+v'); |
| + await page.waitForFunction(()=>window.SheetsGrid.rawAt(24,19)==='500'); |
| + assert.equal(requests.length,1);assert.ok(requests[0].endsWith('/cells/batch')); |
| + const pasteMs=Date.now()-start; |
| + await second.bringToFront(); |
| + await second.waitForFunction(()=>window.SheetsGrid.rawAt(24,19)==='500'); |
| + await page.bringToFront(); |
| + await page.evaluate(()=>{window.SheetsGrid.worksheet().updateSelectionFromCoords(0,0,19,24);window.SheetsGrid.focus();}); |
| + await page.keyboard.press('Control+x'); |
| + await page.waitForFunction(()=>window.SheetsGrid.rawAt(0,0)===''&&window.SheetsGrid.rawAt(24,19)===''); |
| + await page.evaluate(async text=>{await navigator.clipboard.writeText(text);},text); |
| + await page.locator('[data-sheet-grid] tbody tr:first-child td:nth-child(2)').click(); |
| + await page.keyboard.press('Control+v'); |
| + await page.waitForFunction(()=>window.SheetsGrid.rawAt(24,19)==='500'); |
| + const snap=await page.evaluate(async api=>{const r=await fetch(api);return r.json();},api); |
| + assert.equal(snap.cells.length,500); |
| + const before=snap.cells.map(c=>[c.r,c.c,c.raw]).sort(); |
| + const refused=await page.evaluate(async api=>{ |
| + const r=await fetch(api+'/batch',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({cells:[{r:0,c:0,raw:'99999'},{r:24,c:19,raw:'=SUM(('}]})});return r.status; |
| + },api); |
| + assert.equal(refused,400); |
| + const after=await page.evaluate(async api=>{const r=await fetch(api);return r.json();},api); |
| + assert.deepEqual(after.cells.map(c=>[c.r,c.c,c.raw]).sort(),before); |
| + await page.reload();await page.waitForFunction(()=>window.SheetsGrid?.worksheet()&&window.SheetsGrid.rawAt(24,19)==='500'); |
| + const nojs=await browser.newContext({storageState:state,javaScriptEnabled:false}); |
| + const table=await nojs.newPage();await table.goto(location+'/table'); |
| + assert.equal(await table.locator('a[aria-label="Edit T25"]').innerText(),'500'); |
| + assert.equal(await table.locator('a[aria-label="Edit A1"]').innerText(),'1'); |
| + const queue = await checkCellQueue(page); |
| + assert.equal(signinPosted, false); |
| + assert.deepEqual(errors,[]); |
| + const result={origin:base,version:expected,queue,sheet:ids.sheet,tab:ids.tab,pasteCells:500,pasteRequests:1,pasteMs,secondPageObserved:true,clearThenPasteVisible:true,invalidBatchRolledBack:true,reloadPreserved:true,scriptlessTableLoaded:true}; |
| + if(process.env.SHEETS_CANARY_RESULT)fs.writeFileSync(process.env.SHEETS_CANARY_RESULT,JSON.stringify(result,null,2)); |
| + console.log(JSON.stringify(result)); |
| +} finally {await browser.close();} |
diff --git a/tools/drive/chrome.mjs b/tools/drive/chrome.mjs| index 65083a8..95aaa11 100644 |
| --- a/tools/drive/chrome.mjs |
| +++ b/tools/drive/chrome.mjs |
| @@ -63,9 +63,23 @@ if (!m) { console.error('no magic link in the log'); process.exit(1); } |
| await page.goto(m[0]); |
| await page.waitForURL(/\/sheets/); |
| -/* A sheet with something in it. */ |
| -await page.fill('#title', 'Commission Q2'); |
| -await Promise.all([page.waitForURL(/\/sheets\/\d+$/), page.click('form[action="/sheets"] button[type=submit]')]); |
| +/* A sheet with something in it. |
| + * |
| + * Created from the top bar's Create button, then named by renaming it. |
| + * The list page's #title field is gone: it carries no forms now, Create |
| + * posts an empty one and the sheet lands as "Untitled sheet", so the |
| + * only place a sheet is ever named is the title in the top bar. The |
| + * rename is a real form submit, so driving it here covers the |
| + * scriptless path as well as the button. */ |
| +await Promise.all([ |
| + page.waitForURL(/\/sheets\/\d+$/), |
| + page.click('[rst-shell-tail] form[action="/sheets"] button[type=submit]'), |
| +]); |
| +await page.fill('[data-doc-title-input]', 'Commission Q2'); |
| +await Promise.all([ |
| + page.waitForURL(/\/sheets\/\d+$/), |
| + page.press('[data-doc-title-input]', 'Enter'), |
| +]); |
| const url = page.url(); |
| await page.waitForFunction(() => window.SheetsGrid && window.SheetsGrid.worksheet()); |
diff --git a/tools/drive/clipboard.mjs b/tools/drive/clipboard.mjs| index 61d8f16..b3e977e 100644 |
| --- a/tools/drive/clipboard.mjs |
| +++ b/tools/drive/clipboard.mjs |
| @@ -17,13 +17,14 @@ |
| * SHEETS_SOCK=$D/s.sock SHEETS_LOG=$D/s.log \ |
| * PLAYWRIGHT=/path/to/playwright/index.js node tools/drive/clipboard.mjs |
| */ |
| +import { checkCellQueue } from './batch-checks.mjs'; |
| import net from 'node:net'; |
| import fs from 'node:fs'; |
| const pw=await import(process.env.PLAYWRIGHT); const chromium=pw.chromium||(pw.default&&pw.default.chromium); |
| const proxy=net.createServer(c=>{const u=net.connect(process.env.SHEETS_SOCK);c.pipe(u);u.pipe(c);c.on('error',()=>u.destroy());u.on('error',()=>c.destroy());}); |
| await new Promise(r=>proxy.listen(8799,'127.0.0.1',r)); |
| const base='http://127.0.0.1:8799'; |
| -const browser=await chromium.launch(); |
| +const browser=await chromium.launch({executablePath:process.env.CHROMIUM_EXECUTABLE_PATH}); |
| const ctx=await browser.newContext({viewport:{width:1440,height:900},permissions:['clipboard-read','clipboard-write']}); |
| const page=await ctx.newPage(); |
| const errors=[]; page.on('pageerror',e=>errors.push(String(e))); page.on('console',m=>{if(m.type()==='error')errors.push(m.text());}); |
| @@ -33,7 +34,11 @@ await page.goto(`${base}/signin`); await page.fill('input[type=email]','paul@exa |
| await Promise.all([page.waitForLoadState('networkidle'),page.click('button[type=submit]')]); |
| const mm=[...fs.readFileSync(process.env.SHEETS_LOG,'utf8').matchAll(/http:\/\/127\.0\.0\.1:8799\/auth\/verify\?[^\s\\"]+/g)].pop(); |
| await page.goto(mm[0]); await page.waitForURL(/\/sheets/); |
| -await page.fill('#title','Clip2'); await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.click('form[action="/sheets"] button[type=submit]')]); |
| +// Create from the top bar, then name it by renaming: the list's #title |
| +// field is gone, so a sheet is born "Untitled sheet" and named after. |
| +await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.click('[rst-shell-tail] form[action="/sheets"] button[type=submit]')]); |
| +await page.fill('[data-doc-title-input]','Clip2'); |
| +await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.press('[data-doc-title-input]','Enter')]); |
| await page.waitForFunction(()=>window.SheetsGrid&&window.SheetsGrid.worksheet()); |
| const w=(r,c,v)=>page.evaluate(([r,c,v])=>window.SheetsGrid.write(r,c,v),[r,c,v]); |
| const sel=(x1,y1,x2,y2)=>page.evaluate(([a,b,c,d])=>window.SheetsGrid.worksheet().updateSelectionFromCoords(a,b,c,d),[x1,y1,x2,y2]); |
| @@ -123,6 +128,8 @@ await page.click('[data-cmd="paste"]'); await page.waitForTimeout(900); |
| // H8 is 7 rows and 6 columns from B1, so A1 becomes G8. |
| check('menu Paste writes a translated cell', await raw(7,7)==='=G8*2', 'got '+JSON.stringify(await raw(7,7))); |
| console.log('\n note text:',JSON.stringify(await page.textContent('[data-sheet-note]'))); |
| +try { await checkCellQueue(page); ok('atomic paste and failed cut queue'); } |
| +catch (err) { check('atomic paste and failed cut queue', false, err.message); } |
| console.log('ERRORS:',JSON.stringify([...new Set(errors)])); |
| console.log(fails.length?('\nFAILURES: '+fails.join('; ')):'\nall checks passed'); |
| await browser.close(); proxy.close(); process.exit(fails.length?1:0); |
diff --git a/tools/drive/drive.mjs b/tools/drive/drive.mjs| index 3946dfb..01bb025 100644 |
| --- a/tools/drive/drive.mjs |
| +++ b/tools/drive/drive.mjs |
| @@ -38,7 +38,14 @@ |
| import net from 'node:net'; |
| import fs from 'node:fs'; |
| const PLAYWRIGHT = process.env.PLAYWRIGHT || 'playwright'; |
| -const { chromium } = await import(PLAYWRIGHT); |
| +/* A CJS build imported from ESM puts its exports on .default, so a |
| + * plain `const { chromium } = await import(...)` yields undefined and |
| + * dies one line later at chromium.launch — which is how this read until |
| + * the branch that added the checks below tried to run it. chrome.mjs |
| + * already spelled it this way; the two now agree. */ |
| +const pw = await import(PLAYWRIGHT); |
| +const chromium = pw.chromium || (pw.default && pw.default.chromium); |
| +if (!chromium) { console.error('playwright: no chromium export'); process.exit(1); } |
| const SOCK = process.env.SHEETS_SOCK || '/tmp/s.sock'; |
| const LOG = process.env.SHEETS_LOG || '/tmp/s.log'; |
| @@ -78,16 +85,27 @@ if (!m) { console.log('FAIL: no magic link in the log'); process.exit(1); } |
| await page.goto(`${base}/auth/verify?token=${m[1]}`); |
| console.log('signed in as:', (await page.textContent('body')).includes('Signed in as') ? 'yes' : 'NO'); |
| -// 2. Make a sheet. |
| +// 2. Make a sheet, from the top bar's Create button. |
| +// |
| +// This used to fill #title on the list page. That field is gone: the |
| +// list carries no forms now, Create posts an empty one, and the sheet |
| +// lands as "Untitled sheet" — so naming it is a rename, below. |
| await page.goto(base + '/sheets'); |
| -await page.fill('#title', 'Browser drive'); |
| // Not networkidle: the SSE stream is a long-lived connection, so the |
| // network on this page is never idle by design. |
| -await page.click('form[action="/sheets"] button[type=submit]'); |
| +await page.click('[rst-shell-tail] form[action="/sheets"] button[type=submit]'); |
| await page.waitForURL(/\/sheets\/\d+$/, { timeout: 15000 }); |
| const url = page.url(); |
| console.log('sheet url:', url); |
| +// 2b. Rename it from the top bar, which is now the only place a sheet |
| +// is ever named — and a real form submit, so this covers the |
| +// scriptless path too. |
| +await page.fill('[data-doc-title-input]', 'Browser drive'); |
| +await page.press('[data-doc-title-input]', 'Enter'); |
| +await page.waitForURL(/\/sheets\/\d+$/, { timeout: 15000 }); |
| +console.log('renamed to:', JSON.stringify(await page.inputValue('[data-doc-title-input]'))); |
| + |
| // 3. The grid must actually render. |
| await page.waitForSelector('.jexcel, .jss_worksheet, table', { timeout: 15000 }); |
| console.log('grid rendered:', await page.locator('[data-sheet-grid] table').count() > 0); |
diff --git a/tools/drive/drop.mjs b/tools/drive/drop.mjs| new file mode 100644 |
| index 0000000..c96deba |
| --- /dev/null |
| +++ b/tools/drive/drop.mjs |
| @@ -0,0 +1,268 @@ |
| +/* Drive the sheet list's drop overlay in a real browser. |
| + * |
| + * The third of the drives, after drive.mjs (live sync across two tabs) |
| + * and chrome.mjs (the editor chrome). Like both it is NOT part of |
| + * `make test`: it needs a browser and a running instance, neither of |
| + * which CI has. |
| + * |
| + * It is here because the Go suite can assert the markup and read the |
| + * stylesheet, and neither tells you whether the thing works. The list |
| + * has no visible drop box any more — a list of sheets should be a list |
| + * of sheets — so the whole affordance is an overlay that must be |
| + * invisible at rest, appear when a file is over the window, and go away |
| + * again. "Invisible" and "appears" are not things a Go test can see. |
| + * |
| + * It earned its place on the first run. The overlay carries data-drop, |
| + * because that is what drop.js looks for, and so it also matched |
| + * [data-drop][data-drop-over] — 0-2-0 against [rst-modal-overlay]'s |
| + * 0-1-0. The scrim was painted accent-soft and the sheet list vanished |
| + * behind a flat blue screen. Every Go test passed. |
| + * |
| + * Usage, from the repository root — see drive.mjs for the full recipe: |
| + * |
| + * SHEETS_SOCK=$D/s.sock SHEETS_LOG=$D/s.log OUT=$D \ |
| + * SHEETS_FIXTURE=testdata/corpus/xlsx/numbers.xlsx \ |
| + * node tools/drive/drop.mjs |
| + * |
| + * The drag events are synthesised rather than performed: Playwright |
| + * cannot drag a file in from the desktop, and a DragEvent carrying a |
| + * real DataTransfer of real File objects is what the browser delivers |
| + * when it does. The final drop is a genuine multipart upload — the |
| + * import at the end really imports. |
| + */ |
| +import net from 'node:net'; |
| +import fs from 'node:fs'; |
| +const PLAYWRIGHT = process.env.PLAYWRIGHT || 'playwright'; |
| +const pw = await import(PLAYWRIGHT); |
| +/* A CJS build imported from ESM puts its exports on .default. */ |
| +const chromium = pw.chromium || (pw.default && pw.default.chromium); |
| +if (!chromium) { console.error('playwright: no chromium export'); process.exit(1); } |
| + |
| +const SOCK = process.env.SHEETS_SOCK; |
| +const LOG = process.env.SHEETS_LOG; |
| +const PORT = parseInt(process.env.SHEETS_PORT || '8799', 10); |
| +const OUT = process.env.OUT || '.'; |
| +const FIXTURE = process.env.SHEETS_FIXTURE || 'testdata/corpus/xlsx/numbers.xlsx'; |
| + |
| +const proxy = net.createServer((c) => { |
| + const u = net.connect(SOCK); |
| + c.pipe(u); u.pipe(c); |
| + c.on('error', () => u.destroy()); |
| + u.on('error', () => c.destroy()); |
| +}); |
| +await new Promise((r) => proxy.listen(PORT, '127.0.0.1', r)); |
| + |
| +const base = `http://127.0.0.1:${PORT}`; |
| +const browser = await chromium.launch(); |
| +const ctx = await browser.newContext({ viewport: { width: 1280, height: 800 } }); |
| +const page = await ctx.newPage(); |
| + |
| +const errors = []; |
| +page.on('console', (m) => { if (m.type() === 'error') errors.push('console: ' + m.text()); }); |
| +page.on('pageerror', (e) => errors.push('pageerror: ' + e.message)); |
| + |
| +const failed = []; |
| +function check(name, ok, detail) { |
| + console.log((ok ? ' ok ' : ' FAIL ') + name + (detail === undefined ? '' : ' ' + JSON.stringify(detail))); |
| + if (!ok) failed.push(name); |
| +} |
| + |
| +/* Sign in. The magic link is read out of the log, which only works on a |
| + * loopback origin with no SMTP — see internal/sheets/auth.go. */ |
| +const before = fs.readFileSync(LOG, 'utf8').length; |
| +await page.goto(base + '/signin'); |
| +await page.fill('#address', 'paul@example.com'); |
| +await page.click('form[action="/signin"] button[type=submit]'); |
| +await page.waitForURL(/signin\?sent=1/, { timeout: 15000 }); |
| +const link = fs.readFileSync(LOG, 'utf8').slice(before).match(/auth\/verify\?token=([A-Za-z0-9_-]+)/); |
| +if (!link) { console.error('no magic link in the log'); process.exit(1); } |
| +await page.goto(`${base}/auth/verify?token=${link[1]}`); |
| +await page.goto(base + '/sheets'); |
| + |
| +console.log('\n— at rest'); |
| + |
| +const rest = await page.evaluate(() => { |
| + const z = document.querySelector('[data-drop-page]'); |
| + if (!z) return null; |
| + const r = z.getBoundingClientRect(); |
| + const input = z.querySelector('input[type=file]'); |
| + return { |
| + hidden: z.hidden, |
| + display: getComputedStyle(z).display, |
| + area: r.width * r.height, |
| + reachable: input ? input.offsetParent !== null : null, |
| + }; |
| +}); |
| +check('the overlay is in the page', rest !== null); |
| +check('...hidden', rest && rest.hidden === true && rest.display === 'none', rest); |
| +check('...costing no layout', rest && rest.area === 0, rest && rest.area); |
| +check('...and no tab stop', rest && rest.reachable === false, rest && rest.reachable); |
| +await page.screenshot({ path: OUT + '/list-at-rest.png' }); |
| + |
| +/* Synthesised drags. __mkdt builds a DataTransfer of n copies of the |
| + * fixture; __fire dispatches at the centre of the viewport, on whatever |
| + * element is actually there, so the event bubbles the way a real one |
| + * does rather than being handed straight to the listener. */ |
| +const helper = ` |
| + window.__mkdt = (b64, n) => { |
| + const bin = atob(b64); |
| + const bytes = new Uint8Array(bin.length); |
| + for (let i = 0; i < bin.length; i++) bytes[i] = bin.charCodeAt(i); |
| + const dt = new DataTransfer(); |
| + for (let i = 0; i < n; i++) { |
| + dt.items.add(new File([bytes], (i ? i + '-' : '') + 'drop.xlsx', |
| + { type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' })); |
| + } |
| + return dt; |
| + }; |
| + window.__fire = (type, dt) => { |
| + const e = new DragEvent(type, { bubbles: true, cancelable: true, dataTransfer: dt }); |
| + (document.elementFromPoint(640, 400) || document.body).dispatchEvent(e); |
| + }; |
| +`; |
| +const b64 = fs.readFileSync(FIXTURE).toString('base64'); |
| +const drag = (n) => page.evaluate(([b64, n]) => { |
| + window.__dt = window.__mkdt(b64, n); |
| + window.__fire('dragenter', window.__dt); |
| + window.__fire('dragover', window.__dt); |
| +}, [b64, n]); |
| + |
| +console.log('\n— a file over the window'); |
| + |
| +await page.evaluate(helper); |
| +await drag(1); |
| +const dragging = await page.evaluate(() => { |
| + const z = document.querySelector('[data-drop-page]'); |
| + const r = z.getBoundingClientRect(); |
| + return { |
| + hidden: z.hidden, |
| + over: z.hasAttribute('data-drop-over'), |
| + w: Math.round(r.width), h: Math.round(r.height), |
| + scrim: getComputedStyle(z).backgroundColor, |
| + recover: getComputedStyle(z.querySelector('[data-drop-page-recover]')).display, |
| + }; |
| +}); |
| +check('the overlay appears', dragging.hidden === false && dragging.over === true, dragging); |
| +check('...covering the window', dragging.w >= 1200 && dragging.h >= 700, dragging); |
| +/* The list must still be visible behind it. This is the check that |
| + * caught the specificity bug in the header comment: an opaque backdrop |
| + * reads as the page having gone away. */ |
| +check('...over a translucent scrim, with the list still there', |
| + /^rgba\(/.test(dragging.scrim) && parseFloat(dragging.scrim.split(',')[3]) < 0.8, dragging.scrim); |
| +check('...and the chooser out of the way', dragging.recover === 'none', dragging.recover); |
| +await page.screenshot({ path: OUT + '/list-dragging-light.png' }); |
| + |
| +await page.evaluate(() => window.__fire('dragleave', window.__dt)); |
| +check('the overlay goes when the drag leaves', |
| + (await page.evaluate(() => document.querySelector('[data-drop-page]').hidden)) === true); |
| + |
| +console.log('\n— a drop it cannot take'); |
| + |
| +await page.evaluate(([b64]) => { |
| + const dt = window.__mkdt(b64, 2); |
| + window.__fire('dragenter', dt); |
| + window.__fire('drop', dt); |
| +}, [b64]); |
| +const refused = await page.evaluate(() => { |
| + const z = document.querySelector('[data-drop-page]'); |
| + return { |
| + hidden: z.hidden, |
| + hint: z.querySelector('[data-drop-hint]').textContent.trim(), |
| + recover: getComputedStyle(z.querySelector('[data-drop-page-recover]')).display, |
| + path: location.pathname, |
| + }; |
| +}); |
| +check('two files are refused in words', refused.hint === 'Drop one file at a time.', refused); |
| +check('...with the overlay left up to say so', refused.hidden === false, refused); |
| +check('...and the chooser back, to recover with', refused.recover !== 'none', refused.recover); |
| +check('...having submitted nothing', refused.path === '/sheets', refused.path); |
| + |
| +/* The recovery form must be reachable by KEYBOARD, not merely visible. |
| + * |
| + * An independent review found both controls carrying tabindex="-1", |
| + * which put them out of the tab order at the one moment they exist to |
| + * be used: the overlay stays up after a refusal so they can be used to |
| + * recover, and Tab went straight past them into the list behind. It |
| + * looked completely correct in a screenshot, which is why this check |
| + * presses Tab instead of reading a style — the same lesson the menu bar |
| + * taught when it worked from the keyboard and not from a mouse. |
| + * |
| + * Tabbing from the panel must land ON the panel's own controls. */ |
| +const reachable = await page.evaluate(() => { |
| + const rec = document.querySelector('[data-drop-page-recover]'); |
| + const controls = [...rec.querySelectorAll('input,button')]; |
| + return { |
| + negative: controls.filter((c) => c.tabIndex < 0).length, |
| + count: controls.length, |
| + }; |
| +}); |
| +check('the recovery controls are in the tab order', reachable.negative === 0 |
| + && reachable.count === 2, JSON.stringify(reachable)); |
| + |
| +/* And prove it by actually tabbing: focus the panel, press Tab, and the |
| + * focus must be inside the panel rather than on the list underneath. */ |
| +await page.evaluate(() => document.querySelector('[data-drop-page-recover] input').focus()); |
| +const focusedInPanel = await page.evaluate(() => |
| + !!document.activeElement.closest('[data-drop-page-recover]')); |
| +check('...and focus really lands in the panel', focusedInPanel, 'focus escaped the overlay'); |
| +await page.keyboard.press('Tab'); |
| +const nextInPanel = await page.evaluate(() => |
| + !!document.activeElement.closest('[data-drop-page-recover]')); |
| +check('...and Tab moves to Import, not past the overlay', nextInPanel, |
| + 'Tab left the recovery form'); |
| + |
| +await page.screenshot({ path: OUT + '/list-refused.png' }); |
| + |
| +await page.keyboard.press('Escape'); |
| +const dismissed = await page.evaluate(() => { |
| + const z = document.querySelector('[data-drop-page]'); |
| + return { hidden: z.hidden, hint: z.querySelector('[data-drop-hint]').textContent.trim() }; |
| +}); |
| +check('Escape dismisses it', dismissed.hidden === true, dismissed); |
| +check('...and the refusal does not outlive the dismissal', |
| + dismissed.hint.startsWith('Excel'), dismissed.hint); |
| + |
| +console.log('\n— dark'); |
| + |
| +await ctx.addCookies([{ name: 'sheets_mode', value: 'dark', url: base }]); |
| +await page.goto(base + '/sheets'); |
| +await page.evaluate(helper); |
| +await drag(1); |
| +/* The cookie is sheets_mode. Naming it "mode" renders the light page |
| + * and the screenshot proves nothing — which it once did, and the only |
| + * tell was that the two images were pixel-identical. */ |
| +check('dark really is dark', (await page.getAttribute('html', 'data-theme')) === 'dark'); |
| +await page.screenshot({ path: OUT + '/list-dragging-dark.png' }); |
| + |
| +console.log('\n— the drop imports'); |
| + |
| +await ctx.addCookies([{ name: 'sheets_mode', value: 'light', url: base }]); |
| +await page.goto(base + '/sheets'); |
| +await page.evaluate(helper); |
| +await page.evaluate(([b64]) => { |
| + const dt = window.__mkdt(b64, 1); |
| + window.__fire('dragenter', dt); |
| + window.__fire('drop', dt); |
| +}, [b64]); |
| +let landed = 'never left /sheets'; |
| +try { |
| + await page.waitForURL((u) => !u.pathname.endsWith('/sheets'), { timeout: 20000 }); |
| + landed = new URL(page.url()).pathname; |
| +} catch (e) { /* landed stays as it was */ } |
| +check('a drop starts a real import', /^\/(jobs|sheets)\//.test(landed), landed); |
| + |
| +let arrived = false; |
| +for (let i = 0; i < 60; i++) { |
| + await page.goto(base + '/sheets'); |
| + if (await page.locator('[data-sheet-list] li').count() > 0) { arrived = true; break; } |
| + await page.waitForTimeout(500); |
| +} |
| +check('...and the sheet lands in the list', arrived); |
| +await page.screenshot({ path: OUT + '/list-after-import.png' }); |
| + |
| +check('no console errors', errors.length === 0, errors.slice(0, 8)); |
| + |
| +console.log(failed.length ? '\nFAILED: ' + failed.join(', ') : '\nall checks passed'); |
| +await browser.close(); |
| +proxy.close(); |
| +process.exit(failed.length ? 1 : 0); |
diff --git a/tools/drive/menus.mjs b/tools/drive/menus.mjs| index 01c11ab..d508b85 100644 |
| --- a/tools/drive/menus.mjs |
| +++ b/tools/drive/menus.mjs |
| @@ -48,7 +48,13 @@ await page.goto(`${base}/signin`); await page.fill('input[type=email]','paul@exa |
| await Promise.all([page.waitForLoadState('networkidle'),page.click('button[type=submit]')]); |
| const mm=[...fs.readFileSync(LOG,'utf8').matchAll(/http:\/\/127\.0\.0\.1:8799\/auth\/verify\?[^\s\\"]+/g)].pop(); |
| await page.goto(mm[0]); await page.waitForURL(/\/sheets/); |
| -if(!(await page.$('[data-sheet-list]'))){await page.fill('#title','Audit');await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.click('form[action="/sheets"] button[type=submit]')]);} |
| +// No sheets yet: Create from the top bar, then name it by renaming. |
| +// The list's #title field is gone — a sheet is born "Untitled sheet". |
| +if(!(await page.$('[data-sheet-list]'))){ |
| + await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.click('[rst-shell-tail] form[action="/sheets"] button[type=submit]')]); |
| + await page.fill('[data-doc-title-input]','Audit'); |
| + await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.press('[data-doc-title-input]','Enter')]); |
| +} |
| else {await page.click('[data-sheet-list] [rst-row-main] a'); await page.waitForURL(/\/sheets\/\d+$/);} |
| const url=page.url(); |
| await page.waitForFunction(()=>window.SheetsGrid&&window.SheetsGrid.worksheet()); |
diff --git a/tools/drive/tableformat.mjs b/tools/drive/tableformat.mjs| index 31eb994..040da0f 100644 |
| --- a/tools/drive/tableformat.mjs |
| +++ b/tools/drive/tableformat.mjs |
| @@ -26,7 +26,11 @@ await page.goto(`${base}/signin`); await page.fill('input[type=email]','paul@exa |
| await Promise.all([page.waitForLoadState('networkidle'),page.click('button[type=submit]')]); |
| const mm=[...fs.readFileSync(process.env.SHEETS_LOG,'utf8').matchAll(/http:\/\/127\.0\.0\.1:8799\/auth\/verify\?[^\s\\"]+/g)].pop(); |
| await page.goto(mm[0]); await page.waitForURL(/\/sheets/); |
| -await page.fill('#title','Table format'); await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.click('form[action="/sheets"] button[type=submit]')]); |
| +// Create from the top bar, then name it by renaming: the list's #title |
| +// field is gone, so a sheet is born "Untitled sheet" and named after. |
| +await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.click('[rst-shell-tail] form[action="/sheets"] button[type=submit]')]); |
| +await page.fill('[data-doc-title-input]','Table format'); |
| +await Promise.all([page.waitForURL(/\/sheets\/\d+$/),page.press('[data-doc-title-input]','Enter')]); |
| const url=page.url(); |
| await page.waitForFunction(()=>window.SheetsGrid&&window.SheetsGrid.worksheet()); |
| for(const [r,c,v] of [[0,0,'Region'],[0,1,'Revenue'],[1,0,'North'],[1,1,'1200'],[2,0,'South'],[2,1,'900']]) |