From dfbe23e5b89ca617b92af9076a5d94e7fc9c4b56 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 13 Nov 2026 21:16:17 +0100 Subject: [PATCH] fix: one status tile list for both admin cards; clear the copied timer on unmount - The mail import's status tiles (which, labels, tones) were written out in the desktop and the mobile card alike; statTiles in the shared model holds them, each card keeps its own look. - The journey share link set 'copied' back two seconds after a copy even when the dialog had closed, which also broke a client test run (window gone at teardown). useCopied, the trip share's hook, moves to hooks/ and serves both; it clears its timer on unmount. - PlanLlmConfig is not exported: only the union is used outside. --- .../Admin/mailIngest/MailIngestAdminCard.tsx | 20 +++----- .../mailIngest/mailIngestAdminModel.test.ts | 15 +++++- .../Admin/mailIngest/mailIngestAdminModel.ts | 29 ++++++++++++ .../components/Journey/useJourneyShareLink.ts | 10 ++-- client/src/components/Trips/useTripShare.ts | 16 +------ client/src/hooks/useCopied.test.ts | 47 +++++++++++++++++++ client/src/hooks/useCopied.ts | 25 ++++++++++ .../screens/admin/MMailIngestAdminCard.tsx | 20 +++----- server/src/nest/llm-parse/llm-config.ts | 2 +- 9 files changed, 134 insertions(+), 50 deletions(-) create mode 100644 client/src/hooks/useCopied.test.ts create mode 100644 client/src/hooks/useCopied.ts diff --git a/client/src/components/Admin/mailIngest/MailIngestAdminCard.tsx b/client/src/components/Admin/mailIngest/MailIngestAdminCard.tsx index b1b053917..6ab318d5f 100644 --- a/client/src/components/Admin/mailIngest/MailIngestAdminCard.tsx +++ b/client/src/components/Admin/mailIngest/MailIngestAdminCard.tsx @@ -1,7 +1,6 @@ import { AlertTriangle, CheckCircle, - Clock, Copy, Inbox, Loader2, @@ -11,6 +10,7 @@ import { Upload, Webhook, XCircle, + type LucideIcon, } from 'lucide-react'; import type { ReactNode } from 'react'; import { useTranslation } from '../../../i18n'; @@ -33,11 +33,13 @@ import { useToast } from '../../shared/Toast'; import { TRANSPORTS, problemText, + statTiles, statusFacts, transportInfo, transportLabel, type MailAdminForm, type MailAdminProblem, + type StatTone, } from './mailIngestAdminModel'; import { useMailIngestAdmin, type MailIngestAdmin } from './useMailIngestAdmin'; @@ -169,8 +171,6 @@ function WebhookFields({ admin, form, t, error }: FieldsProps) { ); } -type StatTone = 'neutral' | 'success' | 'warning' | 'danger'; - const STAT_TONE: Record = { neutral: { icon: 'bg-surface-tertiary text-content-muted', value: 'text-content' }, success: { icon: 'bg-success-soft text-success', value: 'text-content' }, @@ -185,7 +185,7 @@ function StatTile({ value, tone, }: { - icon: typeof Mail; + icon: LucideIcon; label: string; value: number; tone: StatTone; @@ -229,15 +229,9 @@ function StatusBlock({ admin, t, locale }: { admin: MailIngestAdmin; t: T; local
- - - - 0 ? 'danger' : 'neutral'} - /> + {statTiles(facts, t).map(({ key, ...tile }) => ( + + ))}
{status.last_error && (

diff --git a/client/src/components/Admin/mailIngest/mailIngestAdminModel.test.ts b/client/src/components/Admin/mailIngest/mailIngestAdminModel.test.ts index ccb720323..f01d50cfd 100644 --- a/client/src/components/Admin/mailIngest/mailIngestAdminModel.test.ts +++ b/client/src/components/Admin/mailIngest/mailIngestAdminModel.test.ts @@ -1,4 +1,4 @@ -// FE-ADMIN-MAILINGEST-001 to FE-ADMIN-MAILINGEST-007 +// FE-ADMIN-MAILINGEST-001 to FE-ADMIN-MAILINGEST-008 import type { MailIngestAdminStatusResponse } from '@trek/shared'; import { describe, expect, it } from 'vitest'; import { @@ -10,6 +10,7 @@ import { formOf, problemText, problemsOf, + statTiles, statusFacts, transportInfo, transportLabel, @@ -105,4 +106,16 @@ describe('mailIngestAdminModel', () => { 'mailAdmin.transport.webhookInfo', ]); }); + + it('FE-ADMIN-MAILINGEST-008: the status tiles in order, the failed one red only when something failed', () => { + const t = ((key: string) => key) as never; + const tiles = statTiles({ mails: 4, added: 2, waiting: 1, failed: 0 }, t); + expect(tiles.map(({ key, label, value, tone }) => [key, label, value, tone])).toEqual([ + ['mails', 'mailAdmin.status.mails', 4, 'neutral'], + ['added', 'mailAdmin.status.added', 2, 'success'], + ['waiting', 'mailAdmin.status.waiting', 1, 'warning'], + ['failed', 'mailAdmin.status.failed', 0, 'neutral'], + ]); + expect(statTiles({ mails: 1, added: 0, waiting: 0, failed: 1 }, t)[3].tone).toBe('danger'); + }); }); diff --git a/client/src/components/Admin/mailIngest/mailIngestAdminModel.ts b/client/src/components/Admin/mailIngest/mailIngestAdminModel.ts index 8618464a7..98b2ac036 100644 --- a/client/src/components/Admin/mailIngest/mailIngestAdminModel.ts +++ b/client/src/components/Admin/mailIngest/mailIngestAdminModel.ts @@ -1,4 +1,5 @@ import type { MailIngestAdminStatusResponse, MailIngestImapTls } from '@trek/shared'; +import { CheckCircle, Clock, Mail, XCircle, type LucideIcon } from 'lucide-react'; import type { TranslationFn } from '../../../types'; /** @@ -129,6 +130,34 @@ export function statusFacts(status: MailIngestAdminStatusResponse): { }; } +/** How a status tile is coloured; each card maps it onto its own tokens. */ +export type StatTone = 'neutral' | 'success' | 'warning' | 'danger'; + +/** One number of the status card. */ +export interface StatTile { + key: 'mails' | 'added' | 'waiting' | 'failed'; + icon: LucideIcon; + label: string; + value: number; + tone: StatTone; +} + +/** The day's numbers as the status tiles show them, in order, on desktop and mobile alike. */ +export function statTiles(facts: ReturnType, t: TranslationFn): StatTile[] { + return [ + { key: 'mails', icon: Mail, label: t('mailAdmin.status.mails'), value: facts.mails, tone: 'neutral' }, + { key: 'added', icon: CheckCircle, label: t('mailAdmin.status.added'), value: facts.added, tone: 'success' }, + { key: 'waiting', icon: Clock, label: t('mailAdmin.status.waiting'), value: facts.waiting, tone: 'warning' }, + { + key: 'failed', + icon: XCircle, + label: t('mailAdmin.status.failed'), + value: facts.failed, + tone: facts.failed > 0 ? 'danger' : 'neutral', + }, + ]; +} + /** A problem in the admin's words; a switch of literal keys so the i18n check sees each one. */ export function problemText(problem: MailAdminProblem, t: TranslationFn): string { switch (problem) { diff --git a/client/src/components/Journey/useJourneyShareLink.ts b/client/src/components/Journey/useJourneyShareLink.ts index 692f4448d..cf0d12283 100644 --- a/client/src/components/Journey/useJourneyShareLink.ts +++ b/client/src/components/Journey/useJourneyShareLink.ts @@ -1,8 +1,8 @@ import { useEffect, useState } from 'react'; import { journeyApi } from '../../api/client'; +import { useCopied } from '../../hooks/useCopied'; import { useTranslation } from '../../i18n'; -import { copyText } from '../../utils/clipboard'; import { useToast } from '../shared/Toast'; export interface JourneyShareLink { @@ -41,7 +41,7 @@ export function useJourneyShareLink(journeyId: number) { const toast = useToast(); const [link, setLink] = useState(null); const [loading, setLoading] = useState(true); - const [copied, setCopied] = useState(false); + const { copied, copy } = useCopied(); /** The share token is the owner's to manage; a contributor is refused. */ const [manageable, setManageable] = useState(true); @@ -103,11 +103,7 @@ export function useJourneyShareLink(journeyId: number) { const shareUrl = journeyShareUrl(link, window.location.origin); - const copyLink = async () => { - if (!(await copyText(shareUrl))) return; - setCopied(true); - setTimeout(() => setCopied(false), 2000); - }; + const copyLink = () => copy(shareUrl); return { link, loading, manageable, copied, shareUrl, createLink, togglePerm, deleteLink, copyLink }; } diff --git a/client/src/components/Trips/useTripShare.ts b/client/src/components/Trips/useTripShare.ts index 5075dc4fe..8f4ae05fa 100644 --- a/client/src/components/Trips/useTripShare.ts +++ b/client/src/components/Trips/useTripShare.ts @@ -7,7 +7,7 @@ import { useCanDo } from '../../store/permissionsStore' import { useTripStore } from '../../store/tripStore' import { useTranslation } from '../../i18n' import { getApiErrorMessage } from '../../types' -import { copyText } from '../../utils/clipboard' +import { useCopied } from '../../hooks/useCopied' import { refusedOffline } from '../../utils/needsConnection' // The state behind the share dialog, shared by the desktop dialog and the one the @@ -37,20 +37,6 @@ export const SHARE_OPTIONS: { key: SharePerm; label: string }[] = [ { key: 'share_hide_images', label: 'share.optHideImages' }, ] -/** Copy text and show "Copied" for two seconds, the timer restarting on every copy. */ -function useCopied() { - const [copied, setCopied] = useState(false) - const timer = useRef | null>(null) - useEffect(() => () => { if (timer.current) clearTimeout(timer.current) }, []) - const copy = async (text: string | null) => { - if (!text || !(await copyText(text))) return - setCopied(true) - if (timer.current) clearTimeout(timer.current) - timer.current = setTimeout(() => setCopied(false), 2000) - } - return { copied, copy } -} - /** The public read-only link and what it may show. */ export function useShareLink(tripId: number) { const [token, setToken] = useState(null) diff --git a/client/src/hooks/useCopied.test.ts b/client/src/hooks/useCopied.test.ts new file mode 100644 index 000000000..278c11e04 --- /dev/null +++ b/client/src/hooks/useCopied.test.ts @@ -0,0 +1,47 @@ +// FE-HOOK-COPIED-001 to FE-HOOK-COPIED-003 +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../utils/clipboard', () => ({ copyText: vi.fn(async () => true) })); + +import { copyText } from '../utils/clipboard'; +import { useCopied } from './useCopied'; + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => vi.useRealTimers()); + +describe('useCopied', () => { + it('FE-HOOK-COPIED-001: shows copied for two seconds, a second copy starting the time over', async () => { + const { result } = renderHook(() => useCopied()); + await act(() => result.current.copy('https://x')); + expect(result.current.copied).toBe(true); + act(() => { + vi.advanceTimersByTime(1500); + }); + await act(() => result.current.copy('https://x')); + act(() => { + vi.advanceTimersByTime(1500); + }); + expect(result.current.copied).toBe(true); + act(() => { + vi.advanceTimersByTime(500); + }); + expect(result.current.copied).toBe(false); + }); + + it('FE-HOOK-COPIED-002: nothing to copy or a refused clipboard shows nothing', async () => { + const { result } = renderHook(() => useCopied()); + await act(() => result.current.copy(null)); + expect(copyText).not.toHaveBeenCalledWith(null); + vi.mocked(copyText).mockResolvedValueOnce(false); + await act(() => result.current.copy('https://x')); + expect(result.current.copied).toBe(false); + }); + + it('FE-HOOK-COPIED-003: unmounting clears the timer, so no state is set afterwards', async () => { + const { result, unmount } = renderHook(() => useCopied()); + await act(() => result.current.copy('https://x')); + unmount(); + expect(vi.getTimerCount()).toBe(0); + }); +}); diff --git a/client/src/hooks/useCopied.ts b/client/src/hooks/useCopied.ts new file mode 100644 index 000000000..a647adad5 --- /dev/null +++ b/client/src/hooks/useCopied.ts @@ -0,0 +1,25 @@ +import { useEffect, useRef, useState } from 'react'; +import { copyText } from '../utils/clipboard'; + +/** + * Copy text and show "Copied" for two seconds, the timer restarting on every + * copy. The timer is cleared on unmount, so a dialog closed right after a copy + * sets no state on a component that is gone. + */ +export function useCopied(ms = 2000) { + const [copied, setCopied] = useState(false); + const timer = useRef | null>(null); + useEffect( + () => () => { + if (timer.current) clearTimeout(timer.current); + }, + [] + ); + const copy = async (text: string | null) => { + if (!text || !(await copyText(text))) return; + setCopied(true); + if (timer.current) clearTimeout(timer.current); + timer.current = setTimeout(() => setCopied(false), ms); + }; + return { copied, copy }; +} diff --git a/client/src/mobile/screens/admin/MMailIngestAdminCard.tsx b/client/src/mobile/screens/admin/MMailIngestAdminCard.tsx index a2d86bc41..414e5c0f3 100644 --- a/client/src/mobile/screens/admin/MMailIngestAdminCard.tsx +++ b/client/src/mobile/screens/admin/MMailIngestAdminCard.tsx @@ -1,12 +1,14 @@ -import { CheckCircle, Clock, Inbox, Mail, Upload, Webhook, XCircle } from 'lucide-react'; +import { Inbox, Upload, Webhook, type LucideIcon } from 'lucide-react'; import { TRANSPORTS, problemText, + statTiles, statusFacts, transportInfo, transportLabel, type MailAdminForm, type MailAdminProblem, + type StatTone, } from '../../../components/Admin/mailIngest/mailIngestAdminModel'; import { useMailIngestAdmin, type MailIngestAdmin } from '../../../components/Admin/mailIngest/useMailIngestAdmin'; import { useTranslation } from '../../../i18n'; @@ -111,8 +113,6 @@ function WebhookFields({ admin, form, t }: PartProps) { ); } -type StatTone = 'neutral' | 'success' | 'warning' | 'danger'; - const STAT_COLOR: Record = { neutral: 'var(--m-muted)', success: 'var(--m-st-confirmed)', @@ -127,7 +127,7 @@ function StatTile({ value, tone, }: { - icon: typeof Mail; + icon: LucideIcon; label: string; value: number; tone: StatTone; @@ -173,15 +173,9 @@ function Status({ admin, t, locale }: { admin: MailIngestAdmin; t: TranslationFn {t('mailAdmin.status.lastRead', { time: lastRead })}

- - - - 0 ? 'danger' : 'neutral'} - /> + {statTiles(facts, t).map(({ key, ...tile }) => ( + + ))}
{status.last_error && (

diff --git a/server/src/nest/llm-parse/llm-config.ts b/server/src/nest/llm-parse/llm-config.ts index e98f11a2d..4322e4121 100644 --- a/server/src/nest/llm-parse/llm-config.ts +++ b/server/src/nest/llm-parse/llm-config.ts @@ -31,7 +31,7 @@ export interface ApiLlmConfig { * so it has no address or key here and is read through `read`. Never stored in * the addon config, only resolved from the shared connection. */ -export interface PlanLlmConfig { +interface PlanLlmConfig { provider: 'chatgpt'; model: string; read: StructuredReader;