fix: address mobile dashboard review findings

This commit is contained in:
ajnart committed 2026-08-14 11:32:09 +02:00
1 parent c1efaabd36
commit b4404113c4
11 files changed
+101 -5

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;
@@ -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`
: "?";