desktop: Keep embeds when editing a description

desktop/src/components/Changes.svelte

@@ -22,6 +22,7 @@
     fullFileLoader,
   } from "@app/lib/diffText";
   import { draftReviewStorage } from "@app/lib/draftReviewStorage";
+  import { embedMap } from "@app/lib/embeds";
   import {
     cachedDiffStats,
     cachedGetDiff,
@@ -828,6 +829,7 @@
         <ExtendedTextarea
           {rid}
           body={showRevisionDescription ? revisionDescription : ""}
+          embeds={embedMap(revision.description.slice(-1)[0]?.embeds)}
           focus
           submitCaption="Save"
           submit={async ({ comment, embeds }) => {

desktop/src/components/ExtendedTextarea.svelte

@@ -5,6 +5,7 @@
 
   import { open } from "@tauri-apps/plugin-dialog";
   import debounce from "lodash/debounce";
+  import { untrack } from "svelte";
   import { SvelteMap } from "svelte/reactivity";
 
   import { basename, encodeEmbedUpload } from "@app/lib/embeds";
@@ -79,7 +80,7 @@
     focus = false,
     inline = false,
     body = $bindable(""),
-    embeds = new SvelteMap<string, Embed>(),
+    embeds: initialEmbeds,
     submitInProgress = false,
     disableSubmit = false,
     disallowEmptyBody = false,
@@ -98,6 +99,8 @@
   }: Props = $props();
   /* eslint-enable prefer-const */
 
+  const embeds = new SvelteMap<string, Embed>(untrack(() => initialEmbeds));
+
   const attachEnabled = $derived(attachDisabled === false);
   const attachDisabledReason = $derived(
     typeof attachDisabled === "string" ? attachDisabled : undefined,

desktop/src/components/IssueDescription.svelte

@@ -3,6 +3,8 @@
   import type { Reaction } from "@bindings/cob/Reaction";
   import type { Embed } from "@bindings/cob/thread/Embed";
 
+  import { embedMap } from "@app/lib/embeds";
+
   import ExtendedTextarea from "@app/components/ExtendedTextarea.svelte";
   import Icon from "@app/components/Icon.svelte";
   import Markdown from "@app/components/Markdown.svelte";
@@ -90,6 +92,7 @@
     <ExtendedTextarea
       {rid}
       {body}
+      embeds={embedMap(embeds)}
       focus
       submitCaption="Save"
       submit={async ({ comment, embeds }) => {

desktop/src/components/Revision.svelte

@@ -24,6 +24,7 @@
   import { diffOptions } from "@app/lib/diffOptions.svelte";
   import { fileDiffPath, fileStatusLabel } from "@app/lib/diffText";
   import { draftReviewStorage } from "@app/lib/draftReviewStorage";
+  import { embedMap } from "@app/lib/embeds";
   import { nodeRunning } from "@app/lib/events";
   import { isIgnoredFile } from "@app/lib/ignoredFiles";
   import {
@@ -1043,6 +1044,7 @@
       <ExtendedTextarea
         {rid}
         body={body ?? ""}
+        embeds={embedMap(revision.description.slice(-1)[0]?.embeds)}
         focus
         submitCaption="Save"
         submit={async ({ comment, embeds }) => {
@@ -1159,6 +1161,9 @@
                   <ExtendedTextarea
                     {rid}
                     body={descriptionEditorPrefill}
+                    embeds={embedMap(
+                      targetRev?.description.slice(-1)[0]?.embeds,
+                    )}
                     focus
                     submitCaption="Save"
                     submit={async ({ comment, embeds }) => {

desktop/src/components/Thread.svelte

@@ -7,9 +7,9 @@
   import type { Snippet } from "svelte";
 
   import { tick } from "svelte";
-  import { SvelteMap } from "svelte/reactivity";
 
   import type { Resolution } from "@app/lib/commentResolutions";
+  import { embedMap } from "@app/lib/embeds";
   import { formatResolvedCaption, scrollIntoView } from "@app/lib/utils";
 
   import CommentComponent from "@app/components/Comment.svelte";
@@ -79,15 +79,6 @@
     resolvedBy,
   }: Props = $props();
 
-  function embedsOf(comment: { embeds?: Embed[] }): Map<string, Embed> {
-    return new SvelteMap(
-      (comment.embeds ?? []).map(embed => [
-        embed.content.replace(/^git:/, ""),
-        embed,
-      ]),
-    );
-  }
-
   function resolvedCaption(commentId: string): string | undefined {
     const resolution = resolvedBy?.(commentId);
     return (
@@ -228,7 +219,7 @@
           reactions={reply.reactions}
           timestamp={reply.edits[0].timestamp}
           body={reply.edits.slice(-1)[0].body}
-          embeds={embedsOf(reply)}
+          embeds={embedMap(reply.embeds)}
           editComment={canModifyComment(reply.author.did) &&
             editComment?.bind(null, reply.id)}
           reactOnComment={reactOnComment?.bind(null, reply.id)}
@@ -282,7 +273,7 @@
       reactions={root.reactions}
       timestamp={root.edits.slice(-1)[0].timestamp}
       body={root.edits.slice(-1)[0].body}
-      embeds={embedsOf(root)}
+      embeds={embedMap(root.embeds)}
       editComment={canModifyComment(root.author.did) &&
         editComment?.bind(null, root.id)}
       reactOnComment={reactOnComment?.bind(null, root.id)}

desktop/src/lib/embeds.ts

@@ -1,3 +1,13 @@
+import type { Embed } from "@bindings/cob/thread/Embed";
+
+import { SvelteMap } from "svelte/reactivity";
+
+export function embedMap(embeds: Embed[] = []): Map<string, Embed> {
+  return new SvelteMap(
+    embeds.map(embed => [embed.content.replace(/^git:/, ""), embed]),
+  );
+}
+
 export function basename(path: string): string {
   const parts = path.split(/[\\/]/).filter(p => p.length > 0);
   return parts[parts.length - 1] ?? path;

desktop/tests/e2e/repo/editingEmbeds.spec.ts

@@ -0,0 +1,192 @@
+import type { Issue } from "@bindings/cob/issue/Issue";
+import type { Revision } from "@bindings/cob/patch/Revision";
+import type { Locator, Page } from "@playwright/test";
+import type { RadiclePeer } from "@tests/support/peerManager.js";
+
+import { api, saveEmbed } from "@tests/support/api.js";
+import { createProject } from "@tests/support/collaboration.js";
+import { expect, test, waitForCommand } from "@tests/support/fixtures.js";
+
+// An edit replaces the embed list of what it edits, so saving an edit that
+// doesn't carry the earlier embeds forward drops them, while the body still
+// links them.
+
+const opts = { announce: false };
+
+async function megaphone(page: Page, peer: RadiclePeer, rid: string) {
+  const oid = await saveEmbed(
+    page,
+    peer,
+    rid,
+    "megaphone.txt",
+    new TextEncoder().encode("loud"),
+  );
+  return {
+    embed: { name: "megaphone.txt", content: `git:${oid}` },
+    link: `[megaphone.txt](${oid})`,
+  };
+}
+
+async function storedRevision(
+  page: Page,
+  peer: RadiclePeer,
+  rid: string,
+  patchId: string,
+) {
+  const revisions = (await api(page, peer, "revisions_by_patch", {
+    rid,
+    id: patchId,
+  })) as Revision[];
+  return revisions[0];
+}
+
+async function patchWithEmbed(page: Page, peer: RadiclePeer) {
+  const { rid, patchId } = await createProject(peer);
+  const { embed, link } = await megaphone(page, peer, rid);
+  await api(page, peer, "edit_patch", {
+    rid,
+    cobId: patchId,
+    action: {
+      type: "revision.edit",
+      revision: patchId,
+      description: `Shout the third line\n\nReaders skim, so the key line should stand out.\n\n${link}`,
+      embeds: [embed],
+    },
+    opts,
+  });
+  return { rid, patchId, embed, link };
+}
+
+async function saveEdit(page: Page, command: string, body: string) {
+  await page
+    .getByRole("textbox", { name: "textarea-comment" })
+    .first()
+    .fill(body);
+  await waitForCommand(page, command, () =>
+    page.getByRole("button", { name: /^Save/ }).click(),
+  );
+}
+
+async function pasteFile(textarea: Locator, name: string) {
+  await waitForCommand(textarea.page(), "save_embed_by_bytes", () =>
+    textarea.evaluate((element, name) => {
+      const clipboardData = new DataTransfer();
+      clipboardData.items.add(new File(["loud"], name, { type: "text/plain" }));
+      element.dispatchEvent(
+        new ClipboardEvent("paste", {
+          clipboardData,
+          bubbles: true,
+          cancelable: true,
+        }),
+      );
+    }, name),
+  );
+  const pattern = new RegExp(`\\[${name.replace(".", "\\.")}\\]\\((\\w+)\\)`);
+  await expect(textarea).toHaveValue(pattern);
+  return pattern.exec(await textarea.inputValue())?.[1];
+}
+
+test("editing an issue description keeps its embeds", async ({
+  page,
+  peer,
+}) => {
+  const { rid, issueId } = await createProject(peer);
+  const { embed, link } = await megaphone(page, peer, rid);
+  await api(page, peer, "edit_issue", {
+    rid,
+    cobId: issueId,
+    action: {
+      type: "comment.edit",
+      id: issueId,
+      body: `Nobody reads them.\n\n${link}`,
+      embeds: [embed],
+    },
+    opts,
+  });
+
+  await page.goto(`/repos/${rid}/issues/${issueId}`);
+  await page.getByText("Nobody reads them.").hover();
+  await page.getByTitle("Edit description").click();
+  await saveEdit(page, "edit_issue", `Nobody hears them.\n\n${link}`);
+
+  const issue = (await api(page, peer, "issue_by_id", {
+    rid,
+    id: issueId,
+  })) as Issue;
+  expect(issue.body?.edits.at(-1)?.body).toContain("Nobody hears them.");
+  expect(issue.body?.embeds).toEqual([embed]);
+});
+
+test("a file attached before the issue reloads is still saved", async ({
+  page,
+  peer,
+}) => {
+  const { rid, issueId } = await createProject(peer);
+  await page.goto(`/repos/${rid}/issues/${issueId}`);
+  await page.getByText("Nobody reads them.").hover();
+  await page.getByTitle("Edit description").click();
+  const oid = await pasteFile(
+    page.getByPlaceholder("Leave your comment"),
+    "notes.txt",
+  );
+
+  // Posting a comment reloads the issue while the description is being edited.
+  await page.getByPlaceholder("Leave a comment").fill("Louder, please");
+  await waitForCommand(page, "create_issue_comment", () =>
+    page.getByRole("button", { name: /^Comment/ }).click(),
+  );
+  await expect(page.getByText("Louder, please")).toBeVisible();
+  await waitForCommand(page, "edit_issue", () =>
+    page.getByRole("button", { name: /^Save/ }).click(),
+  );
+
+  const issue = (await api(page, peer, "issue_by_id", {
+    rid,
+    id: issueId,
+  })) as Issue;
+  expect(issue.body?.embeds).toEqual([
+    { name: "notes.txt", content: `git:${oid}` },
+  ]);
+});
+
+test("editing a patch description keeps its embeds", async ({ page, peer }) => {
+  const { rid, patchId, embed, link } = await patchWithEmbed(page, peer);
+
+  await page.goto(`/repos/${rid}/patches/${patchId}`);
+  await page.getByText("Readers skim").first().hover();
+  await page.getByTitle("Edit description").first().click();
+  await saveEdit(
+    page,
+    "edit_patch",
+    `Shout the third line\n\nReaders skim, so make it loud.\n\n${link}`,
+  );
+
+  const description = (await storedRevision(page, peer, rid, patchId))
+    .description;
+  expect(description.at(-1)?.body).toContain("so make it loud");
+  expect(description.at(-1)?.embeds).toEqual([embed]);
+});
+
+test("editing a patch description on the changes tab keeps its embeds", async ({
+  page,
+  peer,
+}) => {
+  const { rid, patchId, embed, link } = await patchWithEmbed(page, peer);
+
+  await page.goto(`/repos/${rid}/patches/${patchId}?view=changes`);
+  // The diff moves the description into place when it first renders, and a
+  // click that lands during the move is dropped.
+  await expect(page.locator("[data-line-type]").first()).toBeVisible();
+  await page.getByText("Readers skim").hover();
+  await page.getByTitle("Edit description").click();
+  await saveEdit(
+    page,
+    "edit_patch",
+    `Readers skim, so make it loud.\n\n${link}`,
+  );
+
+  const description = (await storedRevision(page, peer, rid, patchId))
+    .description;
+  expect(description.at(-1)?.body).toContain("so make it loud");
+  expect(description.at(-1)?.embeds).toEqual([embed]);
+});

desktop/tests/e2e/repo/tasks.spec.ts

@@ -6,6 +6,7 @@ import type { RadiclePeer } from "@tests/support/peerManager.js";
 import * as Fs from "node:fs/promises";
 import * as Path from "node:path";
 
+import { api, saveEmbed } from "@tests/support/api.js";
 import * as issue from "@tests/support/cobs/issue.js";
 import * as patch from "@tests/support/cobs/patch.js";
 import {
@@ -30,29 +31,6 @@ const onePixelPng = Buffer.from(
   "base64",
 );
 
-function api(page: Page, peer: RadiclePeer, command: string, data: object) {
-  return page.request
-    .post(`http://127.0.0.1:${peer.httpdBaseUrl.port}/${command}`, { data })
-    .then(response => response.json());
-}
-
-async function saveEmbed(
-  page: Page,
-  peer: RadiclePeer,
-  rid: string,
-  name: string,
-  bytes: Uint8Array,
-) {
-  const response = await page.request.post(
-    `http://127.0.0.1:${peer.httpdBaseUrl.port}/save_embed_by_bytes`,
-    {
-      headers: { rid, "Content-Type": "application/octet-stream" },
-      data: Buffer.concat([Buffer.from(`${name}\0`), bytes]),
-    },
-  );
-  return (await response.json()) as string;
-}
-
 async function createComment(
   page: Page,
   peer: RadiclePeer,

desktop/tests/support/api.ts

@@ -0,0 +1,32 @@
+import type { Page } from "@playwright/test";
+import type { RadiclePeer } from "@tests/support/peerManager.js";
+
+// Calls a backend command directly, to set up or read back state that the UI
+// test doesn't go through.
+export function api(
+  page: Page,
+  peer: RadiclePeer,
+  command: string,
+  data: object,
+) {
+  return page.request
+    .post(`http://127.0.0.1:${peer.httpdBaseUrl.port}/${command}`, { data })
+    .then(response => response.json());
+}
+
+export async function saveEmbed(
+  page: Page,
+  peer: RadiclePeer,
+  rid: string,
+  name: string,
+  bytes: Uint8Array,
+) {
+  const response = await page.request.post(
+    `http://127.0.0.1:${peer.httpdBaseUrl.port}/save_embed_by_bytes`,
+    {
+      headers: { rid, "Content-Type": "application/octet-stream" },
+      data: Buffer.concat([Buffer.from(`${name}\0`), bytes]),
+    },
+  );
+  return (await response.json()) as string;
+}

desktop/tests/unit/embeds.test.ts

@@ -3,11 +3,30 @@ import { describe, expect, test } from "vitest";
 import {
   basename,
   decodeEmbed,
+  embedMap,
   embedPreviewKind,
   encodeEmbedUpload,
   retinaWidth,
 } from "@app/lib/embeds";
 
+describe("embedMap", () => {
+  test("keys embeds by their oid", () => {
+    const embed = { name: "board.png", content: "git:a1b2c3" };
+    expect(Array.from(embedMap([embed]))).toEqual([["a1b2c3", embed]]);
+  });
+
+  test("is empty without embeds", () => {
+    expect(embedMap().size).toBe(0);
+    expect(embedMap([]).size).toBe(0);
+  });
+
+  test("keeps the last of two embeds with the same oid", () => {
+    const first = { name: "old.png", content: "git:a1b2c3" };
+    const second = { name: "new.png", content: "git:a1b2c3" };
+    expect(Array.from(embedMap([first, second]).values())).toEqual([second]);
+  });
+});
+
 describe("basename", () => {
   test.each([
     ["/home/alice/cat.png", "cat.png"],