From d1753c33f54ebb24f6bd51f41cf68141c3f41a3f Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 31 Mar 2025 16:38:25 +0100 Subject: [PATCH] Use correct rageshake URL when running in embedded package + tests (#3132) * Use correct rageshake URL when running in embedded package It was incorrectly trying to use the one from config.json * Refactor to add tests * Empty mock config --- src/settings/SettingsModal.tsx | 6 +- src/settings/submit-rageshake.test.ts | 68 +------ src/settings/submit-rageshake.ts | 17 +- src/settings/useSubmitRageshake.test.tsx | 222 +++++++++++++++++++++++ 4 files changed, 235 insertions(+), 78 deletions(-) create mode 100644 src/settings/useSubmitRageshake.test.tsx diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 7522983e..de717b02 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -30,7 +30,7 @@ import { PreferencesSettingsTab } from "./PreferencesSettingsTab"; import { Slider } from "../Slider"; import { DeviceSelection } from "./DeviceSelection"; import { DeveloperSettingsTab } from "./DeveloperSettingsTab"; -import { isRageshakeAvailable } from "./submit-rageshake"; +import { useSubmitRageshake } from "./submit-rageshake"; type SettingsTab = | "audio" @@ -71,6 +71,8 @@ export const SettingsModal: FC = ({ const [showDeveloperSettingsTab] = useSetting(developerMode); + const { available: isRageshakeAvailable } = useSubmitRageshake(); + const audioTab: Tab = { key: "audio", name: t("common.audio"), @@ -148,7 +150,7 @@ export const SettingsModal: FC = ({ const tabs = [audioTab, videoTab]; if (widget === null) tabs.push(profileTab); tabs.push(preferencesTab); - if (isRageshakeAvailable() || import.meta.env.VITE_PACKAGE === "full") { + if (isRageshakeAvailable || import.meta.env.VITE_PACKAGE === "full") { // for full package we want to show the analytics consent checkbox // even if rageshake is not available tabs.push(feedbackTab); diff --git a/src/settings/submit-rageshake.test.ts b/src/settings/submit-rageshake.test.ts index 3433856a..1f143a6d 100644 --- a/src/settings/submit-rageshake.test.ts +++ b/src/settings/submit-rageshake.test.ts @@ -15,78 +15,12 @@ import { beforeEach, } from "vitest"; -import { - getRageshakeSubmitUrl, - isRageshakeAvailable, -} from "./submit-rageshake"; +import { getRageshakeSubmitUrl } from "./submit-rageshake"; import { getUrlParams } from "../UrlParams"; import { mockConfig } from "../utils/test"; vi.mock("../UrlParams", () => ({ getUrlParams: vi.fn() })); -describe("isRageshakeAvailable", () => { - beforeEach(() => { - (getUrlParams as Mock).mockReturnValue({}); - mockConfig({}); - }); - - afterEach(() => { - vi.unstubAllEnvs(); - vi.clearAllMocks(); - }); - - describe("embedded package", () => { - beforeEach(() => { - vi.stubEnv("VITE_PACKAGE", "embedded"); - }); - - it("returns false with no rageshakeSubmitUrl URL param", () => { - expect(isRageshakeAvailable()).toBe(false); - }); - - it("ignores config value and returns false with no rageshakeSubmitUrl URL param", () => { - mockConfig({ - rageshake: { - submit_url: "https://config.example.com.localhost", - }, - }); - expect(isRageshakeAvailable()).toBe(false); - }); - - it("returns true with rageshakeSubmitUrl URL param", () => { - (getUrlParams as Mock).mockReturnValue({ - rageshakeSubmitUrl: "https://url.example.com.localhost", - }); - expect(isRageshakeAvailable()).toBe(true); - }); - }); - - describe("full package", () => { - beforeEach(() => { - vi.stubEnv("VITE_PACKAGE", "full"); - }); - it("returns false with no config value", () => { - expect(isRageshakeAvailable()).toBe(false); - }); - - it("ignores rageshakeSubmitUrl URL param and returns false with no config value", () => { - (getUrlParams as Mock).mockReturnValue({ - rageshakeSubmitUrl: "https://url.example.com.localhost", - }); - expect(isRageshakeAvailable()).toBe(false); - }); - - it("returns true with config value", () => { - mockConfig({ - rageshake: { - submit_url: "https://config.example.com.localhost", - }, - }); - expect(isRageshakeAvailable()).toBe(true); - }); - }); -}); - describe("getRageshakeSubmitUrl", () => { beforeEach(() => { (getUrlParams as Mock).mockReturnValue({}); diff --git a/src/settings/submit-rageshake.ts b/src/settings/submit-rageshake.ts index b3138348..bfd55126 100644 --- a/src/settings/submit-rageshake.ts +++ b/src/settings/submit-rageshake.ts @@ -131,11 +131,9 @@ export function getRageshakeSubmitUrl(): string | undefined { return undefined; } -export function isRageshakeAvailable(): boolean { - return !!getRageshakeSubmitUrl(); -} - -export function useSubmitRageshake(): { +export function useSubmitRageshake( + injectedGetRageshakeSubmitUrl = getRageshakeSubmitUrl, +): { submitRageshake: (opts: RageShakeSubmitOptions) => Promise; sending: boolean; sent: boolean; @@ -158,7 +156,8 @@ export function useSubmitRageshake(): { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore async (opts) => { - if (!getRageshakeSubmitUrl()) { + const submitUrl = injectedGetRageshakeSubmitUrl(); + if (!submitUrl) { throw new Error("No rageshake URL is configured"); } @@ -292,7 +291,7 @@ export function useSubmitRageshake(): { ); } - const res = await fetch(Config.get().rageshake!.submit_url, { + const res = await fetch(submitUrl, { method: "POST", body, }); @@ -309,7 +308,7 @@ export function useSubmitRageshake(): { logger.error(error); } }, - [client, sending], + [client, sending, injectedGetRageshakeSubmitUrl], ); return { @@ -317,7 +316,7 @@ export function useSubmitRageshake(): { sending, sent, error, - available: isRageshakeAvailable(), + available: !!injectedGetRageshakeSubmitUrl(), }; } diff --git a/src/settings/useSubmitRageshake.test.tsx b/src/settings/useSubmitRageshake.test.tsx new file mode 100644 index 00000000..b278d4b1 --- /dev/null +++ b/src/settings/useSubmitRageshake.test.tsx @@ -0,0 +1,222 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { + expect, + describe, + it, + vi, + beforeEach, + afterEach, + type Mock, +} from "vitest"; +import { useState, type ReactElement } from "react"; +import { render, screen, waitFor } from "@testing-library/react"; +import { type MatrixClient } from "matrix-js-sdk/lib/client"; + +import { useSubmitRageshake, getRageshakeSubmitUrl } from "./submit-rageshake"; +import { ClientContextProvider } from "../ClientContext"; +import { getUrlParams } from "../UrlParams"; +import { mockConfig } from "../utils/test"; + +vi.mock("../UrlParams", () => ({ getUrlParams: vi.fn() })); + +const TestComponent = ({ + sendLogs, + getRageshakeSubmitUrl, +}: { + sendLogs: boolean; + getRageshakeSubmitUrl: () => string | undefined; +}): ReactElement => { + const [clickError, setClickError] = useState(null); + const { available, sending, sent, submitRageshake, error } = + useSubmitRageshake(getRageshakeSubmitUrl); + + const onClick = (): void => { + submitRageshake({ + sendLogs, + }).catch((e) => { + setClickError(e); + }); + }; + + return ( +
+

{available ? "true" : "false"}

+

{sending ? "true" : "false"}

+

{sent ? "true" : "false"}

+

{error?.message}

+

{clickError?.message}

+ +
+ ); +}; + +function renderWithMockClient( + getRageshakeSubmitUrl: () => string | undefined, + sendLogs: boolean, +): void { + const client = vi.mocked({ + getUserId: vi.fn().mockReturnValue("@user:localhost"), + getUser: vi.fn().mockReturnValue(null), + credentials: { + userId: "@user:localhost", + }, + getCrypto: vi.fn().mockReturnValue(undefined), + } as unknown as MatrixClient); + + render( + + + , + ); +} + +describe("useSubmitRageshake", () => { + describe("available", () => { + beforeEach(() => { + (getUrlParams as Mock).mockReturnValue({}); + mockConfig({}); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + }); + + describe("embedded package", () => { + beforeEach(() => { + vi.stubEnv("VITE_PACKAGE", "embedded"); + }); + + it("returns false with no rageshakeSubmitUrl URL param", () => { + renderWithMockClient(getRageshakeSubmitUrl, false); + expect(screen.getByTestId("available").textContent).toBe("false"); + }); + + it("ignores config value and returns false with no rageshakeSubmitUrl URL param", () => { + mockConfig({ + rageshake: { + submit_url: "https://config.example.com.localhost", + }, + }); + renderWithMockClient(getRageshakeSubmitUrl, false); + expect(screen.getByTestId("available").textContent).toBe("false"); + }); + + it("returns true with rageshakeSubmitUrl URL param", () => { + (getUrlParams as Mock).mockReturnValue({ + rageshakeSubmitUrl: "https://url.example.com.localhost", + }); + renderWithMockClient(getRageshakeSubmitUrl, false); + expect(screen.getByTestId("available").textContent).toBe("true"); + }); + }); + + describe("full package", () => { + beforeEach(() => { + mockConfig({}); + vi.stubEnv("VITE_PACKAGE", "full"); + }); + it("returns false with no config value", () => { + renderWithMockClient(getRageshakeSubmitUrl, false); + expect(screen.getByTestId("available").textContent).toBe("false"); + }); + + it("ignores rageshakeSubmitUrl URL param and returns false with no config value", () => { + (getUrlParams as Mock).mockReturnValue({ + rageshakeSubmitUrl: "https://url.example.com.localhost", + }); + renderWithMockClient(getRageshakeSubmitUrl, false); + expect(screen.getByTestId("available").textContent).toBe("false"); + }); + + it("returns true with config value", () => { + mockConfig({ + rageshake: { + submit_url: "https://config.example.com.localhost", + }, + }); + renderWithMockClient(getRageshakeSubmitUrl, false); + expect(screen.getByTestId("available").textContent).toBe("true"); + }); + }); + }); + + describe("when rageshake is available", () => { + beforeEach(() => { + mockConfig({}); + vi.unstubAllGlobals(); + }); + + it("starts unsent", () => { + renderWithMockClient(() => "https://rageshake.localhost/foo", false); + expect(screen.getByTestId("sending").textContent).toBe("false"); + expect(screen.getByTestId("sent").textContent).toBe("false"); + }); + + it("submitRageshake fetches expected URL", async () => { + const fetchFn = vi.fn().mockResolvedValue({ + status: 200, + }); + vi.stubGlobal("fetch", fetchFn); + + renderWithMockClient(() => "https://rageshake.localhost/foo", false); + screen.getByTestId("submit").click(); + await waitFor(() => { + expect(screen.getByTestId("sent").textContent).toBe("true"); + }); + expect(fetchFn).toHaveBeenCalledExactlyOnceWith( + "https://rageshake.localhost/foo", + expect.objectContaining({ + method: "POST", + }), + ); + expect(screen.getByTestId("clickError").textContent).toBe(""); + expect(screen.getByTestId("error").textContent).toBe(""); + }); + }); + + describe("when rageshake is not available", () => { + it("starts unsent", () => { + renderWithMockClient(() => undefined, false); + expect(screen.getByTestId("sending").textContent).toBe("false"); + expect(screen.getByTestId("sent").textContent).toBe("false"); + }); + + it("submitRageshake throws error", async () => { + renderWithMockClient(() => undefined, false); + screen.getByTestId("submit").click(); + await waitFor(() => { + expect(screen.getByTestId("clickError").textContent).toBe( + "No rageshake URL is configured", + ); + }); + }); + }); +});