From 68e2e0639b7f2d0cf91d50579d29841bcacabbf2 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 09:59:08 +0200 Subject: [PATCH] refactor(airtrail): share the AirTrail connection form between desktop and phone Both settings shells now read their state, the save and test handlers and the canSave rule from one useAirTrailSettings hook and keep only their own markup. --- .../Settings/AirTrailConnectionSection.tsx | 88 ++++---------- .../Settings/useAirTrailSettings.test.ts | 112 ++++++++++++++++++ .../Settings/useAirTrailSettings.ts | 95 +++++++++++++++ .../settings/MAirTrailConnectionSection.tsx | 87 ++++---------- 4 files changed, 249 insertions(+), 133 deletions(-) create mode 100644 client/src/components/Settings/useAirTrailSettings.test.ts create mode 100644 client/src/components/Settings/useAirTrailSettings.ts diff --git a/client/src/components/Settings/AirTrailConnectionSection.tsx b/client/src/components/Settings/AirTrailConnectionSection.tsx index d984d4098..ddb780706 100644 --- a/client/src/components/Settings/AirTrailConnectionSection.tsx +++ b/client/src/components/Settings/AirTrailConnectionSection.tsx @@ -1,8 +1,7 @@ -import React, { useEffect, useState } from 'react' +import React from 'react' import { Loader2, Plane, Save } from 'lucide-react' import { useTranslation } from '../../i18n' -import { useToast } from '../shared/Toast' -import { airtrailApi } from '../../api/client' +import { useAirTrailSettings } from './useAirTrailSettings' import AirTrailIcon from '../shared/AirTrailIcon' import Section from './Section' import ToggleSwitch from './ToggleSwitch' @@ -18,69 +17,24 @@ import { fs } from '../shared/DialogShell' */ export default function AirTrailConnectionSection(): React.ReactElement { const { t } = useTranslation() - const toast = useToast() - const [url, setUrl] = useState('') - const [apiKey, setApiKey] = useState('') - const [allowInsecureTls, setAllowInsecureTls] = useState(false) - const [writeEnabled, setWriteEnabled] = useState(false) - const [connected, setConnected] = useState(false) - const [loading, setLoading] = useState(true) - const [saving, setSaving] = useState(false) - const [testing, setTesting] = useState(false) - - useEffect(() => { - airtrailApi - .getSettings() - .then(d => { - setUrl(d.url || '') - setAllowInsecureTls(!!d.allowInsecureTls) - setWriteEnabled(!!d.writeEnabled) - setConnected(!!d.connected) - }) - .catch(() => {}) - .finally(() => setLoading(false)) - }, []) - - // Send the key only when the user typed a new one — never prefilled, so a blank - // field means "keep the stored key". - const keyPayload = (): { apiKey?: string } => { - const k = apiKey.trim() - return k ? { apiKey: k } : {} - } - - const handleSave = async () => { - setSaving(true) - try { - const d = await airtrailApi.saveSettings({ url: url.trim(), allowInsecureTls, writeEnabled, ...keyPayload() }) - const status = await airtrailApi.status().catch(() => ({ connected: false })) - setConnected(!!status.connected) - setApiKey('') - if (d?.warning) toast.warning(d.warning) - else toast.success(t('settings.airtrail.toast.saved')) - } catch (err) { - const reason = (err as { response?: { data?: { error?: string } } } | null)?.response?.data?.error - toast.error(reason || t('settings.airtrail.toast.saveError')) - } finally { - setSaving(false) - } - } - - const handleTest = async () => { - setTesting(true) - try { - const d = await airtrailApi.test({ url: url.trim(), allowInsecureTls, ...keyPayload() }) - setConnected(!!d.connected) - if (d.connected) toast.success(t('settings.airtrail.test.success', { count: d.flightCount ?? 0 })) - else toast.error(d.error || t('settings.airtrail.test.failed')) - } catch { - toast.error(t('settings.airtrail.test.failed')) - } finally { - setTesting(false) - } - } - - const canSave = !!url.trim() && (connected || !!apiKey.trim()) + const { + url, + setUrl, + apiKey, + setApiKey, + allowInsecureTls, + toggleInsecureTls, + writeEnabled, + toggleWriteEnabled, + connected, + loading, + saving, + testing, + canSave, + handleSave, + handleTest, + } = useAirTrailSettings() const fieldId = (name: string) => `airtrail-${name}` @@ -122,12 +76,12 @@ export default function AirTrailConnectionSection(): React.ReactElement { setAllowInsecureTls(v => !v)} label={t('settings.airtrail.allowInsecureTls')} />} + control={} /> setWriteEnabled(v => !v)} label={t('settings.airtrail.writeBack')} />} + control={} /> diff --git a/client/src/components/Settings/useAirTrailSettings.test.ts b/client/src/components/Settings/useAirTrailSettings.test.ts new file mode 100644 index 000000000..921e2a9df --- /dev/null +++ b/client/src/components/Settings/useAirTrailSettings.test.ts @@ -0,0 +1,112 @@ +// FE-COMP-AIRTRAIL-HOOK-001 to -008: the AirTrail connection form both settings shells share. +import { act, renderHook, waitFor } from '@testing-library/react'; +import { airtrailApi } from '../../api/client'; +import { useAirTrailSettings } from './useAirTrailSettings'; + +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, p?: Record) => (p ? `${k}:${JSON.stringify(p)}` : k) }), +})); + +const STORED = { url: 'https://air.example', allowInsecureTls: true, writeEnabled: false, connected: true }; + +beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(airtrailApi, 'getSettings').mockResolvedValue(STORED); + vi.spyOn(airtrailApi, 'saveSettings').mockResolvedValue({}); + vi.spyOn(airtrailApi, 'status').mockResolvedValue({ connected: true }); + vi.spyOn(airtrailApi, 'test').mockResolvedValue({ connected: true, flightCount: 3 }); +}); +afterEach(() => vi.restoreAllMocks()); + +async function loaded() { + const hook = renderHook(() => useAirTrailSettings()); + await waitFor(() => expect(hook.result.current.loading).toBe(false)); + return hook; +} + +describe('useAirTrailSettings', () => { + it('FE-COMP-AIRTRAIL-HOOK-001: hydrates the form from the stored settings and never prefills the key', async () => { + const { result } = await loaded(); + expect(result.current.url).toBe('https://air.example'); + expect(result.current.allowInsecureTls).toBe(true); + expect(result.current.writeEnabled).toBe(false); + expect(result.current.connected).toBe(true); + expect(result.current.apiKey).toBe(''); + }); + + it('FE-COMP-AIRTRAIL-HOOK-002: a failed read still ends loading and leaves the defaults', async () => { + vi.mocked(airtrailApi.getSettings).mockRejectedValue(new Error('down')); + const { result } = await loaded(); + expect(result.current.url).toBe(''); + expect(result.current.connected).toBe(false); + }); + + it('FE-COMP-AIRTRAIL-HOOK-003: canSave needs a url plus a connection or a typed key', async () => { + vi.mocked(airtrailApi.getSettings).mockResolvedValue({ url: '', connected: false }); + const { result } = await loaded(); + expect(result.current.canSave).toBe(false); + act(() => result.current.setUrl('https://a.example')); + expect(result.current.canSave).toBe(false); + act(() => result.current.setApiKey(' k ')); + expect(result.current.canSave).toBe(true); + }); + + it('FE-COMP-AIRTRAIL-HOOK-004: save leaves a blank key out, refreshes the status and toasts success', async () => { + const { result } = await loaded(); + act(() => result.current.toggleWriteEnabled()); + act(() => result.current.toggleInsecureTls()); + await act(() => result.current.handleSave()); + expect(airtrailApi.saveSettings).toHaveBeenCalledWith({ + url: 'https://air.example', + allowInsecureTls: false, + writeEnabled: true, + }); + expect(airtrailApi.status).toHaveBeenCalled(); + expect(toast.success).toHaveBeenCalledWith('settings.airtrail.toast.saved'); + expect(result.current.saving).toBe(false); + }); + + it('FE-COMP-AIRTRAIL-HOOK-005: save sends a typed key trimmed, clears it, and shows a server warning instead', async () => { + vi.mocked(airtrailApi.saveSettings).mockResolvedValue({ warning: 'careful' }); + vi.mocked(airtrailApi.status).mockRejectedValue(new Error('x')); + const { result } = await loaded(); + act(() => result.current.setApiKey(' secret ')); + await act(() => result.current.handleSave()); + expect(airtrailApi.saveSettings).toHaveBeenCalledWith(expect.objectContaining({ apiKey: 'secret' })); + expect(result.current.apiKey).toBe(''); + expect(result.current.connected).toBe(false); + expect(toast.warning).toHaveBeenCalledWith('careful'); + expect(toast.success).not.toHaveBeenCalled(); + }); + + it('FE-COMP-AIRTRAIL-HOOK-006: a refused save toasts the server reason, else the generic error', async () => { + vi.mocked(airtrailApi.saveSettings).mockRejectedValueOnce({ response: { data: { error: 'bad url' } } }); + const { result } = await loaded(); + await act(() => result.current.handleSave()); + expect(toast.error).toHaveBeenLastCalledWith('bad url'); + vi.mocked(airtrailApi.saveSettings).mockRejectedValueOnce(new Error('x')); + await act(() => result.current.handleSave()); + expect(toast.error).toHaveBeenLastCalledWith('settings.airtrail.toast.saveError'); + }); + + it('FE-COMP-AIRTRAIL-HOOK-007: test reports the flight count, or the probe error, and updates the connection', async () => { + const { result } = await loaded(); + await act(() => result.current.handleTest()); + expect(airtrailApi.test).toHaveBeenCalledWith({ url: 'https://air.example', allowInsecureTls: true }); + expect(toast.success).toHaveBeenCalledWith('settings.airtrail.test.success:{"count":3}'); + vi.mocked(airtrailApi.test).mockResolvedValueOnce({ connected: false, error: 'nope' }); + await act(() => result.current.handleTest()); + expect(result.current.connected).toBe(false); + expect(toast.error).toHaveBeenLastCalledWith('nope'); + }); + + it('FE-COMP-AIRTRAIL-HOOK-008: a test that throws toasts the generic failure and clears testing', async () => { + vi.mocked(airtrailApi.test).mockRejectedValue(new Error('x')); + const { result } = await loaded(); + await act(() => result.current.handleTest()); + expect(toast.error).toHaveBeenCalledWith('settings.airtrail.test.failed'); + expect(result.current.testing).toBe(false); + }); +}); diff --git a/client/src/components/Settings/useAirTrailSettings.ts b/client/src/components/Settings/useAirTrailSettings.ts new file mode 100644 index 000000000..04089bc97 --- /dev/null +++ b/client/src/components/Settings/useAirTrailSettings.ts @@ -0,0 +1,95 @@ +import { useEffect, useState } from 'react'; +import { airtrailApi } from '../../api/client'; +import { useTranslation } from '../../i18n'; +import { useToast } from '../shared/Toast'; + +/** + * The per-user AirTrail connection form behind both settings shells: the desktop + * section and its phone twin render their own markup over this. The key is never + * prefilled, so a blank field means "keep the stored key". Not to be confused with + * hooks/useAirtrailConnection, which only answers whether a trip may offer AirTrail. + */ +export function useAirTrailSettings() { + const { t } = useTranslation(); + const toast = useToast(); + + const [url, setUrl] = useState(''); + const [apiKey, setApiKey] = useState(''); + const [allowInsecureTls, setAllowInsecureTls] = useState(false); + const [writeEnabled, setWriteEnabled] = useState(false); + const [connected, setConnected] = useState(false); + const [loading, setLoading] = useState(true); + const [saving, setSaving] = useState(false); + const [testing, setTesting] = useState(false); + + useEffect(() => { + airtrailApi + .getSettings() + .then((d) => { + setUrl(d.url || ''); + setAllowInsecureTls(!!d.allowInsecureTls); + setWriteEnabled(!!d.writeEnabled); + setConnected(!!d.connected); + }) + .catch(() => {}) + .finally(() => setLoading(false)); + }, []); + + // Send the key only when the user typed a new one, never prefilled, so a blank + // field means "keep the stored key". + const keyPayload = (): { apiKey?: string } => { + const k = apiKey.trim(); + return k ? { apiKey: k } : {}; + }; + + const handleSave = async () => { + setSaving(true); + try { + const d = await airtrailApi.saveSettings({ url: url.trim(), allowInsecureTls, writeEnabled, ...keyPayload() }); + const status = await airtrailApi.status().catch(() => ({ connected: false })); + setConnected(!!status.connected); + setApiKey(''); + if (d?.warning) toast.warning(d.warning); + else toast.success(t('settings.airtrail.toast.saved')); + } catch (err) { + const reason = (err as { response?: { data?: { error?: string } } } | null)?.response?.data?.error; + toast.error(reason || t('settings.airtrail.toast.saveError')); + } finally { + setSaving(false); + } + }; + + const handleTest = async () => { + setTesting(true); + try { + const d = await airtrailApi.test({ url: url.trim(), allowInsecureTls, ...keyPayload() }); + setConnected(!!d.connected); + if (d.connected) toast.success(t('settings.airtrail.test.success', { count: d.flightCount ?? 0 })); + else toast.error(d.error || t('settings.airtrail.test.failed')); + } catch { + toast.error(t('settings.airtrail.test.failed')); + } finally { + setTesting(false); + } + }; + + const canSave = !!url.trim() && (connected || !!apiKey.trim()); + + return { + url, + setUrl, + apiKey, + setApiKey, + allowInsecureTls, + toggleInsecureTls: () => setAllowInsecureTls((v) => !v), + writeEnabled, + toggleWriteEnabled: () => setWriteEnabled((v) => !v), + connected, + loading, + saving, + testing, + canSave, + handleSave, + handleTest, + }; +} diff --git a/client/src/mobile/screens/settings/MAirTrailConnectionSection.tsx b/client/src/mobile/screens/settings/MAirTrailConnectionSection.tsx index 0c7dd7063..c54165e09 100644 --- a/client/src/mobile/screens/settings/MAirTrailConnectionSection.tsx +++ b/client/src/mobile/screens/settings/MAirTrailConnectionSection.tsx @@ -1,8 +1,7 @@ -import React, { useEffect, useState } from 'react' +import React from 'react' import { Plane, Save, RefreshCw } from 'lucide-react' import { useTranslation } from '../../../i18n' -import { useToast } from '../../../components/shared/Toast' -import { airtrailApi } from '../../../api/client' +import { useAirTrailSettings } from '../../../components/Settings/useAirTrailSettings' import { MSetCard, MSetEyebrow, MSetRow, MSetInput, MSetButton, MSetHint } from './MSettingsUi' import MToggle from '../../components/MToggle' @@ -14,68 +13,24 @@ import MToggle from '../../components/MToggle' */ export default function MAirTrailConnectionSection(): React.ReactElement { const { t } = useTranslation() - const toast = useToast() - const [url, setUrl] = useState('') - const [apiKey, setApiKey] = useState('') - const [allowInsecureTls, setAllowInsecureTls] = useState(false) - const [writeEnabled, setWriteEnabled] = useState(false) - const [connected, setConnected] = useState(false) - const [loading, setLoading] = useState(true) - const [saving, setSaving] = useState(false) - const [testing, setTesting] = useState(false) - - useEffect(() => { - airtrailApi - .getSettings() - .then(d => { - setUrl(d.url || '') - setAllowInsecureTls(!!d.allowInsecureTls) - setWriteEnabled(!!d.writeEnabled) - setConnected(!!d.connected) - }) - .catch(() => {}) - .finally(() => setLoading(false)) - }, []) - - // Send the key only when the user typed a new one — never prefilled, so a blank - // field means "keep the stored key". - const keyPayload = (): { apiKey?: string } => { - const k = apiKey.trim() - return k ? { apiKey: k } : {} - } - - const handleSave = async () => { - setSaving(true) - try { - const d = await airtrailApi.saveSettings({ url: url.trim(), allowInsecureTls, writeEnabled, ...keyPayload() }) - const status = await airtrailApi.status().catch(() => ({ connected: false })) - setConnected(!!status.connected) - setApiKey('') - if (d?.warning) toast.warning(d.warning) - else toast.success(t('settings.airtrail.toast.saved')) - } catch (err: any) { - toast.error(err?.response?.data?.error || t('settings.airtrail.toast.saveError')) - } finally { - setSaving(false) - } - } - - const handleTest = async () => { - setTesting(true) - try { - const d = await airtrailApi.test({ url: url.trim(), allowInsecureTls, ...keyPayload() }) - setConnected(!!d.connected) - if (d.connected) toast.success(t('settings.airtrail.test.success', { count: d.flightCount ?? 0 })) - else toast.error(d.error || t('settings.airtrail.test.failed')) - } catch { - toast.error(t('settings.airtrail.test.failed')) - } finally { - setTesting(false) - } - } - - const canSave = !!url.trim() && (connected || !!apiKey.trim()) + const { + url, + setUrl, + apiKey, + setApiKey, + allowInsecureTls, + toggleInsecureTls, + writeEnabled, + toggleWriteEnabled, + connected, + loading, + saving, + testing, + canSave, + handleSave, + handleTest, + } = useAirTrailSettings() return ( @@ -104,7 +59,7 @@ export default function MAirTrailConnectionSection(): React.ReactElement { trailing={ setAllowInsecureTls(v => !v)} + onChange={toggleInsecureTls} ariaLabel={t('settings.airtrail.allowInsecureTls')} /> } @@ -115,7 +70,7 @@ export default function MAirTrailConnectionSection(): React.ReactElement { trailing={ setWriteEnabled(v => !v)} + onChange={toggleWriteEnabled} ariaLabel={t('settings.airtrail.writeBack')} /> }