Let an image show another image's picture

convex/function_spec.json

@@ -48407,6 +48407,12 @@
       "args": {
         "type": "object",
         "value": {
+          "acceptsPictureIds": {
+            "fieldType": {
+              "type": "boolean"
+            },
+            "optional": true
+          },
           "accountSubject": {
             "fieldType": {
               "type": "string"
@@ -144197,6 +144203,12 @@
       "args": {
         "type": "object",
         "value": {
+          "acceptsPictureIds": {
+            "fieldType": {
+              "type": "boolean"
+            },
+            "optional": true
+          },
           "clientProtocolVersion": {
             "fieldType": {
               "type": "number"
@@ -169716,6 +169728,12 @@
       "args": {
         "type": "object",
         "value": {
+          "acceptsPictureIds": {
+            "fieldType": {
+              "type": "boolean"
+            },
+            "optional": true
+          },
           "acceptsTrashedPagesLeftOut": {
             "fieldType": {
               "type": "boolean"

convex/images.ts

@@ -13,6 +13,7 @@ import {
   inferProvider,
   inferUploadStatus,
   isUploadPlaceholder,
+  pictureShownByImage,
   serializeAssetForViewer,
   staleUploadAgeMs,
   type Provider,
@@ -799,16 +800,18 @@ export const getAssetUrl = query({
     const strategy = await getStrategyByPublicId(ctx, args.strategyPublicId);
     await assertStrategyReadable(ctx, strategy, args.shareToken);
 
-    if (
-      !(await strategyReferencesAsset(ctx, strategy._id, args.assetPublicId))
-    ) {
+    // Builds from before pictures had their own id ask by the image's id.
+    const pictureId =
+      (await pictureShownByImage(ctx, strategy._id, args.assetPublicId)) ??
+      args.assetPublicId;
+    if (!(await strategyReferencesAsset(ctx, strategy._id, pictureId))) {
       throw notFoundError("Asset", args.assetPublicId);
     }
 
     const asset = await getActiveAssetForStrategy(
       ctx,
       strategy._id,
-      args.assetPublicId,
+      pictureId,
     );
     if (asset === null) {
       throw notFoundError("Asset", args.assetPublicId);

convex/lib/imageAssets.ts

@@ -46,10 +46,54 @@ export function inferFileExtension(
   return match?.[1]?.toLowerCase() ?? "";
 }
 
+/// The picture an image element shows: its `assetId` when it has one, else
+/// its own id. A copy of an image is a new element showing its original's
+/// picture, so it needs no picture of its own, and is never waiting on an
+/// upload its original is still making. Only images have an `assetId`.
 export function collectAssetIdFromElementPayload(
   payload: Doc<"elements">["payload"],
 ): string | null {
-  return typeof payload.data.id === "string" ? payload.data.id : null;
+  const { assetId, id } = payload.data;
+  if (typeof assetId === "string" && assetId.length > 0) return assetId;
+  return typeof id === "string" ? id : null;
+}
+
+/// [next] keeping the picture [current] shows, when it leaves the picture
+/// out. An image never changes picture, and builds from before pictures had
+/// their own id write an image's whole payload without the field: moving a
+/// copy there must not cut it off from its picture.
+export function keepPictureId(
+  current: Doc<"elements">["payload"],
+  next: Doc<"elements">["payload"],
+): Doc<"elements">["payload"] {
+  const assetId = current.data.assetId;
+  if (typeof assetId !== "string" || "assetId" in next.data) return next;
+  return { ...next, data: { ...next.data, assetId } };
+}
+
+/// [assets] as builds from before pictures had their own id can read them:
+/// they look an image's picture up under the image's own id. Each live image
+/// showing another id's picture gets that picture under its own id too.
+export function withPictureAliases<T extends { publicId: string }>(
+  assets: T[],
+  elements: Doc<"elements">[],
+): T[] {
+  const byId = new Map(assets.map((asset) => [asset.publicId, asset]));
+  const aliases: T[] = [];
+  for (const element of elements) {
+    if (element.deleted || element.elementType !== "image") continue;
+    const id = element.payload.data.id;
+    const pictureId = collectAssetIdFromElementPayload(element.payload);
+    if (typeof id !== "string" || pictureId === null || pictureId === id) {
+      continue;
+    }
+    const picture = byId.get(pictureId);
+    if (picture === undefined || byId.has(id)) continue;
+    const alias = { ...picture, publicId: id };
+    byId.set(id, alias);
+    aliases.push(alias);
+  }
+  return [...assets, ...aliases];
 }
 
 /// The images a lineup group shows: links[*].images[*].id, across every
@@ -141,18 +185,69 @@ export async function getActiveAssetForStrategy(
     return strategyAsset;
   }
 
-  const legacyCandidates = await ctx.db
-    .query("imageAssets")
-    .withIndex("by_publicId", (q) => q.eq("publicId", assetPublicId))
-    .order("desc")
-    .take(20);
-  return (
-    legacyCandidates.find(
-      (asset) =>
-        (asset.strategyId === undefined || asset.strategyId === strategyId) &&
-        isVisibleAsset(asset),
-    ) ?? null
-  );
+  // The other rows that can still show the picture: this strategy's from
+  // before upload statuses, then rows of no strategy, active or from before
+  // statuses. Each set is read from its own index range, so neither copies
+  // in other strategies (which keep pictures' ids) nor failed upload
+  // attempts can crowd it out. Five each, with the active row and an upload
+  // placeholder, stays within the 22 rows a copy's budget charges for
+  // (convex/lib/contentCopy.ts).
+  const ranges = [
+    [strategyId, undefined],
+    [undefined, "active"],
+    [undefined, undefined],
+  ] as const;
+  for (const [owner, uploadStatus] of ranges) {
+    const candidates = await ctx.db
+      .query("imageAssets")
+      .withIndex("by_strategyId_and_publicId_and_uploadStatus", (q) =>
+        q
+          .eq("strategyId", owner)
+          .eq("publicId", assetPublicId)
+          .eq("uploadStatus", uploadStatus),
+      )
+      .order("desc")
+      .take(5);
+    const visible = candidates.find(isVisibleAsset);
+    if (visible !== undefined) return visible;
+  }
+  return null;
+}
+
+/// The picture image [itemPublicId] of the strategy shows, when the image
+/// shows another's (see collectAssetIdFromElementPayload): builds from
+/// before pictures had their own id ask for a picture by the image's id.
+export async function pictureShownByImage(
+  ctx: AnyCtx,
+  strategyId: Id<"strategies">,
+  itemPublicId: string,
+): Promise<string | null> {
+  const element = await ctx.db
+    .query("elements")
+    .withIndex("by_publicId", (q) => q.eq("publicId", itemPublicId))
+    .first();
+  if (
+    element === null ||
+    element.deleted ||
+    element.strategyId !== strategyId ||
+    element.elementType !== "image"
+  ) {
+    return null;
+  }
+  const pictureId = collectAssetIdFromElementPayload(element.payload);
+  return pictureId === itemPublicId ? null : pictureId;
+}
+
+/// [payload] as builds from before pictures had their own id hold it: they
+/// keep no `assetId`, so one sent to them reads as a change they made.
+/// They find the picture by the image's own id (withPictureAliases), and a
+/// write of theirs keeps the stored picture (keepPictureId).
+export function withoutPictureId(
+  payload: Doc<"elements">["payload"],
+): Doc<"elements">["payload"] {
+  if (!("assetId" in payload.data)) return payload;
+  const { assetId: _assetId, ...data } = payload.data;
+  return { ...payload, data };
 }
 
 /// A row recording that a strategy's content shows an image whose upload has

convex/lib/snapshotSerialization.ts

@@ -1,4 +1,5 @@
 import type { Doc } from "../_generated/dataModel";
+import { withoutPictureId } from "./imageAssets";
 
 export function serializeStrategyHeader(
   strategy: Doc<"strategies">,
@@ -45,17 +46,22 @@ export function serializePageContent(pageContent: Doc<"pageContents">) {
   };
 }
 
+/// [element] as a client sees it. One that doesn't [acceptsPictureIds]
+/// gets its payload without the picture id (see withoutPictureId).
 export function serializeElement(
   strategyPublicId: string,
   pagePublicId: string,
   element: Doc<"elements">,
+  acceptsPictureIds: boolean,
 ) {
   return {
     publicId: element.publicId,
     strategyPublicId,
     pagePublicId,
     elementType: element.elementType,
-    payload: element.payload,
+    payload: acceptsPictureIds
+      ? element.payload
+      : withoutPictureId(element.payload),
     sortIndex: element.sortIndex,
     revision: element.revision,
     deleted: element.deleted,

convex/ops.ts

@@ -8,7 +8,9 @@ import {
 } from "./lib/strategyAgentSummary";
 import {
   expectAssets,
+  keepPictureId,
   referencedAssetIds,
+  withoutPictureId,
   staleUploadAgeMs,
 } from "./lib/imageAssets";
 import {
@@ -784,6 +786,7 @@ function isRejectionReason(
 function toPublicResult(
   op: StrategyOp,
   result: OperationResult,
+  acceptsPictureIds: boolean,
 ): PublicOperationResult {
   if (result.status === "failed") {
     return {
@@ -827,7 +830,10 @@ function toPublicResult(
           current: {
             type: currentTargetForOp(op),
             revision: result.latestRevision,
-            value: result.latestPayload,
+            value:
+              op.entityType === "element" && !acceptsPictureIds
+                ? withoutPictureId(result.latestPayload as ElementPayload)
+                : result.latestPayload,
           } as Infer<typeof currentOpSnapshotValidator>,
         }),
   };
@@ -1231,7 +1237,9 @@ async function applyElementOp(
       throw errorWithCode("MISSING_PAGE_PUBLIC_ID", "Missing pagePublicId");
     }
     const page = await requireTargetPage(ctx, strategy, op.pagePublicId);
-    const payload = assertElementPayload(op.payload);
+    const sent = assertElementPayload(op.payload);
+    const payload =
+      existing === null ? sent : keepPictureId(existing.payload, sent);
     if (existing !== null) {
       if (existing.strategyId !== strategy._id) {
         return rejected("element_strategy_mismatch");
@@ -1379,6 +1387,7 @@ async function applyElementOp(
           checkRevision = true;
         }
       }
+      payload = keepPictureId(existing.payload, payload);
       setIfChanged(patch, "payload", existing.payload, payload);
       setIfChanged(patch, "payloadKind", existing.payloadKind, payload.kind);
       setIfChanged(
@@ -1776,6 +1785,11 @@ export const applyBatch = mutation({
     // once the page is back. Older clients get the no-op a deleted page's
     // purged content gave them (see refuseDeleteOffLivePage).
     checkTrashedPageDeletes: v.optional(v.boolean()),
+    // Set by clients that keep an image's picture id (assetId, see
+    // collectAssetIdFromElementPayload). Older clients get image payloads
+    // without it, as they would write them, and find pictures under each
+    // image's own id (withPictureAliases).
+    acceptsPictureIds: v.optional(v.boolean()),
     // Sent by clients on protocol 4, which stored lineups as origin,
     // landing and link rows. Ignored: accepting them lets such a client
     // reach the protocol gate (CLIENT_UPGRADE_REQUIRED) instead of failing
@@ -1845,7 +1859,9 @@ export const applyBatch = mutation({
                   latestPayload: latest?.payload,
                 }
               : noop(existingEvent.appliedRevision);
-        results.push(toPublicResult(op, replayResult));
+        results.push(
+          toPublicResult(op, replayResult, args.acceptsPictureIds === true),
+        );
         continue;
       }
 
@@ -1951,7 +1967,11 @@ export const applyBatch = mutation({
         acceptedStrategyBatchBaseRevision = originalExpectedRevision;
       }
 
-      const publicResult = toPublicResult(op, result);
+      const publicResult = toPublicResult(
+        op,
+        result,
+        args.acceptsPictureIds === true,
+      );
 
       await ctx.db.insert("operationEvents", {
         strategyId: strategy._id,

convex/page.ts

@@ -8,6 +8,7 @@ import {
   collectReferencedAssetIds,
   getViewerAssetForStrategy,
   serializeAssetForViewer,
+  withPictureAliases,
 } from "./lib/imageAssets";
 import {
   serializeElement,
@@ -27,6 +28,11 @@ export const getSnapshot = query({
     strategyPublicId: v.string(),
     pagePublicId: v.string(),
     shareToken: v.optional(v.string()),
+    // Set by clients that keep an image's picture id (assetId, see
+    // collectAssetIdFromElementPayload). Older clients get image payloads
+    // without it, as they would write them, and find pictures under each
+    // image's own id (withPictureAliases).
+    acceptsPictureIds: v.optional(v.boolean()),
   },
   returns: pageSnapshotValidator,
   handler: async (ctx, args) => {
@@ -69,19 +75,25 @@ export const getSnapshot = query({
       elements: elements
         .sort((left, right) => left.sortIndex - right.sortIndex)
         .map((element) =>
-          serializeElement(strategy.publicId, page.publicId, element),
+          serializeElement(
+            strategy.publicId,
+            page.publicId,
+            element,
+            args.acceptsPictureIds === true,
+          ),
         ),
       lineups: lineups
         .sort((left, right) => left.sortIndex - right.sortIndex)
         .map((lineup) =>
           serializeLineup(strategy.publicId, page.publicId, lineup),
         ),
-      assets: (
+      assets: withPictureAliases(
         await Promise.all(
           assets
             .filter((asset): asset is Doc<"imageAssets"> => asset !== null)
             .map((asset) => serializeAssetForViewer(ctx, asset)),
-        )
+        ),
+        elements,
       ).sort((left, right) => left.publicId.localeCompare(right.publicId)),
     };
   },