diff --git a/apps/blocks/src/blocks/feature-packs/data/data-console-module.tsx b/apps/blocks/src/blocks/feature-packs/data/data-console-module.tsx index 78fd8891..6d7e57d0 100644 --- a/apps/blocks/src/blocks/feature-packs/data/data-console-module.tsx +++ b/apps/blocks/src/blocks/feature-packs/data/data-console-module.tsx @@ -8,7 +8,7 @@ import { DatabaseIcon } from 'lucide-react'; import type { SheetsConfig, SheetsExecuteFn } from '@constructive-io/sheets'; import { DATA_FEATURE_PACK } from '../../../feature-packs'; -import { createConsoleIdentityKey } from '../../console-runtime'; +import { createConsoleIdentityKey, type IdentityScopedConsoleTransport } from '../../console-runtime'; import { normalizeConsoleKitError } from '../../console-kit/console-kit-runtime'; import type { ConstructiveCapabilityContribution } from '../../console-kit/constructive/constructive-capabilities'; import type { @@ -42,6 +42,22 @@ function documentSource(document: unknown): string { return String(document); } +function createSheetsExecutor(transport?: IdentityScopedConsoleTransport | null): SheetsExecuteFn | undefined { + if (!transport) return undefined; + return async (document: unknown, variables?: Record) => { + const result = await transport.execute({ + document: documentSource(document), + variables + }); + if (result.ok) return result.data; + const first = result.errors[0]; + const error = new Error(first?.message || 'The GraphQL operation failed.') as Error & { code?: string }; + const code = first?.extensions?.code; + if (typeof code === 'string') error.code = code; + throw error; + }; +} + function DataConsoleFeature({ adapterProps, config, @@ -49,26 +65,14 @@ function DataConsoleFeature({ onError }: ConsoleKitFeatureComponentProps) { const dataEndpoint = runtime.endpoints.data; + const storageEndpoint = runtime.endpoints.storage; const scopedTransport = runtime.transportFor('data'); - const execute = React.useMemo(() => { - if (!scopedTransport) return undefined; - return async (document: unknown, variables?: Record) => { - const result = await scopedTransport.execute({ - document: documentSource(document), - variables - }); - if (result.ok) return result.data; - const first = result.errors[0]; - const error = new Error( - first?.message || 'The GraphQL operation failed.' - ) as Error & { code?: string }; - const code = first?.extensions?.code; - if (typeof code === 'string') error.code = code; - throw error; - }; - }, [scopedTransport]); + const storageTransport = runtime.transportFor('storage'); + const execute = React.useMemo(() => createSheetsExecutor(scopedTransport), [scopedTransport]); + const executeStorage = React.useMemo(() => createSheetsExecutor(storageTransport), [storageTransport]); const sheetsConfig = React.useMemo(() => ({ endpoint: dataEndpoint?.url ?? '', + storage: storageEndpoint?.url && executeStorage ? { endpoint: storageEndpoint.url, execute: executeStorage } : undefined, databaseId: config.databaseId, auth: { mode: 'embedded', @@ -96,7 +100,9 @@ function DataConsoleFeature({ config.databaseId, config.queryClient, dataEndpoint?.url, + storageEndpoint?.url, execute, + executeStorage, onError, runtime.session ]); diff --git a/packages/blocks-ui/package.json b/packages/blocks-ui/package.json index 11d02123..65d18773 100644 --- a/packages/blocks-ui/package.json +++ b/packages/blocks-ui/package.json @@ -46,6 +46,7 @@ ], "scripts": { "build": "tsup", + "prepack": "pnpm build", "dev": "tsup --watch", "lint:types": "tsc --noEmit", "test": "vitest run", diff --git a/packages/data/package.json b/packages/data/package.json index ae16273b..770a75a1 100644 --- a/packages/data/package.json +++ b/packages/data/package.json @@ -35,6 +35,7 @@ ], "scripts": { "build": "tsup", + "prepack": "pnpm build", "dev": "tsup --watch", "lint:types": "tsc --noEmit", "test": "vitest run", diff --git a/packages/schema-builder/src/schema/schema-builder-core/lib/gql/hooks/schema-builder/transformers/field-constraints-mapper.ts b/packages/schema-builder/src/schema/schema-builder-core/lib/gql/hooks/schema-builder/transformers/field-constraints-mapper.ts index 08131245..79d5bfc0 100644 --- a/packages/schema-builder/src/schema/schema-builder-core/lib/gql/hooks/schema-builder/transformers/field-constraints-mapper.ts +++ b/packages/schema-builder/src/schema/schema-builder-core/lib/gql/hooks/schema-builder/transformers/field-constraints-mapper.ts @@ -65,7 +65,7 @@ export function cellTypeToBackendType(cellType: CellType): string { // Media types - use PostgreSQL DOMAINs where available // DOMAINs: image (jsonb), upload (jsonb) - defined in pgpm-types extension - // These DOMAINs enforce { url, mime } structure with URL validation + // These domains accept a JSON object containing a URL, file ID, or storage key. image: 'image', // PostgreSQL DOMAIN over jsonb file: 'text', // No DOMAIN available, uses base type video: 'text', // No DOMAIN available, uses base type diff --git a/packages/sheets/README.md b/packages/sheets/README.md index dbe016ae..c10ca26f 100644 --- a/packages/sheets/README.md +++ b/packages/sheets/README.md @@ -171,10 +171,19 @@ The `SheetsConfig` object passed to ``: | `databaseId` | `string` | No | Multi-tenant database scope | | `fieldTypeOverrides` | `Record` | No | Override detected field types | | `execute` | `SheetsExecuteFn` | No | Custom GraphQL executor | -| `executeUpload` | `SheetsUploadFn` | No | Custom file upload handler | +| `storage` | `{ endpoint: string; execute?: SheetsExecuteFn }` | No | Storage endpoint for presigned uploads and previews | +| `executeUpload` | `SheetsUploadFn` | No | Host uploader returning `{ file: SheetsFileValue; url?: string }`. Takes precedence over the storage uploader | +| `upload` | `{ bucketKey?: string }` | No | Optional explicit bucket override. Sheets requests the tenant's public default when omitted | | `queryClient` | `QueryClient` | No | Bring-your-own TanStack Query client | | `onAuthError` | `() => void` | No | Callback on 401/UNAUTHENTICATED | +Sheets writes media cell values through the data endpoint. Image and upload +cells store the durable file projection, and attachment cells store its public +URL as text. The optional top-level upload result URL can expire. Hosts with +neither storage nor an injected uploader render media cells read-only. See +[Host integration](docs/HOST_INTEGRATION.md#media-uploads) for the migration +from the previous upload API. + ## Components | Component | Description | diff --git a/packages/sheets/docs/FEATURES.md b/packages/sheets/docs/FEATURES.md index 01de7dfd..b8bb8937 100644 --- a/packages/sheets/docs/FEATURES.md +++ b/packages/sheets/docs/FEATURES.md @@ -67,7 +67,7 @@ The range band is a `pointer-events-none` overlay (`data-part-id="sheets-range-b ## 4. Editing ✅ -Inline editing routes through a single `resolveEditIntent`: inline toggle (boolean), portal overlay editor, or no-op (readonly). 12 editors (text, number, date, image, relation, geometry, interval, json, array, inet, tsvector, url, upload) re-hosted in a React portal `OverlayManager`. +Inline editing routes through a single `resolveEditIntent`: inline toggle (boolean), portal overlay editor, or no-op (readonly). Editors run in a React portal `OverlayManager`. The media editor handles image, file, audio, video, upload, and attachment cells. | Aspect | Behavior | |--------|----------| @@ -79,7 +79,9 @@ Inline editing routes through a single `resolveEditIntent`: inline toggle (boole | Optimistic | The cell flips instantly; reverts on server reject | | Drafts | Client-side draft rows with per-row submit, validation styling, relation/FK normalization | -Consumer editors keeping cancel-on-outside (their internal Save/validation/focus): json, url, inet, array, tsvector, interval, date, geometry, relation, upload. *Commit-on-blur for those is a planned follow-up.* +Consumer editors keeping cancel-on-outside (their internal Save/validation/focus): json, url, inet, array, tsvector, interval, date, geometry, relation, and media. *Commit-on-blur for those is a planned follow-up.* + +Media uploads use the host storage endpoint, and cell updates use the data endpoint. Image and upload cells store the file projection and resolve signed previews as needed. Attachment cells store a public URL. Failed draft uploads can retry against the created row, and clearing a cell preserves its old file row. **Deferred:** in-editor `Tab`/`Enter` commit-and-advance (Tab across a row while editing). diff --git a/packages/sheets/docs/HOST_INTEGRATION.md b/packages/sheets/docs/HOST_INTEGRATION.md index e836b1d0..3109c509 100644 --- a/packages/sheets/docs/HOST_INTEGRATION.md +++ b/packages/sheets/docs/HOST_INTEGRATION.md @@ -34,11 +34,36 @@ interface SheetsAuthEmbedded { 1. **HTTP 401** — `config.onAuthError?.()` before `throw createError.unauthorized(...)` 2. **GraphQL UNAUTHENTICATED** — `onAuthError?.()` inside `parseGraphQLResponse` before returning the error -Both paths also apply to upload requests (`createSheetsUpload`). +Both paths also apply to GraphQL requests made by the default storage executor. ### Propagation -`SheetsProvider` passes the full `config` object to `createSheetsExecute` and `createSheetsUpload`. No additional wiring is needed in the provider — adding `onAuthError` to your config is sufficient. +`SheetsProvider` uses the host's token getter and `onAuthError` to build the +default data and storage executors. If the host injects an executor or +uploader, it owns auth handling for requests made through that override. + +## Media uploads + +Configure the data endpoint as `endpoint` and the tenant's storage endpoint as +`storage: { endpoint, execute? }`. Upload discovery and signed preview lookups +use storage, and the cell update uses the data endpoint. An injected +`executeUpload` takes precedence for uploads. Without either an injected uploader +or storage, the media editor is read-only. + +Sheets requests the public default bucket by omitting `bucketKey` and sending +`isPublic: true`. A host can set `upload.bucketKey` to select a particular +bucket. Image and upload fields store the file projection +`{ id, key, bucket_id, mime, size, filename, url? }`. The editor resolves a +signed preview when the projection lacks a public URL. Attachment fields store +only `file.url` as text and require a public bucket with a public URL prefix. + +When migrating a host uploader, return `{ file: SheetsFileValue, url?: string }`. +The optional top-level `url` is for immediate download and may expire. Put a +durable public URL on `file.url` when the storage service provides one. +`executeFieldUpload(target, file, transport)` now takes the table, field, and +typed row identifier in `target`, and the data executor, uploader, and adapter +in `transport`. It returns the saved cell value and accepts `null` to clear. +Use `useSheetsFieldUpload` in place of `useSheetsImageUpload`. ## Auth Error Flow @@ -58,7 +83,7 @@ Both paths also apply to upload requests (`createSheetsUpload`). ┌──────────────────────────────┐ │ SheetsProvider │ │ → createSheetsExecute(cfg) │ -│ → createSheetsUpload(cfg) │ +│ → storage.execute │ └──────────┬───────────────────┘ │ request fails (401 / UNAUTHENTICATED) ▼ diff --git a/packages/sheets/docs/OVERLAY_EDITORS.md b/packages/sheets/docs/OVERLAY_EDITORS.md index baf1991e..ce15843c 100644 --- a/packages/sheets/docs/OVERLAY_EDITORS.md +++ b/packages/sheets/docs/OVERLAY_EDITORS.md @@ -123,7 +123,7 @@ Import from `./overlay-presets`. Don't hardcode width classes — use the preset |--------|----------|-------------| | `sm` | Single input (date, inet, interval) | 320-480px | | `md` | Tag chips, lists (array, relation, tsvector) | 400-560px | -| `lg` | Rich editors (JSON, image) | 480-720px | +| `lg` | Rich editors (JSON, media uploads) | 480-720px | | `xl` | Full-width (geometry/map) | 600px-90vw | ### Always set `displayName` on wrapper components diff --git a/packages/sheets/docs/SHEETS_IMPROVEMENT_ROADMAP.md b/packages/sheets/docs/SHEETS_IMPROVEMENT_ROADMAP.md index 34cac52d..c279c9ef 100644 --- a/packages/sheets/docs/SHEETS_IMPROVEMENT_ROADMAP.md +++ b/packages/sheets/docs/SHEETS_IMPROVEMENT_ROADMAP.md @@ -35,7 +35,7 @@ The rest of the roadmap (backend adapter, typed row model, component API, headle **Critical weaknesses (release blockers in bold):** - **No real extension surface** — the advertised one is dead; the live ones are closed (§4, P0). - **No failure UX** — `` renders unconditionally; the `error` from the data hooks is destructured but **never rendered** (`sheets.tsx:198`), **zero error boundaries** across 113 files, no empty/loading/error states. -- **Data-loss bug** — image draft-submit→upload removes the draft from the store _before_ the upload (`image-editor.tsx:243-263`); an upload failure orphans a server row and drops the draft. +- **Draft upload retry** — the media editor retains its staged file and the created row key so a failed upload can retry after draft removal. - **Backend shape hardcoded everywhere** — `_meta` doc, `buildPostGraphile*`, Relay `{nodes}` parsing, PG filter dialect, relation inflection; ~17 files import `@constructive-io/data` directly. Sheets is a PostGraphile client, not a generic grid. - **951-line god-component** (`sheets.tsx`) fuses two data modes + drafts + optimistic updates + theming + JSX; no headless core. - **Soundness holes at the exact extension seams** — `combinedRows as any[]` (~15×), `__isDraft` magic keys (43 sites), `(cell.data as any)`, the `@ts-expect-error` in `extractCellValue` (`sheets.utils.ts:696`). @@ -64,7 +64,7 @@ Effort: **S**≤1d · **M**≈2-4d · **L**≈1-2wk · **XL**≈3wk+. Impact: | 0.3 | **ESM-scoped code-splitting + lazy heavy editors.** _Verified: `splitting:true` with `format:['esm','cjs']` **fails** esbuild ("splitting only works with esm"). Split `tsup.config.ts` into two configs — ESM with `splitting:true`, CJS non-split. `React.lazy` the 3 heavy editors (geometry→leaflet, json, image) in `editor-registry.ts`; light editors stay eager._ | M | ★★ | ✅ | | 0.4 | **Fix the vendor build.** Drop phantom `noExternal: ['pluralize','sonner']` (not deps); add real dep `'inflekt'`; drop `leaflet`/`react-leaflet` from `noExternal` (already optional peers — removes the ~270KB vendor double-bundle); add the missing `build:vendor` script so CI exercises it (`tsup.vendor.config.ts`). | S | ★★ | ✅ | | 0.5 | **Render real error / empty / loading states.** Surface `error` through `SheetsInner`'s memo (`use-load-grid.ts` already returns `{isError,error}`; `sheets.tsx:169` only pulls `hasCompletedInitialLoad`) and branch before ``. Ship default state components; gate loading on initial load only. _BC note: changes default DOM on error/empty — document it._ | M | ★★★ | ⚠️ behavior | -| 0.6 | **Fix the orphaned-row data-loss path** (`image-editor.tsx:260-263`): on post-submit upload failure, call `onDraftUploadComplete?.()`, flip the editor out of draft mode, capture the created id, and retain the file so a single retry uploads to the now-real row (don't re-enter `onSubmitDraft`). | M | ★★ | ✅ | +| 0.6 | **Keep draft uploads retryable.** Retain the editor and staged file after row creation so retry writes to the created row. | M | ★★ | ✅ | | 0.7 | **Injectable logger + `onError` on `SheetsConfig`** replacing the 11 raw `console.*`. Define **one** config delta (`logger`, `onError`) — consolidate with the error-boundary and architecture proposals to avoid a fractured config. Default `error→console.error`, `debug→no-op`. | S | ★ | ✅ | | 0.8 | **Error boundary** around `SheetsInner` and each overlay editor (`wrapEditor`), routed through `config.onError`; editor boundary calls `onFinishedEditing(undefined)` on reset so a bad editor can't wedge the overlay. Export `SheetsErrorBoundary`. | M | ★★ | ⚠️ swallows | | 0.9 | **Doc fix:** `EMBEDDING.md:166` `onSelect`→`onTableChange` (verified real prop). | S | ★ | ✅ | @@ -150,11 +150,10 @@ export interface SheetsBackendAdapter { createRow/updateRow/deleteRow(...): Promise<...>; buildWhere(field, operator, value): Record | null; // default emits PostGraphile dialect resolveOrderBy(field, dir, table): unknown; - uploadFile?(file, target, execute): Promise<{ url: string }>; } // SheetsConfig: adapter?: SheetsBackendAdapter ``` -*Land incrementally* (`fetchMeta` → `listRows` → mutations) behind the default adapter; add data-hook tests (currently zero) before moving each method. Fold the **redundant-count removal (0.1) into `listRows`** and **route the upload GraphQL patch step through `execute`** (the hand-built `fetch` in `use-sheets-upload.ts` bypasses the canonical error seam) so the adapter boundary doesn't leak. +*Land incrementally* (`fetchMeta` → `listRows` → mutations) behind the default adapter; add data-hook tests (currently zero) before moving each method. Fold the **redundant-count removal (0.1) into `listRows`** and **route the media cell patch through the adapter**. The storage transport uploads the file and resolves previews, while the data adapter persists the projection. **2.2 — One resolution authority · M · ★★ · BC** Make `mapToFrontendCellType(meta, cfg?)` the single resolver with injectable `aliasOverrides`/`scalarOverrides`/`resolve`. Route `createColumnSchemaFromMeta` and the forms layer through it (they diverge today). *Correction:* the divergence currently has **no visible effect** — `createColumnSchemaFromMeta`'s output (`tableSchema`) has **zero live consumers**; the grid already uses the full cascade. So this is **cleanup + an injection hook**, not a live-bug fix — either route it through the cascade or delete the dead `tableSchema`. Move Constructive-specific scalars (`Relation`/`Interval`/`GeoJSON`) into the PostGraphile adapter's default `scalarOverrides`. diff --git a/packages/sheets/package.json b/packages/sheets/package.json index 1f64e27b..27a2a499 100644 --- a/packages/sheets/package.json +++ b/packages/sheets/package.json @@ -68,6 +68,7 @@ ], "scripts": { "build": "rm -rf dist && tsup && cp src/styles.css dist/styles.css", + "prepack": "pnpm build", "build:registry": "tsx scripts/build-registry.ts", "check:registry": "tsx scripts/build-registry.ts --check", "build:vendor": "tsup --config tsup.vendor.config.ts", diff --git a/packages/sheets/registry.json b/packages/sheets/registry.json index 15949282..745b8347 100644 --- a/packages/sheets/registry.json +++ b/packages/sheets/registry.json @@ -8,7 +8,7 @@ "type": "registry:block", "title": "Sheets", "description": "A metadata-driven, spreadsheet-grade CRUD grid with accessible DOM cells and adapter-owned data access.", - "docs": "Import from `@/components/ui/sheets`. A metadata-driven CRUD grid: it reads a table's columns from the endpoint's `_meta`, then loads, edits, creates, and deletes rows itself over GraphQL with the host's session. There's no `data` prop and no save callback. The host owns the endpoint, the session token, and auth errors, and PostgreSQL privileges and RLS at the endpoint decide every read and write.\n\nRender it in a client component. `Sheets` fills its parent, so give the parent a bounded height, and mount `` from `@/components/ui/portal` once as the last child of `` for the overlay editors:\n```tsx\n'use client';\nimport { useMemo } from 'react';\nimport { Sheets, SheetsProvider, type SheetsConfig } from '@/components/ui/sheets';\n\nexport function ProjectsGrid({ endpoint, userId, getToken }: { endpoint: string; userId: string; getToken: () => string | null }) {\n const config = useMemo(\n () => ({ endpoint, auth: { mode: 'embedded', getToken, getIdentityKey: () => userId }, onAuthError: signOut }),\n [endpoint, getToken, userId],\n );\n return (\n \n
\n \n
\n \n );\n}\n```\n`tableName` is a table's `name` in `_meta`, its GraphQL type name. To let people pick one, call `useSheetsMeta()` inside the provider, pass `data?._meta?.tables ?? []` to `selectConsoleDataTables` from `@constructive-io/data`, and give the names to ``.\n\n`SheetsProvider` takes `config: SheetsConfig`; keep it referentially stable.\n- `endpoint` (the app GraphQL endpoint) and `auth` are required. `auth` is `{ mode: 'embedded', getToken: () => string | null, getIdentityKey?: () => string | null }`, where the identity key is a non-secret user id that keeps each user's cache apart, or `{ mode: 'standalone' }` with `authEndpoint` and the grid wrapped in `SheetsAuthGate` from `@/components/ui/sheets/auth`.\n- Optional: `databaseId`, `locale` (default en-US), `fieldTypeOverrides: Record`, `upload: { bucketKey? }` (default \"public\"), `queryClient` (reuse yours), `onAuthError()` (on a 401, before the error throws), `onError(error, { source?: 'grid' | 'editor' | 'mutation' | 'upload', tableName? })`, `logger`, `plugins`.\n- To replace the transport: `execute(document, variables?)`, `executeUpload(file, path?, options?) => Promise<{ url }>`, or a whole `adapter: SheetsBackendAdapter` (`fetchMeta`, `listRows`, `createRow`, `updateRow`, `deleteRow`).\n\n`Sheets` props:\n- `tableName` (required), `pageSize` (default 100), `showSelection` and `showPagination` (default true), `infiniteScroll` (default false), `className`.\n- Observers only, since the grid does its own writes: `onRowSelect(rows)`, `onCellEdit(id, field, value)` once an edit is saved (`id` is the primary key, or an object for a composite key), and `onEvent({ type, tableName, at, meta? })` with `type` load:success | load:error | cell:edit | row:create | row:delete | filter:apply | sort:change | page:change.\n- `slots: { toolbar?, empty?, error?({ error, retry }), loading? }` replace those regions; `cellTypes` and `cellSlots` customize cells; `commands`, `keymap`, `interceptors`, and `onCommand(event, result)` extend or observe grid commands; `theme` and `themeMode` ('light' | 'dark' | 'system') style it.\n- A `ref` gets `SheetsHandle`: `scrollToRow(index)`, `submitDrafts()`, `refetch()`, `getSelectedRows()`, `exportCsv({ columns?, filename? })`.", + "docs": "Import from `@/components/ui/sheets`. A metadata-driven CRUD grid: it reads a table's columns from the endpoint's `_meta`, then loads, edits, creates, and deletes rows itself over GraphQL with the host's session. There's no `data` prop and no save callback. The host owns the endpoint, the session token, and auth errors, and PostgreSQL privileges and RLS at the endpoint decide every read and write.\n\nRender it in a client component. `Sheets` fills its parent, so give the parent a bounded height, and mount `` from `@/components/ui/portal` once as the last child of `` for the overlay editors:\n```tsx\n'use client';\nimport { useMemo } from 'react';\nimport { Sheets, SheetsProvider, type SheetsConfig } from '@/components/ui/sheets';\n\nexport function ProjectsGrid({ endpoint, userId, getToken }: { endpoint: string; userId: string; getToken: () => string | null }) {\n const config = useMemo(\n () => ({ endpoint, auth: { mode: 'embedded', getToken, getIdentityKey: () => userId }, onAuthError: signOut }),\n [endpoint, getToken, userId],\n );\n return (\n \n
\n \n
\n \n );\n}\n```\n`tableName` is a table's `name` in `_meta`, its GraphQL type name. To let people pick one, call `useSheetsMeta()` inside the provider, pass `data?._meta?.tables ?? []` to `selectConsoleDataTables` from `@constructive-io/data`, and give the names to ``.\n\n`SheetsProvider` takes `config: SheetsConfig`; keep it referentially stable.\n- `endpoint` (the app GraphQL endpoint) and `auth` are required. `auth` is `{ mode: 'embedded', getToken: () => string | null, getIdentityKey?: () => string | null }`, where the identity key is a non-secret user id that keeps each user's cache apart, or `{ mode: 'standalone' }` with `authEndpoint` and the grid wrapped in `SheetsAuthGate` from `@/components/ui/sheets/auth`.\n- Optional: `databaseId`, `locale` (default en-US), `fieldTypeOverrides: Record`, `storage: { endpoint, execute? }`, `upload: { bucketKey? }` (defaults to the tenant public bucket), `queryClient` (reuse yours), `onAuthError()` (on a 401, before the error throws), `onError(error, { source?: 'grid' | 'editor' | 'mutation' | 'upload', tableName? })`, `logger`, `plugins`.\n- To replace the transport: `execute(document, variables?)`, `executeUpload(file, path?, options?) => Promise<{ file: SheetsFileValue; url?: string }>`, or a whole `adapter: SheetsBackendAdapter` (`fetchMeta`, `listRows`, `createRow`, `updateRow`, `deleteRow`). The injected uploader takes precedence. Media editors require storage or an injected uploader. Signed download URLs are resolved through storage and stay separate from the stored file projection.\n\n`Sheets` props:\n- `tableName` (required), `pageSize` (default 100), `showSelection` and `showPagination` (default true), `infiniteScroll` (default false), `className`.\n- Observers only, since the grid does its own writes: `onRowSelect(rows)`, `onCellEdit(id, field, value)` once an edit is saved (`id` is the primary key, or an object for a composite key), and `onEvent({ type, tableName, at, meta? })` with `type` load:success | load:error | cell:edit | row:create | row:delete | filter:apply | sort:change | page:change.\n- `slots: { toolbar?, empty?, error?({ error, retry }), loading? }` replace those regions; `cellTypes` and `cellSlots` customize cells; `commands`, `keymap`, `interceptors`, and `onCommand(event, result)` extend or observe grid commands; `theme` and `themeMode` ('light' | 'dark' | 'system') style it.\n- A `ref` gets `SheetsHandle`: `scrollToRow(index)`, `submitDrafts()`, `refetch()`, `getSelectedRows()`, `exportCsv({ columns?, filename? })`.", "categories": [ "blocks", "data" @@ -516,11 +516,6 @@ "target": "@ui/sheets/grid-dom/editors/tsvector-editor.tsx", "type": "registry:component" }, - { - "path": "registry/constructive/blocks/sheets/grid-dom/editors/upload-editor.tsx", - "target": "@ui/sheets/grid-dom/editors/upload-editor.tsx", - "type": "registry:component" - }, { "path": "registry/constructive/blocks/sheets/grid-dom/editors/url-editor.tsx", "target": "@ui/sheets/grid-dom/editors/url-editor.tsx", @@ -626,11 +621,6 @@ "target": "@ui/sheets/grid/editors/tsvector-editor.tsx", "type": "registry:component" }, - { - "path": "registry/constructive/blocks/sheets/grid/editors/upload-editor.tsx", - "target": "@ui/sheets/grid/editors/upload-editor.tsx", - "type": "registry:component" - }, { "path": "registry/constructive/blocks/sheets/grid/editors/url-editor.tsx", "target": "@ui/sheets/grid/editors/url-editor.tsx", @@ -836,6 +826,11 @@ "target": "@ui/sheets/hooks/use-sheets-infinite-table.ts", "type": "registry:lib" }, + { + "path": "registry/constructive/blocks/sheets/hooks/use-sheets-media-url.ts", + "target": "@ui/sheets/hooks/use-sheets-media-url.ts", + "type": "registry:lib" + }, { "path": "registry/constructive/blocks/sheets/hooks/use-sheets-meta.ts", "target": "@ui/sheets/hooks/use-sheets-meta.ts", diff --git a/packages/sheets/scripts/build-registry.ts b/packages/sheets/scripts/build-registry.ts index f68120ab..fd6f193f 100644 --- a/packages/sheets/scripts/build-registry.ts +++ b/packages/sheets/scripts/build-registry.ts @@ -16,7 +16,7 @@ buildSourceRegistry({ title: 'Sheets', description: 'A metadata-driven, spreadsheet-grade CRUD grid with accessible DOM cells and adapter-owned data access.', categories: ['blocks', 'data'], - docs: "Import from `@/components/ui/sheets`. A metadata-driven CRUD grid: it reads a table's columns from the endpoint's `_meta`, then loads, edits, creates, and deletes rows itself over GraphQL with the host's session. There's no `data` prop and no save callback. The host owns the endpoint, the session token, and auth errors, and PostgreSQL privileges and RLS at the endpoint decide every read and write.\n\nRender it in a client component. `Sheets` fills its parent, so give the parent a bounded height, and mount `` from `@/components/ui/portal` once as the last child of `` for the overlay editors:\n```tsx\n'use client';\nimport { useMemo } from 'react';\nimport { Sheets, SheetsProvider, type SheetsConfig } from '@/components/ui/sheets';\n\nexport function ProjectsGrid({ endpoint, userId, getToken }: { endpoint: string; userId: string; getToken: () => string | null }) {\n const config = useMemo(\n () => ({ endpoint, auth: { mode: 'embedded', getToken, getIdentityKey: () => userId }, onAuthError: signOut }),\n [endpoint, getToken, userId],\n );\n return (\n \n
\n \n
\n \n );\n}\n```\n`tableName` is a table's `name` in `_meta`, its GraphQL type name. To let people pick one, call `useSheetsMeta()` inside the provider, pass `data?._meta?.tables ?? []` to `selectConsoleDataTables` from `@constructive-io/data`, and give the names to ``.\n\n`SheetsProvider` takes `config: SheetsConfig`; keep it referentially stable.\n- `endpoint` (the app GraphQL endpoint) and `auth` are required. `auth` is `{ mode: 'embedded', getToken: () => string | null, getIdentityKey?: () => string | null }`, where the identity key is a non-secret user id that keeps each user's cache apart, or `{ mode: 'standalone' }` with `authEndpoint` and the grid wrapped in `SheetsAuthGate` from `@/components/ui/sheets/auth`.\n- Optional: `databaseId`, `locale` (default en-US), `fieldTypeOverrides: Record`, `upload: { bucketKey? }` (default \"public\"), `queryClient` (reuse yours), `onAuthError()` (on a 401, before the error throws), `onError(error, { source?: 'grid' | 'editor' | 'mutation' | 'upload', tableName? })`, `logger`, `plugins`.\n- To replace the transport: `execute(document, variables?)`, `executeUpload(file, path?, options?) => Promise<{ url }>`, or a whole `adapter: SheetsBackendAdapter` (`fetchMeta`, `listRows`, `createRow`, `updateRow`, `deleteRow`).\n\n`Sheets` props:\n- `tableName` (required), `pageSize` (default 100), `showSelection` and `showPagination` (default true), `infiniteScroll` (default false), `className`.\n- Observers only, since the grid does its own writes: `onRowSelect(rows)`, `onCellEdit(id, field, value)` once an edit is saved (`id` is the primary key, or an object for a composite key), and `onEvent({ type, tableName, at, meta? })` with `type` load:success | load:error | cell:edit | row:create | row:delete | filter:apply | sort:change | page:change.\n- `slots: { toolbar?, empty?, error?({ error, retry }), loading? }` replace those regions; `cellTypes` and `cellSlots` customize cells; `commands`, `keymap`, `interceptors`, and `onCommand(event, result)` extend or observe grid commands; `theme` and `themeMode` ('light' | 'dark' | 'system') style it.\n- A `ref` gets `SheetsHandle`: `scrollToRow(index)`, `submitDrafts()`, `refetch()`, `getSelectedRows()`, `exportCsv({ columns?, filename? })`.", + docs: "Import from `@/components/ui/sheets`. A metadata-driven CRUD grid: it reads a table's columns from the endpoint's `_meta`, then loads, edits, creates, and deletes rows itself over GraphQL with the host's session. There's no `data` prop and no save callback. The host owns the endpoint, the session token, and auth errors, and PostgreSQL privileges and RLS at the endpoint decide every read and write.\n\nRender it in a client component. `Sheets` fills its parent, so give the parent a bounded height, and mount `` from `@/components/ui/portal` once as the last child of `` for the overlay editors:\n```tsx\n'use client';\nimport { useMemo } from 'react';\nimport { Sheets, SheetsProvider, type SheetsConfig } from '@/components/ui/sheets';\n\nexport function ProjectsGrid({ endpoint, userId, getToken }: { endpoint: string; userId: string; getToken: () => string | null }) {\n const config = useMemo(\n () => ({ endpoint, auth: { mode: 'embedded', getToken, getIdentityKey: () => userId }, onAuthError: signOut }),\n [endpoint, getToken, userId],\n );\n return (\n \n
\n \n
\n \n );\n}\n```\n`tableName` is a table's `name` in `_meta`, its GraphQL type name. To let people pick one, call `useSheetsMeta()` inside the provider, pass `data?._meta?.tables ?? []` to `selectConsoleDataTables` from `@constructive-io/data`, and give the names to ``.\n\n`SheetsProvider` takes `config: SheetsConfig`; keep it referentially stable.\n- `endpoint` (the app GraphQL endpoint) and `auth` are required. `auth` is `{ mode: 'embedded', getToken: () => string | null, getIdentityKey?: () => string | null }`, where the identity key is a non-secret user id that keeps each user's cache apart, or `{ mode: 'standalone' }` with `authEndpoint` and the grid wrapped in `SheetsAuthGate` from `@/components/ui/sheets/auth`.\n- Optional: `databaseId`, `locale` (default en-US), `fieldTypeOverrides: Record`, `storage: { endpoint, execute? }`, `upload: { bucketKey? }` (defaults to the tenant public bucket), `queryClient` (reuse yours), `onAuthError()` (on a 401, before the error throws), `onError(error, { source?: 'grid' | 'editor' | 'mutation' | 'upload', tableName? })`, `logger`, `plugins`.\n- To replace the transport: `execute(document, variables?)`, `executeUpload(file, path?, options?) => Promise<{ file: SheetsFileValue; url?: string }>`, or a whole `adapter: SheetsBackendAdapter` (`fetchMeta`, `listRows`, `createRow`, `updateRow`, `deleteRow`). The injected uploader takes precedence. Media editors require storage or an injected uploader. Signed download URLs are resolved through storage and stay separate from the stored file projection.\n\n`Sheets` props:\n- `tableName` (required), `pageSize` (default 100), `showSelection` and `showPagination` (default true), `infiniteScroll` (default false), `className`.\n- Observers only, since the grid does its own writes: `onRowSelect(rows)`, `onCellEdit(id, field, value)` once an edit is saved (`id` is the primary key, or an object for a composite key), and `onEvent({ type, tableName, at, meta? })` with `type` load:success | load:error | cell:edit | row:create | row:delete | filter:apply | sort:change | page:change.\n- `slots: { toolbar?, empty?, error?({ error, retry }), loading? }` replace those regions; `cellTypes` and `cellSlots` customize cells; `commands`, `keymap`, `interceptors`, and `onCommand(event, result)` extend or observe grid commands; `theme` and `themeMode` ('light' | 'dark' | 'system') style it.\n- A `ref` gets `SheetsHandle`: `scrollToRow(index)`, `submitDrafts()`, `refetch()`, `getSelectedRows()`, `exportCsv({ columns?, filename? })`.", }, registrySubdirectory: 'blocks/sheets', targetPrefix: '@ui/sheets', diff --git a/packages/sheets/src/adapter/postgraphile-adapter.ts b/packages/sheets/src/adapter/postgraphile-adapter.ts index f5a23f4d..f3524845 100644 --- a/packages/sheets/src/adapter/postgraphile-adapter.ts +++ b/packages/sheets/src/adapter/postgraphile-adapter.ts @@ -1,3 +1,5 @@ +import { print } from 'graphql'; + import { META_CONTRACT_INTROSPECTION_DOCUMENT, META_DOCUMENT, @@ -10,6 +12,7 @@ import { buildPostGraphileUpdate, buildSelect, createError, + getCustomAstForCleanField, normalizeSchemaEnumInputValues, prepareCreateInput, prepareUpdateInput, @@ -195,11 +198,28 @@ export function createPostGraphileAdapter(): SheetsBackendAdapter { options: MutationOptions = {}, ): Promise { const { table, allTables, tableName } = ctx; - const doc = buildPostGraphileUpdate(table, allTables, { - ...options, - fieldSelection: options.fieldSelection || 'display', - }); - const patchFieldName = toPatchFieldName(tableName, table); + const mutationName = toUpdateMutationName(tableName, table); + const returning = options.returning && table.fields + .filter((field) => options.returning?.includes(field.name)) + .map((field) => print(getCustomAstForCleanField(field))) + .join(' '); + // The root lookup field can have a ByKey suffix. The update payload + // and patch fields use the table's singular field name instead. + const payloadTable = returning && table.query + ? { ...table, query: { ...table.query, one: null } } + : table; + const singularName = toCamelCaseSingular(tableName, payloadTable); + const patchFieldName = toPatchFieldName(tableName, payloadTable); + // The current data builder ignores returning fields and derives the + // input type from the table name. Key mutations name it after the operation. + const doc = returning + ? `mutation SheetsUpdateFields($input: ${mutationName[0].toUpperCase()}${mutationName.slice(1)}Input!) { + ${mutationName}(input: $input) { ${singularName} { ${returning} } } + }` + : buildPostGraphileUpdate(table, allTables, { + ...options, + fieldSelection: options.fieldSelection || 'display', + }); const sanitizedPatch = prepareUpdateInput(patch); if (Object.keys(sanitizedPatch).length === 0) return null; const normalizedPatch = normalizeMutationEnums(ctx, sanitizedPatch, execute); @@ -211,8 +231,6 @@ export function createPostGraphileAdapter(): SheetsBackendAdapter { }, }; const result = (await execute(doc, variables)) as Record; - const mutationName = toUpdateMutationName(tableName, table); - const singularName = toCamelCaseSingular(tableName, table); const updatedRow = (result[mutationName] as Record)?.[singularName] ?? null; return updatedRow as T | null; }, diff --git a/packages/sheets/src/cell-model/cell-props.ts b/packages/sheets/src/cell-model/cell-props.ts index 988f9e33..73a7de55 100644 --- a/packages/sheets/src/cell-model/cell-props.ts +++ b/packages/sheets/src/cell-model/cell-props.ts @@ -21,4 +21,5 @@ export interface CellProps { isEditing: boolean; onStartEdit: () => void; disabled: boolean; + readOnlyReason?: string; } diff --git a/packages/sheets/src/cell-model/factories/image.ts b/packages/sheets/src/cell-model/factories/image.ts index 9147d96f..d7410f92 100644 --- a/packages/sheets/src/cell-model/factories/image.ts +++ b/packages/sheets/src/cell-model/factories/image.ts @@ -1,40 +1,26 @@ // Native SheetsCell factory for the MEDIA family (image/file/video/audio/upload). +// Attachment columns hold URL text and use the text factory instead. // -// Ports v1 `ImageCellFactory` (grid/cell-content-factory.ts). The glide cell put -// the resolved URL in BOTH `data` and `displayData` as a single-element array -// (`[url]`); empty/null/'' collapses to `['']`. The neutral SheetsCell keeps the -// SAME `data: [url]` array (so copyText === JSON.stringify([url]) like glide) and -// sets `displayData` to the STRING form of that array (JSON.stringify) so the -// `image` kind — projected via stringifyField, NOT the geometry/custom branch — -// yields the SAME displayText glide produced from its array `displayData`. -// -// URL extraction logic copied verbatim (NOT imported from cell-content-factory, -// which stays glide-only and is deleted at cutover). +// `data` is a single-element array holding the durable URL (`['']` when there is +// none), so copyText === JSON.stringify([url]) as before; `displayData` is the string +// form of that array. A signed preview URL never enters this payload: it expires, so +// copy/export only ever carry the stored URL. import type { CellType } from '../../cell-types/types'; import type { CellCreationMetadata } from '../../grid/grid-cell-types'; import { MEDIA_TYPES } from '../../cell-types/cell-type-groups'; +import { getImageUrl } from '../../utils/file.utils'; import type { SheetsCell } from '../sheets-cell'; import type { SheetsCellFactory } from './types'; -/** Resolve a media URL from a string or an object's url/src/href/path keys (v1 order). */ -function resolveMediaUrl(value: unknown): string { - if (typeof value === 'string') return value; - if (typeof value === 'object' && value) { - const obj = value as Record; - return (obj.url || obj.src || obj.href || obj.path || '') as string; - } - return ''; -} - export const imageSheetsCellFactory: SheetsCellFactory = { canHandle(cellType: string): boolean { - return MEDIA_TYPES.has(cellType); + return cellType !== 'attachment' && MEDIA_TYPES.has(cellType); }, create(value: unknown, metadata: CellCreationMetadata): SheetsCell { - const url = value === null || value === undefined || value === '' ? '' : resolveMediaUrl(value); - const data = [url]; + // Keep the legacy copy/export representation and its durable URL. + const data = [getImageUrl(value) ?? '']; return { kind: 'image', data, diff --git a/packages/sheets/src/cell-model/views/image-view.test.tsx b/packages/sheets/src/cell-model/views/image-view.test.tsx new file mode 100644 index 00000000..6671d46e --- /dev/null +++ b/packages/sheets/src/cell-model/views/image-view.test.tsx @@ -0,0 +1,195 @@ +/* @vitest-environment jsdom */ + +import { act } from 'react'; +import { createRoot } from 'react-dom/client'; +import { QueryClient } from '@tanstack/react-query'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { SheetsConfig } from '../../context/sheets-context'; +import { SheetsProvider } from '../../context/sheets-provider'; +import { __resetUploadCapabilityCache } from '../../context/sheets-upload-presigned'; +import { makeSchemaExecute } from '../../testing/graphql-schema-mock'; +import type { CellColumnDescriptor } from '../cell-props'; +import { imageSheetsCellFactory } from '../factories/image'; +import { ImageCellView } from './image-view'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const READ_SCHEMA = /* GraphQL */ ` + scalar UUID + type AppFile { id: UUID!, downloadUrl: String } + type AppFileConnection { nodes: [AppFile] } + input UUIDFilter { equalTo: UUID, in: [UUID!] } + input AppFileFilter { id: UUIDFilter } + type Query { + appFile(id: UUID!): AppFile + appFiles(where: AppFileFilter, first: Int): AppFileConnection + } +`; +const storageEndpoint = 'https://storage.local/graphql'; +const cdnUrl = 'https://cdn.local/photo.png'; +const signedUrl = (id: string, revision = 1) => `https://s3.local/${id}?signature=${revision}`; +const fileId = (number: number) => `a9240826-717c-4e79-bfc5-${String(number).padStart(12, '0')}`; +const projection = (number: number, mime = 'image/png') => ({ + id: fileId(number), + key: `file-${number}`, + bucket_id: 'bucket-1', + filename: `file-${number}`, + size: 1024, + mime, +}); + +interface FileFilter { + where: { id: { in?: string[]; equalTo?: string } }; + first?: number; +} + +function mediaCell(value: unknown, cellType: CellColumnDescriptor['cellType'], index: number) { + const column = { key: 'photo', name: 'Photo', cellType }; + const cell = imageSheetsCellFactory.create(value, { + cellType, fieldName: column.key, canEdit: true, isReadonly: false, activationBehavior: 'double-click', + }); + return ( + {}} disabled={false} + /> + ); +} + +describe('Sheets media previews', () => { + let container: HTMLDivElement; + let root: ReturnType; + let queryClient: QueryClient; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + queryClient = new QueryClient(); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + queryClient.clear(); + __resetUploadCapabilityCache(); + }); + + async function render( + values: unknown[], + storage?: SheetsConfig['storage'], + identityKey = 'user-1', + cellType: CellColumnDescriptor['cellType'] = 'image', + ) { + await act(async () => { + root.render( + null, getIdentityKey: () => identityKey }, + queryClient, + storage, + }}> + {values.map((value, index) => mediaCell(value, cellType, index))} + , + ); + }); + } + + async function expectPreviews(urls: string[]) { + await act(async () => { + await vi.waitFor(() => { + const signedLookups = queryClient.isFetching(); + expect(signedLookups).toBe(0); + }); + }); + // React Query delivers observer updates on the next task. + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); }); + expect(Array.from(container.querySelectorAll('img'), (img) => img.getAttribute('src'))).toEqual(urls); + } + + it('renders a durable URL without looking up storage and preserves the copied value', async () => { + const storage = vi.fn(); + const value = { ...projection(1), url: cdnUrl }; + expect(mediaCell(value, 'image', 0).props.cell.data).toEqual([cdnUrl]); + await render([value], { endpoint: storageEndpoint, execute: storage }); + await expectPreviews([cdnUrl]); + expect(storage).not.toHaveBeenCalled(); + }); + + it('loads visible image IDs in batches of 100 even when singular reads are available', async () => { + const appFile = vi.fn(); + const appFiles = vi.fn(({ where, first }: FileFilter) => ({ + nodes: where.id.in?.slice(0, first).reverse().map((id) => ({ id, downloadUrl: signedUrl(id) })), + })); + const storage = { endpoint: storageEndpoint, execute: makeSchemaExecute(READ_SCHEMA, { appFile, appFiles }) }; + const images = Array.from({ length: 101 }, (_, index) => projection(index + 1)); + + await render(images, storage); + await expectPreviews(images.map(({ id }) => signedUrl(id))); + expect(appFile).not.toHaveBeenCalled(); + const batches = appFiles.mock.calls.map(([{ where, first }]) => ({ size: where.id.in?.length, first })); + expect(batches).toEqual([{ size: 100, first: 100 }, { size: 1, first: 1 }]); + }); + + it('falls back to equality reads when the connection lacks an in operator', async () => { + const appFiles = vi.fn(({ where }: FileFilter) => { + const id = where.id.equalTo; + return { nodes: id ? [{ id, downloadUrl: signedUrl(id) }] : [] }; + }); + const sdl = READ_SCHEMA.replace(', in: [UUID!]', '').replace('appFile(id: UUID!): AppFile', ''); + const storage = { endpoint: storageEndpoint, execute: makeSchemaExecute(sdl, { appFiles }) }; + + await render([projection(1), projection(2)], storage); + await expectPreviews([signedUrl(fileId(1)), signedUrl(fileId(2))]); + expect(appFiles.mock.calls.map(([{ where }]) => where.id)).toEqual([ + { equalTo: fileId(1) }, { equalTo: fileId(2) }, + ]); + }); + + it('uses the existing chip when storage is absent or a file is not an image', async () => { + await render([projection(1)]); + await expectPreviews([]); + const chip = container.querySelector('[data-slot="file-cell"]'); + expect(chip?.textContent).toContain('file-1'); + expect(chip?.querySelector('[title]')?.getAttribute('title')).toContain('1 KB'); + + const execute = vi.fn(); + await render([projection(2, 'application/pdf')], { endpoint: storageEndpoint, execute }, 'user-1', 'upload'); + await expectPreviews([]); + expect(container.querySelector('[data-slot="file-cell"]')?.textContent).toContain('file-2'); + expect(execute).not.toHaveBeenCalled(); + }); + + it('reauthorizes a file under the new identity on the same endpoint', async () => { + const allowed = makeSchemaExecute(READ_SCHEMA, { + appFiles: () => ({ nodes: [{ id: fileId(1), downloadUrl: signedUrl(fileId(1)) }] }), + }); + const denied = vi.fn(() => ({ nodes: [] })); + await render([projection(1)], { endpoint: storageEndpoint, execute: allowed }); + await expectPreviews([signedUrl(fileId(1))]); + + await render([projection(1)], { endpoint: storageEndpoint, execute: makeSchemaExecute(READ_SCHEMA, { appFiles: denied }) }, 'user-2'); + await expectPreviews([]); + expect(denied).toHaveBeenCalledTimes(1); + expect(container.querySelector('[data-slot="file-cell"]')?.textContent).toContain('file-1'); + }); + + it('refreshes a failed signed image once and uses the chip if the replacement also fails', async () => { + let revision = 0; + const appFiles = vi.fn(() => ({ + nodes: [{ id: fileId(1), downloadUrl: signedUrl(fileId(1), ++revision) }], + })); + const storage = { endpoint: storageEndpoint, execute: makeSchemaExecute(READ_SCHEMA, { appFiles }) }; + + await render([projection(1)], storage); + await expectPreviews([signedUrl(fileId(1))]); + await act(async () => { container.querySelector('img')?.dispatchEvent(new Event('error')); }); + await expectPreviews([signedUrl(fileId(1), 2)]); + await act(async () => { container.querySelector('img')?.dispatchEvent(new Event('error')); }); + await expectPreviews([]); + expect(appFiles).toHaveBeenCalledTimes(2); + expect(container.querySelector('[data-slot="file-cell"]')?.textContent).toContain('file-1'); + }); +}); diff --git a/packages/sheets/src/cell-model/views/image-view.tsx b/packages/sheets/src/cell-model/views/image-view.tsx index c9da99b6..594fc343 100644 --- a/packages/sheets/src/cell-model/views/image-view.tsx +++ b/packages/sheets/src/cell-model/views/image-view.tsx @@ -1,30 +1,24 @@ -// Default native DOM view for the `image` SheetsCell kind. Dependency-light: plain -// JSX + Tailwind v4 + local `cn`, with no shared primitive dependency. Reads the URL -// from the factory payload (`cell.data` is `[url]`). +// Default native DOM view for the media SheetsCell kinds (image/file/video/audio/upload). // -// Mirrors the canvas painter (`grid/draw-image-cell.ts`): a uniform SQUARE -// thumbnail with object-fit:cover (center-crop, NOT contain) and rounded corners, -// inset by a small pad. A muted box sits UNDER the so it shows through -// while the image loads AND stands alone when the url is empty (the canvas drew -// the same placeholder for both the loading and missing-url branches). +// A uniform SQUARE thumbnail with object-fit:cover (center-crop, NOT contain) and +// rounded corners, inset by a small pad. A muted box sits UNDER the so it shows +// through while the image loads AND stands alone when there is no preview. A stored +// projection without a public URL resolves a signed preview through +// `useSheetsMediaUrl`; non-image files and unresolved projections render a file chip. // // Virtualization destroys off-screen rows, so the body is recreated on every -// scroll-back. We retain successfully-loaded URLs in a module LRU (`warmImage`) to -// keep them warm in memory — a remounted paints without a re-fetch. No -// `loading="lazy"` (it forces a fetch-after-mount on every remount). +// scroll-back. Durable URLs are retained in a module LRU (`warmImage`) so a remounted +// paints without a re-fetch; signed URLs live in the query cache and are never +// warmed. No `loading="lazy"` (it forces a fetch-after-mount on every remount). import { FileIcon } from 'lucide-react'; +import { useSheetsMediaUrl } from '../../hooks/use-sheets-media-url'; import type { CellProps } from '../cell-props'; import { cn } from '../../utils/cn'; -import { getImageMetadata } from '../../utils/file.utils'; +import { formatFileSize, getImageMetadata } from '../../utils/file.utils'; import { warmImage } from '../image-cache'; -function readUrl(data: unknown): string { - if (Array.isArray(data) && typeof data[0] === 'string') return data[0]; - return ''; -} - /** Last path segment of a url (sans query/hash), to label a file chip that has no filename. */ function basename(url: string): string { const path = url.split(/[?#]/)[0]; @@ -32,32 +26,28 @@ function basename(url: string): string { return seg || url; } -/** - * On a non-`image` media column (file/video/audio/upload), render a file chip UNLESS we KNOW - * the value is an image (an `image/*` mime). Non-image mimes, unknown mimes and bare URL - * strings all chip — mirroring the editor's stored-value preview so the grid never shows a - * broken for a document. The `image` column is unaffected (always a thumbnail). - */ -function isNonImageFile(cellType: string, value: unknown): boolean { - if (cellType === 'image' || value == null) return false; - const mime = getImageMetadata(value)?.mime; - return !(mime && mime !== 'unknown' && mime.startsWith('image/')); -} - export function ImageCellView(props: CellProps) { - const url = readUrl(props.cell.data); + const meta = getImageMetadata(props.value); + const previewImage = props.column.cellType === 'image' || Boolean(meta?.mime.startsWith('image/')); + const { url, durableUrl, id, isLoading, onImageError, onImageLoad } = useSheetsMediaUrl(props.value, previewImage); - if (url && isNonImageFile(props.column.cellType, props.value)) { - const meta = getImageMetadata(props.value); - const filename = meta?.filename && meta.filename !== 'Unknown' ? meta.filename : basename(url); + if ((!previewImage && (id || durableUrl)) || (id && !url && !isLoading)) { + const filename = meta?.filename && meta.filename !== 'Unknown' ? meta.filename : durableUrl ? basename(durableUrl) : 'File'; + const details = [ + filename, + meta?.size !== undefined ? formatFileSize(meta.size) : null, + meta?.mime !== 'unknown' ? meta?.mime : null, + ].filter(Boolean).join(' • '); return (
@@ -68,15 +58,22 @@ export function ImageCellView(props: CellProps) {
{url ? ( warmImage(url)} + onError={onImageError} + onLoad={() => { + onImageLoad(); + if (durableUrl) warmImage(durableUrl); + }} className={cn( 'absolute inset-0 h-full w-full rounded-sm object-cover outline-1 -outline-offset-1 outline-black/10 dark:outline-white/10', )} diff --git a/packages/sheets/src/cell-model/views/text-view.tsx b/packages/sheets/src/cell-model/views/text-view.tsx index 19ff1683..2637fa8d 100644 --- a/packages/sheets/src/cell-model/views/text-view.tsx +++ b/packages/sheets/src/cell-model/views/text-view.tsx @@ -14,7 +14,9 @@ export function TextCellView(props: CellProps) {
{text} diff --git a/packages/sheets/src/cell-types/cell-type-groups.ts b/packages/sheets/src/cell-types/cell-type-groups.ts index 867c89aa..8234b5e3 100644 --- a/packages/sheets/src/cell-types/cell-type-groups.ts +++ b/packages/sheets/src/cell-types/cell-type-groups.ts @@ -57,7 +57,7 @@ export const STRUCTURED_DATA_TYPES: Set = new Set([ ]); // ─── Media ────────────────────────────────────────────────────────────────── -export const MEDIA_TYPES: Set = new Set(['image', 'file', 'video', 'audio', 'upload']); +export const MEDIA_TYPES: Set = new Set(['image', 'file', 'video', 'audio', 'upload', 'attachment']); // ─── Enhanced scalar types ────────────────────────────────────────────────── export const ENHANCED_TYPES: Set = new Set([ diff --git a/packages/sheets/src/cell-types/type-mapping.ts b/packages/sheets/src/cell-types/type-mapping.ts index 0086d200..b7bc6855 100644 --- a/packages/sheets/src/cell-types/type-mapping.ts +++ b/packages/sheets/src/cell-types/type-mapping.ts @@ -23,7 +23,7 @@ export function mapToFrontendCellType(metadata: { // This must be checked FIRST, even for arrays, to preserve types like 'array' vs 'text-array' if (pgAlias) { const aliasType = mapAliasToCellType(pgAlias); - if (aliasType !== 'unknown') return aliasType; + if (aliasType !== 'unknown' && !(isArray && aliasType === 'attachment')) return aliasType; } // Handle arrays after pgAlias check @@ -80,6 +80,7 @@ function mapAliasToCellType(alias: string): CellType { video: 'video', audio: 'audio', upload: 'upload', + attachment: 'attachment', // Relation types relation: 'relation', @@ -234,6 +235,7 @@ function mapBackendTypeToCellType(pgType: string): CellType { url: 'url', image: 'image', upload: 'upload', + attachment: 'attachment', origin: 'origin', // Arrays are handled by isArray flag diff --git a/packages/sheets/src/cell-types/types.ts b/packages/sheets/src/cell-types/types.ts index 18287091..05975891 100644 --- a/packages/sheets/src/cell-types/types.ts +++ b/packages/sheets/src/cell-types/types.ts @@ -49,6 +49,7 @@ export type CellType = | 'video' | 'audio' | 'upload' // Custom upload type + | 'attachment' // Public file URL stored as text // Special cells | 'uuid' | 'color' diff --git a/packages/sheets/src/context/sheets-context.ts b/packages/sheets/src/context/sheets-context.ts index c76fdc73..06971396 100644 --- a/packages/sheets/src/context/sheets-context.ts +++ b/packages/sheets/src/context/sheets-context.ts @@ -21,6 +21,11 @@ export interface SheetsAuthStandalone { mode: 'standalone'; } +export interface SheetsStorageTransport { + endpoint: string; + execute: SheetsExecuteFn; +} + export interface SheetsConfig { /** Data endpoint (app-public GraphQL) */ endpoint: string; @@ -38,7 +43,9 @@ export interface SheetsConfig { execute?: SheetsExecuteFn; /** Optional: inject custom upload function */ executeUpload?: SheetsUploadFn; - /** Optional upload settings. `bucketKey` defaults to "public". */ + /** Storage endpoint for media uploads and download URL lookups. */ + storage?: { endpoint: string; execute?: SheetsExecuteFn }; + /** Explicit bucket override. Omit to use the tenant's public default. */ upload?: { bucketKey?: string }; /** Optional: inject existing QueryClient */ queryClient?: QueryClient; @@ -61,6 +68,8 @@ export interface SheetsContextValue { config: SheetsConfig; execute: SheetsExecuteFn; executeUpload: SheetsUploadFn; + storage?: SheetsStorageTransport; + canUpload: boolean; scopeKey: SheetsScopeKey; } diff --git a/packages/sheets/src/context/sheets-execute.test.ts b/packages/sheets/src/context/sheets-execute.test.ts index 5447d21d..d1a3a946 100644 --- a/packages/sheets/src/context/sheets-execute.test.ts +++ b/packages/sheets/src/context/sheets-execute.test.ts @@ -4,10 +4,13 @@ import type { SheetsConfig } from './sheets-context'; import { createSheetsExecute, createSheetsUpload } from './sheets-execute'; import { resolveUploadCapability, + resolveDownloadUrl, __resetUploadCapabilityCache, } from './sheets-upload-presigned'; import { buildIntrospection, + graphqlFetchFromSchema, + uploadPayload, makeSchemaExecute, makeSchemaExecuteWithStats, BUCKET_ONLY_SDL, @@ -153,7 +156,7 @@ describe('createSheetsUpload — presigned seam', () => { { uploadAppFile: (args: { input: Record }) => { capturedInput = args.input; - return { uploadUrl: 'https://s3.local/put?sig=abc', fileId: 'file-1', key: 'abc', deduplicated: false }; + return uploadPayload({ uploadUrl: 'https://s3.local/put?sig=abc', fileId: 'file-1', key: 'abc', deduplicated: false }, { mime: 'image/png' }); }, appFile: (args: { id: string }) => { capturedDownloadId = args.id; @@ -172,7 +175,7 @@ describe('createSheetsUpload — presigned seam', () => { // (a) mutation variables — input shape, hash, and NO ownerId expect(capturedInput).toEqual({ - bucketKey: 'public', + isPublic: true, contentHash: expectedHash, contentType: 'image/png', size: file.size, @@ -196,10 +199,14 @@ describe('createSheetsUpload — presigned seam', () => { expect(downloadVars).toEqual({ id: 'file-1' }); expect(result).toMatchObject({ url: 'https://cdn.local/public/abc', - filename: 'hello.txt', - mime: 'image/png', - size: file.size, + file: { + bucket_id: 'bucket-1', + filename: 'hello.txt', + mime: 'image/png', + size: file.size, + }, }); + expect(result.file.url).toBeUndefined(); }); it('relay node fallback: downloadUrl via node(nodeId:) with a base64 global id (not the raw pk)', async () => { @@ -215,7 +222,7 @@ describe('createSheetsUpload — presigned seam', () => { const execute = makeSchemaExecute( NODE_EXEC_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put?sig=n', fileId: 'file-n', key: 'x', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put?sig=n', fileId: 'file-n', key: 'x', deduplicated: false }), node: (args: { nodeId: string }) => { capturedNodeId = args.nodeId; return { downloadUrl: 'https://cdn/node.png' }; @@ -244,7 +251,7 @@ describe('createSheetsUpload — presigned seam', () => { const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { - uploadAppFile: () => ({ uploadUrl: null, fileId: 'file-dup', key: 'abc', deduplicated: true }), + uploadAppFile: () => uploadPayload({ uploadUrl: null, fileId: 'file-dup', key: 'abc', deduplicated: true }), appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/abc' }), }); @@ -269,7 +276,7 @@ describe('createSheetsUpload — presigned seam', () => { const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { uploadAppFile: (args: { input: Record }) => { mutationContentType = args.input.contentType; - return { uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }; + return uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }); }, appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); @@ -314,7 +321,7 @@ describe('createSheetsUpload — presigned seam', () => { ); const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); @@ -336,7 +343,7 @@ describe('createSheetsUpload — presigned seam', () => { }); const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); @@ -361,7 +368,7 @@ describe('createSheetsUpload — presigned seam', () => { const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { uploadAppFile: (args: { input: Record }) => { sentHash = args.input.contentHash; - return { uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }; + return uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }); }, appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); @@ -379,7 +386,7 @@ describe('createSheetsUpload — presigned seam', () => { vi.spyOn(globalThis, 'fetch').mockRejectedValue(new TypeError('Failed to fetch')); const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); @@ -392,13 +399,13 @@ describe('createSheetsUpload — presigned seam', () => { }); }); - it('PUT failure ⇒ deletes the row the upload op created, then rejects with the PUT error', async () => { + it('PUT failure ⇒ leaves the row for server expiry and preserves the PUT error', async () => { stubDigest(); vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('denied', { status: 403, statusText: 'Forbidden' })); const deleted: unknown[] = []; const execute = makeSchemaExecute(UPLOAD_WITH_WRITES_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put', fileId: 'file-orphan', key: 'k', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'file-orphan', key: 'k', deduplicated: false }), deleteAppFile: (args: { input: { id: string } }) => { deleted.push(args.input.id); return { appFile: { id: args.input.id } }; @@ -410,7 +417,7 @@ describe('createSheetsUpload — presigned seam', () => { const upload = createSheetsUpload(config, () => null); await expect(upload(createTestFile())).rejects.toThrow('Upload failed: 403 Forbidden'); - expect(deleted).toEqual(['file-orphan']); + expect(deleted).toEqual([]); }); it('non-deduplicated payload with null uploadUrl ⇒ badRequest, no PUT', async () => { @@ -418,23 +425,119 @@ describe('createSheetsUpload — presigned seam', () => { const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { - uploadAppFile: () => ({ uploadUrl: null, fileId: 'f', key: 'k', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: null, fileId: 'f', key: 'k', deduplicated: false }), appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); const config: SheetsConfig = { endpoint: 'https://badpayload.example.com/graphql', auth: { mode: 'standalone' }, execute }; const upload = createSheetsUpload(config, () => null); - await expect(upload(createTestFile())).rejects.toMatchObject({ code: 'BAD_USER_INPUT' }); + await expect(upload(createTestFile())).rejects.toThrow('no uploadUrl'); expect(fetchSpy).not.toHaveBeenCalled(); }); + it('rejects a schema without the file projection before creating a row', async () => { + const fetch = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); + const uploadAppFile = vi.fn(); + const execute = makeSchemaExecute(ROOT_UPLOAD_SDL.replace('file: JSON', ''), { uploadAppFile }); + const upload = createSheetsUpload({ ...createEmbeddedConfig(), execute }, () => null); + + await expect(upload(createTestFile())).rejects.toThrow('file projection'); + expect(uploadAppFile).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('still resolves an existing file on a schema without an upload projection', async () => { + const execute = makeSchemaExecute(ROOT_UPLOAD_SDL.replace('file: JSON', ''), { + appFile: () => ({ id: 'existing', downloadUrl: 'https://cdn.local/existing' }), + }); + + await expect(resolveDownloadUrl(execute, 'https://older.example.com/graphql', 'existing')) + .resolves.toBe('https://cdn.local/existing'); + }); + + it('resolves an existing file on a read-only storage endpoint with no upload mutation', async () => { + const execute = makeSchemaExecute(NO_UPLOADS_SDL, { + appFile: () => ({ id: 'existing', downloadUrl: 'https://cdn.local/read-only' }), + }); + + await expect(resolveDownloadUrl(execute, 'https://read-only.example.com/graphql', 'existing')) + .resolves.toBe('https://cdn.local/read-only'); + }); + + it('rejects an empty projection before PUT', async () => { + const fetch = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); + const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { + uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false, file: null }), + }); + const upload = createSheetsUpload({ ...createEmbeddedConfig(), execute }, () => null); + + await expect(upload(createTestFile())).rejects.toThrow('file projection'); + expect(fetch).not.toHaveBeenCalled(); + }); + + it.each([false, true])('returns the projection when lookup fails after upload (dedup=%s)', async (deduplicated) => { + const fetch = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); + const payload = uploadPayload({ uploadUrl: deduplicated ? null : 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated }); + const appFile = vi.fn(() => { + throw new Error('File lookup failed'); + }); + const execute = makeSchemaExecute(ROOT_UPLOAD_SDL, { uploadAppFile: () => payload, appFile }); + const upload = createSheetsUpload({ ...createEmbeddedConfig(), execute }, () => null); + + expect(await upload(createTestFile())).toEqual({ file: payload.file }); + expect(fetch).toHaveBeenCalledTimes(deduplicated ? 0 : 1); + expect(appFile).toHaveBeenCalledTimes(1); + }); + + it('uploads to the selected bucket on a storage endpoint with no download lookup', async () => { + const fetch = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); + const payload = uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }); + const uploadAppFile = vi.fn((_args: { input: Record }) => payload); + const sdl = ROOT_UPLOAD_SDL.replace('type Query { appFile(id: UUID!): AppFile }', 'type Query { ready: Boolean }'); + const onOperation = vi.fn(); + const execute = makeSchemaExecute(sdl, { uploadAppFile }, onOperation); + const dataExecute = vi.fn(async (): Promise => { + throw new Error('Upload used the data endpoint'); + }); + const upload = createSheetsUpload({ + ...createEmbeddedConfig(), + execute: dataExecute, + storage: { endpoint: 'https://storage.example.com/graphql', execute }, + upload: { bucketKey: 'selected-bucket' }, + }, () => null); + + expect(await upload(createTestFile())).toEqual({ file: payload.file }); + expect(uploadAppFile.mock.calls[0]?.[0]).toMatchObject({ input: { bucketKey: 'selected-bucket' } }); + expect(uploadAppFile.mock.calls[0]?.[0]).not.toHaveProperty('input.isPublic'); + expect(dataExecute).not.toHaveBeenCalled(); + expect(onOperation).toHaveBeenCalledTimes(2); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('uses the storage URL and current host auth when no storage executor is injected', async () => { + const endpoint = 'https://storage.example.com/graphql'; + const payload = uploadPayload({ uploadUrl: null, fileId: 'f', key: 'k', deduplicated: true }, { url: 'https://cdn.local/k' }); + const fetch = vi.spyOn(globalThis, 'fetch').mockImplementation(graphqlFetchFromSchema(ROOT_UPLOAD_SDL, { + uploadAppFile: () => ({ ...payload, file: { ...payload.file, filename: null } }), + appFile: () => ({ id: 'f', downloadUrl: 'https://cdn.local/k' }), + })); + const upload = createSheetsUpload({ ...createEmbeddedConfig(), storage: { endpoint } }, () => 'host-token'); + + expect(await upload(createTestFile())).toEqual({ file: payload.file, url: 'https://cdn.local/k' }); + expect(fetch).toHaveBeenCalled(); + for (const [url, init] of fetch.mock.calls) { + expect(url).toBe(endpoint); + expect(new Headers(init?.headers).get('authorization')).toBe('Bearer host-token'); + } + }); + it('capability cache: two uploads to the same endpoint introspect once', async () => { stubDigest(); vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('', { status: 200 })); const { execute, introspectionCount } = makeSchemaExecuteWithStats(ROOT_UPLOAD_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put', fileId: 'f', key: 'k', deduplicated: false }), appFile: (args: { id: string }) => ({ id: args.id, downloadUrl: 'https://cdn.local/public/k' }), }); @@ -465,7 +568,7 @@ describe('createSheetsUpload — presigned seam', () => { { uploadAppFile: (args: { input: Record }) => { capturedInput = args.input; - return { uploadUrl: 'https://s3.local/put?sig=z', fileId: 'file-c', key: 'x', deduplicated: false }; + return uploadPayload({ uploadUrl: 'https://s3.local/put?sig=z', fileId: 'file-c', key: 'x', deduplicated: false }, { mime: 'image/png' }); }, appFiles: (args: { where?: Record }) => { capturedWhere = args.where; @@ -484,7 +587,7 @@ describe('createSheetsUpload — presigned seam', () => { // upload input shape (NO ownerId), real hash expect(capturedInput).toEqual({ - bucketKey: 'public', + isPublic: true, contentHash: expectedHash, contentType: 'image/png', size: file.size, @@ -501,10 +604,14 @@ describe('createSheetsUpload — presigned seam', () => { expect(result).toMatchObject({ url: 'https://cdn/x.png', - filename: 'hello.txt', - mime: 'image/png', - size: file.size, + file: { + bucket_id: 'bucket-1', + filename: 'hello.txt', + mime: 'image/png', + size: file.size, + }, }); + expect(result.file.url).toBeUndefined(); }); it('connection condition-style (b): downloadUrl via appFiles(condition:{id:$id}), no operator wrapper', async () => { @@ -522,7 +629,7 @@ describe('createSheetsUpload — presigned seam', () => { const execute = makeSchemaExecute( CONDITION_ONLY_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put?sig=c', fileId: 'file-cond', key: 'x', deduplicated: false }), + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put?sig=c', fileId: 'file-cond', key: 'x', deduplicated: false }), appFiles: (args: { condition?: Record }) => { capturedCondition = args.condition; return { nodes: [{ downloadUrl: 'https://cdn/cond.png' }] }; @@ -555,12 +662,14 @@ describe('createSheetsUpload — presigned seam', () => { // uploadAppFile, so discovery must prefer the appFiles connection (-> AppFile). // This is the live production path; the wrong connection -> 0 nodes -> fail. let appFilesWhere: Record | undefined; + const uploadDataRoomFile = vi.fn(); let dataRoomCalled = false; let downloadSource: string | undefined; const execute = makeSchemaExecute( CONNECTION_TWO_FILES_SDL, { - uploadAppFile: () => ({ uploadUrl: 'https://s3.local/put?sig=m', fileId: 'file-m', key: 'x', deduplicated: false }), + uploadDataRoomFile, + uploadAppFile: () => uploadPayload({ uploadUrl: 'https://s3.local/put?sig=m', fileId: 'file-m', key: 'x', deduplicated: false }), appFiles: (args: { where?: Record }) => { appFilesWhere = args.where; return { nodes: [{ downloadUrl: 'https://cdn/app.png' }] }; @@ -583,6 +692,7 @@ describe('createSheetsUpload — presigned seam', () => { expect(downloadSource).toContain('appFiles('); expect(downloadSource).not.toContain('dataRoomFiles('); expect(dataRoomCalled).toBe(false); + expect(uploadDataRoomFile).not.toHaveBeenCalled(); expect(appFilesWhere).toEqual({ id: { equalTo: 'file-m' } }); expect(result.url).toBe('https://cdn/app.png'); }); @@ -613,25 +723,23 @@ describe('resolveUploadCapability', () => { expect(cap).toMatchObject({ uploadFieldName: 'uploadAppFile', inputTypeName: 'UploadAppFileInput', - fileByIdField: 'appFile', - fileByIdArgName: 'id', - fileTypeName: 'AppFile', + download: { + fileByIdField: 'appFile', + fileByIdArgName: 'id', + fileTypeName: 'AppFile', + }, }); }); - it('discovers the delete-by-id mutation for the uploaded file type, never the update', () => { - const cap = resolveUploadCapability(buildIntrospection(UPLOAD_WITH_WRITES_SDL).__schema); - expect(cap).toMatchObject({ deleteFieldName: 'deleteAppFile', deleteInputTypeName: 'DeleteAppFileInput' }); - expect(resolveUploadCapability(buildIntrospection(ROOT_UPLOAD_SDL).__schema).deleteFieldName).toBeUndefined(); - }); - it('falls back to Relay node(nodeId:) when no id-arg/connection field exists', () => { const cap = resolveUploadCapability(buildIntrospection(RELAY_ONLY_SDL).__schema); expect(cap).toMatchObject({ uploadFieldName: 'uploadAppFile', - fileByIdField: 'node', - fileByIdArgName: 'nodeId', - fileTypeName: 'AppFile', + download: { + fileByIdField: 'node', + fileByIdArgName: 'nodeId', + fileTypeName: 'AppFile', + }, }); }); @@ -639,14 +747,16 @@ describe('resolveUploadCapability', () => { const cap = resolveUploadCapability(buildIntrospection(CONNECTION_ONLY_SDL).__schema); expect(cap).toMatchObject({ uploadFieldName: 'uploadAppFile', - fileByIdField: 'appFiles', - fileByIdArgName: 'connection', - fileTypeName: 'AppFile', - idScalarName: 'UUID', - connectionFilterArgName: 'where', - connectionIdField: 'id', - connectionIdOperator: 'equalTo', - connectionNodesField: 'nodes', + download: { + fileByIdField: 'appFiles', + fileByIdArgName: 'connection', + fileTypeName: 'AppFile', + idScalarName: 'UUID', + connectionFilterArgName: 'where', + connectionIdField: 'id', + connectionIdOperator: 'equalTo', + connectionNodesField: 'nodes', + }, }); }); @@ -654,16 +764,18 @@ describe('resolveUploadCapability', () => { const cap = resolveUploadCapability(buildIntrospection(CONDITION_ONLY_SDL).__schema); expect(cap).toMatchObject({ uploadFieldName: 'uploadAppFile', - fileByIdField: 'appFiles', - fileByIdArgName: 'connection', - fileTypeName: 'AppFile', - idScalarName: 'UUID', - connectionFilterArgName: 'condition', - connectionIdField: 'id', - connectionNodesField: 'nodes', + download: { + fileByIdField: 'appFiles', + fileByIdArgName: 'connection', + fileTypeName: 'AppFile', + idScalarName: 'UUID', + connectionFilterArgName: 'condition', + connectionIdField: 'id', + connectionNodesField: 'nodes', + }, }); // condition style carries NO equality operator - expect(cap.connectionIdOperator).toBeUndefined(); + expect(cap.download?.connectionIdOperator).toBeUndefined(); }); it('multi-storage-table connection: prefers the upload-op-matched connection (appFiles over dataRoomFiles)', () => { @@ -671,11 +783,13 @@ describe('resolveUploadCapability', () => { const cap = resolveUploadCapability(buildIntrospection(CONNECTION_TWO_FILES_SDL).__schema); expect(cap).toMatchObject({ uploadFieldName: 'uploadAppFile', - fileByIdField: 'appFiles', - fileByIdArgName: 'connection', - fileTypeName: 'AppFile', - connectionFilterArgName: 'where', - connectionIdOperator: 'equalTo', + download: { + fileByIdField: 'appFiles', + fileByIdArgName: 'connection', + fileTypeName: 'AppFile', + connectionFilterArgName: 'where', + connectionIdOperator: 'equalTo', + }, }); }); diff --git a/packages/sheets/src/context/sheets-execute.ts b/packages/sheets/src/context/sheets-execute.ts index 6f60ab0e..5f349f2a 100644 --- a/packages/sheets/src/context/sheets-execute.ts +++ b/packages/sheets/src/context/sheets-execute.ts @@ -3,10 +3,9 @@ import { print } from 'graphql'; import { ConstructiveError, createError, TypedDocumentString } from '@constructive-io/data'; -import type { SheetsConfig } from './sheets-context'; +import type { SheetsConfig, SheetsStorageTransport } from './sheets-context'; import { callUploadOp, - deleteUploadedFileRow, discoverUploadCapability, fetchDownloadUrl, presignedPut, @@ -24,13 +23,23 @@ export type SheetsExecuteFn = ( variables?: Record, ) => Promise; -/** - * Result of an upload through the seam. `url` is the only guaranteed field; - * the REST upload endpoint also returns `{ filename, mime, size }` (and may - * return server-internal extras). Keeping it open-ended lets callers read the - * richer metadata while a bare `{ url }` (e.g. the mock) still satisfies it. - */ -export type SheetsUploadResult = { url: string } & Record; +/** Durable file metadata for image and upload columns. */ +export interface SheetsFileValue { + id: string; + key: string; + bucket_id: string; + mime: string; + size: number; + filename: string; + /** Public URL supplied by storage, when available. */ + url?: string; +} + +export interface SheetsUploadResult { + file: SheetsFileValue; + /** Optional download URL for immediate use. It can expire. */ + url?: string; +} /** * Optional upload metadata carried through the seam (third arg). Lets an @@ -181,45 +190,46 @@ export function createSheetsExecute( }; } -/** - * Presigned-URL cell-upload seam. - * - * Replaces the legacy multipart `/upload` POST entirely. The flow: - * 1. Read + SHA-256 hash the file bytes (Web Crypto) - * 2. Discover (cached per endpoint) the upload op + downloadUrl-by-id field - * via GraphQL introspection — the root `uploadFile` mutation, matched - * by FIELD/TYPE SHAPE (LOCKED: no hardcoded names) - * 3. Call the upload op with input `{ bucketKey, contentHash, contentType, - * size, filename }` — NO `ownerId` (app-scoped buckets have no owner_id) - * 4. If `uploadUrl` is non-null, PUT the raw bytes to it with ONLY the signed - * `Content-Type` header (NO Authorization). Skipped when `deduplicated`. - * 5. Resolve the stable public `downloadUrl` for the returned fileId. - * - * Returns `{ url, filename, mime, size, key, fileId }` where `url` is the stable - * download URL — the same cell value shape callers/Step-2 already consume. - * - * `path` is ignored (content-addressed buckets key on `contentHash`). - */ +export function resolveStorageTransport( + config: SheetsConfig, + getToken: () => string | null, +): SheetsStorageTransport | undefined { + const endpoint = config.storage?.endpoint; + if (!endpoint) return undefined; + + return { + endpoint, + execute: config.storage?.execute ?? createSheetsExecute({ ...config, endpoint }, getToken), + }; +} + +/** Creates an uploader for storage. Direct callers may use the supplied endpoint as storage. */ export function createSheetsUpload(config: SheetsConfig, getToken: () => string | null): SheetsUploadFn { - // Reuse the kept GraphQL executor so canonical error normalization + onAuthError - // (401/UNAUTHENTICATED) apply to introspection, the upload op, and the - // downloadUrl query alike. - const execute = config.execute ?? createSheetsExecute(config, getToken); + const transport = resolveStorageTransport(config, getToken) ?? { + endpoint: config.endpoint, + execute: config.execute ?? createSheetsExecute(config, getToken), + }; + return createPresignedSheetsUpload(transport, config.upload?.bucketKey); +} - return async (file: File, _path?: string, options?: SheetsUploadOptions): Promise => { - if (!config.endpoint) { +export function createPresignedSheetsUpload( + { endpoint, execute }: SheetsStorageTransport, + bucketKey?: string, +): SheetsUploadFn { + return async (file, _path, options) => { + if (!endpoint) { throw createError.badRequest('No GraphQL endpoint configured.'); } - const bucketKey = config.upload?.bucketKey ?? 'public'; - const contentType = file.type || 'application/octet-stream'; + const cap = await discoverUploadCapability(endpoint, execute); + if (!cap.hasFileProjection) { + throw createError.badRequest('Uploads require a storage mutation that returns a file projection.'); + } - // Single read of the bytes — reused for both hashing and the PUT body. + const contentType = file.type || 'application/octet-stream'; + // Read the file once for both hashing and the PUT body. const buffer = await file.arrayBuffer(); const contentHash = await sha256HexFromBuffer(buffer); - - const cap = await discoverUploadCapability(config.endpoint, execute); - const payload = await callUploadOp(execute, cap, { bucketKey, contentHash, @@ -228,36 +238,27 @@ export function createSheetsUpload(config: SheetsConfig, getToken: () => string filename: file.name, }); - // Dedup (deduplicated ⇒ uploadUrl === null) means the row + S3 object already - // exist; skip the PUT. A non-deduplicated payload MUST carry an uploadUrl — a - // null there is a server-contract violation, so fail loudly rather than - // resolving a downloadUrl for an object that was never uploaded. if (!payload.deduplicated) { if (!payload.uploadUrl) { throw createError.badRequest('Upload failed: server returned no uploadUrl for a non-deduplicated file.'); } - try { - await presignedPut(payload.uploadUrl, buffer, contentType, options?.onProgress); - } catch (error) { - // The upload op already created the row; drop it so the bucket doesn't list - // a file whose bytes were never stored. - await deleteUploadedFileRow(execute, cap, payload.fileId); - throw error; - } + // The server expires rows whose PUT fails. + await presignedPut(payload.uploadUrl, buffer, contentType, options?.onProgress); } else { options?.onProgress?.({ loaded: file.size, total: file.size, percentage: 100 }); } - const url = await fetchDownloadUrl(execute, cap, payload.fileId); - - return { - url, - filename: file.name, - mime: file.type, - size: file.size, - key: payload.key, - fileId: payload.fileId, + const result: SheetsUploadResult = { + file: { ...payload.file, filename: payload.file.filename ?? file.name }, }; + if (!cap.download) return result; + + try { + return { ...result, url: await fetchDownloadUrl(execute, cap.download, payload.fileId) }; + } catch { + // The upload succeeded. The download URL can be requested again later. + return result; + } }; } diff --git a/packages/sheets/src/context/sheets-provider.lifecycle.test.tsx b/packages/sheets/src/context/sheets-provider.lifecycle.test.tsx index d6ded533..0a9c2f87 100644 --- a/packages/sheets/src/context/sheets-provider.lifecycle.test.tsx +++ b/packages/sheets/src/context/sheets-provider.lifecycle.test.tsx @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { SheetsConfig } from './sheets-context'; import { useSheetsContext, type SheetsScopeKey } from './sheets-context'; import { SheetsProvider } from './sheets-provider'; +import type { SheetsUploadFn } from './sheets-execute'; import { useSheetsStoreApi } from '../store/sheets-store'; import { getStoredToken } from '../auth/utils/token-store'; @@ -111,6 +112,47 @@ describe('SheetsProvider lifecycle', () => { vi.clearAllMocks(); }); + it('uses a host uploader when provided and keeps uploads off the data endpoint otherwise', async () => { + let upload!: SheetsUploadFn; + function UploadProbe() { + upload = useSheetsContext().executeUpload; + return null; + } + + const config = createEmbeddedConfig(); + const execute = vi.fn(); + const executeStorage = vi.fn(); + const executeUpload = vi.fn().mockResolvedValue({ + file: { id: 'file-1', key: 'avatar', bucket_id: 'bucket-1', filename: 'avatar.png', mime: 'image/png', size: 1 }, + }); + await act(async () => { + root.render( + + + , + ); + }); + + const file = new File(['x'], 'avatar.png', { type: 'image/png' }); + await expect(upload(file)).resolves.toMatchObject({ file: { id: 'file-1' } }); + expect(executeUpload).toHaveBeenCalledWith(file); + expect(executeStorage).not.toHaveBeenCalled(); + + await act(async () => { + root.render( + + + , + ); + }); + + await expect(upload(file)).rejects.toThrow('Uploads need a storage endpoint'); + expect(execute).not.toHaveBeenCalled(); + }); + it('restores standalone auth token for the active database', async () => { const expiresAt = new Date(Date.now() + 60_000).toISOString(); mockGetStoredToken.mockReturnValue({ accessToken: 'db-token', expiresAt }); diff --git a/packages/sheets/src/context/sheets-provider.tsx b/packages/sheets/src/context/sheets-provider.tsx index 5c81bb56..c8bb3e8b 100644 --- a/packages/sheets/src/context/sheets-provider.tsx +++ b/packages/sheets/src/context/sheets-provider.tsx @@ -3,7 +3,13 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { useStore as useZustandStore, type StoreApi } from 'zustand'; import { SheetsContext, type SheetsConfig, type SheetsContextValue, type SheetsScopeKey } from './sheets-context'; -import { createSheetsExecute, createSheetsUpload, type SheetsExecuteFn, type SheetsUploadFn } from './sheets-execute'; +import { + createPresignedSheetsUpload, + createSheetsExecute, + resolveStorageTransport, + type SheetsExecuteFn, + type SheetsUploadFn, +} from './sheets-execute'; import { createSheetsStore, SheetsStoreContext, type SheetsStoreState } from '../store/sheets-store'; import { getStoredToken } from '../auth/utils/token-store'; import { setSheetsLogger } from '../utils/sheets-logger'; @@ -15,6 +21,10 @@ interface SheetsProviderProps { children: React.ReactNode; } +const unavailableUpload: SheetsUploadFn = async () => { + throw new Error('Uploads need a storage endpoint'); +}; + export function SheetsProvider({ config, children }: SheetsProviderProps) { const providerInstanceId = useId(); // Create store once @@ -94,11 +104,13 @@ export function SheetsProvider({ config, children }: SheetsProviderProps) { return createSheetsExecute(config, getToken); }, [config, getToken]); - // Create upload function + const storage = useMemo(() => resolveStorageTransport(config, getToken), [config, getToken]); + const bucketKey = config.upload?.bucketKey; const executeUpload: SheetsUploadFn = useMemo(() => { if (config.executeUpload) return config.executeUpload; - return createSheetsUpload(config, getToken); - }, [config, getToken]); + return storage ? createPresignedSheetsUpload(storage, bucketKey) : unavailableUpload; + }, [config.executeUpload, storage, bucketKey]); + const canUpload = Boolean(config.executeUpload || storage); const configuredIdentityKey = config.auth.mode === 'embedded' @@ -142,8 +154,8 @@ export function SheetsProvider({ config, children }: SheetsProviderProps) { }, [queryClient, scopeKey]); const contextValue: SheetsContextValue = useMemo( - () => ({ config, execute, executeUpload, scopeKey }), - [config, execute, executeUpload, scopeKey], + () => ({ config, execute, executeUpload, storage, canUpload, scopeKey }), + [config, execute, executeUpload, storage, canUpload, scopeKey], ); return ( diff --git a/packages/sheets/src/context/sheets-upload-presigned.ts b/packages/sheets/src/context/sheets-upload-presigned.ts index 96259850..a6f26e6d 100644 --- a/packages/sheets/src/context/sheets-upload-presigned.ts +++ b/packages/sheets/src/context/sheets-upload-presigned.ts @@ -6,14 +6,12 @@ * 2. Call the presigned upload op (discovered via introspection) to get a * presigned PUT url + fileId * 3. PUT the raw bytes to that url (only Content-Type, NO Authorization) - * 4. Resolve the stable public `downloadUrl` for the fileId + * 4. Return the file projection and try to resolve an optional download URL * - * `confirmUpload` is intentionally skipped: the backend plugin creates the file - * row inside the upload op, so `downloadUrl` resolves immediately. + * The server confirms PUTs and expires rows that never received one. * - * The upload op is discovered via runtime introspection, matched by FIELD/TYPE - * SHAPE (never a hardcoded name): a root mutation `uploadFile(input: {...})` - * whose input + payload carry the upload shape. + * Runtime introspection finds root mutations with the presigned upload shape + * and prefers `uploadAppFile` when multiple storage planes are exposed. * * The downloadUrl-by-id field is likewise shape-discovered. Three read shapes, * preferred in order: the inflected singular `appFile(id:)`; a filtered @@ -25,7 +23,8 @@ import { getIntrospectionQuery } from 'graphql'; import { createError } from '@constructive-io/data'; -import type { SheetsExecuteFn } from './sheets-execute'; +import type { SheetsStorageTransport } from './sheets-context'; +import type { SheetsExecuteFn, SheetsFileValue } from './sheets-execute'; // ============================================================================ // Capability discovery types @@ -36,7 +35,12 @@ export interface UploadCapability { uploadFieldName: string; /** The upload input object type name, e.g. 'UploadAppFileInput'. */ inputTypeName: string; - /** Query field used to resolve the stable downloadUrl by id, e.g. 'appFile' | 'node' | 'appFiles'. */ + hasFileProjection: boolean; + download: DownloadByIdResult | null; +} + +interface DownloadByIdResult { + /** Query field used to resolve downloadUrl by id, e.g. 'appFile' | 'node' | 'appFiles'. */ fileByIdField: string; /** * Which downloadUrl-by-id resolution shape was discovered: @@ -57,13 +61,13 @@ export interface UploadCapability { connectionIdOperator?: string; /** connection kind only — the field on the connection object holding the file rows, e.g. 'nodes'. */ connectionNodesField?: string; - /** - * Delete-by-id mutation for the uploaded file type, e.g. 'deleteAppFile', when the - * schema grants one. Used to remove the row the upload op created when its PUT fails. - */ - deleteFieldName?: string; - /** The delete input object type name, e.g. 'DeleteAppFileInput'. */ - deleteInputTypeName?: string; + connectionHasFirst?: boolean; +} + +interface StorageCapabilities { + upload: UploadCapability | null; + download: DownloadByIdResult | null; + batch: DownloadByIdResult | null; } interface UploadPayload { @@ -71,6 +75,7 @@ interface UploadPayload { fileId: string; key: string; deduplicated: boolean; + file: Omit & { filename?: string | null }; } // ============================================================================ @@ -99,19 +104,24 @@ export async function sha256HexFromBuffer(buffer: ArrayBuffer): Promise // Introspection-based capability discovery (cached per endpoint) // ============================================================================ -const capabilityCache = new Map>(); +let capabilityCache = new WeakMap>>(); -/** Test-only: clear the per-endpoint capability cache between cases. */ +/** Test-only: clear cached storage capabilities between cases. */ export function __resetUploadCapabilityCache(): void { - capabilityCache.clear(); + capabilityCache = new WeakMap(); } /** - * Discover the upload op + downloadUrl-by-id field for an endpoint, by shape. - * Cached per `endpoint`; concurrent uploads share one in-flight introspection. + * Share storage introspection across uploads and previews using this executor + * and endpoint. A read-only storage endpoint can still resolve previews. */ -export function discoverUploadCapability(endpoint: string, execute: SheetsExecuteFn): Promise { - const cached = capabilityCache.get(endpoint); +async function discoverStorageCapabilities(endpoint: string, execute: SheetsExecuteFn): Promise { + let endpoints = capabilityCache.get(execute); + if (!endpoints) { + endpoints = new Map(); + capabilityCache.set(execute, endpoints); + } + const cached = endpoints.get(endpoint); if (cached) return cached; const promise = (async () => { @@ -120,15 +130,26 @@ export function discoverUploadCapability(endpoint: string, execute: SheetsExecut if (!schema) { throw createError.badRequest('Uploads are not enabled for this endpoint.'); } - return resolveUploadCapability(schema); + return resolveStorageCapabilities(schema); })(); // Don't cache a rejected discovery — let the next attempt retry. - promise.catch(() => capabilityCache.delete(endpoint)); - capabilityCache.set(endpoint, promise); + promise.catch(() => endpoints.delete(endpoint)); + endpoints.set(endpoint, promise); return promise; } +export async function discoverUploadCapability(endpoint: string, execute: SheetsExecuteFn): Promise { + const { upload } = await discoverStorageCapabilities(endpoint, execute); + if (!upload) capabilityCache.get(execute)?.delete(endpoint); + return requireUploadCapability(upload); +} + +function requireUploadCapability(upload: UploadCapability | null): UploadCapability { + if (!upload) throw createError.badRequest('Uploads are not enabled for this endpoint.'); + return upload; +} + // --- Minimal introspection shapes (only what discovery reads) --- // Loose, structural shapes for the slice of introspection that discovery @@ -198,9 +219,13 @@ const PK_FIELD_NAME = 'id'; /** * Pure resolver over an introspection __schema. Exported for direct unit tests - * (no fetch). Matches by field/type SHAPE, never by a literal name. + * (no fetch). Matches by field/type shape, with an AppFile preference. */ export function resolveUploadCapability(schema: IntrospectionSchema): UploadCapability { + return requireUploadCapability(resolveStorageCapabilities(schema).upload); +} + +function resolveStorageCapabilities(schema: IntrospectionSchema): StorageCapabilities { const typeByName = new Map(); for (const t of schema.types) { if (t.name) typeByName.set(t.name, t); @@ -222,7 +247,7 @@ export function resolveUploadCapability(schema: IntrospectionSchema): UploadCapa return names.every((n) => present.has(n)); }; - const PAYLOAD_FIELDS = ['uploadUrl', 'fileId', 'deduplicated']; + const PAYLOAD_FIELDS = ['uploadUrl', 'fileId', 'key', 'deduplicated']; const INPUT_FIELDS = ['contentHash', 'bucketKey', 'contentType', 'size']; const mutationType = schema.mutationType ? typeByName.get(schema.mutationType.name) : null; @@ -232,7 +257,7 @@ export function resolveUploadCapability(schema: IntrospectionSchema): UploadCapa // type carries the upload-input shape and whose payload carries the // upload-payload shape. (The legacy bucket-centric `requestUploadUrl` shape was // removed from the backend on 2026-05-08; only the root upload mutation exists.) - let upload: Pick | null = null; + let upload: Omit | null = null; for (const field of mutationType?.fields ?? []) { const inputArg = field.args.find((a) => a.name === 'input'); @@ -240,102 +265,74 @@ export function resolveUploadCapability(schema: IntrospectionSchema): UploadCapa const inputTypeName = namedTypeName(inputArg.type); const payloadTypeName = namedTypeName(field.type); if ( + inputTypeName && inputHasFields(inputTypeName, INPUT_FIELDS) && objectHasFields(payloadTypeName, PAYLOAD_FIELDS) ) { - upload = { uploadFieldName: field.name, inputTypeName: inputTypeName as string }; - break; + const payloadFields = typeByName.get(payloadTypeName ?? '')?.fields; + const candidate = { + uploadFieldName: field.name, + inputTypeName, + hasFileProjection: payloadFields?.some( + (f) => f.name === 'file' && !isListTypeRef(f.type) && namedTypeName(f.type) === 'JSON', + ) ?? false, + }; + if (field.name === 'uploadAppFile') { + upload = candidate; + break; + } + upload ??= candidate; } } - if (!upload) { - throw createError.badRequest('Uploads are not enabled for this endpoint.'); - } - - // --- File object type + by-id field for downloadUrl --- - const downloadById = resolveDownloadByIdField(schema, typeByName, queryType, upload.uploadFieldName); - - // --- Delete-by-id for the uploaded file type (optional) --- - // A Mutation field whose `input` carries only `id` (+ clientMutationId) and whose - // payload exposes the file type. The update mutation also takes `id` but carries a - // patch field, so it never matches. - const uploadedTypeName = upload.uploadFieldName.replace(/^upload/, ''); - const fileTypeName = typeByName.has(uploadedTypeName) ? uploadedTypeName : downloadById.fileTypeName; - let remove: Pick = {}; - for (const field of mutationType?.fields ?? []) { - const inputArg = field.args.find((a) => a.name === 'input'); - const inputTypeName = namedTypeName(inputArg?.type); - const inputFields = inputTypeName ? typeByName.get(inputTypeName)?.inputFields : null; - if (!inputTypeName || !inputFields?.some((f) => f.name === PK_FIELD_NAME)) continue; - if (!inputFields.every((f) => f.name === PK_FIELD_NAME || f.name === 'clientMutationId')) continue; - const payload = typeByName.get(namedTypeName(field.type) ?? ''); - if (payload?.fields?.some((f) => namedTypeName(f.type) === fileTypeName)) { - remove = { deleteFieldName: field.name, deleteInputTypeName: inputTypeName }; - break; + const fileTypes = new Set(); + for (const t of schema.types) { + if (t.kind === 'OBJECT' && t.name && t.fields?.some((f) => f.name === 'downloadUrl')) { + fileTypes.add(t.name); } } + const uploadType = upload?.uploadFieldName.replace(/^upload/, '') ?? 'AppFile'; + const uploadDownload = resolveDownloadByIdField(typeByName, queryType, fileTypes, uploadType); + const preferredPreviewType = fileTypes.has('AppFile') ? 'AppFile' : uploadType; + const previewType = fileTypes.has(preferredPreviewType) + ? preferredPreviewType + : uploadDownload?.fileTypeName ?? preferredPreviewType; + const download = previewType === uploadType + ? uploadDownload + : resolveDownloadByIdField(typeByName, queryType, fileTypes, previewType); + const batch = resolveConnectionDownloadField(typeByName, queryType, fileTypes, previewType, true); + return { - ...upload, - ...downloadById, - ...remove, + upload: upload ? { ...upload, download: uploadDownload } : null, + download, + batch, }; } -/** - * Best-effort removal of the row the upload op created, for a PUT that failed. - * Without it, a module that never confirms uploads lists a file whose bytes were - * never stored. Failures are swallowed: the caller reports the original PUT error. - */ -export async function deleteUploadedFileRow( - execute: SheetsExecuteFn, - cap: UploadCapability, - fileId: string, -): Promise { - if (!cap.deleteFieldName || !cap.deleteInputTypeName) return; - const mutation = ` - mutation SheetsUploadCleanup($input: ${cap.deleteInputTypeName}!) { - ${cap.deleteFieldName}(input: $input) { __typename } - } - `.trim(); - try { - await execute(mutation, { input: { [PK_FIELD_NAME]: fileId } }); - } catch { - // Best effort: the caller rethrows the PUT error, which is the one to report. - } -} - -type DownloadByIdResult = Omit; - function resolveDownloadByIdField( - schema: IntrospectionSchema, typeByName: Map, queryType: IntrospectionType | null | undefined, - uploadFieldName: string, -): DownloadByIdResult { - // The file object type carries a `downloadUrl` field (grounded on - // download-url-field.ts which adds `downloadUrl: String` to @storageFiles). - const fileTypesWithDownloadUrl = new Set(); - for (const t of schema.types) { - if (t.kind === 'OBJECT' && t.name && t.fields?.some((f) => f.name === 'downloadUrl')) { - fileTypesWithDownloadUrl.add(t.name); - } - } - - // 1. Prefer the inflected by-id form: a Query field whose return type has a + fileTypesWithDownloadUrl: Set, + preferred: string, +): DownloadByIdResult | null { + // 1. Look for an inflected by-id form: a Query field whose return type has a // `downloadUrl` field and that takes a single `id` arg. e.g. appFile(id:). // The `$id` variable type is the actual scalar of that id arg (NOT a // hardcoded UUID) — derived by shape. + let singular: DownloadByIdResult | null = null; for (const field of queryType?.fields ?? []) { const returnTypeName = namedTypeName(field.type); if (!returnTypeName || !fileTypesWithDownloadUrl.has(returnTypeName)) continue; if (field.args.length === 1 && field.args[0].name === PK_FIELD_NAME) { - return { + const candidate: DownloadByIdResult = { fileByIdField: field.name, fileByIdArgName: 'id', fileTypeName: returnTypeName, idScalarName: namedTypeName(field.args[0].type) ?? undefined, }; + if (returnTypeName === preferred) return candidate; + singular ??= candidate; } } @@ -347,8 +344,9 @@ function resolveDownloadByIdField( // nodes field, id scalar) are discovered by SHAPE, never hardcoded. // Preferred over the Relay node path: it consumes the raw fileId directly, // so it needs no client-side global-id construction. - const connection = resolveConnectionDownloadField(typeByName, queryType, fileTypesWithDownloadUrl, uploadFieldName); - if (connection) return connection; + const connection = resolveConnectionDownloadField(typeByName, queryType, fileTypesWithDownloadUrl, preferred); + if (connection?.fileTypeName === preferred) return connection; + if (singular || connection) return singular ?? connection; // 3. Relay fallback: `node(nodeId: ID!)`. Last resort — only when neither a // singular `id` accessor nor a filtered connection exists. Caller spreads @@ -361,19 +359,18 @@ function resolveDownloadByIdField( // after the upload op (uploadAppFile → AppFile); else the first-declared // downloadUrl-bearing type. (Was alphabetical `.sort()[0]`, which could pick // the wrong type when several expose downloadUrl, e.g. DataRoomFile.) - const preferred = uploadFieldName.replace(/^upload/, ''); const fileTypeName = fileTypesWithDownloadUrl.has(preferred) ? preferred : [...fileTypesWithDownloadUrl][0]; return { fileByIdField: 'node', fileByIdArgName: 'nodeId', fileTypeName }; } - throw createError.badRequest('Uploads are not enabled for this endpoint.'); + return null; } /** - * Discover a filtered-connection downloadUrl-by-id path. Returns null if the - * schema exposes no such connection. Supports two filter input styles: + * Discover a filtered connection for single or batched download lookups. + * The single-file path supports two filter input styles: * (a) operator filter — `where: { id: { equalTo: $id } }` (connection-filter * plugin): the id field's input type carries an equality operator. * (b) condition filter — `condition: { id: $id }`: the id field is the scalar @@ -383,7 +380,8 @@ function resolveConnectionDownloadField( typeByName: Map, queryType: IntrospectionType | null | undefined, fileTypesWithDownloadUrl: Set, - uploadFieldName: string, + preferred: string, + batch = false, ): DownloadByIdResult | null { // Equality operator names we recognize on a filter-input field, in priority // order (exact-match equality first). @@ -411,6 +409,9 @@ function resolveConnectionDownloadField( }); if (!nodesField) continue; const fileTypeName = namedTypeName(nodesField.type) as string; + if (batch && (fileTypeName !== preferred || + !typeByName.get(fileTypeName)?.fields?.some((field) => field.name === PK_FIELD_NAME))) continue; + const connectionHasFirst = field.args.some((arg) => arg.name === 'first'); // Find a filter arg whose input type has an `id` field. for (const arg of field.args) { @@ -427,9 +428,11 @@ function resolveConnectionDownloadField( // operator (e.g. UUIDFilter.equalTo). The `$id` scalar is the operator's // type. if (idFieldType?.kind === 'INPUT_OBJECT' && idFieldType.inputFields) { - const opField = EQUALITY_OPERATORS.map((op) => - idFieldType.inputFields!.find((f) => f.name === op), - ).find((f) => f != null); + const opField = batch + ? idFieldType.inputFields.find((f) => f.name === 'in' && isListTypeRef(f.type)) + : EQUALITY_OPERATORS.map((op) => + idFieldType.inputFields!.find((f) => f.name === op), + ).find((f) => f != null); if (opField) { candidates.push({ fileByIdField: field.name, @@ -440,16 +443,17 @@ function resolveConnectionDownloadField( connectionIdField: idField.name, connectionIdOperator: opField.name, connectionNodesField: nodesField.name, + connectionHasFirst, }); break; } - // id field is an input object but with no recognized equality op — skip. + // This filter lacks the operator needed for this lookup. continue; } // Style (b): the id field is a scalar directly (condition style). The // `$id` scalar is that field's type. - if (idFieldType?.kind === 'SCALAR' || (!idFieldType && idFieldTypeName)) { + if (!batch && (idFieldType?.kind === 'SCALAR' || (!idFieldType && idFieldTypeName))) { candidates.push({ fileByIdField: field.name, fileByIdArgName: 'connection', @@ -458,6 +462,7 @@ function resolveConnectionDownloadField( connectionFilterArgName: arg.name, connectionIdField: idField.name, connectionNodesField: nodesField.name, + connectionHasFirst, }); break; } @@ -472,7 +477,6 @@ function resolveConnectionDownloadField( // lookup-disabled tenant API — so resolving the WRONG file connection (e.g. // reading a DataRoomFile id through appFiles) yields 0 nodes and fails every // upload. Mirrors the node-branch determinism, where it matters far more. - const preferred = uploadFieldName.replace(/^upload/, ''); return candidates.find((c) => c.fileTypeName === preferred) ?? candidates[0]; } @@ -481,7 +485,7 @@ function resolveConnectionDownloadField( // ============================================================================ interface UploadOpInput { - bucketKey: string; + bucketKey?: string; contentHash: string; contentType: string; size: number; @@ -504,12 +508,13 @@ export async function callUploadOp( fileId key deduplicated + file } } `.trim(); const data = await execute>(mutation, { input: { - bucketKey: input.bucketKey, + ...(input.bucketKey === undefined ? { isPublic: true } : { bucketKey: input.bucketKey }), contentHash: input.contentHash, contentType: input.contentType, size: input.size, @@ -520,6 +525,13 @@ export async function callUploadOp( if (!payload) { throw createError.badRequest('Upload failed: no payload returned from upload mutation.'); } + const file = payload.file; + if ( + !file || typeof file.id !== 'string' || typeof file.key !== 'string' || + typeof file.bucket_id !== 'string' || typeof file.mime !== 'string' || typeof file.size !== 'number' + ) { + throw createError.badRequest('Upload failed: server returned an invalid file projection.'); + } return payload; } @@ -631,13 +643,12 @@ function toGlobalId(typeName: string, id: string): string { } /** - * Resolve the stable public download URL for a file id via the discovered - * by-id field. For public files the backend returns a permanent public URL - * (its exact path shape is the backend's concern) — stored opaquely in the cell. + * Resolve a file's download URL for immediate use. It can be a signed URL + * with an expiration, so only the storage projection belongs in the cell. */ export async function fetchDownloadUrl( execute: SheetsExecuteFn, - cap: UploadCapability, + cap: DownloadByIdResult, fileId: string, ): Promise { if (cap.fileByIdArgName === 'nodeId') { @@ -678,7 +689,7 @@ export async function fetchDownloadUrl( : `{ ${idField}: $id }`; const query = ` query SheetsDownloadUrl($id: ${idScalar}!) { - ${cap.fileByIdField}(${filterArg}: ${filterLiteral}, first: 1) { + ${cap.fileByIdField}(${filterArg}: ${filterLiteral}${cap.connectionHasFirst ? ', first: 1' : ''}) { ${nodesField} { downloadUrl } @@ -715,6 +726,36 @@ export async function fetchDownloadUrl( return url; } +/** Resolve URLs for one batch of visible images. Missing or unreadable IDs have no entry. */ +export async function fetchMediaDownloadUrls( + { endpoint, execute }: SheetsStorageTransport, + getActiveIds: () => readonly string[], +): Promise> { + const { batch, download } = await discoverStorageCapabilities(endpoint, execute); + const ids = getActiveIds(); + const urls = new Map(); + if (ids.length === 0) return urls; + if (batch) { + const first = batch.connectionHasFirst ? `, first: ${ids.length}` : ''; + const query = `query SheetsMediaDownloadUrls($ids: [${batch.idScalarName ?? 'ID'}!]!) { + ${batch.fileByIdField}(${batch.connectionFilterArgName}: { + ${batch.connectionIdField}: { in: $ids } + }${first}) { ${batch.connectionNodesField} { id downloadUrl } } + }`; + type FileNode = { id: string; downloadUrl: string | null }; + const data = await execute | null>>(query, { ids }); + for (const row of data?.[batch.fileByIdField]?.[batch.connectionNodesField!] ?? []) { + if (row?.downloadUrl) urls.set(row.id, row.downloadUrl); + } + } else if (download) { + const results = await Promise.allSettled(ids.map((id) => fetchDownloadUrl(execute, download, id))); + results.forEach((result, index) => { + if (result.status === 'fulfilled') urls.set(ids[index], result.value); + }); + } + return urls; +} + /** * Resolve a file's signed `downloadUrl` by id — the public entry point for callers * OUTSIDE the upload flow (e.g. a storage browser fetching the preview/copy link for @@ -722,7 +763,7 @@ export async function fetchDownloadUrl( * * `downloadUrl` is a Grafast computed field, so it cannot be selected through the * runtime query builder (`buildSelect` intersects the select against pg columns and - * drops computed fields). This discovers the by-id field once per endpoint (cached) + * drops computed fields). This discovers the by-id field using cached introspection * and reads it through the SAME resolver the upload path uses. Returns null on any * failure — a missing url is not exceptional for the caller (it just disables * copy/download), so the throwing internals are swallowed here. @@ -733,8 +774,8 @@ export async function resolveDownloadUrl( fileId: string, ): Promise { try { - const cap = await discoverUploadCapability(endpoint, execute); - return await fetchDownloadUrl(execute, cap, fileId); + const { download } = await discoverStorageCapabilities(endpoint, execute); + return download ? await fetchDownloadUrl(execute, download, fileId) : null; } catch { return null; } diff --git a/packages/sheets/src/grid-dom/__tests__/media-editor-flow.test.tsx b/packages/sheets/src/grid-dom/__tests__/media-editor-flow.test.tsx new file mode 100644 index 00000000..dee3e81a --- /dev/null +++ b/packages/sheets/src/grid-dom/__tests__/media-editor-flow.test.tsx @@ -0,0 +1,187 @@ +/* @vitest-environment jsdom */ + +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { QueryClient } from '@tanstack/react-query'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { SheetsConfig } from '../../context/sheets-context'; +import type { SheetsExecuteFn, SheetsUploadFn } from '../../context/sheets-execute'; +import { SheetsProvider } from '../../context/sheets-provider'; +import { Sheets } from '../../grid/sheets'; +import { createMockExecute } from '../../testing/mock-sheets-provider'; + +const toast = vi.hoisted(() => ({ error: vi.fn(), success: vi.fn(), info: vi.fn() })); +vi.mock('@constructive-io/ui/toast', () => ({ toast })); + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const projection = { + id: 'file-1', key: 'cat', bucket_id: 'bucket-1', filename: 'cat.png', mime: 'image/png', size: 5, +}; +const attachmentUrl = 'https://cdn.local/attachment.pdf'; + +function mockBackend(rows: Record[] = []) { + return createMockExecute({ tables: [{ + name: 'MediaItem', + fields: [ + { name: 'id', gqlType: 'UUID', pgType: 'uuid' }, + { name: 'photo', gqlType: 'JSON', pgType: 'image' }, + { name: 'link', gqlType: 'String', pgType: 'attachment' }, + ], + rows, + }] }); +} + +function button(label: string, parent: ParentNode = document.body) { + return Array.from(parent.querySelectorAll('button')).find((item) => item.textContent?.trim() === label); +} + +async function waitUntil(read: () => T | null | undefined | false): Promise { + for (let attempt = 0; attempt < 50; attempt++) { + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + const found = read(); + if (found) return found; + } + throw new Error('Expected media editor state did not appear'); +} + +async function click(element: HTMLElement, event = 'click') { + await act(async () => element.dispatchEvent(new MouseEvent(event, { bubbles: true }))); +} + +async function pickFile(file: File) { + const input = await waitUntil(() => document.querySelector('[data-slot="image-editor"] input[type="file"]')); + Object.defineProperty(input, 'files', { configurable: true, value: [file] }); + await act(async () => input.dispatchEvent(new Event('change', { bubbles: true }))); +} + +describe('Sheets media editor flows', () => { + let root: Root; + let container: HTMLDivElement; + let queryClient: QueryClient; + const createObjectUrl = Object.getOwnPropertyDescriptor(URL, 'createObjectURL'); + const revokeObjectUrl = Object.getOwnPropertyDescriptor(URL, 'revokeObjectURL'); + + beforeEach(() => { + vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockReturnValue(1200); + vi.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockReturnValue(800); + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 1200, 33)); + Object.defineProperty(URL, 'createObjectURL', { configurable: true, value: () => 'blob:media-preview' }); + Object.defineProperty(URL, 'revokeObjectURL', { configurable: true, value: () => {} }); + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + queryClient.clear(); + container.remove(); + vi.restoreAllMocks(); + vi.clearAllMocks(); + if (createObjectUrl) Object.defineProperty(URL, 'createObjectURL', createObjectUrl); + else Reflect.deleteProperty(URL, 'createObjectURL'); + if (revokeObjectUrl) Object.defineProperty(URL, 'revokeObjectURL', revokeObjectUrl); + else Reflect.deleteProperty(URL, 'revokeObjectURL'); + }); + + async function render(execute: SheetsExecuteFn, executeUpload?: SheetsUploadFn) { + const config: SheetsConfig = { + endpoint: 'https://data.local/graphql', + auth: { mode: 'embedded', getToken: () => null, getIdentityKey: () => 'user-1' }, + execute, + executeUpload, + queryClient, + }; + await act(async () => root.render( +
+ + + +
, + )); + } + + it('keeps the draft editor and file available after create removes the only draft and upload fails', async () => { + const backend = mockBackend(); + let holdList = true; + const execute: SheetsExecuteFn = async (document: unknown, variables?: Record) => { + if (holdList && backend.mutations.some(({ op }) => op === 'create') && String(document).includes('mediaItems(')) { + return { mediaItems: { nodes: [], totalCount: 0, pageInfo: { hasNextPage: false } } } as T; + } + return backend.execute(document, variables); + }; + const executeUpload = vi.fn() + .mockRejectedValueOnce(new Error('PUT blocked')) + .mockImplementationOnce(async () => { + holdList = false; + return { file: projection }; + }); + + await render(execute, executeUpload); + await click(await waitUntil(() => button('Add row', container))); + const imageCell = await waitUntil(() => container.querySelector('[data-slot="image-cell"]')); + await click(imageCell, 'dblclick'); + const file = new File(['hello'], 'cat.png', { type: 'image/png' }); + await pickFile(file); + await click(await waitUntil(() => button('Create Row & Upload'))); + + await waitUntil(() => toast.error.mock.calls.some(([message]) => message.message === 'PUT blocked')); + const created = backend.mutations.filter(({ op }) => op === 'create'); + expect(created).toHaveLength(1); + const retry = await waitUntil(() => button('Upload & Save')); + await click(retry); + + await waitUntil(() => backend.mutations.find(({ op }) => op === 'update')); + expect(executeUpload.mock.calls.map(([upload]) => upload)).toEqual([file, file]); + expect(backend.mutations.map(({ op }) => op)).toEqual(['create', 'update']); + expect(backend.mutations[1].variables).toEqual({ + input: { id: executeUpload.mock.calls[0][2]?.recordId, mediaItemPatch: { photo: projection } }, + }); + }); + + it('clears a saved file through the row writer without uploading or deleting a file', async () => { + const backend = mockBackend([{ id: 'row-1', photo: projection }]); + const executeUpload = vi.fn(); + await render(backend.execute, executeUpload); + await click(await waitUntil(() => container.querySelector('[data-slot="file-cell"]')), 'dblclick'); + await click(await waitUntil(() => document.querySelector('button[aria-label="Remove file"]'))); + await click(await waitUntil(() => button('Save'))); + + await waitUntil(() => backend.mutations.find(({ op }) => op === 'update')); + expect(backend.mutations.map(({ op }) => op)).toEqual(['update']); + expect(backend.mutations[0].variables).toEqual({ input: { id: 'row-1', mediaItemPatch: { photo: null } } }); + expect(executeUpload).not.toHaveBeenCalled(); + }); + + it('makes media read-only without storage, including the grid Delete action', async () => { + const backend = mockBackend([{ id: 'row-1', photo: projection, link: attachmentUrl }]); + await render(backend.execute); + const fileCell = await waitUntil(() => container.querySelector('[data-slot="file-cell"]')); + expect(fileCell.querySelector('[title]')?.getAttribute('title')).toContain('Uploads need a storage endpoint'); + await click(fileCell); + await click(fileCell, 'dblclick'); + const grid = await waitUntil(() => container.querySelector('[role="grid"]')); + await act(async () => grid.dispatchEvent(new KeyboardEvent('keydown', { key: 'Delete', bubbles: true }))); + expect(document.querySelector('[data-slot="image-editor"]')).toBeNull(); + expect(backend.mutations).toEqual([]); + }); + + it('opens a stored attachment as URL text and keeps the cell unchanged when a new file has no public URL', async () => { + const backend = mockBackend([{ id: 'row-1', link: attachmentUrl }]); + const executeUpload = vi.fn().mockResolvedValue({ file: projection }); + await render(backend.execute, executeUpload); + const attachmentCell = await waitUntil(() => Array.from(container.querySelectorAll('[data-slot="text-cell"]')) + .find((cell) => cell.textContent === attachmentUrl)); + await click(attachmentCell, 'dblclick'); + await pickFile(new File(['hello'], 'attachment.pdf', { type: 'application/pdf' })); + await click(await waitUntil(() => button('Upload & Save'))); + + await waitUntil(() => toast.error.mock.calls.some(([message]) => + message.message.includes('attachment columns need a public bucket with a public URL prefix'))); + expect(backend.mutations).toEqual([]); + expect(attachmentCell.textContent).toBe(attachmentUrl); + }); +}); diff --git a/packages/sheets/src/grid-dom/editors/__tests__/image-editor.test.tsx b/packages/sheets/src/grid-dom/editors/__tests__/image-editor.test.tsx index c3131aa8..0bcb9826 100644 --- a/packages/sheets/src/grid-dom/editors/__tests__/image-editor.test.tsx +++ b/packages/sheets/src/grid-dom/editors/__tests__/image-editor.test.tsx @@ -17,7 +17,14 @@ import type { EditorProps } from '../editor-props'; // The mock also records the props the adapter wired (recordId/isDraftRow/etc.). const lastProps: { current: Record | null } = { current: null }; const SAVE_DATA: ImageSaveData = { - imageData: { url: 'https://cdn.example.com/cat.png', filename: 'cat.png', size: 1234, mime: 'image/png' }, + imageData: { + id: 'file-1', + key: 'cat', + bucket_id: 'bucket-1', + filename: 'cat.png', + size: 1234, + mime: 'image/png', + }, }; const SERVER_ROW = { id: 'real-42', name: 'created' }; @@ -52,12 +59,13 @@ function makeProps(value: unknown, over: Partial = {}): EditorProps cell, colKey: 'photo', rowId: 'row-1', + rowIdentifier: 'row-1', rowIndex: 0, tableName: 'pets', onCommit: vi.fn(), onCommitPatch: vi.fn(), onCancel: vi.fn(), - onSubmitDraftRow: vi.fn().mockResolvedValue(SERVER_ROW), + onSubmitDraftRow: vi.fn().mockResolvedValue({ createdRow: SERVER_ROW }), onInvalidateData: vi.fn(), overlay: { maxHeight: 400, flipped: false }, ...over, @@ -126,10 +134,10 @@ describe('ImageEditorDom (native EditorProps adapter)', () => { }); it('draft row: wires onSubmitDraft -> onSubmitDraftRow(rowId) resolving { createdRow }', async () => { - const onSubmitDraftRow = vi.fn().mockResolvedValue(SERVER_ROW); + const onSubmitDraftRow = vi.fn().mockResolvedValue({ createdRow: SERVER_ROW }); await act(async () => { root.render( - , + , ); }); @@ -137,7 +145,7 @@ describe('ImageEditorDom (native EditorProps adapter)', () => { expect(p.isDraftRow).toBe(true); expect(p.onSubmitDraft).toBeTypeOf('function'); - // Invoke the wrapped draft submit and assert it adapts to a DraftSubmitResult. + // The shell already returns the DraftSubmitResult. const result = await (p.onSubmitDraft as () => Promise<{ createdRow: unknown }>)(); expect(onSubmitDraftRow).toHaveBeenCalledWith('draft:abc'); expect(result).toEqual({ createdRow: SERVER_ROW }); diff --git a/packages/sheets/src/grid-dom/editors/__tests__/upload-editor.test.tsx b/packages/sheets/src/grid-dom/editors/__tests__/upload-editor.test.tsx deleted file mode 100644 index cbce1314..00000000 --- a/packages/sheets/src/grid-dom/editors/__tests__/upload-editor.test.tsx +++ /dev/null @@ -1,161 +0,0 @@ -/* @vitest-environment jsdom */ - -import { act } from 'react'; -import { createRoot, type Root } from 'react-dom/client'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import type { SheetsCell } from '../../../cell-model/sheets-cell'; -import type { EditorProps } from '../editor-props'; -import { UploadEditorDom } from '../upload-editor'; - -(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; - -function makeProps(value: unknown, over: Partial = {}): EditorProps { - const cell: SheetsCell = { kind: 'text', data: value, displayData: String(value ?? ''), readonly: false }; - return { - value, - cell, - colKey: 'attachment', - rowId: 'row-1', - rowIndex: 0, - onCommit: vi.fn(), - onCommitPatch: vi.fn(), - onCancel: vi.fn(), - overlay: { maxHeight: 400, flipped: false }, - ...over, - }; -} - -function typeInto(input: HTMLInputElement, text: string) { - // React tracks the input's value via its own setter; bypass it so the change - // event reflects the new value (jsdom controlled-input idiom). - const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!; - setter.call(input, text); - input.dispatchEvent(new Event('input', { bubbles: true })); -} - -function clickByText(container: HTMLElement, text: string) { - const btn = Array.from(container.querySelectorAll('button')).find((b) => b.textContent?.includes(text)); - btn?.dispatchEvent(new MouseEvent('click', { bubbles: true })); -} - -const SEED_URL = 'https://example.com/old.pdf'; - -describe('UploadEditorDom (native EditorProps re-host)', () => { - let root: Root; - let container: HTMLDivElement; - - beforeEach(() => { - container = document.createElement('div'); - document.body.appendChild(container); - root = createRoot(container); - }); - - afterEach(async () => { - await act(async () => { - root.unmount(); - }); - container.remove(); - vi.clearAllMocks(); - }); - - it('seeds the file preview from the raw value and renders the data-slot wrapper', async () => { - await act(async () => { - root.render(); - }); - // The default `Upload File` tab shows the file preview seeded from the value. - expect(container.textContent).toContain('old.pdf'); - expect(container.querySelector('[data-slot="upload-editor"]')).toBeTruthy(); - }); - - it('Save commits the serialized URL string via onCommit (not onCommitPatch)', async () => { - const onCommit = vi.fn(); - const onCommitPatch = vi.fn(); - const onCancel = vi.fn(); - await act(async () => { - root.render(); - }); - - // The URL `` lives in the (lazy-rendered) `File URL` tab panel — switch - // to it before editing so the input is mounted. - await act(async () => { - clickByText(container, 'File URL'); - }); - const urlInput = container.querySelector('#file-url') as HTMLInputElement; - const nextUrl = 'https://example.com/new.pdf'; - await act(async () => { - typeInto(urlInput, nextUrl); - }); - - await act(async () => { - clickByText(container, 'Save'); - }); - - // URL-only files serialize as a bare URL string into the cell's `.data`; the - // DOM re-host unwraps `.data` back to the raw committed value. - expect(onCommit).toHaveBeenCalledTimes(1); - expect(onCommit).toHaveBeenCalledWith(JSON.stringify(nextUrl)); - expect(onCommitPatch).not.toHaveBeenCalled(); - expect(onCancel).not.toHaveBeenCalled(); - }); - - it('Save with a cleared selection commits an empty string', async () => { - const onCommit = vi.fn(); - await act(async () => { - root.render(); - }); - - // Clearing the URL drops the selected file -> saveData is null -> data = ''. - await act(async () => { - clickByText(container, 'File URL'); - }); - const urlInput = container.querySelector('#file-url') as HTMLInputElement; - await act(async () => { - typeInto(urlInput, ''); - }); - - await act(async () => { - clickByText(container, 'Save'); - }); - - expect(onCommit).toHaveBeenCalledTimes(1); - expect(onCommit).toHaveBeenCalledWith(''); - }); - - it('Cancel button cancels and never commits', async () => { - const onCommit = vi.fn(); - const onCommitPatch = vi.fn(); - const onCancel = vi.fn(); - await act(async () => { - root.render(); - }); - - await act(async () => { - clickByText(container, 'Cancel'); - }); - - expect(onCancel).toHaveBeenCalledTimes(1); - expect(onCommit).not.toHaveBeenCalled(); - expect(onCommitPatch).not.toHaveBeenCalled(); - }); - - it('Escape cancels, never commits', async () => { - const onCommit = vi.fn(); - const onCommitPatch = vi.fn(); - const onCancel = vi.fn(); - await act(async () => { - root.render(); - }); - - // Escape is handled by the source EditorFocusTrap's React onKeyDown (the - // role="dialog" wrapper); dispatch on it so the synthetic handler fires. - const dialog = container.querySelector('[role="dialog"]') as HTMLElement; - await act(async () => { - dialog.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); - }); - - expect(onCancel).toHaveBeenCalledTimes(1); - expect(onCommit).not.toHaveBeenCalled(); - expect(onCommitPatch).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/sheets/src/grid-dom/editors/editor-props.ts b/packages/sheets/src/grid-dom/editors/editor-props.ts index 94f8bd28..19ac357a 100644 --- a/packages/sheets/src/grid-dom/editors/editor-props.ts +++ b/packages/sheets/src/grid-dom/editors/editor-props.ts @@ -11,13 +11,16 @@ import type { FieldMetadata } from '../../cell-types/cell-type-resolver'; import type { SheetsCell } from '../../cell-model/sheets-cell'; +import type { DraftSubmitResult } from '../../grid/draft-types'; import type { RelationInfo } from '../../store/relation-info-slice'; +import type { SheetsRowIdentifier } from '../../row-identity'; export interface EditorProps { value: unknown; cell: SheetsCell; colKey: string; rowId: string; + rowIdentifier?: SheetsRowIdentifier | null; rowIndex: number; tableName?: string; fieldMeta?: FieldMetadata; @@ -38,8 +41,8 @@ export interface EditorProps { onCommitPatch?: (patch: Record) => void; /** Dismiss the overlay with no write. */ onCancel: () => void; - /** Self-committing editors: submit a client-side draft row, resolving to the server row. */ - onSubmitDraftRow?: (draftRowId: string) => Promise; + /** Self-committing editors: submit a client-side draft row and return its creation result. */ + onSubmitDraftRow?: (draftRowId: string) => Promise; /** Self-committing editors: invalidate cached data after an out-of-band mutation. */ onInvalidateData?: () => void; /** Geometry pre-computed by the OverlayManager (reused `computeOverlayGeometry`). */ diff --git a/packages/sheets/src/grid-dom/editors/editor-registry-dom.ts b/packages/sheets/src/grid-dom/editors/editor-registry-dom.ts index 96a9c603..bfd9f2ab 100644 --- a/packages/sheets/src/grid-dom/editors/editor-registry-dom.ts +++ b/packages/sheets/src/grid-dom/editors/editor-registry-dom.ts @@ -96,6 +96,7 @@ export const DOM_EDITOR_REGISTRY: Record = { video: ImageEditorDom, audio: ImageEditorDom, upload: ImageEditorDom, + attachment: ImageEditorDom, // Date/time types date: DateEditorDom, datetime: DateEditorDom, diff --git a/packages/sheets/src/grid-dom/editors/image-editor.tsx b/packages/sheets/src/grid-dom/editors/image-editor.tsx index 1010594d..95e3f478 100644 --- a/packages/sheets/src/grid-dom/editors/image-editor.tsx +++ b/packages/sheets/src/grid-dom/editors/image-editor.tsx @@ -4,11 +4,11 @@ // native) SELF-COMMITTING `ImageEditor` verbatim. The UI (dropzone, upload flow, // OVERLAY_LG width, own EditorFocusTrap, motion crossfade) is reused as-is; the // contract maps directly onto EditorProps: -// • value -> the raw row value, read directly by the source (getImageUrl). -// • recordId -> EditorProps.rowId. -// • isDraftRow -> rowId startsWith 'draft:'. +// • value -> the raw row value, read directly by the editor. +// • recordId -> typed EditorProps.rowIdentifier. +// • isDraftRow -> no server identity and a draft rowId. // • onSaveComplete -> onCommitPatch({ [colKey]: saveData.imageData }) (single field). -// • onSubmitDraft -> onSubmitDraftRow(draftRowId) (mapped to a DraftSubmitResult). +// • onSubmitDraft -> onSubmitDraftRow(draftRowId). // • onDraftUploadComplete -> onInvalidateData. // • onFinishedEditing(next?) -> onCancel() (a CLOSE; the upload self-commits via // onSaveComplete, so the value path is never used for image). @@ -17,7 +17,6 @@ import { Suspense } from 'react'; import { ImageEditor, type ImageSaveData } from '../../grid/editors/image-editor'; -import type { DraftSubmitResult } from '../../grid/draft-types'; import type { EditorProps } from './editor-props'; const DRAFT_PREFIX = 'draft:'; @@ -27,13 +26,14 @@ export function ImageEditorDom({ cell, colKey, rowId, + rowIdentifier, tableName, onCommitPatch, onCancel, onSubmitDraftRow, onInvalidateData, }: EditorProps) { - const isDraftRow = rowId.startsWith(DRAFT_PREFIX); + const isDraftRow = rowIdentifier == null && rowId.startsWith(DRAFT_PREFIX); // The whole MEDIA family routes here (image/file/video/audio/upload). Only `image` // stays image-only; every other media type accepts ANY file (no image guard/cap), // so e.g. an `upload` column can take a PDF. The upload transport is already @@ -49,14 +49,8 @@ export function ImageEditorDom({ ? (saveData: ImageSaveData) => onCommitPatch({ [colKey]: saveData.imageData }) : undefined; - // Draft rows: persist first to obtain a real id. The editor expects a - // DraftSubmitResult ({ createdRow }); EditorProps.onSubmitDraftRow resolves to - // the raw server row, so wrap it into that shape. const handleSubmitDraft = isDraftRow && onSubmitDraftRow - ? async (): Promise => { - const createdRow = (await onSubmitDraftRow(rowId)) as DraftSubmitResult['createdRow']; - return { createdRow }; - } + ? () => onSubmitDraftRow(rowId) : undefined; return ( @@ -68,7 +62,7 @@ export function ImageEditorDom({ onFinishedEditing={() => onCancel()} tableName={tableName} fieldName={colKey} - recordId={rowId} + recordId={rowIdentifier ?? null} onSaveComplete={handleSaveComplete} isDraftRow={isDraftRow} onSubmitDraft={handleSubmitDraft} diff --git a/packages/sheets/src/grid-dom/editors/upload-editor.tsx b/packages/sheets/src/grid-dom/editors/upload-editor.tsx deleted file mode 100644 index cf3be2f6..00000000 --- a/packages/sheets/src/grid-dom/editors/upload-editor.tsx +++ /dev/null @@ -1,27 +0,0 @@ -'use client'; - -// Native upload editor — a thin EditorProps adapter that RE-HOSTS the (now value- -// native) `UploadEditor` verbatim. The UI (upload drop zone / URL tabs, file preview, -// OVERLAY.md width, own EditorFocusTrap + Ctrl/Cmd+Enter save) is reused as-is; the -// contract maps directly to value-native EditorProps. -// -// The editor reads the raw row value and, on save, emits the raw serialized value -// (JSON.stringify(saveData) | ''), so the value flows straight through: -// `onFinishedEditing(next)` -> `onCommit(next)`, `onFinishedEditing(undefined)` -> -// `onCancel()`. This is a VALUE editor (commit model: value); parity note — glide -// mapped `upload` -> the image editor, but the ported UploadEditor remains available -// and is re-hosted here. - -import { UploadEditor } from '../../grid/editors/upload-editor'; -import type { EditorProps } from './editor-props'; - -export function UploadEditorDom({ value, onCommit, onCancel }: EditorProps) { - return ( -
- (next === undefined ? onCancel() : onCommit(next))} - /> -
- ); -} diff --git a/packages/sheets/src/grid-dom/overlay/__tests__/overlay-manager-motion.test.tsx b/packages/sheets/src/grid-dom/overlay/__tests__/overlay-manager-motion.test.tsx new file mode 100644 index 00000000..6aaf6532 --- /dev/null +++ b/packages/sheets/src/grid-dom/overlay/__tests__/overlay-manager-motion.test.tsx @@ -0,0 +1,84 @@ +/* @vitest-environment jsdom */ +// OverlayManager open/close motion: the editor reaches the open state after its +// measured first frame, and closing leaves only an inert, id-free copy that is +// removed after the close transition. Reduced motion skips the copy. +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { OverlayManager } from '../overlay-manager'; +import { SheetsContext, type SheetsContextValue } from '../../../context/sheets-context'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const noop = () => {}; +const STUB_CONTEXT = { + config: { endpoint: '', auth: { mode: 'standalone' }, onError: noop }, + execute: (async () => ({})) as never, + executeUpload: (async () => ({})) as never, + scopeKey: { databaseId: null, endpoint: '', identityKey: null }, +} as unknown as SheetsContextValue; + +const ANCHOR = { + top: 100, bottom: 130, left: 50, height: 30, width: 100, right: 150, x: 50, y: 100, toJSON() {}, +} as DOMRect; + +function mockReducedMotion(reduce: boolean) { + window.matchMedia = vi.fn().mockReturnValue({ matches: reduce }) as unknown as typeof window.matchMedia; +} + +describe('OverlayManager motion', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + vi.useFakeTimers(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + afterEach(async () => { + await act(async () => root.unmount()); + vi.useRealTimers(); + Reflect.deleteProperty(window, 'matchMedia'); + document.body.innerHTML = ''; + }); + + async function render(open: boolean) { + await act(async () => { + root.render( + + + + + , + ); + }); + } + + const overlays = () => Array.from(document.querySelectorAll('[data-slot="sheets-editor-overlay"]')); + + it('opens after the measured frame and closes through an inert copy that is then removed', async () => { + mockReducedMotion(false); + await render(true); + expect(overlays().map((el) => el.dataset.state)).toEqual(['open']); + + await render(false); + const [ghost] = overlays(); + expect(overlays()).toHaveLength(1); + expect(ghost.dataset.state).toBe('closing'); + expect(ghost.getAttribute('aria-hidden')).toBe('true'); + expect(ghost.inert).toBe(true); + expect(ghost.querySelector('[id]')).toBeNull(); + + await act(async () => vi.runAllTimers()); + expect(overlays()).toHaveLength(0); + }); + + it('closes instantly when the user prefers reduced motion', async () => { + mockReducedMotion(true); + await render(true); + await render(false); + expect(overlays()).toHaveLength(0); + }); +}); diff --git a/packages/sheets/src/grid-dom/overlay/overlay-manager.tsx b/packages/sheets/src/grid-dom/overlay/overlay-manager.tsx index a4087883..01dfb6e3 100644 --- a/packages/sheets/src/grid-dom/overlay/overlay-manager.tsx +++ b/packages/sheets/src/grid-dom/overlay/overlay-manager.tsx @@ -8,7 +8,7 @@ // It NEVER mutates `.gdg-clip-region` — that DOM coupling stays in the canvas // `OverlayViewportGuard` so the glide path is untouched. -import React, { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; import { computeOverlayGeometry, OverlayMeasureContext } from '../../grid/editors/overlay-viewport-guard'; @@ -19,6 +19,47 @@ import { cn } from '../../utils/cn'; const MARGIN_PX = 12; const MIN_BELOW_PX = 320; +// Open and close follow the transitions.dev menu-dropdown recipe (05): grow from +// the anchored cell corner from 0.97 to 1 with a 250ms smooth-out, then leave with +// a shorter 150ms fade to 0.99. Each value reads the host's `--dropdown-*` token +// (the Dashboard ships them with its other popovers) and falls back to the recipe +// default. `transform` and `opacity` are the only transitioned properties, so the +// measured left/top placement always applies instantly. +const DROPDOWN_MOTION = [ + 'opacity-0 pointer-events-none will-change-[transform,opacity]', + '[transform:scale(var(--dropdown-pre-scale,0.97))]', + '[transition-property:transform,opacity]', + '[transition-duration:var(--dropdown-open-dur,250ms)]', + '[transition-timing-function:var(--dropdown-ease,cubic-bezier(0.22,1,0.36,1))]', + 'data-[state=open]:opacity-100 data-[state=open]:pointer-events-auto data-[state=open]:[transform:scale(1)]', + 'data-[state=closing]:[transform:scale(var(--dropdown-closing-scale,0.99))]', + 'data-[state=closing]:[transition-duration:var(--dropdown-close-dur,150ms)]', + 'motion-reduce:transition-none!', +].join(' '); + +function prefersReducedMotion(): boolean { + return typeof window.matchMedia !== 'function' || window.matchMedia('(prefers-reduced-motion: reduce)').matches; +} + +/** + * The editor unmounts as soon as it closes, so its exit plays on an inert copy of + * the last open frame. The copy holds no React state or handlers, cannot take + * focus or pointer input, and is removed once the close transition ends. + */ +function playCloseTransition(node: HTMLElement): void { + if (node.dataset.state !== 'open' || prefersReducedMotion()) return; + const host = node.parentNode ?? document.getElementById('portal-root') ?? document.body; + const ghost = node.cloneNode(true) as HTMLElement; + for (const el of [ghost, ...ghost.querySelectorAll('[id]')]) el.removeAttribute('id'); + ghost.setAttribute('aria-hidden', 'true'); + ghost.inert = true; + host.insertBefore(ghost, node.parentNode === host ? node : null); + void ghost.offsetWidth; // Start from the copied open frame. + ghost.dataset.state = 'closing'; + const closeMs = parseFloat(getComputedStyle(ghost).transitionDuration) * 1000; + window.setTimeout(() => ghost.remove(), (Number.isFinite(closeMs) ? closeMs : 150) + 50); +} + // Nested floating layers an editor MIGHT portal outside the overlay (Base UI / Radix // / Floating UI popover, select, menu, dialog, tooltip). A pointerdown inside one of // these must NOT dismiss the overlay. The current editors render their pickers inline @@ -45,12 +86,21 @@ interface OverlayManagerProps { export function OverlayManager({ anchorRect, open, presetClass, onCancel, dismissMode = 'cancel', children }: OverlayManagerProps): React.ReactNode { const containerRef = useRef(null); + // React detaches the ref before it removes the node, so the closing copy still + // sees the editor's last open frame in place. + const setContainer = useCallback((node: HTMLDivElement | null) => { + const previous = containerRef.current; + containerRef.current = node; + if (!node && previous) playCloseTransition(previous); + }, []); // Measured, viewport-clamped position (both axes). Null until the overlay has been // measured once, so first paint stays hidden rather than flashing at an unclamped // spot. Covers the FLIP (no room below -> open above) AND the horizontal shift (an // anchor near the right edge -> slide left so the overlay stays on-screen instead of // overflowing off the viewport, e.g. the last column's editor). - const [pos, setPos] = useState<{ left: number; top: number } | null>(null); + const [pos, setPos] = useState<{ left: number; top: number; originX: number } | null>(null); + // False until the measured position has painted once at the pre-open scale. + const [entered, setEntered] = useState(false); const targetY = anchorRect?.top; const targetH = anchorRect?.height; @@ -71,25 +121,49 @@ export function OverlayManager({ anchorRect, open, presetClass, onCancel, dismis useLayoutEffect(() => { if (!open || !anchorRect) { setPos(null); + setEntered(false); return; } const el = containerRef.current; if (!el) return; - const rect = el.getBoundingClientRect(); - const vw = window.innerWidth; - const vhNow = window.innerHeight; + const place = () => { + // Layout size, not getBoundingClientRect: the pre-open scale would shrink it. + const width = el.offsetWidth; + const height = el.offsetHeight; + const vw = window.innerWidth; + const vhNow = window.innerHeight; - let left = anchorRect.left; - left = Math.max(MARGIN_PX, Math.min(left, vw - MARGIN_PX - rect.width)); - if (!Number.isFinite(left)) left = MARGIN_PX; + let left = anchorRect.left; + left = Math.max(MARGIN_PX, Math.min(left, vw - MARGIN_PX - width)); + if (!Number.isFinite(left)) left = MARGIN_PX; - let top = shouldFlip ? anchorRect.top - rect.height : anchorRect.bottom; - top = Math.max(MARGIN_PX, Math.min(top, vhNow - MARGIN_PX - rect.height)); - if (!Number.isFinite(top)) top = MARGIN_PX; + let top = shouldFlip ? anchorRect.top - height : anchorRect.bottom; + top = Math.max(MARGIN_PX, Math.min(top, vhNow - MARGIN_PX - height)); + if (!Number.isFinite(top)) top = MARGIN_PX; - setPos({ left, top }); + // Grow from the anchor cell's corner, even when the overlay is shifted left. + const originX = Math.max(0, Math.min(anchorRect.left - left, width)); + setPos((prev) => (prev && prev.left === left && prev.top === top && prev.originX === originX + ? prev + : { left, top, originX })); + }; + place(); + // Lazy editors (date) and state changes (image) resize after opening; keep the + // overlay clamped on-screen as its content settles. + if (typeof ResizeObserver === 'undefined') return; + const observer = new ResizeObserver(place); + observer.observe(el); + return () => observer.disconnect(); }, [open, shouldFlip, anchorRect, maxHeight]); + useEffect(() => { + if (!open || pos === null || entered) return; + const el = containerRef.current; + if (!el) return; + void el.offsetWidth; // Commit the pre-open frame so the state change below transitions. + setEntered(true); + }, [open, pos, entered]); + // Dismiss on a pointerdown OUTSIDE the overlay — standard popover behaviour. The // overlay was previously only closable via Esc / commit / scroll, so a click in the // grid or page left it stuck open. Attached in an effect (after commit) so the @@ -125,16 +199,16 @@ export function OverlayManager({ anchorRect, open, presetClass, onCancel, dismis if (!open || !anchorRect || typeof document === 'undefined') return null; - // Animate only once the measured, viewport-clamped position is applied, so the - // entrance transform never fights the layout-effect measure. + // Reveal only once the measured, viewport-clamped position is applied, so the + // entrance never fights the layout-effect measure. const positioned = pos !== null; const style: React.CSSProperties = { position: 'fixed', // Pre-measure default = the ANCHOR position (below, or above when flipping), NOT a // screen corner. So an overlay that fits (the common case) renders at its FINAL spot // with zero left/top change once measured — no travel. It stays hidden until measured - // and `transition-none` keeps the rare off-screen clamp instant, so the overlay is - // never seen sliding to the anchor (the correction happens off-screen, pre-reveal). + // and left/top are never transitioned, so the rare off-screen clamp is instant and the + // overlay is never seen sliding to the anchor (the correction happens pre-reveal). left: pos ? pos.left : anchorRect.left, top: pos ? pos.top : shouldFlip ? anchorRect.top : anchorRect.bottom, maxHeight, @@ -144,6 +218,7 @@ export function OverlayManager({ anchorRect, open, presetClass, onCancel, dismis // arbitrary z, so the editor and any popover/tooltip it opens (e.g. the relation // "Record Data" popover, which portals into #portal-root) stack coherently. zIndex: 'var(--z-layer-floating)', + transformOrigin: `${pos?.originX ?? 0}px ${shouldFlip ? '100%' : '0'}`, }; // Portal into the app's shared floating-overlay host (#portal-root) so the editor and @@ -155,19 +230,11 @@ export function OverlayManager({ anchorRect, open, presetClass, onCancel, dismis return createPortal(
{/* Provide the overlay geometry so editors (relation/image/geometry) can self-bound their internal scroll lists; without it they read maxHeight:0 and render unbounded (the diff --git a/packages/sheets/src/grid-dom/overlay/use-overlay-controller.ts b/packages/sheets/src/grid-dom/overlay/use-overlay-controller.ts index db801df5..aaab43ad 100644 --- a/packages/sheets/src/grid-dom/overlay/use-overlay-controller.ts +++ b/packages/sheets/src/grid-dom/overlay/use-overlay-controller.ts @@ -8,6 +8,8 @@ // TODO(Phase 7): Tab should commit-and-open-next; today Tab is left to default. import { useCallback, useState } from 'react'; +import type { SheetsScopeKey } from '../../context/sheets-context'; +import type { SheetsCellResolution } from '../use-sheets-content'; export interface OverlayCell { rowIndex: number; @@ -15,6 +17,14 @@ export interface OverlayCell { anchorRect: DOMRect; /** Type-to-edit seed: when set, value editors initialize the input to this char (overwrite). */ initialText?: string; + /** Keep the media editor mounted when creating its row removes the draft. */ + mediaDraft?: { + rowId: string; + tableName: string; + scopeKey: SheetsScopeKey; + value: unknown; + resolution: SheetsCellResolution; + }; /** * Edit surface for this active cell. `'overlay'` (default when omitted) renders the portal * editor; `'inline'` edits IN PLACE inside the cell (no portal — the cell host renders the diff --git a/packages/sheets/src/grid-dom/sheets-cell-host.tsx b/packages/sheets/src/grid-dom/sheets-cell-host.tsx index 9cd88901..e396bc9e 100644 --- a/packages/sheets/src/grid-dom/sheets-cell-host.tsx +++ b/packages/sheets/src/grid-dom/sheets-cell-host.tsx @@ -152,6 +152,7 @@ function toCellProps(props: SheetsCellHostInnerProps, onStartEdit: () => void): isEditing: props.isEditing, onStartEdit, disabled: false, + readOnlyReason: props.resolution.readOnlyReason, }; } @@ -219,6 +220,7 @@ const SheetsCellHostInner = memo( ref={wrapperRef} id={`sheets-cell-${props.rowIndex}-${props.columnIndex}`} aria-selected={props.isActive || undefined} + title={props.resolution.readOnlyReason} onClick={activate} onDoubleClick={startEdit} onContextMenu={contextMenu} @@ -274,6 +276,8 @@ const SheetsCellHostInner = memo( prev.onActivateCell === next.onActivateCell && prev.onContextMenu === next.onContextMenu && prev.onSubmitDraftRow === next.onSubmitDraftRow && + prev.resolution.cell.readonly === next.resolution.cell.readonly && + prev.resolution.readOnlyReason === next.resolution.readOnlyReason && // styleHint.draft/error and the draft-action status can flip while `value` is // unchanged (validation result arrives, save in flight), so gate on them too. prev.resolution.cell.styleHint?.draft === next.resolution.cell.styleHint?.draft && diff --git a/packages/sheets/src/grid-dom/use-sheets-content.ts b/packages/sheets/src/grid-dom/use-sheets-content.ts index f8fe2c9e..dd35e70b 100644 --- a/packages/sheets/src/grid-dom/use-sheets-content.ts +++ b/packages/sheets/src/grid-dom/use-sheets-content.ts @@ -43,6 +43,9 @@ export interface SheetsCellResolution { * of the `meta.cellType` the cell itself now self-describes.) */ typeKey: string; + /** Typed server row locator, separate from the grid's string key. */ + rowIdentifier?: SheetsRowIdentifier | null; + readOnlyReason?: string; /** Field metadata for the column — fed to a native overlay editor's EditorProps.fieldMeta. */ fieldMeta?: FieldMetadata; /** Relation info for the column (when relational) — fed to a native overlay editor's EditorProps.relationInfo. */ @@ -64,10 +67,12 @@ export interface UseSheetsContentArgs { serverRowsReadOnly?: boolean; /** Primary-key fields are immutable after insertion because they define row identity. */ serverReadOnlyFields?: ReadonlySet; - /** Resolve the GraphQL mutation identity; self-committing editors require a legacy scalar id. */ + /** Resolve the GraphQL mutation identity. */ getRowIdentifier?: (row: Readonly>) => SheetsRowIdentifier | null; - /** Relation/upload editors still use scalar record APIs and must fail closed otherwise. */ + /** Relation editors still use scalar record APIs. */ supportsScalarSelfCommit?: boolean; + canUpload?: boolean; + canCreate?: boolean; } function isDraftId(value: unknown): boolean { @@ -126,6 +131,8 @@ export function useSheetsContent(args: UseSheetsContentArgs) { serverReadOnlyFields, getRowIdentifier, supportsScalarSelfCommit = true, + canUpload = true, + canCreate = true, } = args; const relationFieldNamesFromMeta = useMemo(() => buildRelationFieldNameSet(meta, tableName), [meta, tableName]); @@ -149,7 +156,7 @@ export function useSheetsContent(args: UseSheetsContentArgs) { // reference-equal guarantees an identical resolution. const cache = useMemo( () => new Map(), - [data, columnKeys, fieldMetaMap, registry, relationFieldNamesFromMeta, relationInfoByField, options, serverRowsReadOnly, serverReadOnlyFields, getRowIdentifier, supportsScalarSelfCommit], + [data, columnKeys, fieldMetaMap, registry, relationFieldNamesFromMeta, relationInfoByField, options, serverRowsReadOnly, serverReadOnlyFields, getRowIdentifier, supportsScalarSelfCommit, canUpload, canCreate], ); const getSheetsCell = useCallback( @@ -248,13 +255,15 @@ export function useSheetsContent(args: UseSheetsContentArgs) { }; const cellType = registry.resolveTypeKey(matchInput, () => route.cellType); const rowIdentifier = !isDraftRow ? getRowIdentifier?.(rowData) : null; - const selfCommittingEditor = isRelation || MEDIA_TYPES.has(cellType); + const isMedia = MEDIA_TYPES.has(cellType); const lacksScalarEditorIdentity = - selfCommittingEditor && + isRelation && (!supportsScalarSelfCommit || ( rowIdentifier != null && typeof rowIdentifier === 'object' )); - const forcedReadOnly = tableForcedReadOnly || lacksScalarEditorIdentity; + const lacksMediaTarget = isMedia && (isDraftRow ? !canCreate || serverRowsReadOnly : rowIdentifier == null); + const readOnlyReason = isMedia && !canUpload ? 'Uploads need a storage endpoint' : undefined; + const forcedReadOnly = tableForcedReadOnly || lacksScalarEditorIdentity || lacksMediaTarget || !!readOnlyReason; const metadata: CellCreationMetadata = { cellType, @@ -292,11 +301,13 @@ export function useSheetsContent(args: UseSheetsContentArgs) { // (relation/image, Phase 5) can read EditorProps.fieldMeta / relationInfo without // re-resolving — no extra work, just exposing what this routing already derived. // `typeKey` is the resolved cellType — the single source of truth for edit-intent. - const resolution: SheetsCellResolution = { cell: styled, component, colKey, typeKey: cellType, fieldMeta, relationInfo }; + const resolution: SheetsCellResolution = { + cell: styled, component, colKey, typeKey: cellType, fieldMeta, relationInfo, rowIdentifier, readOnlyReason, + }; cache.set(cacheKey, { value: rawValue, resolution }); return resolution; }, - [data, columnKeys, fieldMetaMap, registry, relationFieldNamesFromMeta, relationInfoByField, options, serverRowsReadOnly, serverReadOnlyFields, getRowIdentifier, supportsScalarSelfCommit], + [data, columnKeys, fieldMetaMap, registry, relationFieldNamesFromMeta, relationInfoByField, options, serverRowsReadOnly, serverReadOnlyFields, getRowIdentifier, supportsScalarSelfCommit, canUpload, canCreate], ); return { getSheetsCell }; diff --git a/packages/sheets/src/grid/editors/__tests__/image-editor-draft.test.tsx b/packages/sheets/src/grid/editors/__tests__/image-editor-draft.test.tsx new file mode 100644 index 00000000..18632ec8 --- /dev/null +++ b/packages/sheets/src/grid/editors/__tests__/image-editor-draft.test.tsx @@ -0,0 +1,98 @@ +/* @vitest-environment jsdom */ +// Draft-row uploads: once the draft has been persisted the draft is gone, so a +// failure after that point must still tell the host to refetch, even when the +// created row came back without a usable primary-key identity. +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const upload = vi.hoisted(() => ({ + resolveRowIdentifier: vi.fn(), + write: vi.fn(), + mutate: vi.fn(), +})); + +vi.mock('../../../hooks/use-sheets-upload', () => ({ useSheetsFieldUpload: () => upload })); +vi.mock('../../../hooks/use-sheets-media-url', () => ({ + useSheetsMediaUrl: () => ({ url: null, durableUrl: null, id: null, isLoading: false, onImageError() {}, onImageLoad() {} }), +})); +vi.mock('@constructive-io/ui/toast', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })); + +import { ImageEditor } from '../image-editor'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +describe('ImageEditor draft-row upload', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + URL.createObjectURL = vi.fn(() => 'blob:preview'); + URL.revokeObjectURL = vi.fn(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + afterEach(async () => { + await act(async () => root.unmount()); + document.body.innerHTML = ''; + vi.clearAllMocks(); + }); + + async function selectFileAndConfirm() { + const input = container.querySelector('input[type="file"]')!; + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + Object.defineProperty(input, 'files', { value: [file], configurable: true }); + await act(async () => input.dispatchEvent(new Event('change', { bubbles: true }))); + const confirm = Array.from(container.querySelectorAll('button')).find((b) => b.textContent === 'Create Row & Upload')!; + await act(async () => confirm.click()); + } + + it('notifies the host when the created row has no resolvable identity', async () => { + const onSubmitDraft = vi.fn().mockResolvedValue({ createdRow: { name: 'no id' } }); + const onDraftUploadComplete = vi.fn(); + upload.resolveRowIdentifier.mockReturnValue(null); + + await act(async () => { + root.render( + {}} + tableName='posts' + fieldName='cover' + isDraftRow + onSubmitDraft={onSubmitDraft} + onDraftUploadComplete={onDraftUploadComplete} + />, + ); + }); + await selectFileAndConfirm(); + + expect(onSubmitDraft).toHaveBeenCalledOnce(); + expect(upload.write).not.toHaveBeenCalled(); + expect(onDraftUploadComplete).toHaveBeenCalledOnce(); + }); + + it('does not notify the host when creating the draft row fails', async () => { + const onSubmitDraft = vi.fn().mockRejectedValue(new Error('insert denied')); + const onDraftUploadComplete = vi.fn(); + + await act(async () => { + root.render( + {}} + tableName='posts' + fieldName='cover' + isDraftRow + onSubmitDraft={onSubmitDraft} + onDraftUploadComplete={onDraftUploadComplete} + />, + ); + }); + await selectFileAndConfirm(); + + expect(onSubmitDraft).toHaveBeenCalledOnce(); + expect(onDraftUploadComplete).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/sheets/src/grid/editors/image-editor.tsx b/packages/sheets/src/grid/editors/image-editor.tsx index 08b62ab8..6b9f8536 100644 --- a/packages/sheets/src/grid/editors/image-editor.tsx +++ b/packages/sheets/src/grid/editors/image-editor.tsx @@ -3,15 +3,13 @@ import { AlertCircleIcon, FileIcon, ImageIcon, ImageOff, Loader2, UploadIcon, XI import { AnimatePresence, motion } from 'motion/react'; import { toast } from '@constructive-io/ui/toast'; -import { useSheetsContext } from '../../context/sheets-context'; -import { useSheetsStoreApi } from '../../store/sheets-store'; import { useSheetsFieldUpload, - executeFieldUpload, type UploadProgress, type UploadResponse, } from '../../hooks/use-sheets-upload'; -import { formatFileSize, getImageMetadata, getImageUrl } from '../../utils/file.utils'; +import { useSheetsMediaUrl } from '../../hooks/use-sheets-media-url'; +import { formatFileSize, getImageMetadata } from '../../utils/file.utils'; import { sheetsLogger } from '../../utils/sheets-logger'; import { useImageStatus } from '../../utils/use-image-status'; import { Button } from '@constructive-io/ui/button'; @@ -22,29 +20,14 @@ import { EditorFocusTrap } from './editor-focus-trap'; import { OVERLAY } from './overlay-presets'; import { OverlayMeasureContext } from './overlay-viewport-guard'; import type { DraftSubmitResult } from '../draft-types'; +import type { SheetsRowIdentifier } from '../../row-identity'; /** Data passed to onSaveComplete for optimistic updates */ export interface ImageSaveData { - imageData: { - url: string; - filename?: string; - size?: number; - mime?: string; - } | null; + imageData: UploadResponse; } -type UploadState = 'idle' | 'submitting-draft' | 'uploading'; - -interface NormalizedImageData { - url: string; - filename: string; - size: number; - mime: string; -} - -function normalizeUploadResponse(data: UploadResponse): NormalizedImageData { - return { url: data.url, filename: data.filename, size: data.size, mime: data.mime }; -} +type UploadState = 'idle' | 'submitting-draft' | 'uploading' | 'clearing'; /** Snappy crossfade for dropzone content transitions */ const dropzoneTransition = { duration: 0.15, ease: [0.2, 0, 0, 1] as [number, number, number, number] }; @@ -76,6 +59,7 @@ function RemoveButton({ onClick, label }: { onClick: () => void; label: string } /** Centered file chip for non-image files (selected or stored) — icon + name + size/mime. */ function FilePreview({ name, size, mime }: { name?: string; size?: number; mime?: string }) { const hasMime = !!mime && mime !== 'unknown'; + const hasSize = size !== undefined; return (