From 71237cf5cfc0390e324171e3dd648454ae24f767 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:29:37 +0100 Subject: [PATCH 1/3] Add unit test for Tools copyLink utility --- client/src/components/Tool/utilities.test.ts | 29 ++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 client/src/components/Tool/utilities.test.ts diff --git a/client/src/components/Tool/utilities.test.ts b/client/src/components/Tool/utilities.test.ts new file mode 100644 index 000000000000..2a22aaddfbe9 --- /dev/null +++ b/client/src/components/Tool/utilities.test.ts @@ -0,0 +1,29 @@ +import { copyLink } from "./utilities"; + +const writeText = jest.fn(); + +Object.assign(navigator, { + clipboard: { + writeText, + }, +}); + +describe("copyLink", () => { + beforeEach(() => { + (navigator.clipboard.writeText as jest.Mock).mockResolvedValue(undefined); + }); + + it("should copy the link to the clipboard", () => { + const toolId = "MyToolId"; + copyLink(toolId); + expect(writeText).toHaveBeenCalledTimes(1); + expect(writeText).toHaveBeenCalledWith(expect.stringContaining(toolId)); + }); + + it("should encode the tool id", () => { + const toolId = "My Tool Id"; + copyLink(toolId); + expect(writeText).toHaveBeenCalledTimes(1); + expect(writeText).toHaveBeenCalledWith(expect.stringContaining("My%20Tool%20Id")); + }); +}); From 22d507112c72839216490069484b5cee6a468ca6 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:30:49 +0100 Subject: [PATCH 2/3] Fix tool link encoding Makes it easier to share when pasting it in oder contexts outside the web browser. --- client/src/components/Tool/utilities.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/src/components/Tool/utilities.js b/client/src/components/Tool/utilities.js index 1b0a6d5a0ad2..84f9d2bd3fd2 100644 --- a/client/src/components/Tool/utilities.js +++ b/client/src/components/Tool/utilities.js @@ -2,7 +2,9 @@ import { getAppRoot } from "onload/loadConfig"; import { copy } from "utils/clipboard"; export function copyLink(toolId, message) { - copy(`${window.location.origin + getAppRoot()}root?tool_id=${toolId}`, message); + const link = `${window.location.origin + getAppRoot()}root?tool_id=${toolId}`; + // Encode the link to handle special characters in tool id + copy(encodeURI(link), message); } export function copyId(toolId, message) { From d7877275610668b77fb0518d935781ad7733ddbe Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:53:17 +0100 Subject: [PATCH 3/3] Add test case for tool IDs with `+` --- client/src/components/Tool/utilities.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/src/components/Tool/utilities.test.ts b/client/src/components/Tool/utilities.test.ts index 2a22aaddfbe9..8295d1e2bfe6 100644 --- a/client/src/components/Tool/utilities.test.ts +++ b/client/src/components/Tool/utilities.test.ts @@ -20,10 +20,17 @@ describe("copyLink", () => { expect(writeText).toHaveBeenCalledWith(expect.stringContaining(toolId)); }); - it("should encode the tool id", () => { + it("should encode the tool id with spaces", () => { const toolId = "My Tool Id"; copyLink(toolId); expect(writeText).toHaveBeenCalledTimes(1); expect(writeText).toHaveBeenCalledWith(expect.stringContaining("My%20Tool%20Id")); }); + + it("should not encode the character '+' in the tool id", () => { + const toolId = "My Tool Id+1"; + copyLink(toolId); + expect(writeText).toHaveBeenCalledTimes(1); + expect(writeText).toHaveBeenCalledWith(expect.stringContaining("My%20Tool%20Id+1")); + }); });