diff --git a/desktop/src/components/Changes.svelte b/desktop/src/components/Changes.svelte index 234321cee..1a82f9bf1 100644 --- a/desktop/src/components/Changes.svelte +++ b/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 @@ { diff --git a/desktop/src/components/ExtendedTextarea.svelte b/desktop/src/components/ExtendedTextarea.svelte index fc529430c..b92f792c1 100644 --- a/desktop/src/components/ExtendedTextarea.svelte +++ b/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(), + embeds: initialEmbeds, submitInProgress = false, disableSubmit = false, disallowEmptyBody = false, @@ -98,6 +99,8 @@ }: Props = $props(); /* eslint-enable prefer-const */ + const embeds = new SvelteMap(untrack(() => initialEmbeds)); + const attachEnabled = $derived(attachDisabled === false); const attachDisabledReason = $derived( typeof attachDisabled === "string" ? attachDisabled : undefined, diff --git a/desktop/src/components/IssueDescription.svelte b/desktop/src/components/IssueDescription.svelte index e96abc9df..38b140352 100644 --- a/desktop/src/components/IssueDescription.svelte +++ b/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 @@ { diff --git a/desktop/src/components/Revision.svelte b/desktop/src/components/Revision.svelte index 672e9bf60..94356b35d 100644 --- a/desktop/src/components/Revision.svelte +++ b/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 @@ { @@ -1159,6 +1161,9 @@ { diff --git a/desktop/src/components/Thread.svelte b/desktop/src/components/Thread.svelte index 248b12fa7..f5a704f2c 100644 --- a/desktop/src/components/Thread.svelte +++ b/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 { - 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)} diff --git a/desktop/src/lib/embeds.ts b/desktop/src/lib/embeds.ts index aade10274..9d27be118 100644 --- a/desktop/src/lib/embeds.ts +++ b/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 { + 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; diff --git a/desktop/tests/e2e/repo/editingEmbeds.spec.ts b/desktop/tests/e2e/repo/editingEmbeds.spec.ts new file mode 100644 index 000000000..aba9f4a1a --- /dev/null +++ b/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]); +}); diff --git a/desktop/tests/e2e/repo/tasks.spec.ts b/desktop/tests/e2e/repo/tasks.spec.ts index 905c1ff93..ee695005e 100644 --- a/desktop/tests/e2e/repo/tasks.spec.ts +++ b/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, diff --git a/desktop/tests/support/api.ts b/desktop/tests/support/api.ts new file mode 100644 index 000000000..88cbc85df --- /dev/null +++ b/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; +} diff --git a/desktop/tests/unit/embeds.test.ts b/desktop/tests/unit/embeds.test.ts index 8d3e78e96..49957ca6e 100644 --- a/desktop/tests/unit/embeds.test.ts +++ b/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"],