fix: address mobile dashboard review findings
This commit is contained in:
1 parent
438f0d80eb
commit
6a2e7d0610
12 files changed
+107
-20
No files matched your search
@@ -13,7 +13,7 @@
|
||||
padding: 0;
|
||||
margin: -1px;
|
||||
overflow: hidden;
|
||||
clip: rect(0, 0, 0, 0);
|
||||
clip-path: inset(50%);
|
||||
white-space: nowrap;
|
||||
border: 0;
|
||||
}
|
||||
|
||||
@@ -44,6 +44,7 @@ export const beszelRouter = createTRPCRouter({
|
||||
};
|
||||
},
|
||||
{
|
||||
throwOnAllFailure: true,
|
||||
fallback: (integration, error) => ({
|
||||
integrationId: integration.id,
|
||||
integrationName: integration.name,
|
||||
@@ -116,6 +117,7 @@ export const beszelRouter = createTRPCRouter({
|
||||
};
|
||||
},
|
||||
{
|
||||
throwOnAllFailure: true,
|
||||
fallback: (integration, error) => ({
|
||||
integrationId: integration.id,
|
||||
integrationName: integration.name,
|
||||
|
||||
@@ -23,6 +23,22 @@ describe("settleIntegrationQueries", () => {
|
||||
expect(results).toEqual(["Second"]);
|
||||
});
|
||||
|
||||
it("retains fallback and successful results for a partial failure", async () => {
|
||||
const results = await settleIntegrationQueries(
|
||||
integrations,
|
||||
vi.fn(async (integration) => {
|
||||
if (integration.id === "first") throw new Error("offline");
|
||||
return integration.name;
|
||||
}),
|
||||
{
|
||||
throwOnAllFailure: true,
|
||||
fallback: (integration) => `${integration.name} unavailable`,
|
||||
},
|
||||
);
|
||||
|
||||
expect(results).toEqual(["First unavailable", "Second"]);
|
||||
});
|
||||
|
||||
it("throws when every integration fails and throwOnAllFailure is enabled", async () => {
|
||||
await expect(
|
||||
settleIntegrationQueries(
|
||||
@@ -35,6 +51,21 @@ describe("settleIntegrationQueries", () => {
|
||||
).rejects.toThrow("offline");
|
||||
});
|
||||
|
||||
it("throws on total failure even when fallbacks are configured", async () => {
|
||||
await expect(
|
||||
settleIntegrationQueries(
|
||||
integrations,
|
||||
vi.fn(async () => {
|
||||
throw new Error("offline");
|
||||
}),
|
||||
{
|
||||
throwOnAllFailure: true,
|
||||
fallback: (integration) => `${integration.name} unavailable`,
|
||||
},
|
||||
),
|
||||
).rejects.toThrow("offline");
|
||||
});
|
||||
|
||||
it("does not treat an empty integration list as a failure", async () => {
|
||||
await expect(
|
||||
settleIntegrationQueries(
|
||||
|
||||
@@ -5,6 +5,7 @@ import { useI18n } from "@homarr/translation/client";
|
||||
|
||||
import { WidgetMobileLoading, WidgetMobileSummary } from "../../common/mobile-summary";
|
||||
import { useBeszelFilteredSystems } from "./hooks";
|
||||
import { hasStaleIntegrationData } from "./query-status";
|
||||
|
||||
interface BeszelSystemsMobileSummaryProps {
|
||||
integrationIds: string[];
|
||||
@@ -18,12 +19,18 @@ export const BeszelSystemsMobileSummary = ({
|
||||
statusFilter,
|
||||
}: BeszelSystemsMobileSummaryProps) => {
|
||||
const t = useI18n();
|
||||
const { data: results = [], error, isPending } = clientApi.widget.beszel.getSystems.useQuery({ integrationIds });
|
||||
const {
|
||||
data: results = [],
|
||||
error,
|
||||
isPending,
|
||||
isLoadingError,
|
||||
isRefetchError,
|
||||
} = clientApi.widget.beszel.getSystems.useQuery({ integrationIds });
|
||||
const systems = useBeszelFilteredSystems(results, statusFilter);
|
||||
const failedResults = results.filter((result) => "error" in result);
|
||||
|
||||
if (isPending) return <WidgetMobileLoading />;
|
||||
if (error) throw error;
|
||||
if (isLoadingError) throw error;
|
||||
if (results.length > 0 && failedResults.length === results.length) {
|
||||
throw new Error(String(failedResults[0]?.error ?? "Unable to connect to Beszel"));
|
||||
}
|
||||
@@ -38,7 +45,7 @@ export const BeszelSystemsMobileSummary = ({
|
||||
up,
|
||||
attention: systems.length - up,
|
||||
})}
|
||||
isStale={failedResults.length > 0}
|
||||
isStale={hasStaleIntegrationData(isRefetchError, failedResults)}
|
||||
/>
|
||||
);
|
||||
};
|
||||
@@ -0,0 +1,11 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
|
||||
import { hasStaleIntegrationData } from "./query-status";
|
||||
|
||||
describe("Beszel query status", () => {
|
||||
test("marks cached data stale after outer or per-integration failures", () => {
|
||||
expect(hasStaleIntegrationData(true, [])).toBe(true);
|
||||
expect(hasStaleIntegrationData(false, [{ error: "offline" }])).toBe(true);
|
||||
expect(hasStaleIntegrationData(false, [])).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,2 @@
|
||||
export const hasStaleIntegrationData = (isRefetchError: boolean, failedResults: readonly unknown[]) =>
|
||||
isRefetchError || failedResults.length > 0;
|
||||
@@ -20,22 +20,17 @@ import {
|
||||
useSystemChartData,
|
||||
} from "./chart";
|
||||
import { chartAxisFormatters, formatByteRate, formatGB, formatPercent, formatStorageBytes } from "./format";
|
||||
import type { BeszelTooltipLayer } from "./tooltip";
|
||||
import { makeTooltipProps } from "./tooltip";
|
||||
import { useLiveStats } from "./use-live-stats";
|
||||
|
||||
const CHART_HEIGHT = 180;
|
||||
const MB = 1024 * 1024;
|
||||
const makeTooltipSet = (layer: BeszelTooltipLayer) => ({
|
||||
percent: makeTooltipProps(formatPercent, false, layer),
|
||||
gigabytes: makeTooltipProps(formatGB, true, layer),
|
||||
rate: makeTooltipProps(formatByteRate, true, layer),
|
||||
percentTotal: makeTooltipProps(formatPercent, true, layer),
|
||||
bytesTotal: makeTooltipProps(formatStorageBytes, true, layer),
|
||||
});
|
||||
const tooltipSets = {
|
||||
background: makeTooltipSet("background"),
|
||||
modal: makeTooltipSet("modal"),
|
||||
const tooltips = {
|
||||
percent: makeTooltipProps(formatPercent),
|
||||
gigabytes: makeTooltipProps(formatGB, true),
|
||||
rate: makeTooltipProps(formatByteRate, true),
|
||||
percentTotal: makeTooltipProps(formatPercent, true),
|
||||
bytesTotal: makeTooltipProps(formatStorageBytes, true),
|
||||
};
|
||||
|
||||
const ChartSkeleton = () => (
|
||||
@@ -66,7 +61,6 @@ interface BeszelStatsViewProps {
|
||||
visibility: BeszelStatsVisibility;
|
||||
columns: 1 | 2;
|
||||
onSwitchToHistorical?: () => void;
|
||||
tooltipLayer?: BeszelTooltipLayer;
|
||||
}
|
||||
|
||||
const whenVisible = <T,>(visible: boolean, data: T | undefined) => (visible ? data : undefined);
|
||||
@@ -78,14 +72,11 @@ export function BeszelStatsView({
|
||||
visibility,
|
||||
columns,
|
||||
onSwitchToHistorical,
|
||||
tooltipLayer = "background",
|
||||
}: BeszelStatsViewProps) {
|
||||
const t = useScopedI18n("widget.beszelSystemStats");
|
||||
const locale = useCurrentIntlLocale();
|
||||
const showDocker = visibility.dockerCpu || visibility.dockerMemory || visibility.dockerNetwork;
|
||||
const isLive = timePeriod === "1m";
|
||||
const tooltips = tooltipSets[tooltipLayer];
|
||||
|
||||
const historicalQuery = clientApi.widget.beszel.getSystemStats.useQuery(
|
||||
{ integrationIds, systemId, timePeriod, includeDocker: showDocker },
|
||||
{ refetchInterval: isLive ? false : 5_000, enabled: !isLive && systemId !== "" },
|
||||
|
||||
@@ -28,7 +28,7 @@ export const toScopedStorageVolumeValue = (integrationId: string, value: string)
|
||||
return `${integrationId}:${volumeName}`;
|
||||
};
|
||||
|
||||
const storageDeviceNamesMatch = (leftDeviceName: string, rightDeviceName: string): boolean => {
|
||||
export const storageDeviceNamesMatch = (leftDeviceName: string, rightDeviceName: string): boolean => {
|
||||
return (
|
||||
leftDeviceName === rightDeviceName ||
|
||||
normalizeStorageDeviceName(leftDeviceName) === normalizeStorageDeviceName(rightDeviceName)
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
|
||||
import { formatDiskTemperature } from "./mobile-summary";
|
||||
|
||||
describe("formatDiskTemperature", () => {
|
||||
test("preserves a valid zero-degree reading", () => {
|
||||
expect(formatDiskTemperature(0, true)).toBe("0°C");
|
||||
});
|
||||
|
||||
test("omits unavailable or disabled readings", () => {
|
||||
expect(formatDiskTemperature(null, true)).toBeUndefined();
|
||||
expect(formatDiskTemperature(undefined, true)).toBeUndefined();
|
||||
expect(formatDiskTemperature(20, false)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,2 @@
|
||||
export const formatDiskTemperature = (temperature: number | null | undefined, shouldShow: boolean) =>
|
||||
shouldShow && temperature != null ? `${temperature}°C` : undefined;
|
||||
@@ -0,0 +1,20 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
|
||||
import { getPreferredUnit } from "./temperature";
|
||||
|
||||
describe("getPreferredUnit", () => {
|
||||
test("preserves zero in Celsius and Fahrenheit", () => {
|
||||
expect(getPreferredUnit(0)).toBe("0.0°C");
|
||||
expect(getPreferredUnit(0, true)).toBe("32.0°F");
|
||||
});
|
||||
|
||||
test("only uses the unavailable placeholder for missing values", () => {
|
||||
expect(getPreferredUnit(undefined)).toBe("?");
|
||||
});
|
||||
|
||||
test("respects the decimal preference", () => {
|
||||
expect(getPreferredUnit(0, false, true)).toBe("0°C");
|
||||
expect(getPreferredUnit(0, true, true)).toBe("32°F");
|
||||
expect(getPreferredUnit(12.6, false, true)).toBe("13°C");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,6 @@
|
||||
export const getPreferredUnit = (value?: number, isFahrenheit = false, disableTemperatureDecimals = false): string =>
|
||||
value !== undefined
|
||||
? isFahrenheit
|
||||
? `${(value * (9 / 5) + 32).toFixed(disableTemperatureDecimals ? 0 : 1)}°F`
|
||||
: `${value.toFixed(disableTemperatureDecimals ? 0 : 1)}°C`
|
||||
: "?";
|
||||
Reference in new issue
Block a user