diff --git a/client/src/components/Settings/LlmConnectionSection.tsx b/client/src/components/Settings/LlmConnectionSection.tsx index f6db68fa4..34701c629 100644 --- a/client/src/components/Settings/LlmConnectionSection.tsx +++ b/client/src/components/Settings/LlmConnectionSection.tsx @@ -1,17 +1,13 @@ -import React, { useEffect, useMemo, useState } from 'react' +import React from 'react' import { Sparkles, Save } from 'lucide-react' import { useTranslation } from '../../i18n' -import { useToast } from '../shared/Toast' -import { useSettingsStore } from '../../store/settingsStore' -import type { Settings } from '../../types' +import { useLlmConnection, type LlmProvider as Provider } from './useLlmConnection' import Section from './Section' import ToggleSwitch from './ToggleSwitch' import { SETTINGS_BUTTON_PRIMARY, SettingRow, SettingRows, SettingsHint } from './settingsKit' import { EditorField, GRID_2, INPUT, Segmented } from '../shared/dialogParts' import { fs } from '../shared/DialogShell' -type Provider = NonNullable - /** * Settings → Integrations → AI parsing. Per-user model used to extract bookings * from uploaded files. It only takes effect when the admin has not configured an @@ -29,71 +25,21 @@ type Provider = NonNullable */ export default function LlmConnectionSection(): React.ReactElement { const { t } = useTranslation() - const toast = useToast() - const settings = useSettingsStore(s => s.settings) - const isLoaded = useSettingsStore(s => s.isLoaded) - const updateSettings = useSettingsStore(s => s.updateSettings) - const loadSettings = useSettingsStore(s => s.loadSettings) - - const [provider, setProvider] = useState('openai') - const [model, setModel] = useState('') - const [apiKey, setApiKey] = useState('') - const [multimodal, setMultimodal] = useState(false) - const [hasStoredKey, setHasStoredKey] = useState(false) - const [saving, setSaving] = useState(false) - - // Hydrate from the loaded settings. llm_api_key arrives masked, so we only use - // its presence to drive the placeholder, never the value itself. A stored - // 'local' from before #1772 shows as OpenAI (local state only, nothing is - // saved until Save is pressed) so the form never offers a value the server - // would refuse. - useEffect(() => { - if (!isLoaded) return - const stored = settings.llm_provider || 'openai' - setProvider(stored === 'local' ? 'openai' : stored) - setModel(settings.llm_model || '') - setMultimodal(settings.llm_multimodal === true) - setHasStoredKey(!!settings.llm_api_key) - }, [isLoaded, settings.llm_provider, settings.llm_model, settings.llm_multimodal, settings.llm_api_key]) - - const providerOptions = useMemo( - () => [ - { value: 'openai' as const, label: t('settings.aiParsing.providerOpenai') }, - { value: 'anthropic' as const, label: t('settings.aiParsing.providerAnthropic') }, - ], - [t], - ) - - const handleSave = async () => { - setSaving(true) - try { - const payload: Partial = { - llm_provider: provider, - llm_model: model.trim(), - // Always cleared: the endpoint is instance configuration now, and this - // also drops a value left over from before #1772. - llm_base_url: '', - llm_multimodal: multimodal, - } - // Send the key only when the user typed a new one — a blank field means - // "keep the stored key". - const key = apiKey.trim() - if (key) payload.llm_api_key = key - await updateSettings(payload) - setApiKey('') - if (key) setHasStoredKey(true) - toast.success(t('settings.aiParsing.toast.saved')) - } catch { - // updateSettings patches the store before the request and keeps the patch - // when the request fails, so a refused save (the 403 from #1772, or any - // other error) would leave the form showing a value the server never - // stored. Pull the stored settings back in so what is on screen is real. - await loadSettings() - toast.error(t('settings.aiParsing.toast.saveError')) - } finally { - setSaving(false) - } - } + const { + isLoaded, + provider, + setProvider, + providerOptions, + model, + setModel, + apiKey, + setApiKey, + multimodal, + toggleMultimodal, + hasStoredKey, + saving, + handleSave, + } = useLlmConnection() return (
@@ -141,7 +87,7 @@ export default function LlmConnectionSection(): React.ReactElement { setMultimodal(v => !v)} label={t('settings.aiParsing.multimodal')} />} + control={} /> diff --git a/client/src/components/Settings/useLlmConnection.test.ts b/client/src/components/Settings/useLlmConnection.test.ts new file mode 100644 index 000000000..5a4c8fdd4 --- /dev/null +++ b/client/src/components/Settings/useLlmConnection.test.ts @@ -0,0 +1,96 @@ +// FE-COMP-LLM-HOOK-001 to -006: the AI parsing form both settings shells share. +import { act, renderHook } from '@testing-library/react'; +import { buildSettings } from '../../../tests/helpers/factories'; +import { resetAllStores, seedStore } from '../../../tests/helpers/store'; +import { useSettingsStore } from '../../store/settingsStore'; +import type { Settings } from '../../types'; +import { useLlmConnection } from './useLlmConnection'; + +const toast = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() })); +vi.mock('../shared/Toast', () => ({ useToast: () => toast })); +vi.mock('../../i18n', () => ({ useTranslation: () => ({ t: (k: string) => k }) })); + +function seed(over: Partial = {}, isLoaded = true) { + const updateSettings = vi.fn().mockResolvedValue(undefined); + const loadSettings = vi.fn().mockResolvedValue(undefined); + seedStore(useSettingsStore, { settings: buildSettings(over), isLoaded, updateSettings, loadSettings }); + return { updateSettings, loadSettings }; +} + +beforeEach(() => { + resetAllStores(); + vi.clearAllMocks(); +}); + +describe('useLlmConnection', () => { + it('FE-COMP-LLM-HOOK-001: waits for the settings, then hydrates them without the key itself', () => { + seed({ llm_provider: 'anthropic', llm_model: 'claude', llm_multimodal: true, llm_api_key: '****' }, false); + const { result, rerender } = renderHook(() => useLlmConnection()); + expect(result.current.provider).toBe('openai'); + expect(result.current.model).toBe(''); + act(() => useSettingsStore.setState({ isLoaded: true })); + rerender(); + expect(result.current.provider).toBe('anthropic'); + expect(result.current.model).toBe('claude'); + expect(result.current.multimodal).toBe(true); + expect(result.current.hasStoredKey).toBe(true); + expect(result.current.apiKey).toBe(''); + }); + + it('FE-COMP-LLM-HOOK-002: a stored local provider from before #1772 shows as OpenAI', () => { + seed({ llm_provider: 'local' }); + const { result } = renderHook(() => useLlmConnection()); + expect(result.current.provider).toBe('openai'); + }); + + it('FE-COMP-LLM-HOOK-003: offers exactly the two hosted providers', () => { + seed(); + const { result } = renderHook(() => useLlmConnection()); + expect(result.current.providerOptions).toEqual([ + { value: 'openai', label: 'settings.aiParsing.providerOpenai' }, + { value: 'anthropic', label: 'settings.aiParsing.providerAnthropic' }, + ]); + }); + + it('FE-COMP-LLM-HOOK-004: save with a blank key keeps the stored one and always clears the endpoint', async () => { + const { updateSettings } = seed({ llm_provider: 'openai', llm_model: 'm' }); + const { result } = renderHook(() => useLlmConnection()); + act(() => result.current.setModel(' gpt ')); + act(() => result.current.toggleMultimodal()); + await act(() => result.current.handleSave()); + expect(updateSettings).toHaveBeenCalledWith({ + llm_provider: 'openai', + llm_model: 'gpt', + llm_base_url: '', + llm_multimodal: true, + }); + expect(toast.success).toHaveBeenCalledWith('settings.aiParsing.toast.saved'); + expect(result.current.saving).toBe(false); + }); + + it('FE-COMP-LLM-HOOK-005: a typed key is sent trimmed, cleared afterwards and marks a stored key', async () => { + const { updateSettings } = seed(); + const { result } = renderHook(() => useLlmConnection()); + act(() => result.current.setProvider('anthropic')); + act(() => result.current.setApiKey(' sk ')); + await act(() => result.current.handleSave()); + expect(updateSettings).toHaveBeenCalledWith( + expect.objectContaining({ llm_provider: 'anthropic', llm_api_key: 'sk' }) + ); + expect(result.current.apiKey).toBe(''); + expect(result.current.hasStoredKey).toBe(true); + }); + + it('FE-COMP-LLM-HOOK-006: a refused save reloads the stored settings and toasts the error', async () => { + const { updateSettings, loadSettings } = seed(); + updateSettings.mockRejectedValue(new Error('403')); + const { result } = renderHook(() => useLlmConnection()); + act(() => result.current.setApiKey('sk')); + await act(() => result.current.handleSave()); + expect(loadSettings).toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith('settings.aiParsing.toast.saveError'); + expect(toast.success).not.toHaveBeenCalled(); + expect(result.current.apiKey).toBe('sk'); + expect(result.current.saving).toBe(false); + }); +}); diff --git a/client/src/components/Settings/useLlmConnection.ts b/client/src/components/Settings/useLlmConnection.ts new file mode 100644 index 000000000..79983e142 --- /dev/null +++ b/client/src/components/Settings/useLlmConnection.ts @@ -0,0 +1,98 @@ +import { useEffect, useMemo, useState } from 'react'; +import { useTranslation } from '../../i18n'; +import { useSettingsStore } from '../../store/settingsStore'; +import type { Settings } from '../../types'; +import { useToast } from '../shared/Toast'; + +export type LlmProvider = NonNullable; + +/** + * The per-user AI parsing model form behind both settings shells (the desktop + * section and its phone twin render their own markup over this). The key is never + * prefilled: a blank field keeps the stored key. Only the two hosted providers are + * offered (#1772), the endpoint is instance configuration. + */ +export function useLlmConnection() { + const { t } = useTranslation(); + const toast = useToast(); + const settings = useSettingsStore((s) => s.settings); + const isLoaded = useSettingsStore((s) => s.isLoaded); + const updateSettings = useSettingsStore((s) => s.updateSettings); + const loadSettings = useSettingsStore((s) => s.loadSettings); + + const [provider, setProvider] = useState('openai'); + const [model, setModel] = useState(''); + const [apiKey, setApiKey] = useState(''); + const [multimodal, setMultimodal] = useState(false); + const [hasStoredKey, setHasStoredKey] = useState(false); + const [saving, setSaving] = useState(false); + + // Hydrate from the loaded settings. llm_api_key arrives masked, so we only use + // its presence to drive the placeholder, never the value itself. A stored + // 'local' from before #1772 shows as OpenAI (local state only, nothing is + // saved until Save is pressed) so the form never offers a value the server + // would refuse. + useEffect(() => { + if (!isLoaded) return; + const stored = settings.llm_provider || 'openai'; + setProvider(stored === 'local' ? 'openai' : stored); + setModel(settings.llm_model || ''); + setMultimodal(settings.llm_multimodal === true); + setHasStoredKey(!!settings.llm_api_key); + }, [isLoaded, settings.llm_provider, settings.llm_model, settings.llm_multimodal, settings.llm_api_key]); + + const providerOptions = useMemo( + () => [ + { value: 'openai' as const, label: t('settings.aiParsing.providerOpenai') }, + { value: 'anthropic' as const, label: t('settings.aiParsing.providerAnthropic') }, + ], + [t] + ); + + const handleSave = async () => { + setSaving(true); + try { + const payload: Partial = { + llm_provider: provider, + llm_model: model.trim(), + // Always cleared: the endpoint is instance configuration now, and this + // also drops a value left over from before #1772. + llm_base_url: '', + llm_multimodal: multimodal, + }; + // Send the key only when the user typed a new one, a blank field means + // "keep the stored key". + const key = apiKey.trim(); + if (key) payload.llm_api_key = key; + await updateSettings(payload); + setApiKey(''); + if (key) setHasStoredKey(true); + toast.success(t('settings.aiParsing.toast.saved')); + } catch { + // updateSettings patches the store before the request and keeps the patch + // when the request fails, so a refused save (the 403 from #1772, or any + // other error) would leave the form showing a value the server never + // stored. Pull the stored settings back in so what is on screen is real. + await loadSettings(); + toast.error(t('settings.aiParsing.toast.saveError')); + } finally { + setSaving(false); + } + }; + + return { + isLoaded, + provider, + setProvider, + providerOptions, + model, + setModel, + apiKey, + setApiKey, + multimodal, + toggleMultimodal: () => setMultimodal((v) => !v), + hasStoredKey, + saving, + handleSave, + }; +} diff --git a/client/src/mobile/screens/settings/MLlmConnectionSection.tsx b/client/src/mobile/screens/settings/MLlmConnectionSection.tsx index a473e4c2c..e8bd52d45 100644 --- a/client/src/mobile/screens/settings/MLlmConnectionSection.tsx +++ b/client/src/mobile/screens/settings/MLlmConnectionSection.tsx @@ -1,15 +1,11 @@ -import React, { useEffect, useMemo, useState } from 'react' +import React, { useState } from 'react' import { Sparkles, Save, ChevronDown } from 'lucide-react' import { useTranslation } from '../../../i18n' -import { useToast } from '../../../components/shared/Toast' -import { useSettingsStore } from '../../../store/settingsStore' -import type { Settings } from '../../../types' +import { useLlmConnection, type LlmProvider as Provider } from '../../../components/Settings/useLlmConnection' import { MSetCard, MSetEyebrow, MSetSelectRow, MSetRow, MSetInput, MSetButton, MSetHint } from './MSettingsUi' import MToggle from '../../components/MToggle' import MSetPickerSheet from './MSetPickerSheet' -type Provider = NonNullable - /** * Mobile-native twin of components/Settings/LlmConnectionSection. Same per-user * AI-parsing model logic (provider/model/key/multimodal, key never prefilled, @@ -19,73 +15,25 @@ type Provider = NonNullable */ export default function MLlmConnectionSection(): React.ReactElement { const { t } = useTranslation() - const toast = useToast() - const settings = useSettingsStore(s => s.settings) - const isLoaded = useSettingsStore(s => s.isLoaded) - const updateSettings = useSettingsStore(s => s.updateSettings) - const loadSettings = useSettingsStore(s => s.loadSettings) - - const [provider, setProvider] = useState('openai') - const [model, setModel] = useState('') - const [apiKey, setApiKey] = useState('') - const [multimodal, setMultimodal] = useState(false) - const [hasStoredKey, setHasStoredKey] = useState(false) - const [saving, setSaving] = useState(false) + const { + isLoaded, + provider, + setProvider, + providerOptions, + model, + setModel, + apiKey, + setApiKey, + multimodal, + toggleMultimodal, + hasStoredKey, + saving, + handleSave, + } = useLlmConnection() const [providerOpen, setProviderOpen] = useState(false) - // Hydrate from the loaded settings. llm_api_key arrives masked, so we only use - // its presence to drive the placeholder, never the value itself. A stored - // 'local' from before #1772 shows as OpenAI (local state only, nothing is - // saved until Save is pressed). - useEffect(() => { - if (!isLoaded) return - const stored = settings.llm_provider || 'openai' - setProvider(stored === 'local' ? 'openai' : stored) - setModel(settings.llm_model || '') - setMultimodal(settings.llm_multimodal === true) - setHasStoredKey(!!settings.llm_api_key) - }, [isLoaded, settings.llm_provider, settings.llm_model, settings.llm_multimodal, settings.llm_api_key]) - - const providerOptions = useMemo( - () => [ - { value: 'openai', label: t('settings.aiParsing.providerOpenai') }, - { value: 'anthropic', label: t('settings.aiParsing.providerAnthropic') }, - ], - [t], - ) const providerLabel = providerOptions.find(o => o.value === provider)?.label ?? provider - const handleSave = async () => { - setSaving(true) - try { - const payload: Partial = { - llm_provider: provider, - llm_model: model.trim(), - // Always cleared: the endpoint is instance configuration now, and this - // also drops a value left over from before #1772. - llm_base_url: '', - llm_multimodal: multimodal, - } - // Send the key only when the user typed a new one — a blank field means - // "keep the stored key". - const key = apiKey.trim() - if (key) payload.llm_api_key = key - await updateSettings(payload) - setApiKey('') - if (key) setHasStoredKey(true) - toast.success(t('settings.aiParsing.toast.saved')) - } catch { - // updateSettings patches the store before the request and keeps the patch - // when the request fails, so a refused save (the 403 from #1772, or any - // other error) would leave the form showing a value the server never - // stored. Pull the stored settings back in so what is on screen is real. - await loadSettings() - toast.error(t('settings.aiParsing.toast.saveError')) - } finally { - setSaving(false) - } - } - return ( {t('settings.aiParsing.hint')} @@ -127,7 +75,7 @@ export default function MLlmConnectionSection(): React.ReactElement { trailing={ setMultimodal(v => !v)} + onChange={toggleMultimodal} ariaLabel={t('settings.aiParsing.multimodal')} /> }