From 533cfb97bab26433edbe16ec09b1ecd72e88b013 Mon Sep 17 00:00:00 2001 From: edmonday Date: Tue, 5 May 2026 16:11:07 +1200 Subject: [PATCH 1/5] docs(plans): add editor asset library plan (NES-1614) Land the Editor Asset Library plan on main as the durable source of truth, independent of the throwaway prototype PR (#9102) where it currently lives. Covers: - 2026-04-28 approach pivot (per-tab history backed by CloudflareImage / MuxVideo, dropping the Block-table and standalone Library tab approaches). - 2026-05-01 v1 ship state and v1.1 team-shared visibility plan, including the persisted teamId design (asset's home team is derived from journey.teamId at upload time). - mediaLibrary LaunchDarkly flag gating for production rollout. - Quick-start (Template Customization) flow scoped out per Lyuba's feedback. - Historical pre-pivot Relay / imageBlocksConnection material retained as a decision trail and clearly marked as superseded. --- ...4-28-001-feat-editor-asset-library-plan.md | 856 ++++++++++++++++++ 1 file changed, 856 insertions(+) create mode 100644 docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md diff --git a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md new file mode 100644 index 00000000000..41eaa40697e --- /dev/null +++ b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md @@ -0,0 +1,856 @@ +--- +title: Editor Asset Library +type: feat +status: active +date: 2026-04-28 +origin: docs/brainstorms/2026-04-28-editor-asset-library-requirements.md +deepened: 2026-04-28 +revised: 2026-05-01 +--- + +# Editor Asset Library + +> **How to read this plan.** This document is layered chronologically: each section reflects what was known and decided at its date. Earlier sections describe approaches that prototyping later invalidated and are kept for the decision trail, not as implementation guidance. The current source of truth for v1 (shipped) and v1.1 (next) is the **2026-05-01** section. The **2026-04-28 pivot** section above it is also current. Everything from "Enhancement Summary" downward is **historical** — it describes the pre-pivot Relay/Connection-based design that was prototyped, evaluated, and rejected. +> +> Prototype work (PR #9102, NES-1614) is the input that drove the supersession. As we move from prototype → production, treat each prototype finding as a candidate plan revision: capture what shipped, what's deferred, what's been ruled out, and feed those into the Linear milestones that track the path to release. + +## 2026-04-28 Approach pivot — supersedes prior sections + +A prototype pass surfaced a fundamental issue with the original `Block`-table-as-source-of-truth premise: **`Block.src` is overwritten in place** by `imageBlockUpdate` when a user picks a new image on the same card. The first cover creates a `Block` row; every subsequent pick on that card mutates that same row's `src`. Confirmed in `apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/Image/BackgroundMediaImage.tsx:221-263`. + +Consequence: a Library backed by `Block` rows can only show *currently-attached* images. The exact scenario the feature is meant to solve — "I uploaded this last week, replaced it, now I want it back" — is unreachable, because the prior `src` is gone. + +### New approach: per-tab history backed by existing `CloudflareImage` table + +Each existing tab in `ImageBlockEditor` gains its own history surface, fed by its own persistence layer: + +| Tab | History surface | Backing data | Survives Block.src overwrite? | +|-----|-----------------|--------------|------------------------------| +| **Custom** | "Your uploads" grid above the file/URL inputs | `CloudflareImage` (media DB), `userId`-scoped, populated by `createCloudflareUploadByFile` and `createCloudflareUploadByUrl` | ✅ Independent table | +| **AI** | "Your generations" grid above the prompt input | Same `CloudflareImage` (populated by `createImageBySegmindPrompt` / `createCloudflareImageFromPrompt`) | ✅ Independent table | +| **Unsplash** | Deferred. No local persistence today; `triggerUnsplashDownload` only calls Unsplash's tracking API. Picking from Unsplash's existing search/browse remains the path. | — | — | +| **Standalone Library tab** | **Dropped.** Per-tab history covers ~90% of the user's mental model ("I uploaded that" → check Custom; "I generated that" → check AI). Cross-source unification adds little once each tab shows its own history. | — | — | + +### What's already in place to leverage + +- **`getMyCloudflareImages(offset, limit)`** — exists in `apis/api-media/src/schema/cloudflare/image/image.ts:69-85`. Returns all `CloudflareImage` rows for the authenticated user. No backend work required to power Custom and AI history grids. +- `CloudflareImage` URL pattern (`https://imagedelivery.net/{hash}/{id}/public`) is identical across uploads/URL paste/AI, so Custom and AI grids initially show overlapping content. + +### Open question rolled into this pivot + +`CloudflareImage` has no `source` enum (`file | url | ai`) or `prompt` column. Two paths: +- **Cheap:** show all `CloudflareImage` rows in both Custom and AI tabs, accept the overlap. +- **Right:** add `source` enum and (for AI) `prompt` column to `CloudflareImage`. Mutations populate them. Each tab filters accordingly. + +For prototype: take the cheap path. Promote to "right" once the UX is validated. + +### Why this is better product + +- **No new architecture required.** No `MediaAsset` table, no Block-mutation rewiring, no orphan-Block strategy. +- **Matches the user's mental model.** People remember *how* they got an image (uploaded vs generated vs picked), and they're already on that tab when they want to find it again. +- **Skips the Unsplash gap.** Unsplash already provides search/browse; "previously picked Unsplash images" is the weakest of the three use cases and can ship later via a small `UnsplashSelection` table if metrics justify it. +- **Removes the deletion-orphan worry entirely** — `CloudflareImage` rows are independent of `Block`, so deletes don't cascade strangely. + +### Sections below this point + +The original plan (Frontend, Backend, Technical Approach, Acceptance Criteria, etc.) was built around the rejected `Block`-table approach. **Treat them as historical context for the decision trail, not the v1 spec.** New acceptance criteria, file layout, and phasing for the per-tab approach are TODO and will be drafted before implementation resumes. + +The revised v1 work is roughly: +1. Add a "Your uploads" grid component to the Custom tab, backed by `getMyCloudflareImages`. +2. Add a "Your generations" grid component to the AI tab, backed by the same query (or a filtered variant once `source` is added to `CloudflareImage`). +3. Defer Unsplash history. +4. Optional: add `source` enum + `prompt` column to `CloudflareImage`, populate on the relevant mutations. + +--- + +## 2026-05-01 v1 shipped + v1.1 team scope plan — supersedes ACTIVE_TEAM material in prior sections + +### What v1 actually shipped (per-tab history, personal scope only) + +Implementation against the 2026-04-28 pivot landed in PR #9102. Production state: + +**Image picker (`ImageBlockEditor`):** +- "Your uploads" grid in Custom tab. +- "Your generations" grid in AI tab. +- Backed by existing `getMyCloudflareImages(offset, limit, isAi): [CloudflareImage!]!` — additive `isAi` arg only, no schema-breaking change. +- New nullable `CloudflareImage.isAi` column populated by upload/URL/AI mutations. Existing 38k rows stay NULL and are excluded from both grids (no backfill — accepted limitation). +- Frontend: offset/limit pagination with `length >= PAGE_SIZE` heuristic for "Load More". `offsetLimitPagination(['isAi'])` cache config. + +**Video picker (`VideoFromMux`):** +- "Your uploads" grid below the Mux upload form. +- Backed by existing `getMyMuxVideos(offset, limit): [MuxVideo!]!` — no schema change. Resolver only added a deterministic `orderBy [createdAt desc, id desc]` (correctness for offset pagination, not a UI preference). +- Click-a-tile opens existing `VideoDetails` + `MuxDetails` with a Select button. Gallery flow reuses the same preview infrastructure as the active-block flow; `playbackId` is threaded through to avoid a redundant fetch. `MuxDetails.dispose()` fix shipped alongside. +- `MuxVideoUploadProvider` refetches `GetMyMuxVideos` after polling completes so freshly-ready uploads surface in the gallery. +- `VideoLibrary.onSelect` now closes the outer drawer when `shouldCloseDrawer` is true so picking the same video as the active block still navigates back to Properties. + +The "fourth Library tab" with `imageBlocksConnection` from the original plan **is permanently dropped**. Per-tab history covers the user's mental model. Don't reintroduce it without explicit product re-validation. + +The `Connection`-style refactor of `getMyCloudflareImages` and `getMyMuxVideos` was prototyped and **reverted** before merge — overengineered for the actual UX requirement. Both resolvers ship as backward-compatible flat arrays with offset/limit pagination. Documented for posterity so the next person doesn't re-introduce it. + +### Production rollout — `mediaLibrary` LaunchDarkly flag + +v1 is currently merged but not yet generally available. Before going live we gate the new UI behind the existing **`mediaLibrary`** LaunchDarkly flag so it can be tested in production with a controlled cohort before being turned on for everyone. + +- **Flag name:** `mediaLibrary` (already provisioned in LaunchDarkly; serves `true` when on). +- **Access pattern:** `const { mediaLibrary } = useFlags()` from `@core/shared/ui/FlagsProvider` — the same pattern used by `editorAnalytics` in `apps/journeys-admin/src/components/Editor/Toolbar/Toolbar.tsx:92` and similar editor flags. +- **Surfaces to gate:** + - `MyCloudflareImagesGrid` mount in the Custom tab (`CustomImage.tsx`) — render only when `mediaLibrary` is true. + - `MyCloudflareImagesGrid` mount in the AI tab (`AIGallery.tsx`) — same. + - `MyMuxVideosGrid` mount in `VideoFromMux.tsx` — same. +- **What is NOT gated:** + - The `isAi` column on `CloudflareImage` and the `isAi` value written by upload/URL/AI mutations. These are additive, schema-only, and harmless when the UI is off — leaving them on means cohorts who flip the flag on later already have populated history. + - The deterministic `orderBy` on `getMyMuxVideos`. Pure correctness improvement, no behavior change for existing list consumers. + - Resolver argument changes (`isAi`, future `teamId`). Optional args; off-by-default. +- **Default behavior with flag off:** the editor pickers behave exactly as they did pre-PR-#9102 — no "Your uploads" / "Your generations" grids, no behavior change in upload, AI, or Mux flows. +- **Rollout steps:** + 1. Wrap the three grid mount points in `mediaLibrary && (...)`. + 2. Verify with the flag off: existing pickers unchanged. + 3. Verify with the flag on for a test user: grids appear, picking applies via `onChange`, refetch-on-upload works. + 4. Roll out to internal users → wider beta → 100%. +- **Tests:** add a flag-off render test for each gated component to lock in the "feature absent" branch and prevent accidental ungated regression. + +### v1.1 — team-shared visibility (replaces the ACTIVE_TEAM material in original Backend / Architecture / Implementation Phases sections) + +Goal: the same per-tab grids surface assets uploaded by *teammates* in the user's active team, alongside their own. + +#### Locked decisions + +- **Asset has both an uploader (`userId`) and a home team (`teamId`).** `userId` is the canonical owner — it's how "Your uploads" stays personal. `teamId` is the home-team affiliation, persisted at upload time and immutable thereafter; it's how the asset stays visible to a team after the uploader leaves. +- **`teamId` is derived from the journey context at upload time**, not from the user's `useTeam().activeTeam` UI state. All editor upload surfaces (image block picker, video block picker, poster picker, journey logo, social share image) happen inside a journey, and a `Journey` always has a `teamId`. Server-side, the resolver looks up the journey by ID to read its `teamId`; the frontend just needs to pass the `journeyId` it's already editing. +- **Visibility split:** + - "Your uploads" grid: `where: { userId: caller }` — unchanged from v1. + - "Team uploads" grid: `where: { teamId: activeTeamId }` plus an explicit caller-is-member precheck. +- **Active team passed per-request** from the editor for the "Team uploads" grid only. The frontend already knows it via `useTeam()`. +- **Membership precheck**: `api-media` directly imports `prisma-journeys` for the `userTeam.findUnique` check. Same precedent already established by `api-journeys-modern`'s user-delete flow (which imports both `prisma-users` and `prisma-journeys`). Latency ~2–5ms, always fresh, no token-staleness window. **Read path no longer needs the teammate-list fetch** — it filters on the asset's own `teamId`. +- **Schema changes:** new nullable `teamId String?` column on `CloudflareImage` and `MuxVideo`. New rows always populated (every editor upload has a journey context); existing 38k+ rows stay NULL. Indexed on `(teamId, createdAt DESC, id DESC)` to match the visibility query. +- **No backfill.** Existing NULL-`teamId` rows are invisible to all team grids and remain visible only to their original uploader in "Your uploads". Clean slate for team-shared visibility from migration forward. +- **Account deletion stays correct.** Neither `userDeleteConfirm` nor `userDeleteJourneysConfirm` touches `MuxVideo` / `CloudflareImage`. After deletion: `userId` becomes a UUID tombstone with no PII; the asset's `teamId` is unaffected, so it stays in the team's "Team uploads" grid (which is exactly what we want — the team keeps the asset). +- **Team deletion**: out of scope for v1.1. If a team is deleted, its assets persist with a stale `teamId` reference. We don't currently have a journey-team-delete flow that cascades, and the surface area to add it is larger than v1.1 should take on. Document and revisit if/when team deletion ships. +- **Out of scope** for v1.1: deletion/management of teammates' assets, multi-team merged views, signed/private playback IDs, uploader attribution UI (deferred if product asks), retroactive `teamId` assignment for pre-migration rows, transferring an asset between teams, storage quotas per team. +- **Quick-start (Template Customization) flow is out of scope.** Based on Lyuba's feedback, surfacing per-tab history inside the quick-start flow doesn't quite make sense for that flow's intent — first-run template setup is a different mental model from editor re-use. Re-pick from history applies only inside the editor's image and video pickers. We can revisit adding it to the quick-start later if it turns out to help. + +#### Schema change + +Prisma model update (`prisma-media`): + +```prisma +model CloudflareImage { + // ... existing fields + teamId String? + @@index([teamId, createdAt(sort: Desc), id(sort: Desc)]) +} + +model MuxVideo { + // ... existing fields + teamId String? + @@index([teamId, createdAt(sort: Desc), id(sort: Desc)]) +} +``` + +Migration: `ADD COLUMN teamId TEXT NULL` plus the index. No backfill; safe and fast on a populated table because nullable columns don't require a table rewrite in Postgres. + +#### Upload mutation changes + +Each mutation that creates a `CloudflareImage` or `MuxVideo` row gains a required `journeyId` arg (the editor always knows which journey the user is in). Server-side flow per upload: + +1. Look up `journey.teamId` from the journey ID (one Prisma read against `prisma-journeys`). +2. Verify caller has access to that journey via the existing journey-edit auth path (no new auth code). +3. Persist the new row with `userId = caller` and `teamId = journey.teamId`. + +Mutations affected (approximate — finalize during BE-2 scoping): +- `createCloudflareUploadByFile` +- `createCloudflareUploadByUrl` +- `createImageBySegmindPrompt` / `createCloudflareImageFromPrompt` +- The Mux upload mutation (creates `MuxVideo`) + +#### Resolver changes (additive, non-breaking) + +```graphql +getMyMuxVideos(offset: Int, limit: Int, teamId: ID): [MuxVideo!]! +getMyCloudflareImages(offset: Int, limit: Int, isAi: Boolean, teamId: ID): [CloudflareImage!]! +``` + +Both resolvers' implementation: + +1. **`teamId` omitted** → existing behavior (`where: { userId: caller }`). Backward compatible — drives the "Your uploads" grid. +2. **`teamId` provided** → "Team uploads" path: + - **Membership precheck**: `journeysPrisma.userTeam.findUnique({ where: { teamId_userId: { teamId, userId: caller } } })`. If null → throw `GraphQLError('Not a member of this team', { extensions: { code: 'FORBIDDEN' } })`. Don't distinguish from non-existent team — avoids existence enumeration. + - **Asset query**: `where: { teamId, ...(isAi != null ? { isAi } : {}) }`. No teammate-list fetch needed. + - Same `orderBy [createdAt desc, id desc]` and offset/limit pagination as v1. + +#### Frontend changes + +- `MyCloudflareImagesGrid` and `MyMuxVideosGrid` get a `teamId?: string | null` prop. +- Mount each grid twice in its respective tab/screen: + ```tsx + + + ``` + Same pattern for Mux: a "Your uploads" + a "Team uploads" grid stacked under `AddByFile`. +- **Apollo cache** — extend keyArgs so the two grids stay in separate cache entries: + ```ts + getMyCloudflareImages: offsetLimitPagination(['isAi', 'teamId']), + getMyMuxVideos: offsetLimitPagination(['teamId']), + ``` +- **Active team source**: `useTeam()` from `TeamProvider` — already a tree ancestor of the editor. Threaded as a prop into the relevant tab/screen components. +- **Upload mutation call sites**: thread the current `journeyId` (already known from the editor route/state) into each upload mutation invocation. +- **Refetch on upload**: existing `refetchQueries: ['GetMyCloudflareImages']` / `'GetMyMuxVideos'` works as-is. Apollo refetches every active query with that operation name regardless of variables, so both personal and team grids update. + +#### Behavioral table + +| Scenario | "Your uploads" (caller) | "Team uploads" (Team X) | +|---|---|---| +| Caller uploads inside a Team X journey | Visible (`userId = caller`) | Visible (`teamId = X`) | +| Caller leaves Team X | Still visible (`userId = caller`) | **Still visible to remaining Team X members** | +| Caller later joins Team Y, uploads inside a Team Y journey | Visible | Appears only in Team Y's grid (`teamId = Y`); does not retroactively appear in Team X | +| Pre-migration row (NULL `teamId`) | Visible to original uploader only | Not visible in any team grid | +| Caller passes `teamId` they're not a member of | — | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | — | `FORBIDDEN` (same shape — no existence leak) | + +#### Performance considerations + +- "Team uploads" path: one Prisma read against `prisma-journeys` for the membership precheck (~2–5ms, indexed). Single asset query against the new `(teamId, createdAt, id)` index — bounded by `LIMIT`, scales independently of team size. +- "Your uploads" path: unchanged from v1 (`userId` filter). +- Upload path: one extra Prisma read against `prisma-journeys` to look up `journey.teamId`. Same lookup the existing journey-edit auth path already does, so likely cached at the request level — negligible. +- Net effect vs the "compute teammates dynamically" alternative: simpler read path, removes a `findMany` from every "Team uploads" load, and removes the `userId IN (N userIds)` query whose performance degraded with team size. + +#### Authorization edge cases + +| Scenario | Behavior | +|---|---| +| Caller passes `teamId` they're not a member of | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | +| User removed from team mid-session and tries to load "Team uploads" | `FORBIDDEN`; frontend detects and refreshes active-team list | +| User removed from team after uploading | Their asset stays in the team's "Team uploads" grid (the design goal) | +| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | +| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | + +#### Tests (v1.1 specific) + +- Migration: nullable column added; existing rows unchanged; index created. +- Upload mutations: each one writes `teamId = journey.teamId` for new rows. +- Upload mutations: caller without journey-edit access cannot create rows (existing auth path). +- Resolver: `teamId` omitted → returns own uploads (v1 behavior preserved). +- Resolver: `teamId` provided + caller is member → returns rows where `teamId = arg`, ordered correctly. Includes the caller's own rows tagged with that team. +- Resolver: `teamId` provided + caller not member → `FORBIDDEN`. +- Resolver: `teamId` for non-existent team → `FORBIDDEN` (same shape). +- Resolver: rows with NULL `teamId` never appear in any "Team uploads" response. +- Resolver: a row whose `userId` later loses team membership still appears in that team's grid (the core design goal). +- Frontend: both grids render independently with separate cache entries; switching `teamId` triggers the team grid to refetch. +- Frontend: refetch-after-upload updates both grids simultaneously. + +#### Phasing + +- **1.1.1 — Backend**: schema migration (nullable `teamId` + index on both tables), upload mutations write `teamId = journey.teamId`, optional `teamId` arg + membership precheck on both `getMyCloudflareImages` and `getMyMuxVideos`. Tests above. Ships unflagged — additive arg, additive column, backward-compatible. +- **1.1.2 — Frontend**: thread `journeyId` into upload mutation calls, thread `useTeam().activeTeam.id` to the grids, mount second "Team uploads" grid in each affected tab/screen, update Apollo cache keys. Both grids stay inside the existing `mediaLibrary` flag gate — v1.1 does not introduce a separate flag. +- **1.1.3 (optional, deferred)**: uploader attribution chip on each tile (federation to `api-users.User`). Skip until product asks. + +#### Risk surface + +- Schema change is a single nullable column + one index per table — no rewrite, fast on populated tables. +- Cross-domain Prisma read at upload time (`journey.teamId` lookup) adds a build-time coupling between `api-media` and `prisma-journeys` — already established precedent (user-delete), so additive rather than new policy. +- Upload mutation signatures change (`journeyId` becomes required). Backward-compat shim if any callers exist outside the editor; verify before merging the BE PR. +- Existing 38k rows being invisible to team grids is intentional (we can't reconstruct their team affiliation) but worth product confirmation. + +#### Material in original plan that does NOT apply to v1.1 + +The original (pre-pivot) plan and the deepening notes (lines 62–101) reference: `imageBlocksConnection`, `ImageBlockLibraryScope` enum, `ImageBlockLibraryEdge`, `ImageBlockLibraryConnection`, `JourneyProfile.lastActiveTeamId` server derivation, CASL guard / publisher-role tests, `thumbnailSrc`, `relayStylePagination`, partial composite Block index. **None of these apply to v1.1.** v1.1 is an additive arg on existing resolvers — not a new connection, not a new type, not a new auth code path beyond the membership precheck above. Treat the original Backend / Architecture / Acceptance Criteria sections as historical context for the rejected approach. + +--- + +## Enhancement Summary + +> ⚠️ **Historical — superseded by the 2026-04-28 pivot and 2026-05-01 v1/v1.1 sections above.** +> +> Everything from this point onward describes the original Relay-Connection / `imageBlocksConnection` / `ACTIVE_TEAM` server-derivation design. **That approach was prototyped and rejected.** Notably: +> - The Relay Connection migration of `getMyCloudflareImages` / `getMyMuxVideos` was reverted before merge — v1 ships offset/limit on the existing list-returning resolvers. +> - The `Block`-table-as-source-of-truth premise was invalidated (`Block.src` is overwritten in place). +> - The standalone Library tab and `imageBlocksConnection` resolver were dropped in favor of per-tab history grids. +> +> Read this section for context on *why* those decisions were made, not as a spec to implement. New work should be scoped against the v1.1 section. + +**Deepened:** 2026-04-28 with 3 best-practices researchers (Pothos cursor + Apollo cache + MUI tabs) and 5 reviewers (performance, security, simplicity, architecture, TypeScript). + +### Major plan changes from deepening + +1. **Resolver moves to legacy `apis/api-journeys`** (NestJS + SDL). The modern Pothos service has no Relay connection precedent, no `CaslAccessible`/`AppCaslGuard` infrastructure, and no team-scoped image-block resolvers — every piece the original plan assumed lives in legacy. The `visitorsConnection` template that the plan cites is itself in legacy. +2. **Drop the `ImageBlockLibraryEntry` projection.** Return `Connection` directly (mirrors `visitorsConnection { edges { node: Visitor } }`). Avoids parallel-type drift, codegen bloat, Apollo normalization collisions on shared `ImageBlock` ids, and ~150 LOC of mapping code. Three reviewers (architecture, simplicity, TypeScript) converged on this. +3. **Drop client-supplied `activeTeamId`.** Derive from `JourneyProfile.lastActiveTeamId` server-side and add an explicit `userTeam` membership precheck. Closes the cross-team enumeration vector and removes a forgeable client input. +4. **Drop server-side `DISTINCT ON` + raw SQL for v1.** Use vanilla Prisma `findMany` ordered by `updatedAt DESC` with a generous take, dedupe by `src` in resolver memory, and rely on the client `Map` as the cross-page safety net. Realistic library sizes are sub-few-thousand. Re-evaluate raw SQL only if EXPLAIN ANALYZE shows the simpler path is too slow. +5. **Add `thumbnailSrc` field (critical performance).** Resolver computes a thumbnail URL per source: Cloudflare `cdn-cgi/image/width=240,quality=80` for cf-hosted, Unsplash `&w=240&fit=crop`, AI/external passthrough. A 24-tile grid otherwise loads tens of MB of full-size hero images. +6. **Apollo cache strategy: `cache-first` (default) + mutation eviction.** Original plan double-paid by combining `cache-and-network` *and* eviction. With single-source-of-truth mutations, eviction alone is correct. +7. **MUI tabs: switch to `variant="scrollable"` with `iconPosition="top"`** unconditionally (the drawer is always narrow; viewport-conditional toggling adds flicker). +8. **Auth: pick one filter, not two.** "Defense-in-depth" via CASL AND scope filter is double-bookkeeping for cases where both are supposed to compute the same set. Use the explicit scope filter as the authoritative gate, plus an explicit membership check for `ACTIVE_TEAM`. CASL still applies elsewhere (the resolver guard) but is not duplicated in the where-input. EXCEPTION: the `publisher` role can read `template: true` journeys across teams via CASL — for that role explicitly, AND-ing the scope filter is load-bearing. Document and test this. +9. **Cursor and raw-row decoding need runtime validation** (zod) if any raw SQL is reintroduced. Versioned cursor (`v: 1, ...`) for forward compatibility. +10. **Explicit Postgres partial index DDL** required (not implicit): `CREATE INDEX CONCURRENTLY ... ON "Block" ("journeyId", "src", "updatedAt" DESC) WHERE typename='ImageBlock' AND src IS NOT NULL AND deletedAt IS NULL`. + +### Phasing decision (locked in 2026-04-28) + +**v1 ships PERSONAL only. ACTIVE_TEAM follows in v1.1.** This was the user's call after weighing the simplicity review's push to ship team-only against the brainstorm's stated dual-scope plan. + +Concrete implications for v1: +- No `ACTIVE_TEAM` scope, no server-side `lastActiveTeamId` derivation, no `userTeam` membership precheck. +- No feature-flag infrastructure required (Q7 dissolves). +- Schema can drop the `ImageBlockLibraryScope` enum and the `scope` argument entirely; the resolver is unconditionally personal-scoped. +- All publisher-role / cross-team / transfer auth tests (R20, R21) are deferred to v1.1 since they only matter when team scope ships. +- The brainstorm's stated value driver (team-shared brand assets) lands in v1.1, not v1. + +The technical sections below were drafted with both scopes in scope. Where they reference `ACTIVE_TEAM`, treat that material as a v1.1 spec carried in the same plan for continuity. **Phase 1 implements PERSONAL only.** + +### Sections substantially revised below + +- Proposed Solution → Backend +- Technical Approach → Architecture (resolver home, schema, query, authorization, frontend cache) +- Implementation Phases (Phase 1 query path; Phase 2 tab variant; Phase 4 flag dependency) +- Acceptance Criteria (added R19–R23; removed R10/R12/R15 — see notes inline) +- Risk Analysis (publisher role; thumbnail bandwidth) +- Dependencies (feature-flag infra) +- Outstanding Questions (Q6 dual-scope vs single-scope; Q7 feature-flag infra) + +## Overview + +Add a **Library** tab to the Editor's image picker (`ImageBlockEditor`) that surfaces images the creator (or their active team) has previously used in any `ImageBlock`. Picking an image applies it to the current block via the existing `onChange` flow — no upload, no AI prompt, no Cloudflare round-trip. The library is a derived view over `Block` rows of `typename = 'ImageBlock'`; **no new media/asset domain model is introduced**. + +Two scope variants are planned in parallel and gated by a feature flag so the same client code can ship either: +- **Approach A — Personal:** library shows ImageBlocks from journeys the user owns or edits. +- **Approach B — Personal + Active Team (recommended target):** library shows ImageBlocks from any journey in the user's currently-active team. + +Recommended path: build the UI once, ship the resolver dual-scoped behind a server flag (`scope: PERSONAL | ACTIVE_TEAM`). Default to `ACTIVE_TEAM` in staging, validate, then in production. If team-scoped query work runs long, ship `PERSONAL` first and flip without touching the client. + +## Problem Statement + +Today the image picker offers Custom (file/URL upload), AI (prompt-to-image), and Unsplash. Creators have no way to pick from images they've already produced. They re-upload identical files (wasted storage, wasted time) and re-prompt the AI to recreate images they already have (wasted budget, drift in output). Teams suffer doubly: a designer uploads a logo to one journey, a colleague uploads the same logo on theirs because they can't see the first one. + +The constraint that shapes this plan: **deletion is forbidden in v1** — removing a library entry would orphan images on existing journeys that still reference them (see origin: `docs/brainstorms/2026-04-28-editor-asset-library-requirements.md` Key Decisions). + +## Proposed Solution + +### Frontend +A new `` + `` inserted into `ImageBlockEditor.tsx` between the existing tabs, mirroring the `UnsplashGallery` structural pattern. New component lives at `apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/ImageLibraryGallery/` (the name `ImageLibrary` is taken by an unrelated wrapper component — see the Naming section in System-Wide Impact). + +The tab calls a new GraphQL query to list deduplicated `ImageBlock` summaries, ordered by `updatedAt DESC`, paginated as a Relay-style cursor connection. Selecting a tile calls the editor's existing `handleSrcChange` → `onChange(input)` prop with `{ src, alt, width, height, blurhash, scale, focalLeft, focalTop }` — the same path Unsplash and AI use today. **No new client mutations.** + +### Backend +A new resolver `imageBlocksConnection(first, after, scope)` in **`apis/api-journeys`** (legacy NestJS + SDL — `visitorsConnection` lives here, all `ImageBlock` mutations live here, `AppCaslGuard` and `@CaslAccessible` decorators only exist here). Server-side filters: +- `typename = 'ImageBlock' AND src IS NOT NULL AND deletedAt IS NULL` +- Journey scope (single explicit filter, not AND-ed with CASL): + - `PERSONAL`: `journey.userJourneys.some({ userId: me, role: { in: [owner, editor] } })` + - `ACTIVE_TEAM`: `journey.teamId = derivedActiveTeamId` AND **explicit precheck** that `userTeam.findFirst({ teamId: derivedActiveTeamId, userId: me })` is non-null. `derivedActiveTeamId` is read from `JourneyProfile.lastActiveTeamId` server-side; no client-supplied `activeTeamId` argument exists. The membership precheck short-circuits to an empty connection when the user is not a current member of their stored active team (e.g., they were just removed). +- The resolver is still gated by `@UseGuards(AppCaslGuard)` — CASL ensures only authenticated users hit the resolver and provides a sanity check, but the where-input is *not* `AND`-ed with `accessibleBy('Journey')`. Reasoning: for non-publisher users the explicit scope filter is a strict subset of what CASL allows; AND-ing changes nothing. For the `publisher` role specifically, CASL grants `Read` on every published-template journey across all teams (`journey.acl.ts:83-86, :144`) — so adding a scope filter here is necessary for `ACTIVE_TEAM` correctness. Encoded as: scope filter authoritative; CASL guard for auth gate only. +- **Include** journeys with `archivedAt`, `deletedAt`, `trashedAt` set — past work is exactly when reuse matters most (R7). + +**De-duplication strategy (v1, simplified):** Use Prisma `findMany` ordered by `updatedAt DESC` with `take: first * 3` (over-fetch to absorb dedup loss), dedupe in resolver memory keyed by `src` preserving the first occurrence (newest), slice to `first + 1` for `hasNextPage`, encode the cursor as `(updatedAt, id)`. The over-fetch factor of 3 is heuristic — tunable. For v1 library sizes (low thousands per team), this is well under p95 budget. The client `Map` keyed on `src` is kept as a cross-page safety net. **Raw SQL `DISTINCT ON` is deferred** to a v1.x optimization if EXPLAIN ANALYZE on a seeded 50k-block dataset misses the latency target. + +## Technical Approach + +### Architecture + +#### Where the resolver lives +**Decision: legacy `apis/api-journeys` (NestJS + SDL).** Reasoning surfaced during deepening: +- `visitorsConnection` (the Relay connection template the plan mirrors) lives in legacy. +- All `ImageBlock` mutations (`imageBlockCreate`, `imageBlockUpdate`) live in legacy at `apis/api-journeys/src/app/modules/block/image/image.resolver.ts`. +- `apis/api-journeys-modern` (Pothos) only defines the `ImageBlock` *type*; it has no `prismaConnection`/`prismaConnectionHelpers` usage anywhere, no `AppCaslGuard`, no `@CaslAccessible` decorator. Hand-rolled `journeyAcl(Action.X, journey, user)` is the modern auth idiom. +- Building the connection in modern would require porting/inventing all of those pieces. Not v1. + +#### Schema (using existing types) + +```graphql +"""Scope for the image library query.""" +enum ImageBlockLibraryScope { + PERSONAL # journeys the user owns or edits + ACTIVE_TEAM # journeys in the user's currently-active team (server-derived) +} + +type ImageBlockLibraryEdge { + cursor: String! + node: ImageBlock! + """Provider-specific thumbnail variant of node.src.""" + thumbnailSrc: String! +} + +type ImageBlockLibraryConnection { + edges: [ImageBlockLibraryEdge!]! + pageInfo: PageInfo! +} + +extend type Query { + imageBlocksConnection( + first: Int = 24 + after: String + scope: ImageBlockLibraryScope! + ): ImageBlockLibraryConnection! +} +``` + +Notes on the schema shape (changes from v1 of the plan): +- **No `ImageBlockLibraryEntry`**. Edges expose existing `ImageBlock` directly. Reuses the existing `ImageBlockFields` codegen fragment, eliminates a parallel TS type, prevents Apollo cache normalization collisions on shared `ImageBlock` ids. +- **No `activeTeamId` argument**. Server derives from `JourneyProfile.lastActiveTeamId`. Removes a forgeable client input. +- **`thumbnailSrc` on the edge** (not on `ImageBlock`) so it's contextual to the library use case and doesn't pollute the canonical type. + +Pattern reference: `apis/api-journeys/src/app/modules/visitor/visitor.graphql:153–220` and `visitor.resolver.ts:37–42`. + +#### Prisma query (v1 — simplified, no raw SQL) + +```ts +// PSEUDO — actual resolver lives in NestJS + Prisma +const overFetch = first * 3 + 1; +const rows = await prisma.block.findMany({ + where: { + typename: 'ImageBlock', + src: { not: null }, + deletedAt: null, + journey: scopeFilter, // see Authorization below + }, + orderBy: [{ updatedAt: 'desc' }, { id: 'desc' }], + take: overFetch, + // cursor-based pagination on (updatedAt, id) — both indexed + ...(after ? { cursor: { id: decodeCursor(after).id }, skip: 1 } : {}), +}); + +// In-memory dedup by src, preserve first (newest by orderBy) +const seen = new Set(); +const deduped: typeof rows = []; +for (const r of rows) { + if (r.src && !seen.has(r.src)) { seen.add(r.src); deduped.push(r); } + if (deduped.length === first + 1) break; +} +const hasNextPage = deduped.length > first; +const slice = deduped.slice(0, first); +``` + +Cursor encoding: base64url JSON `{ v: 1, id, updatedAt }`. Decoded with a zod schema (`z.object({ v: z.literal(1), id: z.string(), updatedAt: z.string().datetime() })`) — malformed cursors throw `GraphQLError('Invalid cursor')` rather than crashing the resolver. The `v` byte enables non-breaking schema evolution. + +**Why this simplified path is correct for v1:** +- For team libraries up to ~few thousand ImageBlocks, the over-fetch + in-memory dedup completes in single-digit ms after the indexed query. +- Trades a marginal worst-case (over-fetch wastes a row read per duplicate) for orders-of-magnitude less complexity vs. raw SQL `DISTINCT ON` + seek cursor on a compound key. +- Cross-page duplicates are still possible if a duplicate `src` straddles a page boundary; the client `Map` (R15) absorbs them. Severity is cosmetic. +- If EXPLAIN ANALYZE on a seeded 50k-block dataset misses the latency target, swap in the raw SQL `DISTINCT ON (src)` CTE pattern (recommended by deepening research as Pattern B: CTE + keyset seek). Captured as a v1.x optimization. + +#### Index DDL (must ship with the resolver) + +```sql +-- Migration: add to libs/prisma/journeys +CREATE INDEX CONCURRENTLY IF NOT EXISTS block_imageblock_library_idx + ON "Block" ("journeyId", "src", "updatedAt" DESC) + WHERE typename = 'ImageBlock' AND "src" IS NOT NULL AND "deletedAt" IS NULL; +``` + +Partial composite index. The partial predicate is essential — `typename='ImageBlock'` is a small fraction of `Block`, and the `WHERE` matches the resolver's filter exactly so the planner uses an index-only scan. Verify with `EXPLAIN ANALYZE` against a seeded 50k+ block dataset before merge. + +Also confirm a `Journey(teamId)` index exists; if not, add one in the same migration. + +#### Authorization + +```ts +@Resolver() +@UseGuards(AppCaslGuard) // auth gate; not used for where-filtering +class ImageBlockLibraryResolver { + @Query(() => ImageBlockLibraryConnection) + async imageBlocksConnection( + @CurrentUserId() userId: string, + @Args('scope') scope: ImageBlockLibraryScope, + @Args('first', { defaultValue: 24 }) first: number, + @Args('after', { nullable: true }) after?: string, + ) { + let scopeFilter: Prisma.JourneyWhereInput; + + if (scope === 'PERSONAL') { + scopeFilter = { + userJourneys: { some: { userId, role: { in: ['owner', 'editor'] } } }, + }; + } else { + // ACTIVE_TEAM — derive teamId server-side, then verify membership + const profile = await this.prisma.journeyProfile.findUnique({ + where: { userId }, + select: { lastActiveTeamId: true }, + }); + const teamId = profile?.lastActiveTeamId; + if (!teamId) return emptyConnection(); // R10: fall back to empty (or PERSONAL — see Q3) + + const isMember = await this.prisma.userTeam.findFirst({ + where: { teamId, userId }, + select: { id: true }, + }); + if (!isMember) return emptyConnection(); // R13 enforcement at resolver entry + + scopeFilter = { teamId }; + } + + // ... findMany with where: { ..., journey: scopeFilter } + } +} +``` + +Decisions encoded: +- **Scope filter is authoritative** for where-filtering. CASL guard handles auth-gate only. +- **`activeTeamId` is server-derived** (`JourneyProfile.lastActiveTeamId`). No client argument. +- **Membership precheck** short-circuits to empty connection — matches R13 ("user removed from team T returns 0 images") at the resolver entry, also closes the timing-side-channel. +- **Publisher-role cross-team test required** (see Acceptance Criteria R20): a publisher reading `ACTIVE_TEAM` must not see template ImageBlocks from teams they're not a member of, because the membership precheck blocks the resolver before the scope filter is even applied. + +#### Client query and cache strategy + +```ts +// apps/journeys-admin/src/libs/useImageLibraryQuery/ +import { relayStylePagination } from '@apollo/client/utilities'; + +// In Apollo cache config (centralised): +typePolicies: { + Query: { + fields: { + imageBlocksConnection: relayStylePagination(['scope']), + }, + }, + ImageBlock: { + // Avoid normalisation collision with the canonical ImageBlock cached + // by the editor's other queries — library entries are read-only views. + keyFields: false, + }, +} + +// Hook: +const { activeTeam, loading: teamLoading } = useTeam(); + +const { data, fetchMore, networkStatus, refetch } = useQuery( + ImageBlocksConnectionDocument, + { + variables: { first: 24, scope: /* from feature flag */ }, + fetchPolicy: 'cache-first', // NOT cache-and-network + skip: teamLoading, + notifyOnNetworkStatusChange: true, // required for NetworkStatus.fetchMore + }, +); + +const isFetchingMore = networkStatus === NetworkStatus.fetchMore; +const { endCursor, hasNextPage } = data?.imageBlocksConnection.pageInfo ?? {}; +``` + +Mutation eviction (single source of truth for refresh): + +```ts +const [createImageBlock] = useJourneyImageBlockCreateMutation({ + update(cache) { + cache.evict({ fieldName: 'imageBlocksConnection' }); + cache.gc(); + }, +}); +// Same wrapper added to useJourneyImageBlockUpdateMutation. +``` + +On `activeTeam.id` change in the same session, also evict — Apollo will re-fetch on next read because the field is empty: + +```ts +useEffect(() => { + client.cache.evict({ fieldName: 'imageBlocksConnection' }); + client.cache.gc(); +}, [activeTeam?.id]); +``` + +Why this differs from v1 of the plan: +- **`cache-first`, not `cache-and-network`.** With single-source-of-truth mutation eviction, every drawer open is either a cache hit (fresh) or a fresh fetch (just evicted). `cache-and-network` would double-pay every open. +- **`relayStylePagination(['scope'])` as the field policy** — handles edge merging, cursor stability, dedup-by-cursor automatically. Source: https://www.apollographql.com/docs/react/pagination/cursor-based#relay-style-cursor-pagination +- **`ImageBlock.keyFields: false`** prevents the library's `ImageBlock` nodes from sharing cache identity with the editor's canonical `ImageBlock` queries (where they would otherwise overwrite richer field selections). +- **Concurrent `fetchMore` guard** via `notifyOnNetworkStatusChange: true` + `NetworkStatus.fetchMore` (R14). + +#### Frontend file layout + +``` +apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/ + ImageBlockEditor.tsx # add 4th tab, beacon param, extend TabParams + ImageLibraryGallery/ + ImageLibraryGallery.tsx # tab body, useImageLibraryQuery, dedup, render list + ImageLibraryGallery.spec.tsx + ImageLibraryGallery.stories.tsx + index.ts + ImageLibraryList/ + ImageLibraryList.tsx # grid renderer, click → onChange + ImageLibraryList.spec.tsx + ImageLibraryList.stories.tsx + ImageLibraryEmptyState/ + ImageLibraryEmptyState.tsx # 3 variants: never-uploaded, team-empty, error + ImageLibraryEmptyState.spec.tsx + +apps/journeys-admin/src/libs/useImageLibraryQuery/ + useImageLibraryQuery.ts + useImageLibraryQuery.spec.tsx + useImageLibraryQuery.mock.tsx + index.ts +``` + +### Implementation Phases + +#### Phase 1 — Backend resolver + schema (legacy api-journeys) + +- **1.1** Add migration with the partial composite index (DDL above) and confirm `Journey(teamId)` index. Run `EXPLAIN ANALYZE` against a seeded dataset (50k blocks across 100 journeys / 1 team). +- **1.2** Add SDL definitions: `ImageBlockLibraryScope` enum, `ImageBlockLibraryEdge`, `ImageBlockLibraryConnection`, and the `imageBlocksConnection` query. Edges expose existing `ImageBlock` and a new `thumbnailSrc: String!` field. +- **1.3** Implement resolver in `apis/api-journeys/src/app/modules/block/image/imageBlockLibrary.resolver.ts` (or extend `image.resolver.ts`). Vanilla Prisma `findMany` with over-fetch + in-memory dedup. Server-derived `lastActiveTeamId`. Membership precheck for `ACTIVE_TEAM`. Versioned cursor with zod validation on decode. +- **1.4** `thumbnailSrc` resolver: provider detection by URL prefix (`imagedelivery.net` → Cloudflare variant; `images.unsplash.com` → `&w=240&fit=crop`; AI provider URLs that already include sizing → passthrough; otherwise → passthrough with a documented bandwidth caveat). +- **1.5** Tests: + - Unit: dedup correctness (same `src` in 3 ImageBlocks → 1 entry, newest wins). + - Auth: user not on team T cannot fetch ACTIVE_TEAM images of T (membership precheck blocks). + - Auth: user removed from team mid-test → fetch returns 0. + - **Auth (publisher role): a publisher reading ACTIVE_TEAM must not see template ImageBlocks from a team they are not a member of.** Specifically guards the cross-team CASL leak vector. + - Auth (transfer): journey transferred from T1→T2; T1 members no longer see its ImageBlocks under `ACTIVE_TEAM=T1`; T2 members do under `ACTIVE_TEAM=T2`. Audit `userJourneys` cleanup on transfer for PERSONAL scope. + - Soft-delete: archived/deleted/trashed journeys' images are included; deleted ImageBlocks are excluded. + - Pagination: 60 unique `src`s, page size 24 → three pages, no duplicates across pages. + - Cursor: malformed cursor returns `Invalid cursor` GraphQLError, not 500. +- **1.6** Codegen: run apollo codegen to produce TS types in journeys-admin. + +**Deliverables:** new query merged behind a feature flag (no client UI yet). +**Success criteria:** integration test green; auth tests green; query callable from GraphQL Playground. +**Estimated effort:** 2–3 days (Approach A and B share the same query; the scope arg differentiates them). + +#### Phase 2 — Frontend Library tab (UI shell) + +- **2.1** Create `ImageLibraryGallery/` skeleton with `next/dynamic` import in `ImageBlockEditor.tsx`. Add 4th tab, extend `TabParams`, emit `setBeaconPageViewed('library-image')`. Decide tab position (Outstanding Q2; recommend index 0 once stable, after empty-state polish). +- **2.2** Switch the `` in `ImageBlockEditor.tsx` from `variant="fullWidth"` to **`variant="scrollable" scrollButtons="auto" allowScrollButtonsMobile`** unconditionally — the drawer is always narrow regardless of viewport, and 4 fullWidth tabs at ~360px truncate i18n labels (German "Bibliothek" doesn't fit). Add `aria-label="Image source"` on the Tabs container. Pair each `` with `iconPosition="top"` and a small icon for visual scanning at narrow widths. +- **2.3** Wire `useImageLibraryQuery` with `cache-first` policy, `notifyOnNetworkStatusChange: true`, `skip: useTeam().loading`, and a `useEffect` that evicts the connection field when `activeTeam.id` changes. +- **2.4** Render grid using the same image-tile aesthetic as `UnsplashList.tsx:60–90`. **Use `edge.thumbnailSrc` (not `edge.node.src`) for the `` source** — full-size images would tank a 24-tile grid. Add `loading="lazy"` and `decoding="async"` to every tile img. +- **2.5** Selection highlight: `selectedBlock?.src === edge.node.src` → outlined state. Clicking the currently-applied tile is a no-op via `handleSrcChange`'s existing short-circuit — accept that rather than introducing the `Current` badge / non-clickable state (R12 dropped per simplicity review). +- **2.6** Client-side dedup safety net: accumulate edges in a `Map` (in `useRef` inside `useImageLibraryQuery`) keyed by `src`, reset in a `useEffect` on `[scope, activeTeam?.id]`. Cap at 500 entries — when reached, disable "Load More". Add a test asserting `Map.size <= 500` after 30 paginations. +- **2.7** Concurrent `fetchMore` guard: disable the "Load More" button while `networkStatus === NetworkStatus.fetchMore`. +- **2.8** Tests + stories. Stories must include: empty, loading, error, populated, populated-with-current-selected, end-of-list, error-on-load-more. + +**Deliverables:** Library tab functional with team scope behind a flag; selection applies images correctly. +**Success criteria:** R1–R6 pass acceptance tests; manual smoke test in Editor. +**Estimated effort:** 2–3 days. + +#### Phase 3 — States, pagination, polish + +- **3.1** Empty states (3 variants per SpecFlow #1, #3, #4): never-uploaded, team-with-no-uploads, error. All translated. +- **3.2** Loading skeleton (6–12 placeholder tiles matching grid template). +- **3.3** Error state with `Try again` button → `refetch()`. +- **3.4** "Load More" button with concurrent-fetch guard (SpecFlow #17). Terminal sentinel "End of library" when `hasNextPage === false` (SpecFlow #16). +- **3.5** `` handler hides broken tiles (Cloudflare/AI URL purge — SpecFlow #12). +- **3.6** A11y: `role="listbox"`, `role="option"` per tile, roving tabindex, descriptive aria-label, focus-on-tab-activate (SpecFlow #27). +- **3.7** Translation strings extracted via `i18next-parser`. Verify `apps-journeys-admin` namespace. +- **3.8** Mutation cache invalidation: `useJourneyImageBlockCreateMutation` / `useJourneyImageBlockUpdateMutation` evict `imageBlocksConnection` on success. +- **3.9** Active-team-null fallback for Approach B (Outstanding Q3 — recommend: fall back to `PERSONAL` scope client-side when `activeTeam == null && !loading`). + +**Deliverables:** production-ready Library tab. +**Success criteria:** all acceptance criteria pass; manual cross-browser + mobile smoke; analytics fires. +**Estimated effort:** 2 days. + +#### Phase 4 — Rollout + +**Prerequisite:** confirm feature-flag infrastructure exists in this monorepo. Architecture review found no GrowthBook/Unleash/`isFeatureEnabled` pattern; if absent, dual-scope rollout is not viable as planned. Two paths: +- **Path A (flag infra exists or is added):** as below. +- **Path B (no flag infra; pragmatic):** ship `ACTIVE_TEAM`-only — that's the actual product value driver per the brainstorm. Drop the `scope` arg or hard-default it. Saves all flag work. Origin doc's "personal vs personal+team" comparison resolves in favor of team for the same reason. + +This plan defaults to Path A and surfaces Path B as Outstanding Q7. + +- **4.1** Server feature flag `imageLibraryScope` defaulting to `PERSONAL`. +- **4.2** Stage with `ACTIVE_TEAM` enabled; QA cross-team auth test plan from Phase 1 manually. +- **4.3** Production: enable for internal teams first, observe analytics (Library-tab applies vs. Custom-tab uploads ratio — see Success Metrics). +- **4.4** Roll to all teams. +- **4.5** Document v2 candidates: search, hide/delete, multi-team filter, video assets, raw SQL `DISTINCT ON` optimization if EXPLAIN ANALYZE shows the v1 over-fetch path is too slow. + +**Deliverables:** GA. +**Success criteria:** non-zero Library applies within first week; no auth incidents. +**Estimated effort:** 1 day of in-product work + flag-flip cycles. + +## Alternative Approaches Considered + +These come from the origin doc and are preserved here for context. + +1. **Standalone library management page** — rejected for v1 in origin; UI tab inside the existing picker has lower friction and lower scope. +2. **Opt-in "Save to library" action** — rejected; auto-save matches the "don't re-upload" goal exactly. Already implicitly happens since uploads create ImageBlocks. +3. **New server-side asset model (`MediaAsset`)** — rejected; every uploaded/generated image already creates an `ImageBlock` immediately. A separate model adds backfill, sync, and complexity without product value for v1. +4. **All teams the user belongs to** — rejected for B; matches journey scoping and avoids cross-org bleed. +5. **Approach A first, B later (phased)** — still a viable fallback if Phase 1 effort balloons. The pragmatic flag-based path subsumes this — code is the same, scope flips at the resolver. + +## System-Wide Impact + +### Interaction Graph + +When a creator clicks a Library tile: +1. `ImageLibraryList` `onClick(item)` fires. +2. → `ImageLibraryGallery` calls `props.onChange({ src, alt, width, height, blurhash, scale, focalLeft, focalTop })`. +3. → `ImageBlockEditor.handleSrcChange` short-circuits if `src === selectedBlock.src` (no-op), otherwise validates `src`, defaults `alt` from filename, calls parent `onChange`. +4. → Parent (e.g. `BackgroundMediaImage.handleChange`) decides create-vs-update: + - No existing `coverBlock` → `useJourneyImageBlockCreateMutation` → server inserts `Block`, then `journeyUpdate` to wire the cover association. + - Existing `coverBlock` → `useJourneyImageBlockUpdateMutation` → server updates `Block.src` etc., bumping `updatedAt`. +5. → Mutation `update()` evicts `imageBlocksConnection` → next library open shows the just-applied image first. +6. → Editor canvas re-renders the new image; preview iframe (Journeys preview) re-renders via existing block subscription. +7. → Beacon event for tab view fires on the next tab switch. + +### Error & Failure Propagation + +- **Resolver query failure** (DB error): Apollo error returned; client shows error state with retry. No partial UI corruption. +- **Auth failure**: `@CaslAccessible('Journey')` returns empty journey set; resolver returns empty connection. User sees the "team has no images" empty state — non-fatal, slightly misleading. Acceptable; auth failure on Journey read is already tested elsewhere. +- **Cloudflare/Unsplash URL 404 at render**: `` hides the broken tile. Other tiles unaffected. If user already applied that image to a journey before it 404'd, that journey is broken regardless — pre-existing behavior, not regressed. +- **Apply mutation failure**: existing path's error handling applies (snackbar, no canvas mutation). Library tab does not introduce new error paths. +- **Cache eviction race** (mutation evicts cache while a `fetchMore` is in-flight): Apollo handles by treating evicted cache as miss; the `fetchMore` resolves with the latest server state. No corruption. + +### State Lifecycle Risks + +- **Picking an image does not create new persistent state in Library** — it creates/updates an `ImageBlock` on a journey via existing mutations. No orphan risk introduced. +- **Cross-page client dedup state** lives only inside the `useImageLibraryQuery` consumer. Resets on tab close (component unmount). No persistent state to leak. +- **Apollo cache for `imageBlocksConnection`** can grow unbounded if user paginates through 1000s of tiles in a session. Mitigate by capping client accumulation at 500 entries (configurable). Memory budget: ~50KB. + +### API Surface Parity + +- `imageBlocksConnection` is a new query — no existing surface to keep parity with. The resolver intentionally mirrors `visitorsConnection` style for codebase consistency. +- The client `onChange` contract is shared by Custom, Unsplash, AI tabs. Library follows the same shape — no parity work needed. +- No mobile-app surface (this is admin-only). + +### Integration Test Scenarios + +1. **Cross-tab freshness**: Upload an image via Custom tab → switch to Library tab → expect the just-uploaded image as the first tile (mutation cache invalidation works end-to-end). +2. **Apply-from-library on empty slot**: Open card with no cover → Library → click → expect `ImageBlock` created and associated; reopen card → cover renders. +3. **Apply-from-library on populated slot**: Open card with an existing cover → Library → click a different image → expect `Block.src` updated in place (no new id); preview re-renders. +4. **Cross-team auth (B)**: User U on teams T1+T2 with images on both. `activeTeam = T1` → Library returns only T1 images. Switch to T2 → Library refetches with T2 images, no T1 leak. Remove U from T2 → Library returns 0 with `activeTeam = T2`. +5. **Soft-delete inclusion**: Seed 3 ImageBlocks: one on active journey, one on archived journey, one on trashed journey. All 3 appear. Hard-delete an ImageBlock (sets `Block.deletedAt`) → it disappears from the next fetch. +6. **De-dup race**: Seed 5 ImageBlocks with same `src` across 5 journeys; assert exactly 1 tile rendered. Insert a 6th with the same `src` between page 1 fetch and page 2 fetch → assert no visible duplicate (client `Map` dedup catches it). + +## Acceptance Criteria + +### Functional Requirements + +- [ ] **R1** A "Library" tab is visible in `ImageBlockEditor` alongside Custom, AI, Unsplash. Test: render `ImageBlockEditor` with mocks, assert 4 `Tab` elements with the expected labels. +- [ ] **R2** Library shows ImageBlocks reverse-chronologically with cursor pagination. Test: seed 50 entries, assert order matches `updatedAt DESC`; assert "Load More" triggers next page. +- [ ] **R3** Selecting an image calls `onChange({ src, alt, width, height, blurhash, scale, focalLeft, focalTop })` matching the source ImageBlock. Test: click tile, assert mock `onChange` payload. +- [ ] **R4** All four sources (Cloudflare upload, URL paste, AI-generated, Unsplash) appear when ever placed in an `ImageBlock`. Test: seed one of each, assert all visible. +- [ ] **R5** No "Save to library" UI control exists; uploads/generations auto-appear after apply (cache invalidation). Test: assert no save button; integration test: apply via Custom tab → switch to Library → first tile is the new image. +- [ ] **R6** De-dup by `src`. Test: seed two ImageBlocks with identical `src`, assert one tile. +- [ ] **R7** Includes images from archived/deleted/trashed journeys. Test: seed each, assert all visible. +- [ ] **R8** No delete, hide, rename, or label affordance on tiles. Test: assert no per-tile menu rendered. + +### New Acceptance Criteria from Flow Analysis & Deepening + +- [ ] **R9** Empty states render distinct copy for: no-images-anywhere/team-with-no-images (collapsed per simplicity review — same UX in v1 since team scope means an empty team library reads the same as "no images") and fetch-error. Two variants total, not three. +- [ ] **R10** When `JourneyProfile.lastActiveTeamId` is null in `ACTIVE_TEAM` scope, resolver returns an empty connection. Client may *optionally* render a hint "Set an active team to see shared images" — leave to design. +- [ ] **R11** When `activeTeam.id` changes client-side, the connection field is evicted and re-fetched from cursor null; accumulated client `Map` is reset. +- [ ] ~~**R12**~~ *(removed: existing selection-highlight via `selectedBlock?.src === edge.node.src` covers this; `handleSrcChange` already short-circuits on identical src; no Current badge needed)* +- [ ] **R13** Cross-team auth test passes: user removed from team T → resolver returns empty connection for `scope=ACTIVE_TEAM` (membership precheck). +- [ ] **R14** Concurrent "Load More" clicks are guarded — only one in-flight fetch at a time (via `NetworkStatus.fetchMore`). +- [ ] **R15** Client-side dedup `Map` absorbs cross-page race duplicates and is capped at 500 entries; "Load More" disables on cap. +- [ ] **R16** A11y: grid is keyboard-navigable; tiles have descriptive aria-labels. +- [ ] **R17** Tab strip uses `variant="scrollable"` with `iconPosition="top"`; remains usable at all drawer widths. +- [ ] **R18** Beacon event `library-image` fires on tab activation. +- [ ] **R19** Each edge exposes `thumbnailSrc` returning a provider-appropriate small variant; client renders `` with `loading="lazy"` + `decoding="async"`. Bandwidth per page-load < 2MB at 24 tiles. +- [ ] **R20** Publisher-role auth test: a user with `publisher` role reading `ACTIVE_TEAM` scope does not see template ImageBlocks from teams they are not a member of. +- [ ] **R21** Journey-transfer auth test: a journey moved from team T1 to T2 stops appearing in T1 members' libraries and starts appearing in T2 members'. PERSONAL scope: confirm `userJourneys` cleanup behavior on transfer. +- [ ] **R22** Cursor decode is zod-validated; malformed cursors return `Invalid cursor` GraphQLError (not 500). +- [ ] **R23** Apollo `ImageBlock` cache normalization for library results does not collide with the editor's canonical `ImageBlock` queries (`keyFields: false` typePolicy on the library's read path, or equivalent). + +### Non-Functional Requirements + +- [ ] Initial fetch p95 < 500ms with team library of 500 unique images. +- [ ] Pagination fetch p95 < 300ms. +- [ ] No N+1 queries — all data in one Prisma round-trip. +- [ ] Bundle impact for the new tab + hook < 8KB gzipped. + +### Quality Gates + +- [ ] Test coverage matches existing `UnsplashGallery` (≥80% line in new files). +- [ ] All new strings in `apps-journeys-admin` translation namespace. +- [ ] Stories rendered for: empty, loading, error, populated, populated-with-current-selected. +- [ ] Manual mobile drawer smoke (`xs` viewport). +- [ ] Storybook accessibility audit on new components (no new violations). + +## Success Metrics + +**Primary metric (lock in before launch — SpecFlow #33):** +- Ratio of Library-tab applies to Custom-tab uploads per session. Target: ≥30% within four weeks of GA. If ≥50%, declare clear success. + +**Secondary metrics:** +- Library tab open-rate (% of editor sessions that activate the Library tab). Target: ≥40% within four weeks. +- Re-upload rate: count of new `ImageBlock` rows where `src` already exists in the user's (or team's) prior ImageBlocks. Should drop measurably post-launch. +- AI re-prompt rate: heuristic match on similar prompts producing similar images. Stretch metric — instrument only if cheap. + +## Dependencies & Prerequisites + +- **D1** Active-team context via `useTeam()` is reliable inside the Editor's component tree (verified — `TeamProvider` is a tree ancestor). Server reads `JourneyProfile.lastActiveTeamId` independently. +- **D2** Soft-delete semantics confirmed: `Journey.archivedAt`, `Journey.deletedAt`, `Journey.trashedAt`, `Block.deletedAt` all soft-delete via timestamp; rows persist. R7 stands. +- **D3** GraphQL codegen pipeline picks up new types automatically (apollo.config.js present). +- **D4** Apollo cache eviction API works against the legacy API's connection field (verified by `visitorsConnection` patterns in legacy). +- **D5** Cloudflare/Unsplash/AI delivery URLs are stable enough that `src` equality is a reasonable dedup key — see Risks for the caveat. +- **D6** Postgres index migration tooling supports `CREATE INDEX CONCURRENTLY` in this monorepo's Prisma migration flow. If not, the migration must be applied out-of-band to avoid table locks on the production `Block` table. +- **D7** **Feature-flag infrastructure (open).** See Outstanding Q7. Phase 1 must verify or add this before Phase 4's dual-scope rollout works. +- **D8** **`userJourneys` cleanup on journey transfer (open).** See Outstanding Q8. Phase 1 verifies behavior; PERSONAL scope correctness depends on it. + +## Risk Analysis & Mitigation + +- **R-1 — Cross-team data leak via publisher role (Critical / Low likelihood).** CASL grants `publisher` users `Read` on every published-template journey across all teams (`journey.acl.ts:83-86, :144`). Without an explicit scope filter and membership precheck, a publisher's `ACTIVE_TEAM` library would surface template images from teams they don't belong to. Mitigation: explicit scope filter + membership precheck + dedicated publisher-role auth test (R20). +- **R-2 — Visible duplicates from `src` normalization gaps (Important / Medium likelihood).** Same Unsplash photo with different query strings, or AI image variants, look identical but have different `src`. Mitigation: accept some duplicates in v1; document as known limitation; optionally apply a normalization rule (strip query string for unsplash.com domain) if QA flags it as common. +- **R-3 — Cross-page dedup race (Important / Low likelihood).** Concurrent ImageBlock creation between page fetches can produce a duplicate `src` in the visible grid. Mitigation: client-side `Map`-based dedup (R15). +- **R-4 — Library tab UX in narrow drawer with 4 tabs (Medium / High likelihood).** Tab labels truncate. Mitigation: switch to scrollable tabs at `xs` if needed (R17). +- **R-5 — AI generations abandoned mid-flow are invisible to Library (Low / Low likelihood).** Documented as v1 limitation; rare and self-inflicted by the user. +- **R-6 — Library opens before `useTeam()` hydrates (Medium / Medium likelihood).** Approach B query runs with `activeTeamId = undefined` and either errors or returns empty. Mitigation: skip the query while `useTeam().loading`; fall back to PERSONAL when `activeTeam` is null (R10). +- **R-7 — Cloudflare object purged but ImageBlock row remains (Low / Low likelihood).** Library shows broken thumb. Mitigation: `` hides; pre-existing systemic issue, not regressed. +- **R-8 — Thumbnail bandwidth (Critical / High likelihood without mitigation).** A 24-tile grid loading full-size hero images can transfer 50–100MB per page on real-world Cloudflare assets. Mitigation: `thumbnailSrc` field on edges with provider-specific variants (Cloudflare `cdn-cgi/image/width=240,quality=80`, Unsplash query param, AI passthrough). R19 enforces this. Without it, mobile users on cellular suffer disproportionately and p95 latency is dominated by network, not query. +- **R-9 — Apollo cache identity collision on `ImageBlock` (Important / Medium likelihood).** The library's `ImageBlock` nodes share `__typename:id` with the editor's canonical `ImageBlock` queries. Default normalization could overwrite richer field selections in the editor's cache with the library's thinner projection — manifesting as missing fields elsewhere in the app. Mitigation: `ImageBlock.keyFields: false` typePolicy in the library's cache config (R23). If that turns out to disable normalization too broadly, fall back to a synthetic `__typename` like `LibraryImageBlock` for the library's read path. +- **R-10 — Feature-flag infrastructure absent (Important / Unknown likelihood).** Architecture review found no flag system in the monorepo. If true, Phase 4's dual-scope rollout strategy is non-functional. Mitigation: confirm in Phase 1; if absent, take Path B (ship `ACTIVE_TEAM` only). Addressed in Outstanding Q7. + +## Resource Requirements + +- 1 frontend engineer, ~5 days (Phases 2 + 3). +- 1 backend engineer, ~3 days (Phase 1). +- 1 designer review for empty states, currently-applied affordance, and tab strip layout (~2 hours). +- 1 product/analytics check to lock in success metric and verify the `library-image` beacon event is consumed by the analytics pipeline (~1 hour). +- Total wall-clock: ~1 week if frontend and backend run in parallel after Phase 1.2. + +## Future Considerations + +- **v2 candidates** (deliberately out of v1 per origin doc): + - Search by filename / AI prompt / tag. + - Hide entries (without deleting the underlying image — flag-only soft-hide). + - Multi-team filter (let the user opt to see assets from any team they belong to). + - Video assets and generic media model — only if a separate brief justifies it. + - Standalone library management page outside the Editor. +- **Server-side `MediaAsset` model**: re-evaluate when the v2 features above (especially hide/label) require fields the `Block` table can't carry without bloat. +- **AI prompt-as-label**: only meaningful with a `MediaAsset` model; deferred. + +## Documentation Plan + +- **No public docs** required for v1 — feature is in-product and self-explanatory. +- **CHANGELOG** entry: "Editor: pick from previously used images via the new Library tab." +- **Storybook** entries function as the component spec. +- **Origin and plan docs** (this file) are the durable record. +- **Outstanding question resolutions** are captured inline in this plan as they're answered, not in a separate doc. + +## Outstanding Questions + +(All deferred-to-planning questions from the origin doc are addressed inline above. Remaining open items, all resolvable in implementation:) + +- **Q1** ~~Modern vs. legacy API home~~ — **Resolved during deepening: legacy `apis/api-journeys`** (modern lacks Pothos connection patterns, CASL guard, and image-block mutations all live in legacy). +- **Q2** Position of the Library tab in the strip and whether it becomes the new default. Recommend: insert at index 0 once Phase 3 (empty states) is solid; keep Unsplash as default for the first week post-launch. +- **Q3** Behavior when `JourneyProfile.lastActiveTeamId == null` in `ACTIVE_TEAM` scope. Recommend: resolver returns empty (not silent fall-through to PERSONAL — different scopes are different intents). Client may render a small "set an active team" hint. +- **Q4** `src` normalization rule for Unsplash query strings — apply now or accept duplicates. Recommend: accept v1; revisit if QA flags >5% visible duplicates. +- **Q5** Whether the `library-image` beacon event name is consumed by the analytics pipeline before launch. Quick confirmation with analytics owner. +- **Q6** ~~Dual-scope vs. single-scope~~ — **Resolved: PERSONAL only for v1, ACTIVE_TEAM in v1.1.** +- **Q7** ~~Feature-flag infrastructure~~ — **Resolved: not required for v1** (single scope means no flag needed). Will be re-evaluated when v1.1 lands; if no flag infra exists by then, v1.1 ships team scope unconditionally. +- **Q8** `userJourneys` cleanup on journey transfer between teams — does it happen? Verify so PERSONAL scope doesn't quietly leak access to ImageBlocks on a journey transferred away from the user. **Now applies to v1** (PERSONAL is the v1 scope). R21 (PERSONAL portion) covers the test. +- **Q9** Whether to over-fetch by `first * 3` (current proposal) or compute the factor adaptively from observed dedup ratio per query. Recommend: fixed factor for v1, instrument the ratio, revisit if observed dedup density is high. + +## Sources & References + +### Origin + +- **Origin document:** [docs/brainstorms/2026-04-28-editor-asset-library-requirements.md](../brainstorms/2026-04-28-editor-asset-library-requirements.md) + - Key decisions carried forward: single picker tab (not standalone page); auto-saved via existing ImageBlock creation; no management actions in v1; de-dup by `src`; include archived/deleted journeys (R7); active-team-only scope for Approach B. + +### Internal References + +- `apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/ImageBlockEditor.tsx` — tab structure, `handleSrcChange` (line 90), `TabParams` (line 67), beacon emission (lines 73–75). +- `apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/UnsplashGallery/UnsplashGallery.tsx` — closest pattern to mirror (network-fetched grid, "Load More"). +- `apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/UnsplashGallery/UnsplashList/UnsplashList.tsx:40–90` — tile render + `onChange` payload shape. +- `apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/AIGallery/AIGallery.tsx:41–77` — `onChange` payload contract. +- `apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/Image/BackgroundMediaImage.tsx:337–345` — canonical create-vs-update wiring at the parent. +- `apps/journeys-admin/src/libs/useJourneyImageBlockCreateMutation/`, `.../useJourneyImageBlockUpdateMutation/` — mutation hooks; add cache eviction here. +- `libs/journeys/ui/src/components/TeamProvider/TeamProvider.tsx` — `useTeam()` source of `activeTeam.id`. +- `apis/api-journeys/src/app/modules/journey/journey.resolver.ts:132–180` — pagination + CASL idiom to replicate. +- `apis/api-journeys/src/app/modules/journey/journey.acl.ts` — `INCLUDE_JOURNEY_ACL`, ability matchers. +- `apis/api-journeys/src/app/modules/visitor/visitor.graphql:153–220` and `visitor.resolver.ts:37–42` — Relay connection template. +- `apis/api-journeys-modern/src/schema/block/image/image.ts` — modern Pothos definition of `ImageBlock`. +- `libs/prisma/journeys/db/schema.prisma` — `Block` (line 536), `Journey` (line 406), `UserJourney` (line 352), `UserTeam` (line 307), `JourneyProfile.lastActiveTeamId` (line 493). + +### External References + +- Apollo Client cache eviction: https://www.apollographql.com/docs/react/caching/garbage-collection +- Relay-style cursor connection spec: https://relay.dev/graphql/connections.htm + +### Related Work + +- No prior PRs or feature attempts for an asset library / media picker found in the codebase. +- No `docs/solutions/` entries — institutional knowledge gap to seed post-launch (`/ce:compound`). From 4254eaf198a974e0cb49a82b85b01b9a25fa1614 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 04:14:52 +0000 Subject: [PATCH 2/5] fix: lint issues --- ...4-28-001-feat-editor-asset-library-plan.md | 169 ++++++++++-------- 1 file changed, 94 insertions(+), 75 deletions(-) diff --git a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md index 41eaa40697e..e54946ae486 100644 --- a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md +++ b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md @@ -18,18 +18,18 @@ revised: 2026-05-01 A prototype pass surfaced a fundamental issue with the original `Block`-table-as-source-of-truth premise: **`Block.src` is overwritten in place** by `imageBlockUpdate` when a user picks a new image on the same card. The first cover creates a `Block` row; every subsequent pick on that card mutates that same row's `src`. Confirmed in `apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Card/BackgroundMedia/Image/BackgroundMediaImage.tsx:221-263`. -Consequence: a Library backed by `Block` rows can only show *currently-attached* images. The exact scenario the feature is meant to solve — "I uploaded this last week, replaced it, now I want it back" — is unreachable, because the prior `src` is gone. +Consequence: a Library backed by `Block` rows can only show _currently-attached_ images. The exact scenario the feature is meant to solve — "I uploaded this last week, replaced it, now I want it back" — is unreachable, because the prior `src` is gone. ### New approach: per-tab history backed by existing `CloudflareImage` table Each existing tab in `ImageBlockEditor` gains its own history surface, fed by its own persistence layer: -| Tab | History surface | Backing data | Survives Block.src overwrite? | -|-----|-----------------|--------------|------------------------------| -| **Custom** | "Your uploads" grid above the file/URL inputs | `CloudflareImage` (media DB), `userId`-scoped, populated by `createCloudflareUploadByFile` and `createCloudflareUploadByUrl` | ✅ Independent table | -| **AI** | "Your generations" grid above the prompt input | Same `CloudflareImage` (populated by `createImageBySegmindPrompt` / `createCloudflareImageFromPrompt`) | ✅ Independent table | -| **Unsplash** | Deferred. No local persistence today; `triggerUnsplashDownload` only calls Unsplash's tracking API. Picking from Unsplash's existing search/browse remains the path. | — | — | -| **Standalone Library tab** | **Dropped.** Per-tab history covers ~90% of the user's mental model ("I uploaded that" → check Custom; "I generated that" → check AI). Cross-source unification adds little once each tab shows its own history. | — | — | +| Tab | History surface | Backing data | Survives Block.src overwrite? | +| -------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | ----------------------------- | +| **Custom** | "Your uploads" grid above the file/URL inputs | `CloudflareImage` (media DB), `userId`-scoped, populated by `createCloudflareUploadByFile` and `createCloudflareUploadByUrl` | ✅ Independent table | +| **AI** | "Your generations" grid above the prompt input | Same `CloudflareImage` (populated by `createImageBySegmindPrompt` / `createCloudflareImageFromPrompt`) | ✅ Independent table | +| **Unsplash** | Deferred. No local persistence today; `triggerUnsplashDownload` only calls Unsplash's tracking API. Picking from Unsplash's existing search/browse remains the path. | — | — | +| **Standalone Library tab** | **Dropped.** Per-tab history covers ~90% of the user's mental model ("I uploaded that" → check Custom; "I generated that" → check AI). Cross-source unification adds little once each tab shows its own history. | — | — | ### What's already in place to leverage @@ -39,6 +39,7 @@ Each existing tab in `ImageBlockEditor` gains its own history surface, fed by it ### Open question rolled into this pivot `CloudflareImage` has no `source` enum (`file | url | ai`) or `prompt` column. Two paths: + - **Cheap:** show all `CloudflareImage` rows in both Custom and AI tabs, accept the overlap. - **Right:** add `source` enum and (for AI) `prompt` column to `CloudflareImage`. Mutations populate them. Each tab filters accordingly. @@ -47,7 +48,7 @@ For prototype: take the cheap path. Promote to "right" once the UX is validated. ### Why this is better product - **No new architecture required.** No `MediaAsset` table, no Block-mutation rewiring, no orphan-Block strategy. -- **Matches the user's mental model.** People remember *how* they got an image (uploaded vs generated vs picked), and they're already on that tab when they want to find it again. +- **Matches the user's mental model.** People remember _how_ they got an image (uploaded vs generated vs picked), and they're already on that tab when they want to find it again. - **Skips the Unsplash gap.** Unsplash already provides search/browse; "previously picked Unsplash images" is the weakest of the three use cases and can ship later via a small `UnsplashSelection` table if metrics justify it. - **Removes the deletion-orphan worry entirely** — `CloudflareImage` rows are independent of `Block`, so deletes don't cascade strangely. @@ -56,6 +57,7 @@ For prototype: take the cheap path. Promote to "right" once the UX is validated. The original plan (Frontend, Backend, Technical Approach, Acceptance Criteria, etc.) was built around the rejected `Block`-table approach. **Treat them as historical context for the decision trail, not the v1 spec.** New acceptance criteria, file layout, and phasing for the per-tab approach are TODO and will be drafted before implementation resumes. The revised v1 work is roughly: + 1. Add a "Your uploads" grid component to the Custom tab, backed by `getMyCloudflareImages`. 2. Add a "Your generations" grid component to the AI tab, backed by the same query (or a filtered variant once `source` is added to `CloudflareImage`). 3. Defer Unsplash history. @@ -70,6 +72,7 @@ The revised v1 work is roughly: Implementation against the 2026-04-28 pivot landed in PR #9102. Production state: **Image picker (`ImageBlockEditor`):** + - "Your uploads" grid in Custom tab. - "Your generations" grid in AI tab. - Backed by existing `getMyCloudflareImages(offset, limit, isAi): [CloudflareImage!]!` — additive `isAi` arg only, no schema-breaking change. @@ -77,6 +80,7 @@ Implementation against the 2026-04-28 pivot landed in PR #9102. Production state - Frontend: offset/limit pagination with `length >= PAGE_SIZE` heuristic for "Load More". `offsetLimitPagination(['isAi'])` cache config. **Video picker (`VideoFromMux`):** + - "Your uploads" grid below the Mux upload form. - Backed by existing `getMyMuxVideos(offset, limit): [MuxVideo!]!` — no schema change. Resolver only added a deterministic `orderBy [createdAt desc, id desc]` (correctness for offset pagination, not a UI preference). - Click-a-tile opens existing `VideoDetails` + `MuxDetails` with a Select button. Gallery flow reuses the same preview infrastructure as the active-block flow; `playbackId` is threaded through to avoid a redundant fetch. `MuxDetails.dispose()` fix shipped alongside. @@ -111,7 +115,7 @@ v1 is currently merged but not yet generally available. Before going live we gat ### v1.1 — team-shared visibility (replaces the ACTIVE_TEAM material in original Backend / Architecture / Implementation Phases sections) -Goal: the same per-tab grids surface assets uploaded by *teammates* in the user's active team, alongside their own. +Goal: the same per-tab grids surface assets uploaded by _teammates_ in the user's active team, alongside their own. #### Locked decisions @@ -158,6 +162,7 @@ Each mutation that creates a `CloudflareImage` or `MuxVideo` row gains a require 3. Persist the new row with `userId = caller` and `teamId = journey.teamId`. Mutations affected (approximate — finalize during BE-2 scoping): + - `createCloudflareUploadByFile` - `createCloudflareUploadByUrl` - `createImageBySegmindPrompt` / `createCloudflareImageFromPrompt` @@ -198,14 +203,14 @@ Both resolvers' implementation: #### Behavioral table -| Scenario | "Your uploads" (caller) | "Team uploads" (Team X) | -|---|---|---| -| Caller uploads inside a Team X journey | Visible (`userId = caller`) | Visible (`teamId = X`) | -| Caller leaves Team X | Still visible (`userId = caller`) | **Still visible to remaining Team X members** | -| Caller later joins Team Y, uploads inside a Team Y journey | Visible | Appears only in Team Y's grid (`teamId = Y`); does not retroactively appear in Team X | -| Pre-migration row (NULL `teamId`) | Visible to original uploader only | Not visible in any team grid | -| Caller passes `teamId` they're not a member of | — | `FORBIDDEN` | -| Caller passes a non-existent `teamId` | — | `FORBIDDEN` (same shape — no existence leak) | +| Scenario | "Your uploads" (caller) | "Team uploads" (Team X) | +| ---------------------------------------------------------- | --------------------------------- | ------------------------------------------------------------------------------------- | +| Caller uploads inside a Team X journey | Visible (`userId = caller`) | Visible (`teamId = X`) | +| Caller leaves Team X | Still visible (`userId = caller`) | **Still visible to remaining Team X members** | +| Caller later joins Team Y, uploads inside a Team Y journey | Visible | Appears only in Team Y's grid (`teamId = Y`); does not retroactively appear in Team X | +| Pre-migration row (NULL `teamId`) | Visible to original uploader only | Not visible in any team grid | +| Caller passes `teamId` they're not a member of | — | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | — | `FORBIDDEN` (same shape — no existence leak) | #### Performance considerations @@ -216,14 +221,14 @@ Both resolvers' implementation: #### Authorization edge cases -| Scenario | Behavior | -|---|---| -| Caller passes `teamId` they're not a member of | `FORBIDDEN` | -| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | -| User removed from team mid-session and tries to load "Team uploads" | `FORBIDDEN`; frontend detects and refreshes active-team list | -| User removed from team after uploading | Their asset stays in the team's "Team uploads" grid (the design goal) | -| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | -| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | +| Scenario | Behavior | +| -------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | +| Caller passes `teamId` they're not a member of | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | +| User removed from team mid-session and tries to load "Team uploads" | `FORBIDDEN`; frontend detects and refreshes active-team list | +| User removed from team after uploading | Their asset stays in the team's "Team uploads" grid (the design goal) | +| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | +| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | #### Tests (v1.1 specific) @@ -263,11 +268,12 @@ The original (pre-pivot) plan and the deepening notes (lines 62–101) reference > ⚠️ **Historical — superseded by the 2026-04-28 pivot and 2026-05-01 v1/v1.1 sections above.** > > Everything from this point onward describes the original Relay-Connection / `imageBlocksConnection` / `ACTIVE_TEAM` server-derivation design. **That approach was prototyped and rejected.** Notably: +> > - The Relay Connection migration of `getMyCloudflareImages` / `getMyMuxVideos` was reverted before merge — v1 ships offset/limit on the existing list-returning resolvers. > - The `Block`-table-as-source-of-truth premise was invalidated (`Block.src` is overwritten in place). > - The standalone Library tab and `imageBlocksConnection` resolver were dropped in favor of per-tab history grids. > -> Read this section for context on *why* those decisions were made, not as a spec to implement. New work should be scoped against the v1.1 section. +> Read this section for context on _why_ those decisions were made, not as a spec to implement. New work should be scoped against the v1.1 section. **Deepened:** 2026-04-28 with 3 best-practices researchers (Pothos cursor + Apollo cache + MUI tabs) and 5 reviewers (performance, security, simplicity, architecture, TypeScript). @@ -278,7 +284,7 @@ The original (pre-pivot) plan and the deepening notes (lines 62–101) reference 3. **Drop client-supplied `activeTeamId`.** Derive from `JourneyProfile.lastActiveTeamId` server-side and add an explicit `userTeam` membership precheck. Closes the cross-team enumeration vector and removes a forgeable client input. 4. **Drop server-side `DISTINCT ON` + raw SQL for v1.** Use vanilla Prisma `findMany` ordered by `updatedAt DESC` with a generous take, dedupe by `src` in resolver memory, and rely on the client `Map` as the cross-page safety net. Realistic library sizes are sub-few-thousand. Re-evaluate raw SQL only if EXPLAIN ANALYZE shows the simpler path is too slow. 5. **Add `thumbnailSrc` field (critical performance).** Resolver computes a thumbnail URL per source: Cloudflare `cdn-cgi/image/width=240,quality=80` for cf-hosted, Unsplash `&w=240&fit=crop`, AI/external passthrough. A 24-tile grid otherwise loads tens of MB of full-size hero images. -6. **Apollo cache strategy: `cache-first` (default) + mutation eviction.** Original plan double-paid by combining `cache-and-network` *and* eviction. With single-source-of-truth mutations, eviction alone is correct. +6. **Apollo cache strategy: `cache-first` (default) + mutation eviction.** Original plan double-paid by combining `cache-and-network` _and_ eviction. With single-source-of-truth mutations, eviction alone is correct. 7. **MUI tabs: switch to `variant="scrollable"` with `iconPosition="top"`** unconditionally (the drawer is always narrow; viewport-conditional toggling adds flicker). 8. **Auth: pick one filter, not two.** "Defense-in-depth" via CASL AND scope filter is double-bookkeeping for cases where both are supposed to compute the same set. Use the explicit scope filter as the authoritative gate, plus an explicit membership check for `ACTIVE_TEAM`. CASL still applies elsewhere (the resolver guard) but is not duplicated in the where-input. EXCEPTION: the `publisher` role can read `template: true` journeys across teams via CASL — for that role explicitly, AND-ing the scope filter is load-bearing. Document and test this. 9. **Cursor and raw-row decoding need runtime validation** (zod) if any raw SQL is reintroduced. Versioned cursor (`v: 1, ...`) for forward compatibility. @@ -289,6 +295,7 @@ The original (pre-pivot) plan and the deepening notes (lines 62–101) reference **v1 ships PERSONAL only. ACTIVE_TEAM follows in v1.1.** This was the user's call after weighing the simplicity review's push to ship team-only against the brainstorm's stated dual-scope plan. Concrete implications for v1: + - No `ACTIVE_TEAM` scope, no server-side `lastActiveTeamId` derivation, no `userTeam` membership precheck. - No feature-flag infrastructure required (Q7 dissolves). - Schema can drop the `ImageBlockLibraryScope` enum and the `scope` argument entirely; the resolver is unconditionally personal-scoped. @@ -312,6 +319,7 @@ The technical sections below were drafted with both scopes in scope. Where they Add a **Library** tab to the Editor's image picker (`ImageBlockEditor`) that surfaces images the creator (or their active team) has previously used in any `ImageBlock`. Picking an image applies it to the current block via the existing `onChange` flow — no upload, no AI prompt, no Cloudflare round-trip. The library is a derived view over `Block` rows of `typename = 'ImageBlock'`; **no new media/asset domain model is introduced**. Two scope variants are planned in parallel and gated by a feature flag so the same client code can ship either: + - **Approach A — Personal:** library shows ImageBlocks from journeys the user owns or edits. - **Approach B — Personal + Active Team (recommended target):** library shows ImageBlocks from any journey in the user's currently-active team. @@ -326,17 +334,20 @@ The constraint that shapes this plan: **deletion is forbidden in v1** — removi ## Proposed Solution ### Frontend + A new `` + `` inserted into `ImageBlockEditor.tsx` between the existing tabs, mirroring the `UnsplashGallery` structural pattern. New component lives at `apps/journeys-admin/src/components/Editor/Slider/Settings/Drawer/ImageBlockEditor/ImageLibraryGallery/` (the name `ImageLibrary` is taken by an unrelated wrapper component — see the Naming section in System-Wide Impact). The tab calls a new GraphQL query to list deduplicated `ImageBlock` summaries, ordered by `updatedAt DESC`, paginated as a Relay-style cursor connection. Selecting a tile calls the editor's existing `handleSrcChange` → `onChange(input)` prop with `{ src, alt, width, height, blurhash, scale, focalLeft, focalTop }` — the same path Unsplash and AI use today. **No new client mutations.** ### Backend + A new resolver `imageBlocksConnection(first, after, scope)` in **`apis/api-journeys`** (legacy NestJS + SDL — `visitorsConnection` lives here, all `ImageBlock` mutations live here, `AppCaslGuard` and `@CaslAccessible` decorators only exist here). Server-side filters: + - `typename = 'ImageBlock' AND src IS NOT NULL AND deletedAt IS NULL` - Journey scope (single explicit filter, not AND-ed with CASL): - `PERSONAL`: `journey.userJourneys.some({ userId: me, role: { in: [owner, editor] } })` - `ACTIVE_TEAM`: `journey.teamId = derivedActiveTeamId` AND **explicit precheck** that `userTeam.findFirst({ teamId: derivedActiveTeamId, userId: me })` is non-null. `derivedActiveTeamId` is read from `JourneyProfile.lastActiveTeamId` server-side; no client-supplied `activeTeamId` argument exists. The membership precheck short-circuits to an empty connection when the user is not a current member of their stored active team (e.g., they were just removed). -- The resolver is still gated by `@UseGuards(AppCaslGuard)` — CASL ensures only authenticated users hit the resolver and provides a sanity check, but the where-input is *not* `AND`-ed with `accessibleBy('Journey')`. Reasoning: for non-publisher users the explicit scope filter is a strict subset of what CASL allows; AND-ing changes nothing. For the `publisher` role specifically, CASL grants `Read` on every published-template journey across all teams (`journey.acl.ts:83-86, :144`) — so adding a scope filter here is necessary for `ACTIVE_TEAM` correctness. Encoded as: scope filter authoritative; CASL guard for auth gate only. +- The resolver is still gated by `@UseGuards(AppCaslGuard)` — CASL ensures only authenticated users hit the resolver and provides a sanity check, but the where-input is _not_ `AND`-ed with `accessibleBy('Journey')`. Reasoning: for non-publisher users the explicit scope filter is a strict subset of what CASL allows; AND-ing changes nothing. For the `publisher` role specifically, CASL grants `Read` on every published-template journey across all teams (`journey.acl.ts:83-86, :144`) — so adding a scope filter here is necessary for `ACTIVE_TEAM` correctness. Encoded as: scope filter authoritative; CASL guard for auth gate only. - **Include** journeys with `archivedAt`, `deletedAt`, `trashedAt` set — past work is exactly when reuse matters most (R7). **De-duplication strategy (v1, simplified):** Use Prisma `findMany` ordered by `updatedAt DESC` with `take: first * 3` (over-fetch to absorb dedup loss), dedupe in resolver memory keyed by `src` preserving the first occurrence (newest), slice to `first + 1` for `hasNextPage`, encode the cursor as `(updatedAt, id)`. The over-fetch factor of 3 is heuristic — tunable. For v1 library sizes (low thousands per team), this is well under p95 budget. The client `Map` keyed on `src` is kept as a cross-page safety net. **Raw SQL `DISTINCT ON` is deferred** to a v1.x optimization if EXPLAIN ANALYZE on a seeded 50k-block dataset misses the latency target. @@ -346,25 +357,31 @@ A new resolver `imageBlocksConnection(first, after, scope)` in **`apis/api-journ ### Architecture #### Where the resolver lives + **Decision: legacy `apis/api-journeys` (NestJS + SDL).** Reasoning surfaced during deepening: + - `visitorsConnection` (the Relay connection template the plan mirrors) lives in legacy. - All `ImageBlock` mutations (`imageBlockCreate`, `imageBlockUpdate`) live in legacy at `apis/api-journeys/src/app/modules/block/image/image.resolver.ts`. -- `apis/api-journeys-modern` (Pothos) only defines the `ImageBlock` *type*; it has no `prismaConnection`/`prismaConnectionHelpers` usage anywhere, no `AppCaslGuard`, no `@CaslAccessible` decorator. Hand-rolled `journeyAcl(Action.X, journey, user)` is the modern auth idiom. +- `apis/api-journeys-modern` (Pothos) only defines the `ImageBlock` _type_; it has no `prismaConnection`/`prismaConnectionHelpers` usage anywhere, no `AppCaslGuard`, no `@CaslAccessible` decorator. Hand-rolled `journeyAcl(Action.X, journey, user)` is the modern auth idiom. - Building the connection in modern would require porting/inventing all of those pieces. Not v1. #### Schema (using existing types) ```graphql -"""Scope for the image library query.""" +""" +Scope for the image library query. +""" enum ImageBlockLibraryScope { - PERSONAL # journeys the user owns or edits - ACTIVE_TEAM # journeys in the user's currently-active team (server-derived) + PERSONAL # journeys the user owns or edits + ACTIVE_TEAM # journeys in the user's currently-active team (server-derived) } type ImageBlockLibraryEdge { cursor: String! node: ImageBlock! - """Provider-specific thumbnail variant of node.src.""" + """ + Provider-specific thumbnail variant of node.src. + """ thumbnailSrc: String! } @@ -374,15 +391,12 @@ type ImageBlockLibraryConnection { } extend type Query { - imageBlocksConnection( - first: Int = 24 - after: String - scope: ImageBlockLibraryScope! - ): ImageBlockLibraryConnection! + imageBlocksConnection(first: Int = 24, after: String, scope: ImageBlockLibraryScope!): ImageBlockLibraryConnection! } ``` Notes on the schema shape (changes from v1 of the plan): + - **No `ImageBlockLibraryEntry`**. Edges expose existing `ImageBlock` directly. Reuses the existing `ImageBlockFields` codegen fragment, eliminates a parallel TS type, prevents Apollo cache normalization collisions on shared `ImageBlock` ids. - **No `activeTeamId` argument**. Server derives from `JourneyProfile.lastActiveTeamId`. Removes a forgeable client input. - **`thumbnailSrc` on the edge** (not on `ImageBlock`) so it's contextual to the library use case and doesn't pollute the canonical type. @@ -393,34 +407,38 @@ Pattern reference: `apis/api-journeys/src/app/modules/visitor/visitor.graphql:15 ```ts // PSEUDO — actual resolver lives in NestJS + Prisma -const overFetch = first * 3 + 1; +const overFetch = first * 3 + 1 const rows = await prisma.block.findMany({ where: { typename: 'ImageBlock', src: { not: null }, deletedAt: null, - journey: scopeFilter, // see Authorization below + journey: scopeFilter // see Authorization below }, orderBy: [{ updatedAt: 'desc' }, { id: 'desc' }], take: overFetch, // cursor-based pagination on (updatedAt, id) — both indexed - ...(after ? { cursor: { id: decodeCursor(after).id }, skip: 1 } : {}), -}); + ...(after ? { cursor: { id: decodeCursor(after).id }, skip: 1 } : {}) +}) // In-memory dedup by src, preserve first (newest by orderBy) -const seen = new Set(); -const deduped: typeof rows = []; +const seen = new Set() +const deduped: typeof rows = [] for (const r of rows) { - if (r.src && !seen.has(r.src)) { seen.add(r.src); deduped.push(r); } - if (deduped.length === first + 1) break; + if (r.src && !seen.has(r.src)) { + seen.add(r.src) + deduped.push(r) + } + if (deduped.length === first + 1) break } -const hasNextPage = deduped.length > first; -const slice = deduped.slice(0, first); +const hasNextPage = deduped.length > first +const slice = deduped.slice(0, first) ``` Cursor encoding: base64url JSON `{ v: 1, id, updatedAt }`. Decoded with a zod schema (`z.object({ v: z.literal(1), id: z.string(), updatedAt: z.string().datetime() })`) — malformed cursors throw `GraphQLError('Invalid cursor')` rather than crashing the resolver. The `v` byte enables non-breaking schema evolution. **Why this simplified path is correct for v1:** + - For team libraries up to ~few thousand ImageBlocks, the over-fetch + in-memory dedup completes in single-digit ms after the indexed query. - Trades a marginal worst-case (over-fetch wastes a row read per duplicate) for orders-of-magnitude less complexity vs. raw SQL `DISTINCT ON` + seek cursor on a compound key. - Cross-page duplicates are still possible if a duplicate `src` straddles a page boundary; the client `Map` (R15) absorbs them. Severity is cosmetic. @@ -443,37 +461,32 @@ Also confirm a `Journey(teamId)` index exists; if not, add one in the same migra ```ts @Resolver() -@UseGuards(AppCaslGuard) // auth gate; not used for where-filtering +@UseGuards(AppCaslGuard) // auth gate; not used for where-filtering class ImageBlockLibraryResolver { @Query(() => ImageBlockLibraryConnection) - async imageBlocksConnection( - @CurrentUserId() userId: string, - @Args('scope') scope: ImageBlockLibraryScope, - @Args('first', { defaultValue: 24 }) first: number, - @Args('after', { nullable: true }) after?: string, - ) { - let scopeFilter: Prisma.JourneyWhereInput; + async imageBlocksConnection(@CurrentUserId() userId: string, @Args('scope') scope: ImageBlockLibraryScope, @Args('first', { defaultValue: 24 }) first: number, @Args('after', { nullable: true }) after?: string) { + let scopeFilter: Prisma.JourneyWhereInput if (scope === 'PERSONAL') { scopeFilter = { - userJourneys: { some: { userId, role: { in: ['owner', 'editor'] } } }, - }; + userJourneys: { some: { userId, role: { in: ['owner', 'editor'] } } } + } } else { // ACTIVE_TEAM — derive teamId server-side, then verify membership const profile = await this.prisma.journeyProfile.findUnique({ where: { userId }, - select: { lastActiveTeamId: true }, - }); - const teamId = profile?.lastActiveTeamId; - if (!teamId) return emptyConnection(); // R10: fall back to empty (or PERSONAL — see Q3) + select: { lastActiveTeamId: true } + }) + const teamId = profile?.lastActiveTeamId + if (!teamId) return emptyConnection() // R10: fall back to empty (or PERSONAL — see Q3) const isMember = await this.prisma.userTeam.findFirst({ where: { teamId, userId }, - select: { id: true }, - }); - if (!isMember) return emptyConnection(); // R13 enforcement at resolver entry + select: { id: true } + }) + if (!isMember) return emptyConnection() // R13 enforcement at resolver entry - scopeFilter = { teamId }; + scopeFilter = { teamId } } // ... findMany with where: { ..., journey: scopeFilter } @@ -482,6 +495,7 @@ class ImageBlockLibraryResolver { ``` Decisions encoded: + - **Scope filter is authoritative** for where-filtering. CASL guard handles auth-gate only. - **`activeTeamId` is server-derived** (`JourneyProfile.lastActiveTeamId`). No client argument. - **Membership precheck** short-circuits to empty connection — matches R13 ("user removed from team T returns 0 images") at the resolver entry, also closes the timing-side-channel. @@ -529,10 +543,10 @@ Mutation eviction (single source of truth for refresh): ```ts const [createImageBlock] = useJourneyImageBlockCreateMutation({ update(cache) { - cache.evict({ fieldName: 'imageBlocksConnection' }); - cache.gc(); - }, -}); + cache.evict({ fieldName: 'imageBlocksConnection' }) + cache.gc() + } +}) // Same wrapper added to useJourneyImageBlockUpdateMutation. ``` @@ -540,12 +554,13 @@ On `activeTeam.id` change in the same session, also evict — Apollo will re-fet ```ts useEffect(() => { - client.cache.evict({ fieldName: 'imageBlocksConnection' }); - client.cache.gc(); -}, [activeTeam?.id]); + client.cache.evict({ fieldName: 'imageBlocksConnection' }) + client.cache.gc() +}, [activeTeam?.id]) ``` Why this differs from v1 of the plan: + - **`cache-first`, not `cache-and-network`.** With single-source-of-truth mutation eviction, every drawer open is either a cache hit (fresh) or a fresh fetch (just evicted). `cache-and-network` would double-pay every open. - **`relayStylePagination(['scope'])` as the field policy** — handles edge merging, cursor stability, dedup-by-cursor automatically. Source: https://www.apollographql.com/docs/react/pagination/cursor-based#relay-style-cursor-pagination - **`ImageBlock.keyFields: false`** prevents the library's `ImageBlock` nodes from sharing cache identity with the editor's canonical `ImageBlock` queries (where they would otherwise overwrite richer field selections). @@ -633,6 +648,7 @@ apps/journeys-admin/src/libs/useImageLibraryQuery/ #### Phase 4 — Rollout **Prerequisite:** confirm feature-flag infrastructure exists in this monorepo. Architecture review found no GrowthBook/Unleash/`isFeatureEnabled` pattern; if absent, dual-scope rollout is not viable as planned. Two paths: + - **Path A (flag infra exists or is added):** as below. - **Path B (no flag infra; pragmatic):** ship `ACTIVE_TEAM`-only — that's the actual product value driver per the brainstorm. Drop the `scope` arg or hard-default it. Saves all flag work. Origin doc's "personal vs personal+team" comparison resolves in favor of team for the same reason. @@ -663,6 +679,7 @@ These come from the origin doc and are preserved here for context. ### Interaction Graph When a creator clicks a Library tile: + 1. `ImageLibraryList` `onClick(item)` fires. 2. → `ImageLibraryGallery` calls `props.onChange({ src, alt, width, height, blurhash, scale, focalLeft, focalTop })`. 3. → `ImageBlockEditor.handleSrcChange` short-circuits if `src === selectedBlock.src` (no-op), otherwise validates `src`, defaults `alt` from filename, calls parent `onChange`. @@ -718,9 +735,9 @@ When a creator clicks a Library tile: ### New Acceptance Criteria from Flow Analysis & Deepening - [ ] **R9** Empty states render distinct copy for: no-images-anywhere/team-with-no-images (collapsed per simplicity review — same UX in v1 since team scope means an empty team library reads the same as "no images") and fetch-error. Two variants total, not three. -- [ ] **R10** When `JourneyProfile.lastActiveTeamId` is null in `ACTIVE_TEAM` scope, resolver returns an empty connection. Client may *optionally* render a hint "Set an active team to see shared images" — leave to design. +- [ ] **R10** When `JourneyProfile.lastActiveTeamId` is null in `ACTIVE_TEAM` scope, resolver returns an empty connection. Client may _optionally_ render a hint "Set an active team to see shared images" — leave to design. - [ ] **R11** When `activeTeam.id` changes client-side, the connection field is evicted and re-fetched from cursor null; accumulated client `Map` is reset. -- [ ] ~~**R12**~~ *(removed: existing selection-highlight via `selectedBlock?.src === edge.node.src` covers this; `handleSrcChange` already short-circuits on identical src; no Current badge needed)* +- [ ] ~~**R12**~~ _(removed: existing selection-highlight via `selectedBlock?.src === edge.node.src` covers this; `handleSrcChange` already short-circuits on identical src; no Current badge needed)_ - [ ] **R13** Cross-team auth test passes: user removed from team T → resolver returns empty connection for `scope=ACTIVE_TEAM` (membership precheck). - [ ] **R14** Concurrent "Load More" clicks are guarded — only one in-flight fetch at a time (via `NetworkStatus.fetchMore`). - [ ] **R15** Client-side dedup `Map` absorbs cross-page race duplicates and is capped at 500 entries; "Load More" disables on cap. @@ -751,9 +768,11 @@ When a creator clicks a Library tile: ## Success Metrics **Primary metric (lock in before launch — SpecFlow #33):** + - Ratio of Library-tab applies to Custom-tab uploads per session. Target: ≥30% within four weeks of GA. If ≥50%, declare clear success. **Secondary metrics:** + - Library tab open-rate (% of editor sessions that activate the Library tab). Target: ≥40% within four weeks. - Re-upload rate: count of new `ImageBlock` rows where `src` already exists in the user's (or team's) prior ImageBlocks. Should drop measurably post-launch. - AI re-prompt rate: heuristic match on similar prompts producing similar images. Stretch metric — instrument only if cheap. From e3c5cb43887b4364eebf7e871a66f92dd9dcdd9c Mon Sep 17 00:00:00 2001 From: edmonday Date: Wed, 6 May 2026 16:13:12 +1200 Subject: [PATCH 3/5] docs(plans): merge personal and team uploads into single grid for v1.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the two-grid v1.1 design ("Your uploads" + "Team uploads") with a single merged grid titled "Uploads" that surfaces both the caller's own uploads and the active team's uploads in one chronological feed. Why: clearer mental model — users see everything they can re-pick from the active team without scanning two stacked grids. Changes: - Resolver: when teamId arg is provided, predicate becomes `userId = caller OR teamId = arg` (a row matching both sides appears once). - Schema: add (userId, createdAt, id) index alongside the (teamId, createdAt, id) index so Postgres can bitmap-OR efficiently across the merged predicate. - Frontend: a single grid mount per tab/screen instead of two; takes activeTeam?.id ?? null. No active team → personal-only fallback. - Title: "Uploads". - Behavioral table, performance, auth, tests, phasing all updated to match. --- ...4-28-001-feat-editor-asset-library-plan.md | 112 +++++++++--------- 1 file changed, 59 insertions(+), 53 deletions(-) diff --git a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md index e54946ae486..1b7e28db295 100644 --- a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md +++ b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md @@ -115,22 +115,21 @@ v1 is currently merged but not yet generally available. Before going live we gat ### v1.1 — team-shared visibility (replaces the ACTIVE_TEAM material in original Backend / Architecture / Implementation Phases sections) -Goal: the same per-tab grids surface assets uploaded by _teammates_ in the user's active team, alongside their own. +Goal: the existing per-tab "Uploads" grid surfaces assets uploaded by _teammates_ in the user's active team alongside the caller's own uploads, in a single chronological feed. #### Locked decisions -- **Asset has both an uploader (`userId`) and a home team (`teamId`).** `userId` is the canonical owner — it's how "Your uploads" stays personal. `teamId` is the home-team affiliation, persisted at upload time and immutable thereafter; it's how the asset stays visible to a team after the uploader leaves. +- **Asset has both an uploader (`userId`) and a home team (`teamId`).** `userId` is the canonical owner — used to surface the asset back to the uploader regardless of team membership. `teamId` is the home-team affiliation, persisted at upload time and immutable thereafter; it's how the asset stays accessible to a team after the uploader leaves. - **`teamId` is derived from the journey context at upload time**, not from the user's `useTeam().activeTeam` UI state. All editor upload surfaces (image block picker, video block picker, poster picker, journey logo, social share image) happen inside a journey, and a `Journey` always has a `teamId`. Server-side, the resolver looks up the journey by ID to read its `teamId`; the frontend just needs to pass the `journeyId` it's already editing. -- **Visibility split:** - - "Your uploads" grid: `where: { userId: caller }` — unchanged from v1. - - "Team uploads" grid: `where: { teamId: activeTeamId }` plus an explicit caller-is-member precheck. -- **Active team passed per-request** from the editor for the "Team uploads" grid only. The frontend already knows it via `useTeam()`. -- **Membership precheck**: `api-media` directly imports `prisma-journeys` for the `userTeam.findUnique` check. Same precedent already established by `api-journeys-modern`'s user-delete flow (which imports both `prisma-users` and `prisma-journeys`). Latency ~2–5ms, always fresh, no token-staleness window. **Read path no longer needs the teammate-list fetch** — it filters on the asset's own `teamId`. -- **Schema changes:** new nullable `teamId String?` column on `CloudflareImage` and `MuxVideo`. New rows always populated (every editor upload has a journey context); existing 38k+ rows stay NULL. Indexed on `(teamId, createdAt DESC, id DESC)` to match the visibility query. -- **No backfill.** Existing NULL-`teamId` rows are invisible to all team grids and remain visible only to their original uploader in "Your uploads". Clean slate for team-shared visibility from migration forward. -- **Account deletion stays correct.** Neither `userDeleteConfirm` nor `userDeleteJourneysConfirm` touches `MuxVideo` / `CloudflareImage`. After deletion: `userId` becomes a UUID tombstone with no PII; the asset's `teamId` is unaffected, so it stays in the team's "Team uploads" grid (which is exactly what we want — the team keeps the asset). +- **Single merged grid per tab — titled "Uploads".** v1's "Your uploads" grid is upgraded in place to also include the active team's assets. No second grid is added. Predicate: `userId = caller OR teamId = activeTeamId`. The user sees a single chronological feed of everything they can re-pick from the active team — their own uploads (in any team) plus teammates' uploads in the active team. +- **No active team selected → personal-only fallback.** When `useTeam().activeTeam` is `null` the grid degrades cleanly to the v1 personal-only behavior (`userId = caller`), so the merged grid never shows nothing. +- **Active team passed per-request** from the editor when present. The frontend already knows it via `useTeam()`. +- **Membership precheck on the team-scoped path.** `api-media` directly imports `prisma-journeys` for the `userTeam.findUnique` check before honoring the `teamId` arg. Same precedent already established by `api-journeys-modern`'s user-delete flow. Latency ~2–5ms, always fresh, no token-staleness window. **Read path does not fetch the teammate list** — it filters on the asset's own `teamId` instead. +- **Schema changes:** new nullable `teamId String?` column on `CloudflareImage` and `MuxVideo`. New rows always populated (every editor upload has a journey context); existing 38k+ rows stay NULL. Two composite indexes per table — `(userId, createdAt DESC, id DESC)` and `(teamId, createdAt DESC, id DESC)` — so Postgres can bitmap-OR efficiently across the merged predicate. +- **No backfill.** Existing NULL-`teamId` rows are invisible to the team side of the merged predicate and remain visible only to their original uploader (via `userId`). Clean slate for team-shared visibility from migration forward. +- **Account deletion stays correct.** Neither `userDeleteConfirm` nor `userDeleteJourneysConfirm` touches `MuxVideo` / `CloudflareImage`. After deletion: `userId` becomes a UUID tombstone with no PII; the asset's `teamId` is unaffected, so it stays accessible via the team predicate (which is exactly what we want — the team keeps the asset). - **Team deletion**: out of scope for v1.1. If a team is deleted, its assets persist with a stale `teamId` reference. We don't currently have a journey-team-delete flow that cascades, and the surface area to add it is larger than v1.1 should take on. Document and revisit if/when team deletion ships. -- **Out of scope** for v1.1: deletion/management of teammates' assets, multi-team merged views, signed/private playback IDs, uploader attribution UI (deferred if product asks), retroactive `teamId` assignment for pre-migration rows, transferring an asset between teams, storage quotas per team. +- **Out of scope** for v1.1: deletion/management of teammates' assets, multi-team merged views (scope is limited to the single active team at a time), signed/private playback IDs, uploader attribution UI (deferred if product asks), retroactive `teamId` assignment for pre-migration rows, transferring an asset between teams, storage quotas per team. - **Quick-start (Template Customization) flow is out of scope.** Based on Lyuba's feedback, surfacing per-tab history inside the quick-start flow doesn't quite make sense for that flow's intent — first-run template setup is a different mental model from editor re-use. Re-pick from history applies only inside the editor's image and video pickers. We can revisit adding it to the quick-start later if it turns out to help. #### Schema change @@ -141,17 +140,19 @@ Prisma model update (`prisma-media`): model CloudflareImage { // ... existing fields teamId String? + @@index([userId, createdAt(sort: Desc), id(sort: Desc)]) @@index([teamId, createdAt(sort: Desc), id(sort: Desc)]) } model MuxVideo { // ... existing fields teamId String? + @@index([userId, createdAt(sort: Desc), id(sort: Desc)]) @@index([teamId, createdAt(sort: Desc), id(sort: Desc)]) } ``` -Migration: `ADD COLUMN teamId TEXT NULL` plus the index. No backfill; safe and fast on a populated table because nullable columns don't require a table rewrite in Postgres. +Migration: `ADD COLUMN teamId TEXT NULL` plus the two indexes per table. No backfill; safe and fast on a populated table because nullable columns don't require a table rewrite in Postgres. Both indexes are needed so the planner can bitmap-OR efficiently across the merged `userId OR teamId` predicate. #### Upload mutation changes @@ -177,85 +178,90 @@ getMyCloudflareImages(offset: Int, limit: Int, isAi: Boolean, teamId: ID): [Clou Both resolvers' implementation: -1. **`teamId` omitted** → existing behavior (`where: { userId: caller }`). Backward compatible — drives the "Your uploads" grid. -2. **`teamId` provided** → "Team uploads" path: +1. **`teamId` omitted** → existing v1 behavior: `where: { userId: caller, ...(isAi != null ? { isAi } : {}) }`. Backward compatible. Used by the personal-only fallback when the user has no active team selected. +2. **`teamId` provided** → merged-grid path: - **Membership precheck**: `journeysPrisma.userTeam.findUnique({ where: { teamId_userId: { teamId, userId: caller } } })`. If null → throw `GraphQLError('Not a member of this team', { extensions: { code: 'FORBIDDEN' } })`. Don't distinguish from non-existent team — avoids existence enumeration. - - **Asset query**: `where: { teamId, ...(isAi != null ? { isAi } : {}) }`. No teammate-list fetch needed. + - **Asset query**: `where: { OR: [{ userId: caller }, { teamId }], ...(isAi != null ? { isAi } : {}) }`. A row matching both sides of the OR appears once (single row, two filter keys). - Same `orderBy [createdAt desc, id desc]` and offset/limit pagination as v1. #### Frontend changes -- `MyCloudflareImagesGrid` and `MyMuxVideosGrid` get a `teamId?: string | null` prop. -- Mount each grid twice in its respective tab/screen: +- `MyCloudflareImagesGrid` and `MyMuxVideosGrid` get a `teamId?: string | null` prop. When provided, the underlying query passes it; when absent, the query falls back to personal-only. +- Mount **once** per tab/screen — there is no second grid: ```tsx - - + ``` - Same pattern for Mux: a "Your uploads" + a "Team uploads" grid stacked under `AddByFile`. -- **Apollo cache** — extend keyArgs so the two grids stay in separate cache entries: + Same pattern for Mux: one grid titled `"Uploads"` under `AddByFile`, taking the active team id. +- **Apollo cache** keyed on the merged shape: ```ts getMyCloudflareImages: offsetLimitPagination(['isAi', 'teamId']), getMyMuxVideos: offsetLimitPagination(['teamId']), ``` -- **Active team source**: `useTeam()` from `TeamProvider` — already a tree ancestor of the editor. Threaded as a prop into the relevant tab/screen components. +- **Active team source**: `useTeam()` from `TeamProvider` — already a tree ancestor of the editor. Threaded as a prop. Switching the active team triggers a refetch under the new cache key. - **Upload mutation call sites**: thread the current `journeyId` (already known from the editor route/state) into each upload mutation invocation. -- **Refetch on upload**: existing `refetchQueries: ['GetMyCloudflareImages']` / `'GetMyMuxVideos'` works as-is. Apollo refetches every active query with that operation name regardless of variables, so both personal and team grids update. +- **Refetch on upload**: existing `refetchQueries: ['GetMyCloudflareImages']` / `'GetMyMuxVideos'` works as-is. Apollo refetches every active query with that operation name regardless of variables. #### Behavioral table -| Scenario | "Your uploads" (caller) | "Team uploads" (Team X) | -| ---------------------------------------------------------- | --------------------------------- | ------------------------------------------------------------------------------------- | -| Caller uploads inside a Team X journey | Visible (`userId = caller`) | Visible (`teamId = X`) | -| Caller leaves Team X | Still visible (`userId = caller`) | **Still visible to remaining Team X members** | -| Caller later joins Team Y, uploads inside a Team Y journey | Visible | Appears only in Team Y's grid (`teamId = Y`); does not retroactively appear in Team X | -| Pre-migration row (NULL `teamId`) | Visible to original uploader only | Not visible in any team grid | -| Caller passes `teamId` they're not a member of | — | `FORBIDDEN` | -| Caller passes a non-existent `teamId` | — | `FORBIDDEN` (same shape — no existence leak) | +| Scenario | What the merged "Uploads" grid shows (active team = X) | +| --- | --- | +| Caller's own asset uploaded inside a Team X journey | Visible (matches both `userId` and `teamId = X`) | +| Caller's own asset uploaded inside a Team Y journey (Y is a different team they're in) | Visible via `userId = caller` | +| Teammate's asset uploaded inside a Team X journey | Visible via `teamId = X` | +| Caller leaves Team X | Their own assets still visible via `userId`; teammates' Team X assets disappear (active team is no longer X) | +| Asset uploaded by someone who later left Team X | Still visible to remaining Team X members via `teamId = X` (the design goal) | +| Pre-migration row (NULL `teamId`) | Visible only via `userId = caller` (i.e., to the original uploader) | +| Caller has no active team selected | Personal-only fallback — only `userId = caller` rows | +| Caller passes `teamId` they're not a member of | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | `FORBIDDEN` (same shape — no existence leak) | #### Performance considerations -- "Team uploads" path: one Prisma read against `prisma-journeys` for the membership precheck (~2–5ms, indexed). Single asset query against the new `(teamId, createdAt, id)` index — bounded by `LIMIT`, scales independently of team size. -- "Your uploads" path: unchanged from v1 (`userId` filter). +- Merged-grid path: one Prisma read against `prisma-journeys` for the membership precheck (~2–5ms, indexed). Single asset query with `userId OR teamId` predicate. Postgres uses bitmap OR across the two new composite indexes — both index branches feed into a heap fetch, then sorted by the leading sort columns of each index (matching `createdAt DESC, id DESC`). Bounded by `LIMIT`, scales independently of team size. +- Personal-only path: unchanged from v1 (`userId` filter, hits the `(userId, createdAt, id)` index directly). - Upload path: one extra Prisma read against `prisma-journeys` to look up `journey.teamId`. Same lookup the existing journey-edit auth path already does, so likely cached at the request level — negligible. -- Net effect vs the "compute teammates dynamically" alternative: simpler read path, removes a `findMany` from every "Team uploads" load, and removes the `userId IN (N userIds)` query whose performance degraded with team size. +- Net effect vs the "compute teammates dynamically" alternative: simpler read path, removes a `findMany` from every team-scoped load, and removes the `userId IN (N userIds)` query whose performance degraded with team size. #### Authorization edge cases -| Scenario | Behavior | -| -------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | -| Caller passes `teamId` they're not a member of | `FORBIDDEN` | -| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | -| User removed from team mid-session and tries to load "Team uploads" | `FORBIDDEN`; frontend detects and refreshes active-team list | -| User removed from team after uploading | Their asset stays in the team's "Team uploads" grid (the design goal) | -| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | -| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | +| Scenario | Behavior | +| --- | --- | +| Caller passes `teamId` they're not a member of | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | +| Caller has no active team | Frontend omits `teamId` arg; resolver returns personal-only | +| User removed from team mid-session and tries to load with `teamId` | `FORBIDDEN`; frontend detects and refreshes active-team list (next request omits `teamId` once `activeTeam` clears) | +| User removed from team after uploading | Their asset stays accessible to that team via `teamId` (the design goal) | +| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | +| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | #### Tests (v1.1 specific) -- Migration: nullable column added; existing rows unchanged; index created. +- Migration: nullable `teamId` column added; existing rows unchanged; both composite indexes created. - Upload mutations: each one writes `teamId = journey.teamId` for new rows. - Upload mutations: caller without journey-edit access cannot create rows (existing auth path). -- Resolver: `teamId` omitted → returns own uploads (v1 behavior preserved). -- Resolver: `teamId` provided + caller is member → returns rows where `teamId = arg`, ordered correctly. Includes the caller's own rows tagged with that team. +- Resolver: `teamId` omitted → returns own uploads only (v1 behavior preserved; drives the no-active-team fallback). +- Resolver: `teamId` provided + caller is member → returns the union of `userId = caller` and `teamId = arg`, ordered by `createdAt DESC, id DESC`. A row matching both sides appears exactly once. - Resolver: `teamId` provided + caller not member → `FORBIDDEN`. - Resolver: `teamId` for non-existent team → `FORBIDDEN` (same shape). -- Resolver: rows with NULL `teamId` never appear in any "Team uploads" response. -- Resolver: a row whose `userId` later loses team membership still appears in that team's grid (the core design goal). -- Frontend: both grids render independently with separate cache entries; switching `teamId` triggers the team grid to refetch. -- Frontend: refetch-after-upload updates both grids simultaneously. +- Resolver: NULL-`teamId` rows are excluded from the team side of the OR (only the original uploader sees them via the `userId` side). +- Resolver: a row whose uploader later loses team membership still appears in the team's merged grid (the core design goal). +- Frontend: switching active team triggers a refetch under the new cache key. +- Frontend: clearing active team (no team selected) triggers a refetch into the personal-only mode. +- Frontend: refetch-after-upload updates the merged grid. #### Phasing -- **1.1.1 — Backend**: schema migration (nullable `teamId` + index on both tables), upload mutations write `teamId = journey.teamId`, optional `teamId` arg + membership precheck on both `getMyCloudflareImages` and `getMyMuxVideos`. Tests above. Ships unflagged — additive arg, additive column, backward-compatible. -- **1.1.2 — Frontend**: thread `journeyId` into upload mutation calls, thread `useTeam().activeTeam.id` to the grids, mount second "Team uploads" grid in each affected tab/screen, update Apollo cache keys. Both grids stay inside the existing `mediaLibrary` flag gate — v1.1 does not introduce a separate flag. +- **1.1.1 — Backend**: schema migration (nullable `teamId` + two composite indexes per table), upload mutations write `teamId = journey.teamId`, optional `teamId` arg + membership precheck on both `getMyCloudflareImages` and `getMyMuxVideos`, resolver uses the merged `userId OR teamId` predicate when `teamId` is provided. Tests above. Ships unflagged — additive arg, additive column, backward-compatible. +- **1.1.2 — Frontend**: thread `journeyId` into upload mutation calls, thread `useTeam().activeTeam?.id ?? null` into the existing single grid mount in each tab/screen, update the grid title to "Uploads", update Apollo cache keys. The grid stays inside the existing `mediaLibrary` flag gate — v1.1 does not introduce a separate flag. - **1.1.3 (optional, deferred)**: uploader attribution chip on each tile (federation to `api-users.User`). Skip until product asks. #### Risk surface -- Schema change is a single nullable column + one index per table — no rewrite, fast on populated tables. +- Schema change is a single nullable column + two indexes per table — no rewrite, fast on populated tables. - Cross-domain Prisma read at upload time (`journey.teamId` lookup) adds a build-time coupling between `api-media` and `prisma-journeys` — already established precedent (user-delete), so additive rather than new policy. - Upload mutation signatures change (`journeyId` becomes required). Backward-compat shim if any callers exist outside the editor; verify before merging the BE PR. -- Existing 38k rows being invisible to team grids is intentional (we can't reconstruct their team affiliation) but worth product confirmation. +- Existing 38k rows being invisible to the team side of the predicate is intentional (we can't reconstruct their team affiliation) but worth product confirmation. +- Bitmap-OR query plan should be verified with EXPLAIN ANALYZE on a representative dataset before BE-2 ships, in case the planner picks a less efficient strategy at small or large team sizes. #### Material in original plan that does NOT apply to v1.1 From d66d58eea2592cb03c90e3a05745eb5e59cba32e Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 6 May 2026 04:16:35 +0000 Subject: [PATCH 4/5] fix: lint issues --- ...4-28-001-feat-editor-asset-library-plan.md | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md index 1b7e28db295..d61ce0564a8 100644 --- a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md +++ b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md @@ -203,17 +203,17 @@ Both resolvers' implementation: #### Behavioral table -| Scenario | What the merged "Uploads" grid shows (active team = X) | -| --- | --- | -| Caller's own asset uploaded inside a Team X journey | Visible (matches both `userId` and `teamId = X`) | -| Caller's own asset uploaded inside a Team Y journey (Y is a different team they're in) | Visible via `userId = caller` | -| Teammate's asset uploaded inside a Team X journey | Visible via `teamId = X` | -| Caller leaves Team X | Their own assets still visible via `userId`; teammates' Team X assets disappear (active team is no longer X) | -| Asset uploaded by someone who later left Team X | Still visible to remaining Team X members via `teamId = X` (the design goal) | -| Pre-migration row (NULL `teamId`) | Visible only via `userId = caller` (i.e., to the original uploader) | -| Caller has no active team selected | Personal-only fallback — only `userId = caller` rows | -| Caller passes `teamId` they're not a member of | `FORBIDDEN` | -| Caller passes a non-existent `teamId` | `FORBIDDEN` (same shape — no existence leak) | +| Scenario | What the merged "Uploads" grid shows (active team = X) | +| -------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | +| Caller's own asset uploaded inside a Team X journey | Visible (matches both `userId` and `teamId = X`) | +| Caller's own asset uploaded inside a Team Y journey (Y is a different team they're in) | Visible via `userId = caller` | +| Teammate's asset uploaded inside a Team X journey | Visible via `teamId = X` | +| Caller leaves Team X | Their own assets still visible via `userId`; teammates' Team X assets disappear (active team is no longer X) | +| Asset uploaded by someone who later left Team X | Still visible to remaining Team X members via `teamId = X` (the design goal) | +| Pre-migration row (NULL `teamId`) | Visible only via `userId = caller` (i.e., to the original uploader) | +| Caller has no active team selected | Personal-only fallback — only `userId = caller` rows | +| Caller passes `teamId` they're not a member of | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | `FORBIDDEN` (same shape — no existence leak) | #### Performance considerations @@ -224,15 +224,15 @@ Both resolvers' implementation: #### Authorization edge cases -| Scenario | Behavior | -| --- | --- | -| Caller passes `teamId` they're not a member of | `FORBIDDEN` | -| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | -| Caller has no active team | Frontend omits `teamId` arg; resolver returns personal-only | -| User removed from team mid-session and tries to load with `teamId` | `FORBIDDEN`; frontend detects and refreshes active-team list (next request omits `teamId` once `activeTeam` clears) | -| User removed from team after uploading | Their asset stays accessible to that team via `teamId` (the design goal) | -| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | -| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | +| Scenario | Behavior | +| -------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | +| Caller passes `teamId` they're not a member of | `FORBIDDEN` | +| Caller passes a non-existent `teamId` | `FORBIDDEN` (same response) | +| Caller has no active team | Frontend omits `teamId` arg; resolver returns personal-only | +| User removed from team mid-session and tries to load with `teamId` | `FORBIDDEN`; frontend detects and refreshes active-team list (next request omits `teamId` once `activeTeam` clears) | +| User removed from team after uploading | Their asset stays accessible to that team via `teamId` (the design goal) | +| Teammate uploads while caller paginates | Asset appears at top of next refetch (`createdAt DESC`); insertion drift across already-fetched pages is possible but mitigated by refetch-on-upload | +| Caller tries to upload into a journey they don't have edit access to | Existing journey-edit auth fails before the row is written; no new auth surface | #### Tests (v1.1 specific) From 1387dd4d6d0f3d359d0d609fd816dbb7399f545a Mon Sep 17 00:00:00 2001 From: edmonday Date: Wed, 6 May 2026 23:26:03 +0000 Subject: [PATCH 5/5] docs(plans): drop id tiebreaker from orderBy/index references NES-1627 ships orderBy: { createdAt: 'desc' } (no id tiebreaker) and indexes without the id column. Update the plan's v1 references and the v1.1 composite-index design to match. Microsecond-precision createdAt + single-row inserts make the id tiebreaker unnecessary in practice. Smaller index footprint, simpler resolver. If a future bulk-insert path lands, the tiebreaker can be added back at that point. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-04-28-001-feat-editor-asset-library-plan.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md index d61ce0564a8..7a4d4d1f939 100644 --- a/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md +++ b/docs/plans/2026-04-28-001-feat-editor-asset-library-plan.md @@ -82,7 +82,7 @@ Implementation against the 2026-04-28 pivot landed in PR #9102. Production state **Video picker (`VideoFromMux`):** - "Your uploads" grid below the Mux upload form. -- Backed by existing `getMyMuxVideos(offset, limit): [MuxVideo!]!` — no schema change. Resolver only added a deterministic `orderBy [createdAt desc, id desc]` (correctness for offset pagination, not a UI preference). +- Backed by existing `getMyMuxVideos(offset, limit): [MuxVideo!]!` — no schema change. Resolver only added a deterministic `orderBy { createdAt: 'desc' }` (correctness for offset pagination, not a UI preference). - Click-a-tile opens existing `VideoDetails` + `MuxDetails` with a Select button. Gallery flow reuses the same preview infrastructure as the active-block flow; `playbackId` is threaded through to avoid a redundant fetch. `MuxDetails.dispose()` fix shipped alongside. - `MuxVideoUploadProvider` refetches `GetMyMuxVideos` after polling completes so freshly-ready uploads surface in the gallery. - `VideoLibrary.onSelect` now closes the outer drawer when `shouldCloseDrawer` is true so picking the same video as the active block still navigates back to Properties. @@ -125,7 +125,7 @@ Goal: the existing per-tab "Uploads" grid surfaces assets uploaded by _teammates - **No active team selected → personal-only fallback.** When `useTeam().activeTeam` is `null` the grid degrades cleanly to the v1 personal-only behavior (`userId = caller`), so the merged grid never shows nothing. - **Active team passed per-request** from the editor when present. The frontend already knows it via `useTeam()`. - **Membership precheck on the team-scoped path.** `api-media` directly imports `prisma-journeys` for the `userTeam.findUnique` check before honoring the `teamId` arg. Same precedent already established by `api-journeys-modern`'s user-delete flow. Latency ~2–5ms, always fresh, no token-staleness window. **Read path does not fetch the teammate list** — it filters on the asset's own `teamId` instead. -- **Schema changes:** new nullable `teamId String?` column on `CloudflareImage` and `MuxVideo`. New rows always populated (every editor upload has a journey context); existing 38k+ rows stay NULL. Two composite indexes per table — `(userId, createdAt DESC, id DESC)` and `(teamId, createdAt DESC, id DESC)` — so Postgres can bitmap-OR efficiently across the merged predicate. +- **Schema changes:** new nullable `teamId String?` column on `CloudflareImage` and `MuxVideo`. New rows always populated (every editor upload has a journey context); existing 38k+ rows stay NULL. Two composite indexes per table — `(userId, createdAt DESC)` and `(teamId, createdAt DESC)` — so Postgres can bitmap-OR efficiently across the merged predicate. - **No backfill.** Existing NULL-`teamId` rows are invisible to the team side of the merged predicate and remain visible only to their original uploader (via `userId`). Clean slate for team-shared visibility from migration forward. - **Account deletion stays correct.** Neither `userDeleteConfirm` nor `userDeleteJourneysConfirm` touches `MuxVideo` / `CloudflareImage`. After deletion: `userId` becomes a UUID tombstone with no PII; the asset's `teamId` is unaffected, so it stays accessible via the team predicate (which is exactly what we want — the team keeps the asset). - **Team deletion**: out of scope for v1.1. If a team is deleted, its assets persist with a stale `teamId` reference. We don't currently have a journey-team-delete flow that cascades, and the surface area to add it is larger than v1.1 should take on. Document and revisit if/when team deletion ships. @@ -182,7 +182,7 @@ Both resolvers' implementation: 2. **`teamId` provided** → merged-grid path: - **Membership precheck**: `journeysPrisma.userTeam.findUnique({ where: { teamId_userId: { teamId, userId: caller } } })`. If null → throw `GraphQLError('Not a member of this team', { extensions: { code: 'FORBIDDEN' } })`. Don't distinguish from non-existent team — avoids existence enumeration. - **Asset query**: `where: { OR: [{ userId: caller }, { teamId }], ...(isAi != null ? { isAi } : {}) }`. A row matching both sides of the OR appears once (single row, two filter keys). - - Same `orderBy [createdAt desc, id desc]` and offset/limit pagination as v1. + - Same `orderBy { createdAt: 'desc' }` and offset/limit pagination as v1. #### Frontend changes @@ -217,8 +217,8 @@ Both resolvers' implementation: #### Performance considerations -- Merged-grid path: one Prisma read against `prisma-journeys` for the membership precheck (~2–5ms, indexed). Single asset query with `userId OR teamId` predicate. Postgres uses bitmap OR across the two new composite indexes — both index branches feed into a heap fetch, then sorted by the leading sort columns of each index (matching `createdAt DESC, id DESC`). Bounded by `LIMIT`, scales independently of team size. -- Personal-only path: unchanged from v1 (`userId` filter, hits the `(userId, createdAt, id)` index directly). +- Merged-grid path: one Prisma read against `prisma-journeys` for the membership precheck (~2–5ms, indexed). Single asset query with `userId OR teamId` predicate. Postgres uses bitmap OR across the two new composite indexes — both index branches feed into a heap fetch, then sorted by the leading sort columns of each index (matching `createdAt DESC`). Bounded by `LIMIT`, scales independently of team size. +- Personal-only path: unchanged from v1 (`userId` filter, hits the `(userId, createdAt)` index directly). - Upload path: one extra Prisma read against `prisma-journeys` to look up `journey.teamId`. Same lookup the existing journey-edit auth path already does, so likely cached at the request level — negligible. - Net effect vs the "compute teammates dynamically" alternative: simpler read path, removes a `findMany` from every team-scoped load, and removes the `userId IN (N userIds)` query whose performance degraded with team size. @@ -240,7 +240,7 @@ Both resolvers' implementation: - Upload mutations: each one writes `teamId = journey.teamId` for new rows. - Upload mutations: caller without journey-edit access cannot create rows (existing auth path). - Resolver: `teamId` omitted → returns own uploads only (v1 behavior preserved; drives the no-active-team fallback). -- Resolver: `teamId` provided + caller is member → returns the union of `userId = caller` and `teamId = arg`, ordered by `createdAt DESC, id DESC`. A row matching both sides appears exactly once. +- Resolver: `teamId` provided + caller is member → returns the union of `userId = caller` and `teamId = arg`, ordered by `createdAt DESC`. A row matching both sides appears exactly once. - Resolver: `teamId` provided + caller not member → `FORBIDDEN`. - Resolver: `teamId` for non-existent team → `FORBIDDEN` (same shape). - Resolver: NULL-`teamId` rows are excluded from the team side of the OR (only the original uploader sees them via the `userId` side).