From ba3733da4dd384a30c32564679b7148b14abaf42 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 18 Jul 2026 19:53:28 +0200 Subject: [PATCH] fix(settings): make fresh-instance unit defaults internally consistent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A brand-new instance — no admin-set defaults, no saved value — rendered temperatures in Fahrenheit and times on the 12-hour clock while distances defaulted to metric, a mix that matches no locale. The store also seeded 'fahrenheit'/'12h' while DisplaySettingsTab's fallback said 'celsius', so the two paths disagreed about the intended default in the first place. Default to one system — celsius / metric / 24h — matching the already-metric distance_unit. The unit defaults now live in a single exported DEFAULT_SETTINGS that both the store and DisplaySettingsTab's fallback read, so they can't drift apart again. Admin-set user defaults (getAdminUserDefaults) and any value a user has already saved still take precedence; only the code-level default changed. --- .../Settings/DisplaySettingsTab.tsx | 10 ++-- client/src/store/settingsStore.test.ts | 21 +++++++ client/src/store/settingsStore.ts | 56 +++++++++++-------- 3 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 client/src/store/settingsStore.test.ts diff --git a/client/src/components/Settings/DisplaySettingsTab.tsx b/client/src/components/Settings/DisplaySettingsTab.tsx index 09b9cf2ad..3627beed9 100644 --- a/client/src/components/Settings/DisplaySettingsTab.tsx +++ b/client/src/components/Settings/DisplaySettingsTab.tsx @@ -1,7 +1,7 @@ import React, { useState, useEffect, useRef } from 'react' import { Languages, Map, ChevronDown, Check } from 'lucide-react' import { SUPPORTED_LANGUAGES, useTranslation } from '../../i18n' -import { useSettingsStore } from '../../store/settingsStore' +import { useSettingsStore, DEFAULT_SETTINGS } from '../../store/settingsStore' import { useToast } from '../shared/Toast' import CustomSelect from '../shared/CustomSelect' import { SYMBOLS, currenciesWith } from '../Budget/BudgetPanel.constants' @@ -12,8 +12,8 @@ export default function DisplaySettingsTab(): React.ReactElement { const { settings, updateSetting } = useSettingsStore() const { t } = useTranslation() const toast = useToast() - const [tempUnit, setTempUnit] = useState(settings.temperature_unit || 'celsius') - const [distanceUnit, setDistanceUnit] = useState(settings.distance_unit || 'metric') + const [tempUnit, setTempUnit] = useState(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit) + const [distanceUnit, setDistanceUnit] = useState(settings.distance_unit || DEFAULT_SETTINGS.distance_unit) const [langOpen, setLangOpen] = useState(false) const langDropdownRef = useRef(null) @@ -27,11 +27,11 @@ export default function DisplaySettingsTab(): React.ReactElement { }, [langOpen]) useEffect(() => { - setTempUnit(settings.temperature_unit || 'celsius') + setTempUnit(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit) }, [settings.temperature_unit]) useEffect(() => { - setDistanceUnit(settings.distance_unit || 'metric') + setDistanceUnit(settings.distance_unit || DEFAULT_SETTINGS.distance_unit) }, [settings.distance_unit]) return ( diff --git a/client/src/store/settingsStore.test.ts b/client/src/store/settingsStore.test.ts new file mode 100644 index 000000000..77a20d521 --- /dev/null +++ b/client/src/store/settingsStore.test.ts @@ -0,0 +1,21 @@ +import { describe, it, expect } from 'vitest' +import { DEFAULT_SETTINGS, useSettingsStore } from './settingsStore' + +// A fresh instance sends no value for a setting an admin hasn't defaulted, so DEFAULT_SETTINGS +// is what a brand-new user actually sees. These guard against the two regressions in the +// original bug: unit defaults that mix measurement systems (°F alongside kilometres), and a +// store default that silently disagrees with DisplaySettingsTab's fallback. +describe('settings defaults', () => { + it('SETTINGS-DEFAULTS-001: the shipped unit defaults belong to one consistent system', () => { + expect(DEFAULT_SETTINGS.temperature_unit).toBe('celsius') + expect(DEFAULT_SETTINGS.distance_unit).toBe('metric') + expect(DEFAULT_SETTINGS.time_format).toBe('24h') + }) + + it('SETTINGS-DEFAULTS-002: the store initialises from DEFAULT_SETTINGS, the same constant DisplaySettingsTab falls back to, so the two cannot drift apart', () => { + const settings = useSettingsStore.getState().settings + expect(settings.temperature_unit).toBe(DEFAULT_SETTINGS.temperature_unit) + expect(settings.distance_unit).toBe(DEFAULT_SETTINGS.distance_unit) + expect(settings.time_format).toBe(DEFAULT_SETTINGS.time_format) + }) +}) diff --git a/client/src/store/settingsStore.ts b/client/src/store/settingsStore.ts index 36f687e93..e58499106 100644 --- a/client/src/store/settingsStore.ts +++ b/client/src/store/settingsStore.ts @@ -21,31 +21,39 @@ interface SettingsState { export const hasStoredLanguage = (): boolean => typeof localStorage !== 'undefined' && !!localStorage.getItem('app_language') +// The effective client-side defaults for a fresh instance. The server sends no value for +// a setting an admin hasn't defaulted (see settingsService.getAdminUserDefaults), so these +// are what a brand-new user actually sees. Keep them internally consistent — one +// measurement system, not °F alongside kilometres — and note that DisplaySettingsTab +// imports these same values for its fallbacks, so the store default and the UI fallback +// can't drift apart again. +export const DEFAULT_SETTINGS: Settings = { + map_tile_url: '', + dark_mode: false, + // Empty = no personal display currency, so Costs falls back to the trip's own. + default_currency: '', + language: localStorage.getItem('app_language') || 'en', + temperature_unit: 'celsius', + distance_unit: 'metric', + time_format: '24h', + show_place_description: false, + optimize_from_accommodation: true, + map_provider: 'leaflet', + map_poi_pill_enabled: true, + mapbox_access_token: '', + mapbox_style: 'mapbox://styles/mapbox/standard', + maplibre_style: '', + mapbox_3d_enabled: true, + mapbox_quality_mode: false, + dashboard_fx_from: 'EUR', + dashboard_fx_to: 'USD', + appearance: DEFAULT_APPEARANCE, + // dashboard_timezones is intentionally left unset so the widget can tell "never + // chosen" (fall back to home + defaults) from an explicitly emptied list. +} + export const useSettingsStore = create((set, get) => ({ - settings: { - map_tile_url: '', - dark_mode: false, - // Empty = no personal display currency, so Costs falls back to the trip's own. - default_currency: '', - language: localStorage.getItem('app_language') || 'en', - temperature_unit: 'fahrenheit', - distance_unit: 'metric', - time_format: '12h', - show_place_description: false, - optimize_from_accommodation: true, - map_provider: 'leaflet', - map_poi_pill_enabled: true, - mapbox_access_token: '', - mapbox_style: 'mapbox://styles/mapbox/standard', - maplibre_style: '', - mapbox_3d_enabled: true, - mapbox_quality_mode: false, - dashboard_fx_from: 'EUR', - dashboard_fx_to: 'USD', - appearance: DEFAULT_APPEARANCE, - // dashboard_timezones is intentionally left unset so the widget can tell "never - // chosen" (fall back to home + defaults) from an explicitly emptied list. - }, + settings: { ...DEFAULT_SETTINGS }, isLoaded: false, loadSettings: async () => {