fix(site): handle missing default chart time (#2397)

Co-authored-by: henrygd <hank@henrygd.me>

internal/site/src/components/network-monitors-table/network-monitors-table.tsx

@@ -35,7 +35,7 @@ import { useToast } from "@/components/ui/use-toast"
 import { isReadOnlyUser } from "@/lib/api"
 import { pb } from "@/lib/api"
 import { SystemStatus } from "@/lib/enums"
-import { $allSystemsById, $direction, $userSettings } from "@/lib/stores"
+import { $allSystemsById, $direction, getUserChartTime } from "@/lib/stores"
 import {
 	cn,
 	isVisuallyLonger,
@@ -503,7 +503,7 @@ function NetworkMonitorSheetContent({
 }) {
 	// Keep monitor exploration independent of the system charts' time range.
 	const [chartTimeStore] = useState(() => {
-		const defaultTime = $userSettings.get().chartTime
+		const defaultTime = getUserChartTime()
 		return atom(defaultTime === "1m" ? "1h" : defaultTime)
 	})
 	const chartTime = useStore(chartTimeStore)

internal/site/src/components/routes/settings/general.tsx

@@ -12,7 +12,7 @@ import Slider from "@/components/ui/slider"
 import { HourFormat, Unit } from "@/lib/enums"
 import { dynamicActivate } from "@/lib/i18n"
 import languages from "@/lib/languages"
-import { $userSettings, defaultLayoutWidth } from "@/lib/stores"
+import { $chartTime, $userSettings, defaultLayoutWidth, getUserChartTime } from "@/lib/stores"
 import { chartTimeData, currentHour12 } from "@/lib/utils"
 import type { UserSettings } from "@/types"
 import { saveSettings } from "./layout"
@@ -22,6 +22,9 @@ export default function SettingsProfilePage({ userSettings }: { userSettings: Us
 	const { i18n } = useLingui()
 	const currentUserSettings = useStore($userSettings)
 	const layoutWidth = currentUserSettings.layoutWidth ?? defaultLayoutWidth
+	// without a value the hidden select submits an empty string, which would persist
+	// a chart time that no longer loads any data (#2104)
+	const chartTime = getUserChartTime(userSettings)
 
 	async function handleSubmit(e: React.FormEvent<HTMLFormElement>) {
 		e.preventDefault()
@@ -29,6 +32,8 @@ export default function SettingsProfilePage({ userSettings }: { userSettings: Us
 		const formData = new FormData(e.target as HTMLFormElement)
 		const data = Object.fromEntries(formData) as Partial<UserSettings>
 		await saveSettings(data)
+		// apply the saved default time period to the active charts
+		$chartTime.set(getUserChartTime())
 		setIsLoading(false)
 	}
 
@@ -122,7 +127,7 @@ export default function SettingsProfilePage({ userSettings }: { userSettings: Us
 							<Label className="block" htmlFor="chartTime">
 								<Trans>Default time period</Trans>
 							</Label>
-							<Select name="chartTime" key={userSettings.chartTime} defaultValue={userSettings.chartTime}>
+							<Select name="chartTime" key={chartTime} defaultValue={chartTime}>
 								<SelectTrigger id="chartTime">
 									<SelectValue />
 								</SelectTrigger>

internal/site/src/components/routes/system/use-system-data.ts

@@ -14,6 +14,7 @@ import {
 	$maxValues,
 	$systems,
 	$userSettings,
+	getUserChartTime,
 } from "@/lib/stores"
 import { chartTimeData, listen, parseSemVer } from "@/lib/utils"
 import type {
@@ -90,7 +91,7 @@ export function useSystemData(id: string) {
 	useEffect(() => {
 		return () => {
 			if (!persistChartTime.current) {
-				$chartTime.set($userSettings.get().chartTime)
+				$chartTime.set(getUserChartTime())
 			}
 			persistChartTime.current = false
 			setSystemStats([])

internal/site/src/lib/api.ts

@@ -4,7 +4,7 @@ import { basePath } from "@/components/router"
 import { toast } from "@/components/ui/use-toast"
 import { dynamicActivate, getLocale } from "@/lib/i18n"
 import type { ChartTimes, UserSettings } from "@/types"
-import { $alerts, $allSystemsById, $allSystemsByName, $userSettings } from "./stores"
+import { $alerts, $allSystemsById, $allSystemsByName, $userSettings, hydrateUserSettings } from "./stores"
 import { chartTimeData, debounce } from "./utils"
 
 /** PocketBase JS Client */
@@ -90,7 +90,7 @@ export function queueUserSettings(newSettings: Partial<UserSettings>) {
 export async function updateUserSettings() {
 	try {
 		const req = await pb.collection("user_settings").getFirstListItem("", { fields: "settings" })
-		$userSettings.set(req.settings)
+		hydrateUserSettings(req.settings)
 		dynamicActivate(req.settings.lang || getLocale())
 		return
 	} catch (e) {
@@ -99,7 +99,7 @@ export async function updateUserSettings() {
 	// create user settings if error fetching existing
 	try {
 		const createdSettings = await pb.collection("user_settings").create({ user: pb.authStore.record?.id })
-		$userSettings.set(createdSettings.settings)
+		hydrateUserSettings(createdSettings.settings)
 		dynamicActivate(createdSettings.settings.lang || getLocale())
 	} catch (e) {
 		console.error("create settings", e)

internal/site/src/lib/stores.ts

@@ -1,4 +1,4 @@
-import { atom, computed, listenKeys, map, type ReadableAtom } from "nanostores"
+import { atom, computed, map, type ReadableAtom } from "nanostores"
 import type { AlertMap, ChartTimes, SystemRecord, UpdateInfo, UserSettings } from "@/types"
 import { pb } from "./api"
 import { Unit } from "./enums"
@@ -31,8 +31,11 @@ export const $publicKey = atom("")
 /** New version info if an update is available, otherwise undefined */
 export const $newVersion = atom<UpdateInfo | undefined>()
 
+/** Chart time period used when user settings don't provide one */
+export const defaultChartTime: ChartTimes = "1h"
+
 /** Chart time period */
-export const $chartTime = atom<ChartTimes>("1h")
+export const $chartTime = atom<ChartTimes>(defaultChartTime)
 
 /** Whether to display average or max chart values */
 export const $maxValues = atom(false)
@@ -50,13 +53,25 @@ export const $maxValues = atom(false)
 
 /** User settings */
 export const $userSettings = map<UserSettings>({
-	chartTime: "1h",
+	chartTime: defaultChartTime,
 	emails: [pb.authStore.record?.email || ""],
 	unitNet: Unit.Bytes,
 	unitTemp: Unit.Celsius,
 })
-// update chart time on change
+
-listenKeys($userSettings, ["chartTime"], ({ chartTime }) => $chartTime.set(chartTime))
+/** Chart time period stored in user settings, or the default if it's missing */
+export function getUserChartTime(settings: UserSettings = $userSettings.get()): ChartTimes {
+	return settings.chartTime || defaultChartTime
+}
+
+/**
+ * Apply settings loaded from the database, including the default chart time.
+ * Other settings writes don't touch $chartTime so they can't reset the active chart range.
+ */
+export function hydrateUserSettings(settings: UserSettings) {
+	$userSettings.set(settings)
+	$chartTime.set(getUserChartTime(settings))
+}
 
 /** Container chart filter */
 export const $containerFilter = atom("")

internal/site/src/types.d.ts

@@ -361,7 +361,8 @@ export interface ChartTimeData {
 }
 
 export interface UserSettings {
-	chartTime: ChartTimes
+	/** may be missing in settings stored by older versions -- use getUserChartTime() */
+	chartTime?: ChartTimes
 	emails?: string[]
 	webhooks?: string[]
 	unitTemp?: Unit

internal/site/tests/chart-time.test.ts

@@ -0,0 +1,41 @@
+import { beforeEach, describe, expect, mock, test } from "bun:test"
+import type { UserSettings } from "../src/types"
+
+// lib/api needs a browser and the lingui macro transform, and stores only uses pb for the auth record
+mock.module("../src/lib/api", () => ({ pb: { authStore: { isValid: false, record: null } } }))
+
+const { $chartTime, $userSettings, defaultChartTime, getUserChartTime, hydrateUserSettings } = await import(
+	"../src/lib/stores"
+)
+
+describe("chart time from user settings", () => {
+	beforeEach(() => {
+		$chartTime.set(defaultChartTime)
+	})
+
+	test("hydration uses the stored chart time", () => {
+		hydrateUserSettings({ chartTime: "24h", emails: [] } as UserSettings)
+		expect($chartTime.get()).toBe("24h")
+	})
+
+	test("hydration falls back to the default when settings have no chart time", () => {
+		$chartTime.set("24h")
+		hydrateUserSettings({ emails: [] } as UserSettings)
+		expect($chartTime.get()).toBe(defaultChartTime)
+		expect(getUserChartTime()).toBe(defaultChartTime)
+	})
+
+	test("hydration falls back to the default when the stored chart time is empty", () => {
+		$chartTime.set("24h")
+		hydrateUserSettings({ chartTime: "", emails: [] } as unknown as UserSettings)
+		expect($chartTime.get()).toBe(defaultChartTime)
+		expect(getUserChartTime()).toBe(defaultChartTime)
+	})
+
+	test("other settings writes don't change the active chart time", () => {
+		$chartTime.set("12h")
+		$userSettings.set({ chartTime: "24h", emails: [], grid: true } as UserSettings)
+		$userSettings.setKey("chartTime", "7d")
+		expect($chartTime.get()).toBe("12h")
+	})
+})