fix: scope config default fallback to user mode, skip null user values on load merge, submit via component ref

- InterfaceSettings.loadSettingsFromSource now only falls through to
  \$config.default_models when !isAdminMode. In admin-defaults mode the
  fallback clobbered whatever the admin last saved with the system-
  wide default on every reopen, so the modal could show (and
  inadvertently re-save) the wrong model.
- Added stripNullValues util and applied it to userUI in +layout.svelte
  before deepMerge. deepMerge treats source null as an explicit
  override, so common "unset / inherit" shapes in user settings
  (textScale, webSearch, etc.) would suppress admin defaults instead
  of letting them through. Stripping nulls preserves the PR's stated
  behaviour (defaults apply for fields the user hasn't customized)
  without changing deepMerge semantics for other callers.
- InterfaceSettings exposes submitSettings() and the admin defaults
  modal now binds the component instance via bind:this and calls
  submitSettings() through that ref. Replaces the document.getElementById
  ('tab-interface') lookup, which was ambiguous the moment a second
  tab-interface form existed in the DOM.
This commit is contained in:
DrMelone 2026-04-13 01:11:42 +02:00
parent 5835cd93cf
commit 2fb4e48319
4 changed files with 60 additions and 15 deletions

View file

@ -16,6 +16,11 @@
let loading = false;
let saving = false;
let adminDefaults: Record<string, any> = {};
// Bound via bind:this on the embedded InterfaceSettings so submitHandler
// can flush submit-scoped writes through a specific component instance
// rather than via a document.getElementById('tab-interface') lookup,
// which would be ambiguous as soon as another InterfaceSettings mounted.
let interfaceSettings: { submitSettings?: () => void } | null = null;
const saveAdminSettings = async (updates: object) => {
adminDefaults = { ...adminDefaults, ...updates };
@ -48,17 +53,13 @@
const submitHandler = async () => {
saving = true;
try {
// The embedded InterfaceSettings form has submit-scoped writes
// (updateInterfaceHandler -> saveSettings({ models, imageCompressionSize }))
// that don't fire on per-control change. Request-submit the child
// form first so those fields land in adminDefaults via the
// saveAdminSettings callback, then wait one tick for reactivity
// to flush before we POST. Without this, admins could save
// defaults and silently lose submit-only fields.
const innerForm = document.getElementById('tab-interface') as HTMLFormElement | null;
if (innerForm && typeof innerForm.requestSubmit === 'function') {
innerForm.requestSubmit();
}
// The embedded InterfaceSettings component exposes submitSettings()
// for this exact flow: it runs the submit-scoped writes (models,
// imageCompressionSize) through saveAdminSettings so they land
// in adminDefaults before we POST. We call it through the
// bind:this ref to avoid the ambiguity of a global
// document.getElementById lookup.
interfaceSettings?.submitSettings?.();
await tick();
await setInterfaceDefaults(localStorage.token, prepareForBackend(adminDefaults));
@ -111,7 +112,11 @@
-->
<div class="flex flex-col w-full">
<div class="interface-defaults-wrapper">
<InterfaceSettings initialSettings={adminDefaults} saveSettings={saveAdminSettings} />
<InterfaceSettings
bind:this={interfaceSettings}
initialSettings={adminDefaults}
saveSettings={saveAdminSettings}
/>
</div>
<div class="flex justify-end pt-3 text-sm font-medium">

View file

@ -193,6 +193,15 @@
});
};
// Exported so the admin-defaults modal can flush submit-scoped writes
// (fields that only flow through form submit, not per-control change)
// without reaching into the DOM via document.getElementById('tab-interface').
// That DOM-global lookup would be ambiguous as soon as another
// InterfaceSettings instance existed on the page.
export const submitSettings = () => {
updateInterfaceHandler();
};
const toggleWebSearch = async () => {
webSearch = webSearch === null ? 'always' : null;
saveSettings({ webSearch: webSearch });
@ -282,7 +291,11 @@
imageCompressionInChannels = source?.imageCompressionInChannels ?? true;
defaultModelId = source?.models?.at(0) ?? '';
if ($config?.default_models) {
// In admin-defaults mode the $config.default_models fallback would
// clobber whatever the admin last saved with the system-wide default
// on every reopen, so admins could see (and inadvertently re-save)
// the wrong model. Preserve the per-user fallback as-is.
if (!isAdminMode && $config?.default_models) {
defaultModelId = $config.default_models.split(',')[0];
}

View file

@ -1864,6 +1864,26 @@ export const displayFileHandler = (
* @param source - The source object (overrides)
* @returns A new object with deep-merged properties
*/
/**
* Return a copy of `value` with any explicit null properties removed,
* recursing into nested plain objects. Arrays are returned as-is. Useful
* before a deepMerge where the source treats null as "inherit / unset"
* rather than "explicit override" (e.g. merging user settings over admin
* defaults, where null on user means "fall through to the default").
*/
export const stripNullValues = (value: any): any => {
if (value === null || typeof value !== 'object') return value;
if (Array.isArray(value)) return value;
const result: Record<string, any> = {};
for (const key in value) {
if (!Object.prototype.hasOwnProperty.call(value, key)) continue;
const v = value[key];
if (v === null) continue;
result[key] = stripNullValues(v);
}
return result;
};
export const deepMerge = (target: any, source: any): any => {
// Handle null/undefined cases - distinguish between "not provided" and "explicitly null"
if (source === undefined) return target;

View file

@ -71,7 +71,7 @@
import Spinner from '$lib/components/common/Spinner.svelte';
import { getUserSettings } from '$lib/apis/users';
import { getInterfaceDefaults } from '$lib/apis/configs';
import { deepMerge } from '$lib/utils';
import { deepMerge, stripNullValues } from '$lib/utils';
import dayjs from 'dayjs';
import { getChannels } from '$lib/apis/channels';
@ -957,7 +957,14 @@
}
}
const userUI = userSettings?.ui ?? localStorageSettings ?? {};
settings.set(deepMerge(adminDefaults ?? {}, userUI));
// User settings commonly carry explicit null for "inherit /
// unset" style fields (e.g. textScale, webSearch), and
// deepMerge treats source null as an explicit override — so
// merging userUI directly would suppress the admin default
// for any such field. Strip nulls out of userUI (recursively
// for nested objects) so "no value" really means "inherit
// the admin default" here.
settings.set(deepMerge(adminDefaults ?? {}, stripNullValues(userUI)));
setTextScale($settings?.textScale ?? 1);
// Set up the token expiry check