From c09ed9697811fee40949db83e91e4fd15021f300 Mon Sep 17 00:00:00 2001 From: Thomas Camlong Date: Sun, 2 Aug 2026 09:27:31 +0200 Subject: [PATCH] fix: address final widget review findings --- apps/docs/docs/widgets/timetable/index.mdx | 2 + e2e/health-checks.spec.ts | 9 ++- .../src/router/test/widgets/rssFeed.spec.ts | 20 ++++++ packages/api/src/router/widgets/rssFeed.ts | 25 ++++---- .../src/beszel-system-stats/component.tsx | 61 +++++++++++++++---- packages/widgets/src/rssFeed/component.tsx | 46 +++++++++++--- packages/widgets/src/timetable/component.tsx | 14 ++++- packages/widgets/src/timetable/index.ts | 9 ++- 8 files changed, 149 insertions(+), 37 deletions(-) diff --git a/apps/docs/docs/widgets/timetable/index.mdx b/apps/docs/docs/widgets/timetable/index.mdx index 75a7221e4..37f5abf05 100644 --- a/apps/docs/docs/widgets/timetable/index.mdx +++ b/apps/docs/docs/widgets/timetable/index.mdx @@ -36,3 +36,5 @@ import streetDepartures from "./img/street-departures.png"; For security, all custom base URLs must resolve to a public IP address whenever Homarr uses them. Private and local hosts are rejected. + +If a timetable refresh fails after departures have loaded, the widget keeps the previous departures visible and marks them with their last update time. diff --git a/e2e/health-checks.spec.ts b/e2e/health-checks.spec.ts index 411d8e29c..d2b179f4f 100644 --- a/e2e/health-checks.spec.ts +++ b/e2e/health-checks.spec.ts @@ -4,9 +4,12 @@ import { createHomarrContainer } from "./shared/create-homarr-container"; import { createRedisContainer } from "./shared/redis-container"; describe("Health checks", () => { - test("ready and live should return 200 OK with normal image and no extra configuration", async () => { + test("ready and live should return 200 OK with normal image and no extra configuration", async ({ + onTestFinished, + }) => { // Arrange const homarrContainer = await createHomarrContainer().start(); + onTestFinished(() => homarrContainer.stop()); try { // Act @@ -21,9 +24,10 @@ describe("Health checks", () => { } }, 60_000); - test("ready and live should return 200 OK with external redis", async () => { + test("ready and live should return 200 OK with external redis", async ({ onTestFinished }) => { // Arrange const redisContainer = await createRedisContainer().start(); + onTestFinished(() => redisContainer.stop()); const homarrContainer = await createHomarrContainer({ environment: { REDIS_IS_EXTERNAL: "true", @@ -32,6 +36,7 @@ describe("Health checks", () => { REDIS_PASSWORD: redisContainer.getPassword(), }, }).start(); + onTestFinished(() => homarrContainer.stop()); try { // Act diff --git a/packages/api/src/router/test/widgets/rssFeed.spec.ts b/packages/api/src/router/test/widgets/rssFeed.spec.ts index 74b2d318b..d5fef09b6 100644 --- a/packages/api/src/router/test/widgets/rssFeed.spec.ts +++ b/packages/api/src/router/test/widgets/rssFeed.spec.ts @@ -65,6 +65,26 @@ describe("rssFeedRouter", () => { expect(logged).not.toContain("token"); expect(logged).not.toContain("secret"); }); + + test("returns successful entries and only a failure count when some feeds fail", async () => { + const failedUrl = "https://user:password@example.com/feed?token=secret"; + mocks.getFeed.mockImplementation(async (input: { url: string }) => { + if (input.url === failedUrl) throw new Error(`Failed to fetch ${failedUrl}`); + return { data: { entries: [{ id: "entry" }] } }; + }); + const caller = rssFeedRouter.createCaller({ + db: createDb(), + deviceType: undefined, + session: createSession(["board-create"]), + }); + + const result = await caller.getFeeds({ urls: ["https://example.com/feed", failedUrl], maximumAmountPosts: 5 }); + + expect(result).toEqual({ entries: [{ id: "entry" }], failedFeedCount: 1 }); + expect(JSON.stringify(result)).not.toContain("password"); + expect(JSON.stringify(result)).not.toContain("token"); + expect(JSON.stringify(result)).not.toContain("secret"); + }); }); describe("canAccessAllFeedsAsync", () => { diff --git a/packages/api/src/router/widgets/rssFeed.ts b/packages/api/src/router/widgets/rssFeed.ts index 1dfb170df..5894dd8ac 100644 --- a/packages/api/src/router/widgets/rssFeed.ts +++ b/packages/api/src/router/widgets/rssFeed.ts @@ -65,17 +65,20 @@ export const rssFeedRouter = createTRPCRouter({ }); } - return settled - .flatMap((result) => { - if (result.status === "fulfilled") return result.value.data.entries; - return []; - }) - .toSorted((entryA, entryB) => { - return entryA.published && entryB.published - ? new Date(entryB.published).getTime() - new Date(entryA.published).getTime() - : 0; - }) - .slice(0, input.maximumAmountPosts); + return { + entries: settled + .flatMap((result) => { + if (result.status === "fulfilled") return result.value.data.entries; + return []; + }) + .toSorted((entryA, entryB) => { + return entryA.published && entryB.published + ? new Date(entryB.published).getTime() - new Date(entryA.published).getTime() + : 0; + }) + .slice(0, input.maximumAmountPosts), + failedFeedCount: failures.length, + }; }), }); diff --git a/packages/widgets/src/beszel-system-stats/component.tsx b/packages/widgets/src/beszel-system-stats/component.tsx index a3144e5b3..6e035a27c 100644 --- a/packages/widgets/src/beszel-system-stats/component.tsx +++ b/packages/widgets/src/beszel-system-stats/component.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useMemo } from "react"; +import { useCallback, useMemo, useRef } from "react"; import { Box, Button, Center, Loader, Menu, ScrollArea, SegmentedControl, Select, Stack, Text } from "@mantine/core"; import { IconQuestionMark, IconServer, IconServerOff } from "@tabler/icons-react"; @@ -36,13 +36,15 @@ export default function BeszelSystemStatsWidget({ const board = useOptionalBoard(); const { data: session } = useSession(); const hasChangeAccess = board ? constructBoardPermissions(board, session).hasChangeAccess : false; - const { mutate: saveItemOptions } = clientApi.widget.options.saveItemOptions.useMutation({ - onError: () => - showErrorNotification({ - title: t("error.selectionSaveTitle"), - message: t("error.selectionSaveMessage"), - }), - }); + const selectionSavePendingRef = useRef(false); + const { mutate: saveItemOptions, isPending: isSelectionSavePending } = + clientApi.widget.options.saveItemOptions.useMutation({ + onError: () => + showErrorNotification({ + title: t("error.selectionSaveTitle"), + message: t("error.selectionSaveMessage"), + }), + }); const { data: systemsResult = [], isPending: systemsPending, @@ -76,22 +78,51 @@ export default function BeszelSystemStatsWidget({ const handleSelectSystem = useCallback( (value: string) => { + if (selectionSavePendingRef.current) return; + + const previousValue = options.systemId; setOptions({ newOptions: { systemId: value } }); if (hasChangeAccess && boardId && itemId) { - saveItemOptions({ boardId, itemId, newOptions: { systemId: value } }); + selectionSavePendingRef.current = true; + saveItemOptions( + { boardId, itemId, newOptions: { systemId: value } }, + { + onError: () => { + setOptions({ newOptions: { systemId: previousValue } }); + }, + onSettled: () => { + selectionSavePendingRef.current = false; + }, + }, + ); } }, - [setOptions, hasChangeAccess, boardId, itemId, saveItemOptions], + [boardId, hasChangeAccess, itemId, options.systemId, saveItemOptions, setOptions], ); const handleTimePeriod = useCallback( (value: string) => { - setOptions({ newOptions: { timePeriod: value as BeszelTimePeriod } }); + if (selectionSavePendingRef.current) return; + + const timePeriod = value as BeszelTimePeriod; + const previousValue = options.timePeriod; + setOptions({ newOptions: { timePeriod } }); if (hasChangeAccess && boardId && itemId) { - saveItemOptions({ boardId, itemId, newOptions: { timePeriod: value } }); + selectionSavePendingRef.current = true; + saveItemOptions( + { boardId, itemId, newOptions: { timePeriod } }, + { + onError: () => { + setOptions({ newOptions: { timePeriod: previousValue } }); + }, + onSettled: () => { + selectionSavePendingRef.current = false; + }, + }, + ); } }, - [boardId, hasChangeAccess, itemId, saveItemOptions, setOptions], + [boardId, hasChangeAccess, itemId, options.timePeriod, saveItemOptions, setOptions], ); if (systemsError) throw systemsError; @@ -131,6 +162,7 @@ export default function BeszelSystemStatsWidget({ data={systems} value={selectedValue} placeholder={t("selectSystem")} + disabled={isSelectionSavePending} onChange={(value) => value && handleSelectSystem(value)} /> @@ -157,6 +189,7 @@ export default function BeszelSystemStatsWidget({ size="compact-xs" leftSection={} className={classes.beszelStatsSystemToggle} + disabled={isSelectionSavePending} > {selectedLabel} @@ -168,6 +201,7 @@ export default function BeszelSystemStatsWidget({ fz="xs" fw={system.value === selectedValue ? 600 : 400} c={system.value !== selectedValue ? "dimmed" : undefined} + disabled={isSelectionSavePending} onClick={() => handleSelectSystem(system.value)} > {system.label} @@ -181,6 +215,7 @@ export default function BeszelSystemStatsWidget({ size="xs" value={options.timePeriod} onChange={handleTimePeriod} + disabled={isSelectionSavePending} data={[ { value: "1m", label: t("period.live") }, { value: "1h", label: t("period.oneHour") }, diff --git a/packages/widgets/src/rssFeed/component.tsx b/packages/widgets/src/rssFeed/component.tsx index 1a30e0352..b09a94623 100644 --- a/packages/widgets/src/rssFeed/component.tsx +++ b/packages/widgets/src/rssFeed/component.tsx @@ -1,30 +1,55 @@ "use client"; -import { Card, Flex, Group, Image, ScrollArea, SimpleGrid, Text } from "@mantine/core"; -import { IconClock } from "@tabler/icons-react"; +import { Alert, Card, Flex, Group, Image, ScrollArea, SimpleGrid, Text } from "@mantine/core"; +import { IconAlertTriangle, IconClock } from "@tabler/icons-react"; import dayjs from "dayjs"; -import type { RouterInputs } from "@homarr/api"; +import type { RouterInputs, RouterOutputs } from "@homarr/api"; import { clientApi } from "@homarr/api/client"; import { useRequiredBoard } from "@homarr/boards/context"; +import { useScopedI18n } from "@homarr/translation/client"; import type { WidgetComponentProps } from "../definition"; import classes from "./component.module.scss"; -const useLiveFeedEntries = (input: RouterInputs["widget"]["rssFeed"]["getFeeds"]) => { - const { data: feedEntries, error } = clientApi.widget.rssFeed.getFeeds.useQuery(input); - if (error && feedEntries === undefined) throw error; +const useLiveFeed = (input: RouterInputs["widget"]["rssFeed"]["getFeeds"]) => { + const { data, error } = clientApi.widget.rssFeed.getFeeds.useQuery(input); + if (error && data === undefined) throw error; - return feedEntries ?? []; + // Persisted query caches can still contain the array response used before failed-feed metadata was added. + const normalizedData = data as typeof data | RouterOutputs["widget"]["rssFeed"]["getFeeds"]["entries"]; + if (Array.isArray(normalizedData)) { + return { + entries: normalizedData, + failedFeedCount: 0, + isStale: Boolean(error), + }; + } + + return { + entries: normalizedData?.entries ?? [], + failedFeedCount: normalizedData?.failedFeedCount ?? 0, + isStale: Boolean(error), + }; }; export default function RssFeed({ options, width, displayMode }: WidgetComponentProps<"rssFeed">) { - const feedEntries = useLiveFeedEntries({ + const { + entries: feedEntries, + failedFeedCount, + isStale, + } = useLiveFeed({ urls: options.feedUrls, maximumAmountPosts: typeof options.maximumAmountPosts === "number" ? options.maximumAmountPosts : 100, }); const board = useRequiredBoard(); + const t = useScopedI18n("widget.rssFeed"); + const warning = isStale + ? t("warning.stale") + : failedFeedCount > 0 + ? t("warning.partial", { count: failedFeedCount }) + : undefined; const languageDir = options.enableRtl ? "RTL" : "LTR"; @@ -34,6 +59,11 @@ export default function RssFeed({ options, width, displayMode }: WidgetComponent return ( + {warning && ( + } p="xs" mb="xs"> + {warning} + + )} {feedEntries.map((feedEntry) => ( { - const { data: timetable } = clientApi.widget.timetable.getTimetable.useQuery({ + const { data: timetable, error } = clientApi.widget.timetable.getTimetable.useQuery({ baseUrl, itemId, stationId: station.value, @@ -47,11 +48,20 @@ const TimetableWidgetInner = ({ station, baseUrl, itemId, displayMode, width }: }); const t = useScopedI18n("widget.timetable"); + if (error && timetable === undefined) throw error; + const entries = timetable?.entries ?? []; + const staleWarning = + error && timetable ? t("warning.stale", { time: dayjs(timetable.timestamp).format("HH:mm:ss") }) : undefined; return ( {t("title", { station: station.label })} + {staleWarning && ( + } p="xs"> + {staleWarning} + + )} = 760 ? 2 : 1} spacing="xs"> {entries.map((entry) => ( diff --git a/packages/widgets/src/timetable/index.ts b/packages/widgets/src/timetable/index.ts index 212e2746b..81ec81db8 100644 --- a/packages/widgets/src/timetable/index.ts +++ b/packages/widgets/src/timetable/index.ts @@ -1,4 +1,4 @@ -import { IconBusStop } from "@tabler/icons-react"; +import { IconBusStop, IconWorldOff } from "@tabler/icons-react"; import { z } from "zod/v4"; import { clientApi } from "@homarr/api/client"; @@ -9,6 +9,13 @@ import { optionsBuilder } from "../options"; export const { componentLoader, definition } = createWidgetDefinition("timetable", { icon: IconBusStop, + errors: { + BAD_REQUEST: { + icon: IconWorldOff, + message: (t) => t("widget.timetable.error.customSourceUnavailable"), + hideLogsLink: true, + }, + }, createOptions() { return optionsBuilder.from((factory) => ({ baseUrl: factory.text({