fix(settings): let an empty user value fall back to the admin default

The admin "user defaults" merge kept any user value that was merely
present, so an empty string shadowed the system-wide default. The
client's Settings save always writes every field — a blank Mapbox token
included — so a user who ever opened Map Settings got '' instead of the
admin token, and clearing their own token could never restore the
fallback (#1634).

Treat an empty/null user value for a defaultable key as "not set" when an
admin default exists, so it falls through. Non-defaultable keys and keys
without an admin default keep their exact stored value.
This commit is contained in:
fbnlrz
2026-07-21 22:46:36 +02:00
committed by Maurice
parent 617ffae351
commit 4b73fa80a9
2 changed files with 61 additions and 2 deletions
+20 -2
View File
@@ -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<string>(DEFAULTABLE_USER_SETTING_KEYS);
const VALID_VALUES: Partial<Record<DefaultableKey, unknown[]>> = {
temperature_unit: ['fahrenheit', 'celsius'],
distance_unit: ['metric', 'imperial'],
@@ -132,8 +134,24 @@ export function getUserSettings(userId: number): Record<string, unknown> {
}
}
// 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<string, unknown> = { ...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 {
@@ -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 ─────────────────────────────────────────────────────────────