diff --git a/desktop/crates/radicle-types/bindings/cob/patch/Review.ts b/desktop/crates/radicle-types/bindings/cob/patch/Review.ts index 53bcaf083..c2de3d933 100644 --- a/desktop/crates/radicle-types/bindings/cob/patch/Review.ts +++ b/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>; timestamp: number; labels: Array; + embeds: Array; }; diff --git a/desktop/crates/radicle-types/src/domain/patch/models/patch.rs b/desktop/crates/radicle-types/src/domain/patch/models/patch.rs index 25b8382b2..f73bab8c3 100644 --- a/desktop/crates/radicle-types/src/domain/patch/models/patch.rs +++ b/desktop/crates/radicle-types/src/domain/patch/models/patch.rs @@ -325,6 +325,7 @@ pub struct Review { timestamp: cob::common::Timestamp, #[ts(as = "Vec")] labels: Vec, + embeds: Vec, } impl Review { @@ -346,6 +347,7 @@ impl Review { }) .collect::>(), timestamp: review.timestamp(), + embeds: review.embeds().iter().cloned().map(Into::into).collect(), } } } diff --git a/desktop/src/components/ReviewPage.svelte b/desktop/src/components/ReviewPage.svelte index 26487e393..66a2172b2 100644 --- a/desktop/src/components/ReviewPage.svelte +++ b/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 @@ { - await saveSummary(comment); + submit={async ({ comment, embeds }) => { + await saveSummary(comment, Array.from(embeds.values())); }} close={() => (editingSummary = false)} /> @@ -974,7 +978,7 @@ content={review.summary ?? ""} toggleTaskItem={isOwnPublishedReview ? async content => { - await saveSummary(content); + await saveSummary(content, review.embeds); } : undefined} /> {:else} diff --git a/desktop/src/components/Revision.svelte b/desktop/src/components/Revision.svelte index 94356b35d..11d96bcec 100644 --- a/desktop/src/components/Revision.svelte +++ b/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} { - await editReviewSummary(opId, summary, reviewOp); + await editReviewSummary(opId, summary, reviewOp, reviewEmbeds); } : undefined} onViewFullReview={hasReviewComments diff --git a/desktop/tests/e2e/repo/editingEmbeds.spec.ts b/desktop/tests/e2e/repo/editingEmbeds.spec.ts index aba9f4a1a..ce911dab3 100644 --- a/desktop/tests/e2e/repo/editingEmbeds.spec.ts +++ b/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]); +}); diff --git a/desktop/tests/unit/support/cobs.ts b/desktop/tests/unit/support/cobs.ts index d5114f486..8306610bb 100644 --- a/desktop/tests/unit/support/cobs.ts +++ b/desktop/tests/unit/support/cobs.ts @@ -56,6 +56,7 @@ export function review(props: Partial = {}): Review { comments: [], timestamp: 0, labels: [], + embeds: [], ...props, }; }