diff --git a/server/src/services/settingsService.ts b/server/src/services/settingsService.ts index d58f53495..7f0580021 100644 --- a/server/src/services/settingsService.ts +++ b/server/src/services/settingsService.ts @@ -36,6 +36,8 @@ export const DEFAULTABLE_USER_SETTING_KEYS = [ type DefaultableKey = typeof DEFAULTABLE_USER_SETTING_KEYS[number]; +const DEFAULTABLE_USER_SETTING_KEY_SET = new Set(DEFAULTABLE_USER_SETTING_KEYS); + const VALID_VALUES: Partial> = { temperature_unit: ['fahrenheit', 'celsius'], distance_unit: ['metric', 'imperial'], @@ -132,8 +134,24 @@ export function getUserSettings(userId: number): Record { } } - // Admin defaults fill in only for keys the user hasn't explicitly set - return { ...adminDefaults, ...userSettings }; + // Admin defaults fill in for keys the user hasn't explicitly set. For a + // defaultable key an *empty* user value counts as "not set": the client's + // Settings save always writes every field (a blank Mapbox token included), so + // an empty string would otherwise shadow the admin/system-wide default and the + // user could never fall back to it — even after clearing their own value + // (#1634). Non-defaultable keys keep their exact stored value. + const merged: Record = { ...adminDefaults }; + for (const [key, value] of Object.entries(userSettings)) { + if ( + DEFAULTABLE_USER_SETTING_KEY_SET.has(key) && + (value === '' || value === null || value === undefined) && + key in adminDefaults + ) { + continue; // empty user value → keep the admin default + } + merged[key] = value; + } + return merged; } function serializeValue(key: string, value: unknown): string { diff --git a/server/tests/unit/services/settingsService.test.ts b/server/tests/unit/services/settingsService.test.ts index d7e9f1da1..92f92d2a7 100644 --- a/server/tests/unit/services/settingsService.test.ts +++ b/server/tests/unit/services/settingsService.test.ts @@ -34,6 +34,7 @@ vi.mock('../../../src/config', () => ({ // Passthrough crypto — value comes back unchanged for most tests vi.mock('../../../src/services/apiKeyCrypto', () => ({ maybe_encrypt_api_key: (v: string) => v, + decrypt_api_key: (v: string) => v, })); import { createTables } from '../../../src/db/schema'; @@ -111,6 +112,46 @@ describe('getUserSettings', () => { expect(s).toHaveProperty('key_a'); expect(s).not.toHaveProperty('key_b'); }); + + // Admin "user defaults" fall-through (#1634) — a system-wide Mapbox token must + // reach a user who left their own token blank. + const setAdminDefault = (settingKey: string, value: string) => + testDb.prepare( + "INSERT INTO app_settings (key, value) VALUES (?, ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value", + ).run(`default_user_setting_${settingKey}`, value); + + it('SET-SVC-020 — new user with no rows inherits the admin default token', () => { + const { user } = createUser(testDb); + setAdminDefault('mapbox_access_token', 'pk.admin'); + expect(getUserSettings(user.id).mapbox_access_token).toBe('pk.admin'); + }); + + it('SET-SVC-021 — an empty user token falls through to the admin default', () => { + const { user } = createUser(testDb); + setAdminDefault('mapbox_access_token', 'pk.admin'); + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'mapbox_access_token', '')").run(user.id); + expect(getUserSettings(user.id).mapbox_access_token).toBe('pk.admin'); + }); + + it('SET-SVC-022 — a non-empty user token overrides the admin default', () => { + const { user } = createUser(testDb); + setAdminDefault('mapbox_access_token', 'pk.admin'); + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'mapbox_access_token', 'pk.user')").run(user.id); + expect(getUserSettings(user.id).mapbox_access_token).toBe('pk.user'); + }); + + it('SET-SVC-023 — an empty value with no admin default stays empty (no regression)', () => { + const { user } = createUser(testDb); + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'mapbox_style', '')").run(user.id); + expect(getUserSettings(user.id).mapbox_style).toBe(''); + }); + + it('SET-SVC-024 — a non-defaultable empty value is preserved even against a same-named default', () => { + const { user } = createUser(testDb); + // 'theme' is not a defaultable key: an empty stored value must be returned as-is. + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'theme', '')").run(user.id); + expect(getUserSettings(user.id).theme).toBe(''); + }); }); // ── upsertSetting ─────────────────────────────────────────────────────────────