desktop: Save embeds when editing a review summary

The summary editor allowed attaching files but never sent them, so the
link ended up in the summary while the file never reached the COB.
Verdict changes and task toggles also sent no embeds, which would drop
them again once a summary has some.

desktop/crates/radicle-types/bindings/cob/patch/Review.ts

@@ -2,6 +2,7 @@
 import type { Author } from "../Author";
 import type { CodeLocation } from "../thread/CodeLocation";
 import type { Comment } from "../thread/Comment";
+import type { Embed } from "../thread/Embed";
 import type { Verdict } from "./Verdict";
 
 export type Review = {
@@ -12,4 +13,5 @@ export type Review = {
   comments: Array<Comment<CodeLocation>>;
   timestamp: number;
   labels: Array<string>;
+  embeds: Array<Embed>;
 };

desktop/crates/radicle-types/src/domain/patch/models/patch.rs

@@ -325,6 +325,7 @@ pub struct Review {
     timestamp: cob::common::Timestamp,
     #[ts(as = "Vec<String>")]
     labels: Vec<cob::Label>,
+    embeds: Vec<cobs::thread::Embed>,
 }
 
 impl Review {
@@ -346,6 +347,7 @@ impl Review {
                 })
                 .collect::<Vec<_>>(),
             timestamp: review.timestamp(),
+            embeds: review.embeds().iter().cloned().map(Into::into).collect(),
         }
     }
 }

desktop/src/components/ReviewPage.svelte

@@ -29,6 +29,7 @@
     fullFileLoader,
     gitStatusEntries,
   } from "@app/lib/diffText";
+  import { embedMap } from "@app/lib/embeds";
   import { nodeRunning } from "@app/lib/events";
   import {
     cachedGetDiff,
@@ -180,7 +181,7 @@
     }
   }
 
-  async function saveSummary(newSummary: string) {
+  async function saveSummary(newSummary: string, embeds: Embed[]) {
     try {
       savingSummary = true;
       await invoke("edit_patch", {
@@ -192,6 +193,7 @@
           summary: newSummary,
           verdict: review.verdict,
           labels: review.labels,
+          embeds,
         },
         opts: { announce: $nodeRunning && $announce },
       });
@@ -241,6 +243,7 @@
           summary: review.summary ?? "",
           verdict,
           labels: review.labels,
+          embeds: review.embeds,
         },
         opts: { announce: $nodeRunning && $announce },
       });
@@ -957,11 +960,12 @@
         <ExtendedTextarea
           {rid}
           body={review.summary ?? ""}
+          embeds={embedMap(review.embeds)}
           focus
           submitCaption={savingSummary ? "Saving…" : "Save"}
           disableSubmit={savingSummary}
-          submit={async ({ comment }) => {
-            await saveSummary(comment);
+          submit={async ({ comment, embeds }) => {
+            await saveSummary(comment, Array.from(embeds.values()));
           }}
           close={() => (editingSummary = false)} />
       </div>
@@ -974,7 +978,7 @@
             content={review.summary ?? ""}
             toggleTaskItem={isOwnPublishedReview
               ? async content => {
-                  await saveSummary(content);
+                  await saveSummary(content, review.embeds);
                 }
               : undefined} />
         {:else}

desktop/src/components/Revision.svelte

@@ -606,6 +606,7 @@
     reviewId: string,
     summary: string,
     review: { verdict?: Verdict; labels?: string[] },
+    embeds: Embed[],
   ) {
     try {
       await invoke("edit_patch", {
@@ -617,6 +618,7 @@
           summary,
           verdict: review.verdict,
           labels: review.labels ?? [],
+          embeds,
         },
         opts: { announce: $nodeRunning && $announce },
       });
@@ -1426,6 +1428,10 @@
           r => r.id === opId,
         )}
         {@const hasReviewComments = (reviewRecord?.comments?.length ?? 0) > 0}
+        {@const reviewEmbeds =
+          [revision, ...revisions]
+            .flatMap(r => r.reviews ?? [])
+            .find(r => r.id === opId)?.embeds ?? []}
         {@const reviewOp = data.op}
         <ReviewItem
           {rid}
@@ -1436,7 +1442,7 @@
           toggleTaskItem={data.op.author.did ===
           didFromPublicKey(config.publicKey)
             ? async summary => {
-                await editReviewSummary(opId, summary, reviewOp);
+                await editReviewSummary(opId, summary, reviewOp, reviewEmbeds);
               }
             : undefined}
           onViewFullReview={hasReviewComments

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

@@ -3,6 +3,9 @@ import type { Revision } from "@bindings/cob/patch/Revision";
 import type { Locator, Page } from "@playwright/test";
 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 { createProject } from "@tests/support/collaboration.js";
 import { expect, test, waitForCommand } from "@tests/support/fixtures.js";
@@ -57,6 +60,43 @@ async function patchWithEmbed(page: Page, peer: RadiclePeer) {
   return { rid, patchId, embed, link };
 }
 
+async function reviewWithEmbed(page: Page, peer: RadiclePeer) {
+  const { rid, patchId } = await createProject(peer);
+  const { embed, link } = await megaphone(page, peer, rid);
+  const reviewId = (await api(page, peer, "create_patch_review", {
+    args: {
+      rid,
+      revision: patchId,
+      verdict: "accept",
+      summary: "Looks fine",
+      labels: [],
+      // Only reviews with comments get a page of their own.
+      comments: [{ body: "Loud enough", location: null }],
+    },
+  })) as string;
+  await api(page, peer, "edit_patch", {
+    rid,
+    cobId: patchId,
+    action: {
+      type: "review.edit",
+      review: reviewId,
+      summary: `Looks fine\n\n${link}`,
+      verdict: "accept",
+      labels: [],
+      embeds: [embed],
+    },
+    opts,
+  });
+  await page.goto(`/repos/${rid}/patches/${patchId}?review=${reviewId}`);
+  await expect(page.getByText("Looks fine")).toBeVisible();
+
+  const storedEmbeds = async () =>
+    (await storedRevision(page, peer, rid, patchId)).reviews?.find(
+      review => review.id === reviewId,
+    )?.embeds;
+  return { embed, link, storedEmbeds };
+}
+
 async function saveEdit(page: Page, command: string, body: string) {
   await page
     .getByRole("textbox", { name: "textarea-comment" })
@@ -190,3 +230,87 @@ test("editing a patch description on the changes tab keeps its embeds", async ({
   expect(description.at(-1)?.body).toContain("so make it loud");
   expect(description.at(-1)?.embeds).toEqual([embed]);
 });
+
+test("editing a review summary keeps its embeds", async ({ page, peer }) => {
+  const { embed, link, storedEmbeds } = await reviewWithEmbed(page, peer);
+
+  await page.getByText("Looks fine").hover();
+  await page.getByTitle("Edit summary").click();
+  await saveEdit(page, "edit_patch", `Too loud\n\n${link}`);
+
+  await expect(page.getByText("Too loud")).toBeVisible();
+  expect(await storedEmbeds()).toEqual([embed]);
+});
+
+test("ticking a task in a review of an older revision keeps its embeds", async ({
+  page,
+  peer,
+}) => {
+  const { rid, repoFolder, patchId } = await createProject(peer);
+  const { embed, link } = await megaphone(page, peer, rid);
+  const reviewId = (await api(page, peer, "create_patch_review", {
+    args: {
+      rid,
+      revision: patchId,
+      verdict: "accept",
+      summary: "Before merging",
+      labels: [],
+      comments: [],
+    },
+  })) as string;
+  await api(page, peer, "edit_patch", {
+    rid,
+    cobId: patchId,
+    action: {
+      type: "review.edit",
+      review: reviewId,
+      summary: `Before merging:\n\n- [ ] Rename the file\n\n${link}`,
+      verdict: "accept",
+      labels: [],
+      embeds: [embed],
+    },
+    opts,
+  });
+  await Fs.writeFile(
+    Path.join(repoFolder, "notes.txt"),
+    "one\ntwo\nTHREE!\nfour\nfive\n",
+  );
+  await peer.git(["commit", "-am", "Shout louder"], { cwd: repoFolder });
+  await peer.git(
+    [
+      "push",
+      "-o",
+      "patch.message=Shout louder",
+      "rad",
+      `HEAD:refs/heads/patches/${patchId}`,
+    ],
+    { cwd: repoFolder },
+  );
+
+  await page.goto(`/repos/${rid}/patches/${patchId}`);
+  const rename = page.getByRole("checkbox", { name: "Rename the file" });
+  await waitForCommand(page, "edit_patch", () => rename.click());
+  await expect(rename).toBeChecked();
+
+  const revisions = (await api(page, peer, "revisions_by_patch", {
+    rid,
+    id: patchId,
+  })) as Revision[];
+  const review = revisions
+    .flatMap(revision => revision.reviews ?? [])
+    .find(review => review.id === reviewId);
+  expect(review?.summary).toContain("- [x] Rename the file");
+  expect(review?.embeds).toEqual([embed]);
+});
+
+test("changing a review's verdict keeps its embeds", async ({ page, peer }) => {
+  const { embed, storedEmbeds } = await reviewWithEmbed(page, peer);
+
+  await page.getByRole("button", { name: /Accepted/ }).click();
+  await waitForCommand(page, "edit_patch", () =>
+    page.getByRole("button", { name: /Reject$/ }).click(),
+  );
+
+  await expect(page.getByRole("button", { name: /Rejected/ })).toBeVisible();
+  expect(await storedEmbeds()).toEqual([embed]);
+});

desktop/tests/unit/support/cobs.ts

@@ -56,6 +56,7 @@ export function review(props: Partial<Review> = {}): Review {
     comments: [],
     timestamp: 0,
     labels: [],
+    embeds: [],
     ...props,
   };
 }