fix: address final widget review findings
This commit is contained in:
1 parent
44b55a401c
commit
c09ed96978
8 files changed
+149
-37
No files matched your search
@@ -36,3 +36,5 @@ import streetDepartures from "./img/street-departures.png";
|
||||
<WidgetConfig configuration={timetableWidget.configuration} />
|
||||
|
||||
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.
|
||||
@@ -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
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}),
|
||||
});
|
||||
|
||||
|
||||
@@ -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)}
|
||||
/>
|
||||
</Stack>
|
||||
@@ -157,6 +189,7 @@ export default function BeszelSystemStatsWidget({
|
||||
size="compact-xs"
|
||||
leftSection={<IconServer size={14} />}
|
||||
className={classes.beszelStatsSystemToggle}
|
||||
disabled={isSelectionSavePending}
|
||||
>
|
||||
{selectedLabel}
|
||||
</Button>
|
||||
@@ -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") },
|
||||
|
||||
@@ -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 (
|
||||
<ScrollArea className="scroll-area-w100" w="100%" h="100%" p={isAdvanced ? "md" : "xs"}>
|
||||
{warning && (
|
||||
<Alert role="status" color="orange" icon={<IconAlertTriangle aria-hidden size={16} />} p="xs" mb="xs">
|
||||
{warning}
|
||||
</Alert>
|
||||
)}
|
||||
<SimpleGrid cols={columns} w="100%" spacing={isAdvanced ? "md" : "xs"} verticalSpacing={isAdvanced ? "md" : "xs"}>
|
||||
{feedEntries.map((feedEntry) => (
|
||||
<Card
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { Badge, Center, Group, ScrollArea, SimpleGrid, Stack, Text } from "@mantine/core";
|
||||
import { Alert, Badge, Center, Group, ScrollArea, SimpleGrid, Stack, Text } from "@mantine/core";
|
||||
import { IconAlertTriangle } from "@tabler/icons-react";
|
||||
import dayjs from "dayjs";
|
||||
|
||||
import { clientApi } from "@homarr/api/client";
|
||||
@@ -39,7 +40,7 @@ interface TimetableWidgetInnerProps {
|
||||
}
|
||||
|
||||
const TimetableWidgetInner = ({ station, baseUrl, itemId, displayMode, width }: TimetableWidgetInnerProps) => {
|
||||
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 (
|
||||
<Stack w="100%" h="100%" gap="xs" p="sm">
|
||||
<Text fw="bold">{t("title", { station: station.label })}</Text>
|
||||
{staleWarning && (
|
||||
<Alert role="status" color="orange" icon={<IconAlertTriangle aria-hidden size={16} />} p="xs">
|
||||
{staleWarning}
|
||||
</Alert>
|
||||
)}
|
||||
<ScrollArea style={{ flex: 1, minHeight: 0 }}>
|
||||
<SimpleGrid cols={displayMode === "advanced" && width >= 760 ? 2 : 1} spacing="xs">
|
||||
{entries.map((entry) => (
|
||||
|
||||
@@ -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({
|
||||
|
||||
Reference in new issue
Block a user