From 2b17fc3221b77147db52e47d44cc5015f53926e1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 8 Aug 2023 15:08:17 +0100 Subject: [PATCH] Show error when searching public rooms fails (#11378) * Show error when searching public rooms fails * Fix types * Improve test coverage * Improve coverage --- .../dialogs/spotlight/SpotlightDialog.tsx | 29 ++++++++++----- src/hooks/usePublicRoomDirectory.ts | 7 +++- src/i18n/strings/en_EN.json | 3 +- .../views/dialogs/SpotlightDialog-test.tsx | 37 ++++++++++++++++++- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx index 9e66c3fd88f..1ac24c9e7be 100644 --- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx +++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx @@ -311,6 +311,7 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n config, setConfig, search: searchPublicRooms, + error: publicRoomsError, } = usePublicRoomDirectory(); const [showRooms, setShowRooms] = useState(true); const [showSpaces, setShowSpaces] = useState(false); @@ -757,6 +758,23 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n let publicRoomsSection: JSX.Element | undefined; if (filter === Filter.PublicRooms) { + let content: JSX.Element | JSX.Element[]; + if (!showRooms && !showSpaces) { + content = ( +
+ {_t("You cannot search for rooms that are neither a room nor a space")} +
+ ); + } else if (publicRoomsError) { + content = ( +
+ {_t("Failed to query public rooms")} +
+ ); + } else { + content = results[Section.PublicRooms].slice(0, SECTION_LIMIT).map(resultMapper); + } + publicRoomsSection = (
= ({ initialText = "", initialFilter = n
-
- {" "} - {showRooms || showSpaces ? ( - results[Section.PublicRooms].slice(0, SECTION_LIMIT).map(resultMapper) - ) : ( -
- {_t("You cannot search for rooms that are neither a room nor a space")} -
- )}{" "} -
+
{content}
); } diff --git a/src/hooks/usePublicRoomDirectory.ts b/src/hooks/usePublicRoomDirectory.ts index 531ae8d1ac7..3b15b9d7ab2 100644 --- a/src/hooks/usePublicRoomDirectory.ts +++ b/src/hooks/usePublicRoomDirectory.ts @@ -51,6 +51,7 @@ export const usePublicRoomDirectory = (): { config?: IPublicRoomDirectoryConfig | null; setConfig(config: IPublicRoomDirectoryConfig | null): void; search(opts: IPublicRoomsOpts): Promise; + error?: Error | true; // true if an unknown error is encountered } => { const [publicRooms, setPublicRooms] = useState([]); @@ -60,6 +61,7 @@ export const usePublicRoomDirectory = (): { const [ready, setReady] = useState(false); const [loading, setLoading] = useState(false); + const [error, setError] = useState(); const [updateQuery, updateResult] = useLatestResult(setPublicRooms); @@ -112,12 +114,14 @@ export const usePublicRoomDirectory = (): { } updateQuery(opts); + setLoading(true); + setError(undefined); try { - setLoading(true); const { chunk } = await MatrixClientPeg.safeGet().publicRooms(opts); updateResult(opts, showNsfwPublicRooms ? chunk : chunk.filter(cheapNsfwFilter)); return true; } catch (e) { + setError(e instanceof Error ? e : true); console.error("Could not fetch public rooms for params", opts, e); updateResult(opts, []); return false; @@ -183,5 +187,6 @@ export const usePublicRoomDirectory = (): { config, search, setConfig, + error, } as const; }; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 911d5f47eaa..f88443d625b 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3138,9 +3138,10 @@ "Search for": "Search for", "View": "View", "Spaces you're in": "Spaces you're in", + "You cannot search for rooms that are neither a room nor a space": "You cannot search for rooms that are neither a room nor a space", + "Failed to query public rooms": "Failed to query public rooms", "Show rooms": "Show rooms", "Show spaces": "Show spaces", - "You cannot search for rooms that are neither a room nor a space": "You cannot search for rooms that are neither a room nor a space", "Other rooms in %(spaceName)s": "Other rooms in %(spaceName)s", "Join %(roomAddress)s": "Join %(roomAddress)s", "Some results may be hidden for privacy": "Some results may be hidden for privacy", diff --git a/test/components/views/dialogs/SpotlightDialog-test.tsx b/test/components/views/dialogs/SpotlightDialog-test.tsx index 9c987e9c1ca..d9699983785 100644 --- a/test/components/views/dialogs/SpotlightDialog-test.tsx +++ b/test/components/views/dialogs/SpotlightDialog-test.tsx @@ -16,7 +16,14 @@ limitations under the License. import React from "react"; import { mocked } from "jest-mock"; -import { IProtocol, IPublicRoomsChunkRoom, MatrixClient, Room, RoomMember } from "matrix-js-sdk/src/matrix"; +import { + ConnectionError, + IProtocol, + IPublicRoomsChunkRoom, + MatrixClient, + Room, + RoomMember, +} from "matrix-js-sdk/src/matrix"; import sanitizeHtml from "sanitize-html"; import { fireEvent, render, screen } from "@testing-library/react"; @@ -495,4 +502,32 @@ describe("Spotlight Dialog", () => { expect(screen.getByText(potatoRoom.name!)).toBeInTheDocument(); }); }); + + it("should show error if /publicRooms API failed", async () => { + mocked(mockedClient.publicRooms).mockRejectedValue(new ConnectionError("Failed to fetch")); + render( null} />); + + jest.advanceTimersByTime(200); + await flushPromisesWithFakeTimers(); + + expect(screen.getByText("Failed to query public rooms")).toBeInTheDocument(); + }); + + it("should show error both 'Show rooms' and 'Show spaces' are unchecked", async () => { + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName, roomId, excludeDefault) => { + if (settingName === "feature_exploring_public_spaces") { + return true; + } else { + return []; // SpotlightSearch.recentSearches + } + }); + render( null} />); + + jest.advanceTimersByTime(200); + await flushPromisesWithFakeTimers(); + + fireEvent.click(screen.getByText("Show rooms")); + + expect(screen.getByText("You cannot search for rooms that are neither a room nor a space")).toBeInTheDocument(); + }); });