diff --git a/client/src/components/Files/FileManager.test.tsx b/client/src/components/Files/FileManager.test.tsx index 0ff151f32..7bd24504e 100644 --- a/client/src/components/Files/FileManager.test.tsx +++ b/client/src/components/Files/FileManager.test.tsx @@ -1,10 +1,11 @@ -// FE-COMP-FILEMANAGER-001 to FE-COMP-FILEMANAGER-012 +// FE-COMP-FILEMANAGER-001 to FE-COMP-FILEMANAGER-038 import { render, screen, waitFor, fireEvent } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; import { useAuthStore } from '../../store/authStore'; import { useTripStore } from '../../store/tripStore'; +import { useDocSyncOfferStore } from '../../store/docSyncOfferStore'; import { resetAllStores, seedStore } from '../../../tests/helpers/store'; import { buildUser, buildTrip } from '../../../tests/helpers/factories'; import type { TripFile } from '../../types'; @@ -104,6 +105,15 @@ beforeEach(() => { }), ); + // Document sync, as a fresh install has it: every provider off, nothing bound. + useDocSyncOfferStore.setState({ bound: {}, providers: null }); + server.use( + http.get('/api/trips/:tripId/docsync/providers', () => HttpResponse.json([])), + http.get('/api/trips/:tripId/docsync/links', () => HttpResponse.json([])), + http.get('/api/trips/:tripId/docsync/connections', () => HttpResponse.json([])), + http.get('/api/trips/:tripId/docsync/status', () => HttpResponse.json({ links: [], items: {} })), + ); + // Stub window.confirm vi.spyOn(window, 'confirm').mockReturnValue(true); }); @@ -631,4 +641,60 @@ describe('FileManager', () => { expect(call[0]).toBeInstanceOf(FormData); }); }); + + describe('document sync button', () => { + const paperless = { id: 'paperless', name: 'Paperless-ngx', description: null, icon: 'paperless', available: true, fields: [] }; + const trashButton = () => screen.getByText('Trash').closest('button') as HTMLButtonElement; + + it('FE-COMP-FILEMANAGER-036: with no provider on and nothing bound there is no sync button, and the trash sits flush right', async () => { + const asked: string[] = []; + server.use( + http.get('/api/trips/:tripId/docsync/links', () => { + asked.push('links'); + return HttpResponse.json([]); + }), + ); + render(); + + await waitFor(() => expect(asked).toContain('links')); + expect(screen.queryByTitle('Document sync')).not.toBeInTheDocument(); + expect(trashButton()).toHaveStyle({ marginLeft: 'auto' }); + }); + + it('FE-COMP-FILEMANAGER-037: somebody who may bind the trip gets it once a provider is on, and it opens the panel', async () => { + server.use(http.get('/api/trips/:tripId/docsync/providers', () => HttpResponse.json([paperless]))); + render(); + const user = userEvent.setup(); + + const button = await screen.findByTitle('Document sync'); + expect(button).toHaveStyle({ marginLeft: 'auto' }); + expect(trashButton().style.marginLeft).toBe(''); + + await user.click(button); + expect(await screen.findByText('Connect a provider')).toBeInTheDocument(); + }); + + it('FE-COMP-FILEMANAGER-038: a member sees it on a bound trip only', async () => { + seedStore(useAuthStore, { user: buildUser({ id: 999, role: 'user' }), isAuthenticated: true }); + server.use( + http.get('/api/trips/:tripId/docsync/providers', () => HttpResponse.json([paperless])), + http.get('/api/trips/:tripId/docsync/links', () => HttpResponse.json([{ id: 1, providerId: 'paperless' }])), + ); + const { unmount } = render(); + expect(await screen.findByTitle('Document sync')).toBeInTheDocument(); + unmount(); + + useDocSyncOfferStore.setState({ bound: {}, providers: null }); + const asked: string[] = []; + server.use( + http.get('/api/trips/:tripId/docsync/links', () => { + asked.push('links'); + return HttpResponse.json([]); + }), + ); + render(); + await waitFor(() => expect(asked).toContain('links')); + expect(screen.queryByTitle('Document sync')).not.toBeInTheDocument(); + }); + }); }); diff --git a/client/src/components/Files/FileManager.tsx b/client/src/components/Files/FileManager.tsx index 7e103cbfa..cf5db4418 100644 --- a/client/src/components/Files/FileManager.tsx +++ b/client/src/components/Files/FileManager.tsx @@ -33,7 +33,7 @@ export default function FileManager(props: FileManagerProps) { S.setShowDocSync(false)} /> )} diff --git a/client/src/components/Files/FileManagerToolbar.tsx b/client/src/components/Files/FileManagerToolbar.tsx index af4b7bcc6..9e0d33d24 100644 --- a/client/src/components/Files/FileManagerToolbar.tsx +++ b/client/src/components/Files/FileManagerToolbar.tsx @@ -2,7 +2,7 @@ import { FolderSync, Star, Trash2 } from 'lucide-react' import type { FileManagerState } from './useFileManager' export function FileManagerToolbar(S: FileManagerState) { - const { showTrash, t, files, filterType, setFilterType, toggleTrash, setShowDocSync } = S + const { showTrash, t, files, filterType, setFilterType, toggleTrash, setShowDocSync, docSyncOffered } = S return (
setShowDocSync(true)} title={t('docsync.title')} style={{ - appearance: 'none', border: '1px solid var(--edge)', cursor: 'pointer', fontFamily: 'inherit', - display: 'inline-flex', alignItems: 'center', gap: 6, - padding: '9px 12px', borderRadius: 10, fontSize: 'calc(13px * var(--fs-scale-body, 1))', fontWeight: 500, - background: 'transparent', color: 'var(--content-muted)', flexShrink: 0, marginLeft: 'auto', - }}> - {t('docsync.title')} - + belongs on the same screen as the documents. Only where there is + something behind it: see useDocSyncOffered. */} + {docSyncOffered && ( + + )} + {/* The first of the two takes the free space, so without the sync + button the trash sits flush right the way it always has. */} - {isOwner && ( + {canManage && (
- {isOwner && ( + {canManage && ( <>
@@ -138,7 +140,7 @@ function Sidebar({ providers, available, activeId, - isOwner, + canManage, onSelect, onAdd, }: { @@ -146,7 +148,7 @@ function Sidebar({ providers: DocSyncProvider[] available: DocSyncProvider[] activeId: number | null - isOwner: boolean + canManage: boolean onSelect: (id: number) => void onAdd: (p: DocSyncProvider) => void }) { @@ -163,7 +165,7 @@ function Sidebar({
  • p.id === link.providerId)?.name || link.providerId} + title={storeName(link, providers)} subtitle={link.remoteLabel || link.remoteRootPath || link.scopeKey} state={link.lastSyncState} active={link.id === activeId} @@ -175,7 +177,7 @@ function Sidebar({ )} - {isOwner && available.length > 0 && ( + {canManage && available.length > 0 && (

    {links.length === 0 ? t('docsync.addProvider') : t('docsync.addAnother')} @@ -340,7 +342,7 @@ function Attention({ } /** The detail column before anything is bound: an invitation, not a blank. */ -function NothingBound({ isOwner }: { isOwner: boolean }) { +function NothingBound({ canManage }: { canManage: boolean }) { const { t } = useTranslation() return (
    @@ -350,7 +352,7 @@ function NothingBound({ isOwner }: { isOwner: boolean }) {

    {t('docsync.empty.title')}

    - {isOwner ? t('docsync.empty.hintOwner') : t('docsync.empty.hintMember')} + {canManage ? t('docsync.empty.hintOwner') : t('docsync.empty.hintMember')}

    diff --git a/client/src/components/Files/docsync/DocSyncScopeModal.tsx b/client/src/components/Files/docsync/DocSyncScopeModal.tsx index 811b76f91..daa036819 100644 --- a/client/src/components/Files/docsync/DocSyncScopeModal.tsx +++ b/client/src/components/Files/docsync/DocSyncScopeModal.tsx @@ -12,7 +12,7 @@ import type { DocSyncConnection, DocSyncScope, useDocSync } from './useDocSync' * would be slower than the filter it replaces. * * "Create" is a first-class action rather than a fallback at the bottom, - * because for a new trip it is the common case — there is no folder yet. + * because for a new trip it is the common case: there is no folder yet. */ export default function DocSyncScopeModal({ connection, @@ -37,20 +37,22 @@ export default function DocSyncScopeModal({ const [newName, setNewName] = useState(suggestedName) const [working, setWorking] = useState(null) + // `loadScopes`, not `sync`: the hook hands back a fresh object on every + // render of the panel above, so depending on it re-listed the provider's + // folders each time anything up there changed. The callback itself is + // stable. Taken out of `sync` first, because calling it as `sync.loadScopes` + // inside the effect makes the whole object a dependency again. + const { loadScopes } = sync useEffect(() => { - // `sync.loadScopes`, not `sync`: the hook hands back a fresh object on - // every render of the panel above, so depending on it re-listed the - // provider's folders each time anything up there changed. The callback - // itself is stable. let cancelled = false void (async () => { - const res = await sync.loadScopes(connection.id) + const res = await loadScopes(connection.id) if (cancelled) return setScopes(res.scopes) setError(res.error ?? null) })() return () => { cancelled = true } - }, [connection.id, sync.loadScopes]) + }, [connection.id, loadScopes]) const shown = useMemo(() => { const needle = query.trim().toLowerCase() diff --git a/client/src/components/Files/docsync/useDocSync.test.ts b/client/src/components/Files/docsync/useDocSync.test.ts index c94989d67..68e4d93ed 100644 --- a/client/src/components/Files/docsync/useDocSync.test.ts +++ b/client/src/components/Files/docsync/useDocSync.test.ts @@ -1,4 +1,4 @@ -// FE-DOCSYNC-HOOK-001 to FE-DOCSYNC-HOOK-022 +// FE-DOCSYNC-HOOK-001 to FE-DOCSYNC-HOOK-024 /** * The document-sync hook. @@ -40,7 +40,7 @@ vi.mock('../../../api/client', async (importOriginal) => { } }) -import { useDocSync, canManageDocSync } from './useDocSync' +import { useDocSync, canManageDocSync, storeName, type DocSyncLink, type DocSyncProvider } from './useDocSync' const TRIP = 3 @@ -179,6 +179,21 @@ describe('useDocSync initial load', () => { expect(result.current.loading).toBe(false) }) + it('FE-DOCSYNC-HOOK-023: a reload that finds the addon gone drops the bindings it had loaded', async () => { + statusApi.mockResolvedValue({ items: { conflict: 1 }, links: [] }) + const { result } = await mountLoaded() + expect(result.current.links).toHaveLength(1) + + providersApi.mockRejectedValue(Object.assign(new Error('not found'), { response: { status: 404 } })) + await act(async () => { await result.current.load() }) + + expect(result.current.providers).toEqual([]) + expect(result.current.connections).toEqual([]) + expect(result.current.links).toEqual([]) + expect(result.current.itemCounts).toEqual({}) + expect(result.current.error).toBeNull() + }) + it('FE-DOCSYNC-HOOK-006: asks the server nothing while the panel is closed', async () => { const { result } = mount(TRIP, false) @@ -374,6 +389,18 @@ describe('useDocSync run and save feedback', () => { }) }) +describe('storeName', () => { + it('FE-DOCSYNC-HOOK-024: prefers the provider list, then the name on the link, then the raw id', () => { + const bound = link() as DocSyncLink + const listed = [provider({ name: 'Nextcloud Hub' }) as DocSyncProvider] + + expect(storeName({ ...bound, providerName: 'Nextcloud' }, listed)).toBe('Nextcloud Hub') + // Switched off by an admin, so the providers route no longer lists it. + expect(storeName({ ...bound, providerName: 'Nextcloud' }, [])).toBe('Nextcloud') + expect(storeName(bound, [])).toBe('nextcloud') + }) +}) + describe('canManageDocSync', () => { it('FE-DOCSYNC-HOOK-019: the trip owner may repoint a binding', () => { expect(canManageDocSync({ id: 4, role: 'user' }, { user_id: 4 })).toBe(true) diff --git a/client/src/components/Files/docsync/useDocSync.ts b/client/src/components/Files/docsync/useDocSync.ts index e2f63c70b..c400872ee 100644 --- a/client/src/components/Files/docsync/useDocSync.ts +++ b/client/src/components/Files/docsync/useDocSync.ts @@ -64,10 +64,28 @@ export interface DocSyncLink { lastSyncState: string lastSyncError: string | null webhookUrl: string | null + /** + * The store's display name as the server knows it. Carried on the link + * because the providers route only lists what an admin has switched on, and + * a binding outlives its provider being switched off. + */ + providerName?: string /** Standing counts per side, from the status route. */ holdings?: { inTrek: number; atProvider: number; paired: number; missing: number } } +/** + * What to call the store a binding points at. + * + * One lookup for both shells, so the list row, the card header and the flow + * summary cannot name the same store three different ways. The providers list + * wins because the rest of the dialog uses its names; the link's own name + * covers a provider that has since been switched off. + */ +export function storeName(link: DocSyncLink, providers: readonly DocSyncProvider[]): string { + return providers.find(p => p.id === link.providerId)?.name || link.providerName || link.providerId +} + /** * Whether this person may change a sync binding. * @@ -76,7 +94,7 @@ export interface DocSyncLink { * share a folder the owner never chose to share. Instance admins are included * because they already override every other permission check in the client. * - * Shared by both shells rather than written out twice — the desktop file + * Shared by both shells rather than written out twice: the desktop file * manager and the phone sheet must not be able to drift on who may do this. */ export function canManageDocSync( @@ -100,7 +118,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { * Whether the first load is through. * * Every mutation refreshes, and a refresh that flips `loading` swaps the whole - * dialog for a spinner and back — a white flash on something as small as + * dialog for a spinner and back: a white flash on something as small as * toggling a direction. Only the first load has nothing to show yet. */ const loadedOnce = useRef(false) @@ -108,7 +126,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { /** * Which load is the current one. * - * Four requests go out per load and several loads overlap — every mutation + * Four requests go out per load and several loads overlap: every mutation * starts one, and so does every ping. Without a generation stamp the slowest * response wins, which on a binding somebody just changed means the screen * settles on the state from before the change. @@ -144,15 +162,20 @@ export function useDocSync(tripId: number | string, enabled: boolean) { loadFailed.current = false loadedOnce.current = true } catch (e: unknown) { - // A 403 or 404 really does mean "nothing to configure here" — the addon is - // off or this person lost access — and blanking the panel is the honest + // A 403 or 404 really does mean "nothing to configure here" (the addon is + // off or this person lost access), and blanking the panel is the honest // answer. Anything else is a failure to say so: blanking on a dropped // connection told the user their provider list was empty. if (mine !== generation.current) return const status = (e as { response?: { status?: number } })?.response?.status loadFailed.current = true if (status === 403 || status === 404) { + // The bindings go too. Both shells keep a binding on screen with no + // provider left, so emptying the providers alone blanked nothing. setProviders([]) + setConnections([]) + setLinks([]) + setItemCounts({}) setError(null) } else { setError(readError(e)) @@ -191,7 +214,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { }, [tripId, load]) /** - * Probe without saving. Always resolves to a verdict object — the route + * Probe without saving. Always resolves to a verdict object: the route * answers 200 even for an unreachable instance, because a form showing a typo * needs a field to render, not an exception. */ @@ -216,7 +239,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { /** * Make a container at the provider. * - * Reports failure the same way every other write here does — a rejected + * Reports failure the same way every other write here does. A rejected * create used to escape as an unhandled rejection, so the button blinked and * nothing happened and nothing was said. Null rather than a throw, because * the caller's next step is to bind what came back, and there is nothing to @@ -254,7 +277,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { * Change one binding, optimistically. * * The reload in `finally` puts the server's answer back either way, so a - * refused change corrects itself on screen — but it used to do that in + * refused change corrects itself on screen, but it used to do that in * silence, with the rejection escaping into a `void` call site. The switch * simply flipped back and nobody said why. */ @@ -266,8 +289,8 @@ export function useDocSync(tripId: number | string, enabled: boolean) { await load() return true } catch (e: unknown) { - // Reload first, so the optimistic value is corrected, then say why — - // the other way round the reload cleared the message again. + // Reload first, so the optimistic value is corrected, then say why. + // The other way round the reload cleared the message again. await load() setError(readError(e)) return false @@ -299,8 +322,8 @@ export function useDocSync(tripId: number | string, enabled: boolean) { await load() return res } catch (e: unknown) { - // A refused run is a thing the person asked for and did not get — an - // orphaned binding answers 409 here — so it has to be said out loud + // A refused run is a thing the person asked for and did not get (an + // orphaned binding answers 409 here), so it has to be said out loud // rather than escaping into the `void` at the call site. After the // reload, which would otherwise clear the message. await load() @@ -334,7 +357,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { * The error code a failed call carries, or `unknown`. * * Both shells render this as `docsync.error.`, so anything that is not - * one of the codes reaches the screen as a raw translation key — and the server + * one of the codes reaches the screen as a raw translation key, and the server * answers plenty of routes with a sentence ("Link not found") rather than a * code. A general message is worse than a specific one and far better than * `docsync.error.Link not found`. @@ -342,7 +365,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) { function readError(e: unknown): string { const res = (e as { response?: { data?: { error?: string; message?: string } } })?.response const raw = res?.data?.error || res?.data?.message || '' - // A code passes through — the sync codes and the handful of keys the routes + // A code passes through: the sync codes and the handful of keys the routes // answer with are all of this shape. A sentence does not: several routes // answer with prose ("Link not found"), and both shells render this as // `docsync.error.`, so that reached the screen as a raw translation diff --git a/client/src/components/Files/docsync/useDocSyncOffered.test.ts b/client/src/components/Files/docsync/useDocSyncOffered.test.ts new file mode 100644 index 000000000..5e984cdce --- /dev/null +++ b/client/src/components/Files/docsync/useDocSyncOffered.test.ts @@ -0,0 +1,190 @@ +// FE-DOCSYNC-OFFER-001 to FE-DOCSYNC-OFFER-013 + +/** + * Whether the Files screen offers document sync. + * + * The providers ship switched off, so the rule is what keeps an upgraded + * instance from growing a button into an empty dialog on every trip: a bound + * trip shows it to everybody, an unbound one only to somebody who could bind + * it, and only once the admin has switched a provider on. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { http, HttpResponse } from 'msw' +import { act, renderHook, waitFor } from '../../../../tests/helpers/render' +import { server } from '../../../../tests/helpers/msw/server' +import { useDocSyncOfferStore } from '../../../store/docSyncOfferStore' +import { setForcedOffline } from '../../../sync/networkMode' +import { useDocSyncOffered } from './useDocSyncOffered' + +const listeners = new Set<(e: Record) => void>() + +vi.mock('../../../api/websocket', async (importOriginal) => ({ + ...(await importOriginal()), + addListener: (fn: (e: Record) => void) => { listeners.add(fn) }, + removeListener: (fn: (e: Record) => void) => { listeners.delete(fn) }, +})) + +/** Deliver an event the way the websocket layer would. */ +function emit(type: string): void { + for (const fn of [...listeners]) fn({ type }) +} + +const TRIP = 4 + +let providerCalls = 0 +let linkCalls = 0 + +function routes({ providers = [] as unknown[], links = [] as unknown[], linksStatus = 200 } = {}) { + server.use( + http.get('/api/trips/:tripId/docsync/providers', () => { + providerCalls += 1 + return HttpResponse.json(providers) + }), + http.get('/api/trips/:tripId/docsync/links', () => { + linkCalls += 1 + return linksStatus === 200 + ? HttpResponse.json(links) + : HttpResponse.json({ error: 'nope' }, { status: linksStatus }) + }), + ) +} + +const paperless = { id: 'paperless', name: 'Paperless-ngx', available: true } +const binding = { id: 1, providerId: 'paperless' } + +beforeEach(() => { + providerCalls = 0 + linkCalls = 0 + listeners.clear() + useDocSyncOfferStore.setState({ bound: {}, providers: null }) +}) + +afterEach(() => setForcedOffline(false)) + +describe('useDocSyncOffered', () => { + it('FE-DOCSYNC-OFFER-001: somebody who may bind the trip sees it once a provider is on, without asking about bindings', async () => { + routes({ providers: [paperless] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, true)) + + await waitFor(() => expect(result.current).toBe(true)) + expect(providerCalls).toBe(1) + expect(linkCalls).toBe(0) + }) + + it('FE-DOCSYNC-OFFER-002: with every provider off and nothing bound, nobody sees it', async () => { + routes({ providers: [] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, true)) + + await waitFor(() => expect(linkCalls).toBe(1)) + expect(result.current).toBe(false) + }) + + it('FE-DOCSYNC-OFFER-003: a provider row without an adapter does not count as offered', async () => { + routes({ providers: [{ ...paperless, available: false }] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, true)) + + await waitFor(() => expect(linkCalls).toBe(1)) + expect(result.current).toBe(false) + }) + + it('FE-DOCSYNC-OFFER-004: a member sees a bound trip, and is never asked about providers', async () => { + routes({ providers: [paperless], links: [binding] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, false)) + + await waitFor(() => expect(result.current).toBe(true)) + expect(providerCalls).toBe(0) + }) + + it('FE-DOCSYNC-OFFER-005: a member does not see an unbound trip, providers or not', async () => { + routes({ providers: [paperless], links: [] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, false)) + + await waitFor(() => expect(linkCalls).toBe(1)) + expect(result.current).toBe(false) + }) + + it('FE-DOCSYNC-OFFER-006: a bound trip keeps the button after the admin switched every provider off', async () => { + routes({ providers: [], links: [binding] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, true)) + + await waitFor(() => expect(result.current).toBe(true)) + }) + + it('FE-DOCSYNC-OFFER-007: the last answer renders at once on the next visit, then is checked again', async () => { + routes({ links: [binding] }) + const first = renderHook(() => useDocSyncOffered(TRIP, false)) + await waitFor(() => expect(first.result.current).toBe(true)) + first.unmount() + + const second = renderHook(() => useDocSyncOffered(TRIP, false)) + expect(second.result.current).toBe(true) + await waitFor(() => expect(linkCalls).toBe(2)) + }) + + it('FE-DOCSYNC-OFFER-008: offline it asks nothing and keeps what it knew, and asks again once back', async () => { + routes({ links: [] }) + useDocSyncOfferStore.setState({ bound: { [TRIP]: true } }) + act(() => setForcedOffline(true)) + const { result } = renderHook(() => useDocSyncOffered(TRIP, false)) + + expect(result.current).toBe(true) + expect(linkCalls).toBe(0) + + act(() => setForcedOffline(false)) + await waitFor(() => expect(result.current).toBe(false)) + expect(linkCalls).toBe(1) + }) + + it('FE-DOCSYNC-OFFER-009: a request that never got an answer leaves the last one standing', async () => { + server.use(http.get('/api/trips/:tripId/docsync/links', () => HttpResponse.error())) + useDocSyncOfferStore.setState({ bound: { [TRIP]: true } }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, false)) + + await act(() => useDocSyncOfferStore.getState().refresh(TRIP, false)) + expect(result.current).toBe(true) + }) + + it('FE-DOCSYNC-OFFER-010: the Documents addon switched off is a real no', async () => { + routes({ linksStatus: 403 }) + useDocSyncOfferStore.setState({ bound: { [TRIP]: true } }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, false)) + + await waitFor(() => expect(result.current).toBe(false)) + }) + + it('FE-DOCSYNC-OFFER-011: a refresh asked for while offline sends nothing and changes nothing', async () => { + routes({ providers: [paperless] }) + useDocSyncOfferStore.setState({ providers: false }) + setForcedOffline(true) + + await useDocSyncOfferStore.getState().refresh(TRIP, true) + + expect(providerCalls).toBe(0) + expect(useDocSyncOfferStore.getState().providers).toBe(false) + }) + + it('FE-DOCSYNC-OFFER-012: a member on the Files tab gets the button when the owner binds the trip', async () => { + routes({ links: [] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, false)) + await waitFor(() => expect(linkCalls).toBe(1)) + expect(result.current).toBe(false) + + // The owner binds it: nothing remounts here, the server only pings. + routes({ links: [binding] }) + act(() => emit('docsync:changed')) + + await waitFor(() => expect(result.current).toBe(true)) + expect(linkCalls).toBe(2) + }) + + it('FE-DOCSYNC-OFFER-013: the button goes when the last binding does, with every provider off', async () => { + routes({ providers: [], links: [binding] }) + const { result } = renderHook(() => useDocSyncOffered(TRIP, true)) + await waitFor(() => expect(result.current).toBe(true)) + + routes({ providers: [], links: [] }) + act(() => emit('docsync:changed')) + + await waitFor(() => expect(result.current).toBe(false)) + }) +}) diff --git a/client/src/components/Files/docsync/useDocSyncOffered.ts b/client/src/components/Files/docsync/useDocSyncOffered.ts new file mode 100644 index 000000000..7d73e70be --- /dev/null +++ b/client/src/components/Files/docsync/useDocSyncOffered.ts @@ -0,0 +1,34 @@ +import { useEffect } from 'react' +import { useNetworkMode } from '../../../hooks/useNetworkMode' +import { useDocSyncOfferStore } from '../../../store/docSyncOfferStore' +import { useServerPing } from '../../../sync/useServerPing' + +/** + * Whether this trip's Files screen shows the "Document sync" button. + * + * One hook for both shells, so the toolbar and the phone header cannot drift + * on who sees it. The last known answer renders straight away; the server is + * asked again when the screen mounts and when the connection comes back, and + * never while offline, where the button keeps whatever it last knew. + * + * It is also asked on every `docsync:changed`, which the server sends when a + * binding comes or goes. A member can sit on the Files tab while the owner + * binds the trip, and on desktop that tab may never remount before a reload. + */ +export function useDocSyncOffered(tripId: number | string, canManage: boolean): boolean { + const offered = useDocSyncOfferStore( + s => s.bound[String(tripId)] === true || (canManage && s.providers === true), + ) + const refresh = useDocSyncOfferStore(s => s.refresh) + const { offline } = useNetworkMode() + + useEffect(() => { + if (!offline) void refresh(tripId, canManage) + }, [tripId, canManage, offline, refresh]) + + // Off while offline: the effect above already asks on the way back, and the + // ping's own return-to-online refetch would only ask a second time. + useServerPing('docsync:changed', !offline, () => refresh(tripId, canManage)) + + return offered +} diff --git a/client/src/components/Files/useFileManager.ts b/client/src/components/Files/useFileManager.ts index 5b008cf46..9970931ba 100644 --- a/client/src/components/Files/useFileManager.ts +++ b/client/src/components/Files/useFileManager.ts @@ -8,6 +8,7 @@ import { useCanDo } from '../../store/permissionsStore' import { useTripStore } from '../../store/tripStore' import { useAuthStore } from '../../store/authStore' import { canManageDocSync } from './docsync/useDocSync' +import { useDocSyncOffered } from './docsync/useDocSyncOffered' import { getAuthUrl } from '../../api/authUrl' import { isImage, isMedia, isWalletPass } from './FileManager.helpers' import { openFile as openFileInTab } from '../../utils/fileDownload' @@ -42,7 +43,8 @@ export function useFileManager({ files = [], onUpload, onDelete, onUpdate, place const can = useCanDo() const trip = useTripStore((s) => s.trip) const currentUser = useAuthStore((s) => s.user) - const isTripOwner = canManageDocSync(currentUser, trip) + const canManageSync = canManageDocSync(currentUser, trip) + const docSyncOffered = useDocSyncOffered(tripId, canManageSync) const { t, locale } = useTranslation() const loadTrash = useCallback(async () => { @@ -215,7 +217,7 @@ export function useFileManager({ files = [], onUpload, onDelete, onUpdate, place previewFile, setPreviewFile, previewFileUrl, assignFileId, setAssignFileId, getRootProps, getInputProps, isDragActive, handlePaste, filteredFiles, handleDelete, handleAssign, mediaFiles, openFile, - showDocSync, setShowDocSync, isTripOwner, + showDocSync, setShowDocSync, canManageSync, docSyncOffered, } } diff --git a/client/src/mobile/screens/admin/MAdminAddonManager.tsx b/client/src/mobile/screens/admin/MAdminAddonManager.tsx index b5eaf61b7..99ef39e61 100644 --- a/client/src/mobile/screens/admin/MAdminAddonManager.tsx +++ b/client/src/mobile/screens/admin/MAdminAddonManager.tsx @@ -10,6 +10,7 @@ import { } from 'lucide-react' import DawarichIcon from '../../../components/shared/DawarichIcon' import AirTrailIcon from '../../../components/shared/AirTrailIcon' +import { DOCUMENT_PROVIDER_ICONS } from '../../../components/shared/DocumentProviderIcons' import MToggle from '../../components/MToggle' import { MAdminButton, MAdminCard, MAdminField, MAdminInput, MAdminSecretInput } from './MAdminUi' @@ -37,6 +38,7 @@ function SynologyIcon({ size = 14 }: { size?: number }) { const PROVIDER_ICONS: Record> = { immich: ImmichIcon, synologyphotos: SynologyIcon, + ...DOCUMENT_PROVIDER_ICONS, } interface Addon { @@ -121,8 +123,9 @@ export default function MAdminAddonManager({ bagTrackingEnabled, onToggleBagTrac refreshGlobalAddons() // Journey off disables every photo provider with it, and the response carries // only Journey. Without re-reading, switching Journey back on brings the shelf - // up with providers the database has long since turned off. - if (addon.id === 'journey') await loadAddons() + // up with providers the database has long since turned off. Documents does + // the same to the document providers. + if (addon.id === 'journey' || addon.id === 'documents') await loadAddons() toast.success(t('admin.addons.toast.updated')) } @@ -152,6 +155,7 @@ export default function MAdminAddonManager({ bagTrackingEnabled, onToggleBagTrac } const photoProviderAddons = addons.filter(isPhotoProviderAddon) + const documentProviderAddons = addons.filter(a => a.type === 'document_provider') const tripAddons = addons.filter(a => a.type === 'trip' && !isPhotosAddon(a)) const globalAddons = addons.filter(a => a.type === 'global') const integrationAddons = addons.filter(a => a.type === 'integration') @@ -162,6 +166,16 @@ export default function MAdminAddonManager({ bagTrackingEnabled, onToggleBagTrac enabled: provider.enabled, toggle: () => handleTogglePhotoProvider(provider), })) + // No credential form under these, unlike the photo providers: a document + // connection belongs to a trip and is entered there. The admin only decides + // whether a provider may be offered at all. + const documentProviderOptions: ProviderOption[] = documentProviderAddons.map((provider) => ({ + key: provider.id, + label: provider.name, + description: provider.description, + enabled: provider.enabled, + toggle: () => handleToggle(provider), + })) if (loading) { return ( @@ -207,6 +221,7 @@ export default function MAdminAddonManager({ bagTrackingEnabled, onToggleBagTrac onToggle={onToggleBagTracking} /> )} + {addon.id === 'documents' && addon.enabled && } {addon.id === 'collab' && addon.enabled && collabFeatures && onToggleCollabFeature && ( <> {COLLAB_SUB_FEATURES.map(feat => ( @@ -234,23 +249,7 @@ export default function MAdminAddonManager({ bagTrackingEnabled, onToggleBagTrac
    {/* Memories providers as sub-items under Journey addon — only while it is on */} - {addon.id === 'journey' && addon.enabled && providerOptions.length > 0 && ( - <> - {providerOptions.map(provider => { - const ProviderIcon = PROVIDER_ICONS[provider.key] - return ( - - ) - })} - - )} + {addon.id === 'journey' && addon.enabled && }
    ))} @@ -332,6 +331,24 @@ function MAddonRow({ addon, onToggle, t, first }: MAddonRowProps) { ) } +/** A provider shelf under its addon: Journey's photo providers, Documents' document providers. */ +function MProviderShelf({ options }: { options: ProviderOption[] }) { + return ( + <> + {options.map(provider => ( + + ))} + + ) +} + interface MSubRowProps { icon?: typeof Luggage providerIcon?: ComponentType<{ size?: number }> diff --git a/client/src/mobile/screens/trip/MTripShell.tsx b/client/src/mobile/screens/trip/MTripShell.tsx index 51347fbe7..cffc7685c 100644 --- a/client/src/mobile/screens/trip/MTripShell.tsx +++ b/client/src/mobile/screens/trip/MTripShell.tsx @@ -19,7 +19,10 @@ import { stageOf } from '../../../components/Roadtrip/roadtripRowModel' import { badgeLabel, distanceBadge } from './roadtrip/stageBadges' import type { CorridorReach } from '../../../components/Roadtrip/corridorSearchModel' import { useSettingsStore } from '../../../store/settingsStore' -import type { Day } from '../../../types' +import { useAuthStore } from '../../../store/authStore' +import { canManageDocSync } from '../../../components/Files/docsync/useDocSync' +import { useDocSyncOffered } from '../../../components/Files/docsync/useDocSyncOffered' +import type { Day, Trip } from '../../../types' /** * Mobile trip screen frame. Owns the chrome the design shares across every @@ -675,9 +678,7 @@ export default function MTripShell({ {trTab === 'dateien' && (
    } label={t('common.upload')} onClick={() => setUploadFilesSignal(s => s + 1)} /> - setOpenDocSyncSignal(s => s + 1)} size={40} className="text-m-muted backdrop-blur-[24px] backdrop-saturate-[1.7]"> - - + setOpenDocSyncSignal(s => s + 1)} /> setOpenFilesTrashSignal(s => s + 1)} size={40} className="text-m-muted backdrop-blur-[24px] backdrop-saturate-[1.7]"> @@ -775,6 +776,23 @@ function PrimaryPill({ label, onClick, icon }: { label: string; onClick: () => v ) } +/** + * The Files header's sync button, only where there is something behind it. + * + * A component of its own so the question is asked while the Files header is + * up, not on every tab the shell renders. + */ +function DocSyncButton({ tripId, trip, label, onOpen }: { tripId: number; trip: Trip; label: string; onOpen: () => void }) { + const user = useAuthStore(s => s.user) + const offered = useDocSyncOffered(tripId, canManageDocSync(user, trip)) + if (!offered) return null + return ( + + + + ) +} + /** 40px list-density toggle on the transports/bookings headers. */ function CompactToggle({ active, onToggle, label }: { active: boolean; onToggle: () => void; label: string }) { return ( diff --git a/client/src/mobile/screens/trip/tabs/MDocSyncSheet.test.tsx b/client/src/mobile/screens/trip/tabs/MDocSyncSheet.test.tsx index 25b168c9f..b688a01a8 100644 --- a/client/src/mobile/screens/trip/tabs/MDocSyncSheet.test.tsx +++ b/client/src/mobile/screens/trip/tabs/MDocSyncSheet.test.tsx @@ -1,4 +1,4 @@ -// FE-DOCSYNC-MOBILE-001 to FE-DOCSYNC-MOBILE-016 +// FE-DOCSYNC-MOBILE-001 to FE-DOCSYNC-MOBILE-017 import type { ComponentProps } from 'react' import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { render, screen, fireEvent, waitFor, within } from '../../../../../tests/helpers/render' @@ -18,7 +18,7 @@ const TRIP_ID = 3 * Shaped like a row of document_provider_fields, which matters more than it looks. * * `field_key` is snake_case and `label` already carries the full key suffix - * (`providerApiKey`, not `apiKey`) — see server/src/db/document-provider-seed.ts. + * (`providerApiKey`, not `apiKey`), see server/src/db/document-provider-seed.ts. * An earlier version of this fixture invented both, which made a sheet that * built label keys the wrong way look correct: the fixture's `apiKey` and the * sheet's `docsync.provider${cap(...)}` happened to meet in the middle, while @@ -97,7 +97,7 @@ const onClose = vi.fn() function renderSheet(over: Partial> = {}) { return render( - , + , ) } @@ -141,7 +141,7 @@ afterEach(() => { vi.restoreAllMocks() }) -describe('MDocSyncSheet — opening and the store list', () => { +describe('MDocSyncSheet: opening and the store list', () => { it('FE-DOCSYNC-MOBILE-001: renders nothing and asks the server nothing while it is closed', () => { renderSheet({ open: false }) @@ -180,14 +180,34 @@ describe('MDocSyncSheet — opening and the store list', () => { it('FE-DOCSYNC-MOBILE-004: an instance with no providers says so instead of listing nothing', async () => { providers = [] + links = [] renderSheet() expect(await screen.findByText('No document providers are available')).toBeInTheDocument() expect(screen.queryByText('This trip')).not.toBeInTheDocument() }) + + it('FE-DOCSYNC-MOBILE-017: a bound trip keeps showing its store after every provider was switched off', async () => { + providers = [] + links = [link({ providerName: 'Paperless-ngx' })] + renderSheet({ canManage: false }) + + const row = await screen.findByRole('button', { name: /TREK trip 3/ }) + expect(screen.queryByText('No document providers are available')).not.toBeInTheDocument() + expect(screen.queryByText('Add another')).not.toBeInTheDocument() + // The providers route no longer names it, so the name comes off the link, + // in the list and again as the detail view's title and far end. + expect(row).toHaveAccessibleName(expect.stringContaining('Paperless-ngx')) + + fireEvent.click(row) + await screen.findByRole('button', { name: 'Sync now' }) + expect(screen.getAllByText('Paperless-ngx')).toHaveLength(2) + expect(screen.queryByText('Document sync')).not.toBeInTheDocument() + expect(screen.queryByText('paperless')).not.toBeInTheDocument() + }) }) -describe('MDocSyncSheet — moving between the views', () => { +describe('MDocSyncSheet: moving between the views', () => { it('FE-DOCSYNC-MOBILE-005: tapping a bound store opens its detail view', async () => { renderSheet() @@ -219,8 +239,8 @@ describe('MDocSyncSheet — moving between the views', () => { const { rerender } = renderSheet() await openDetail() - rerender() - rerender() + rerender() + rerender() expect(await screen.findByText('Add another')).toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Sync now' })).not.toBeInTheDocument() @@ -255,7 +275,7 @@ describe('MDocSyncSheet — moving between the views', () => { }) }) -describe('MDocSyncSheet — the two lanes', () => { +describe('MDocSyncSheet: the two lanes', () => { it('FE-DOCSYNC-MOBILE-010: switching a lane off narrows the direction to the other one', async () => { renderSheet() await openDetail() @@ -291,7 +311,7 @@ describe('MDocSyncSheet — the two lanes', () => { }) }) -describe('MDocSyncSheet — what the owner may do', () => { +describe('MDocSyncSheet: what the owner may do', () => { it('FE-DOCSYNC-MOBILE-012: the auto-sync switch patches the binding', async () => { renderSheet() await openDetail() @@ -330,9 +350,9 @@ describe('MDocSyncSheet — what the owner may do', () => { }) }) -describe('MDocSyncSheet — what a member may do', () => { +describe('MDocSyncSheet: what a member may do', () => { it('FE-DOCSYNC-MOBILE-015: a member sees no settings and no disconnect, and the lanes are inert', async () => { - renderSheet({ isOwner: false }) + renderSheet({ canManage: false }) await openDetail() expect(screen.queryByRole('switch')).not.toBeInTheDocument() @@ -350,7 +370,7 @@ describe('MDocSyncSheet — what a member may do', () => { it('FE-DOCSYNC-MOBILE-016: a member is not offered stores to add, and an unbound trip explains who sets it up', async () => { links = [] - renderSheet({ isOwner: false }) + renderSheet({ canManage: false }) expect(await screen.findByText('Nothing connected yet')).toBeInTheDocument() expect(screen.getByText('The trip owner sets this up. Documents stay in TREK either way.')).toBeInTheDocument() diff --git a/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx b/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx index 05a66e40b..1b49da858 100644 --- a/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx +++ b/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx @@ -12,7 +12,7 @@ import { useTranslation } from '../../../../i18n' import { DOCUMENT_PROVIDER_ICONS } from '../../../../components/shared/DocumentProviderIcons' import TrekIcon from '../../../../components/shared/TrekIcon' import { - useDocSync, type DocSyncLink, type DocSyncProvider, + storeName, useDocSync, type DocSyncLink, type DocSyncProvider, } from '../../../../components/Files/docsync/useDocSync' import { useConnectForm } from '../../../../components/Files/docsync/useConnectForm' import { conflictPolicyKey, nextConflictPolicy } from '../../../../components/Files/docsync/DocSyncBits' @@ -23,7 +23,7 @@ import { relativeTime } from '../../../../utils/relativeTime' * Document sync on the phone. * * Everything that decides anything lives in `useDocSync`, which the desktop - * panel uses unchanged — this file is markup in the phone's design language and + * panel uses unchanged. This file is markup in the phone's design language and * nothing else. The two shells are a deliberate mirror, and duplicated logic * across them is what the 3% duplication budget is spent on. * @@ -36,13 +36,13 @@ type View = 'list' | 'detail' | 'connect' | 'scope' export default function MDocSyncSheet({ tripId, tripTitle, - isOwner, + canManage, open, onClose, }: { tripId: number | string tripTitle?: string - isOwner: boolean + canManage: boolean open: boolean onClose: () => void }) { @@ -77,8 +77,8 @@ export default function MDocSyncSheet({ const title = view === 'connect' || view === 'scope' ? pending?.name ?? t('docsync.title') - : view === 'detail' - ? sync.providers.find(p => p.id === link?.providerId)?.name ?? t('docsync.title') + : view === 'detail' && link + ? storeName(link, sync.providers) : t('docsync.title') return ( @@ -108,7 +108,8 @@ export default function MDocSyncSheet({
    - ) : sync.providers.length === 0 ? ( + ) : sync.providers.length === 0 && sync.links.length === 0 ? ( + // Only with nothing bound, as on the desktop panel. ) : view === 'connect' && pending ? ( p.id === link.providerId)?.name ?? link.providerId} + providerName={storeName(link, sync.providers)} sync={sync} - isOwner={isOwner} + canManage={canManage} onUnlink={() => setConfirmUnlink(true)} /> ) : ( @@ -137,7 +138,7 @@ export default function MDocSyncSheet({ links={sync.links} providers={sync.providers} available={available} - isOwner={isOwner} + canManage={canManage} onOpen={id => { setSelected(id); setView('detail') }} onAdd={openStore} /> @@ -167,14 +168,14 @@ function ListView({ links, providers, available, - isOwner, + canManage, onOpen, onAdd, }: { links: DocSyncLink[] providers: DocSyncProvider[] available: DocSyncProvider[] - isOwner: boolean + canManage: boolean onOpen: (id: number) => void onAdd: (p: DocSyncProvider) => void }) { @@ -189,7 +190,7 @@ function ListView({ p.id === l.providerId)?.name || l.providerId} + title={storeName(l, providers)} sub={l.remoteLabel || l.remoteRootPath || l.scopeKey} state={l.lastSyncState} onClick={() => onOpen(l.id)} @@ -199,7 +200,7 @@ function ListView({ )} - {isOwner && available.length > 0 && ( + {canManage && available.length > 0 && (
    {links.length === 0 ? t('docsync.addProvider') : t('docsync.addAnother')}
    @@ -217,7 +218,7 @@ function ListView({
    )} - {links.length === 0 && !isOwner && ( + {links.length === 0 && !canManage && ( )}
    @@ -230,14 +231,14 @@ function DetailView({ tripId, providerName, sync, - isOwner, + canManage, onUnlink, }: { link: DocSyncLink tripId: number | string providerName: string sync: ReturnType - isOwner: boolean + canManage: boolean onUnlink: () => void }) { const { t, language } = useTranslation() @@ -249,7 +250,7 @@ function DetailView({ /** The last remaining direction cannot be switched off. */ const toggle = (lane: 'push' | 'pull') => { - if (!isOwner) return + if (!canManage) return const nextPush = lane === 'push' ? !pushOn : pushOn const nextPull = lane === 'pull' ? !pullOn : pullOn if (!nextPush && !nextPull) return @@ -278,14 +279,14 @@ function DetailView({ active={pushOn} label={t('docsync.flow.toProvider')} icon={} - disabled={!isOwner} + disabled={!canManage} onClick={() => toggle('push')} /> } - disabled={!isOwner} + disabled={!canManage} onClick={() => toggle('pull')} /> @@ -316,7 +317,7 @@ function DetailView({ {busy ? t('docsync.syncing') : t('docsync.syncNow')} - {isOwner && ( + {canManage && ( <> (null) + // `loadScopes`, not `sync`: the hook hands back a fresh object on every + // render of the panel above, so depending on it re-listed the provider's + // folders each time anything up there changed. The callback itself is + // stable. Taken out of `sync` first, because calling it as `sync.loadScopes` + // inside the effect makes the whole object a dependency again. + const { loadScopes } = sync useEffect(() => { - // `sync.loadScopes`, not `sync`: the hook hands back a fresh object on - // every render of the panel above, so depending on it re-listed the - // provider's folders each time anything up there changed. The callback - // itself is stable. let cancelled = false void (async () => { - const res = await sync.loadScopes(connectionId) + const res = await loadScopes(connectionId) if (cancelled) return setScopes(res.scopes) setError(res.error ?? null) })() return () => { cancelled = true } - }, [connectionId, sync.loadScopes]) + }, [connectionId, loadScopes]) const bind = async (scope: { scopeKey: string; label: string; remoteRootId: string | null; remoteRootPath: string | null }) => { setWorking(scope.scopeKey) @@ -757,7 +760,7 @@ function slugFor(title: string | undefined, tripId: number | string): string { /** * The documents that changed in both places, with the way out. * - * The same three answers the panel offers, through the same hook — only the + * The same three answers the panel offers, through the same hook. Only the * markup is a phone's. */ function MConflicts({ diff --git a/client/src/mobile/screens/trip/tabs/MFilesTab.tsx b/client/src/mobile/screens/trip/tabs/MFilesTab.tsx index df33cf658..6d79fe723 100644 --- a/client/src/mobile/screens/trip/tabs/MFilesTab.tsx +++ b/client/src/mobile/screens/trip/tabs/MFilesTab.tsx @@ -228,7 +228,7 @@ export default function MFilesTab({ planner, shell }: MTabScreenProps) { setDocSyncOpen(false)} /> diff --git a/client/src/repo/docSyncRepo.ts b/client/src/repo/docSyncRepo.ts new file mode 100644 index 000000000..4d3e89a2b --- /dev/null +++ b/client/src/repo/docSyncRepo.ts @@ -0,0 +1,44 @@ +import { docsyncApi } from '../api/client' +import { isEffectivelyOffline } from '../sync/networkMode' + +/** + * The two reads behind the Files screen's "Document sync" button. + * + * Online-only with no Dexie table, for the reason `dawarichRepo` gives: both + * answers are the server's (which providers the admin switched on, whether the + * owner bound this trip), and the dialog the button opens cannot load offline + * either. The store in front of this keeps the last answer for the session. + */ +export class DocSyncOfflineError extends Error { + constructor() { + super('Document sync needs a connection') + this.name = 'DocSyncOfflineError' + } +} + +function requireOnline(): void { + if (isEffectivelyOffline()) throw new DocSyncOfflineError() +} + +export const docSyncRepo = { + /** + * Whether the instance offers a provider somebody could connect. + * + * The list is instance-wide; the route sits under a trip only because every + * document-sync route does. `available` is the same filter the dialog + * applies, so a provider row without an adapter does not bring up a button + * into an empty list. + */ + async offersProvider(tripId: number | string): Promise { + requireOnline() + const providers = (await docsyncApi.providers(tripId)) as Array<{ available?: boolean }> + return providers.some(p => p.available) + }, + + /** Whether the trip is bound to at least one store. */ + async isBound(tripId: number | string): Promise { + requireOnline() + const links = (await docsyncApi.listLinks(tripId)) as unknown[] + return links.length > 0 + }, +} diff --git a/client/src/store/docSyncOfferStore.ts b/client/src/store/docSyncOfferStore.ts new file mode 100644 index 000000000..935cec347 --- /dev/null +++ b/client/src/store/docSyncOfferStore.ts @@ -0,0 +1,48 @@ +import { create } from 'zustand' +import { docSyncRepo } from '../repo/docSyncRepo' + +/** + * Whether a trip's Files screen offers document sync, remembered for the session. + * + * The seed ships every document provider switched off, so a button on every + * trip led, straight after the upgrade, into "No document providers are + * available". It shows where there is something behind it: a trip that is + * already bound (every member needs to see where their documents go), or + * somebody who may bind it on an instance that offers a provider. + * + * Remembered so that coming back to the tab renders the last answer at once, + * instead of the button popping in after a request every time. + */ +interface DocSyncOfferState { + /** Per trip id, whether it has a binding. Missing until asked. */ + bound: Record + /** Whether the instance offers any provider. Null until asked. */ + providers: boolean | null + refresh: (tripId: number | string, canManage: boolean) => Promise +} + +export const useDocSyncOfferStore = create((set) => ({ + bound: {}, + providers: null, + + refresh: async (tripId, canManage) => { + const key = String(tripId) + try { + // Somebody who may bind the trip gets the button as soon as a provider + // is on, bound or not, so the second request is for everybody else. + if (canManage) { + const providers = await docSyncRepo.offersProvider(tripId) + set({ providers }) + if (providers) return + } + const bound = await docSyncRepo.isBound(tripId) + set(s => ({ bound: { ...s.bound, [key]: bound } })) + } catch (err) { + // 403 is the Documents addon switched off and 404 a trip this person no + // longer reaches: both a real "no". Anything else, offline included, is + // no answer at all, and the last one stands. + const status = (err as { response?: { status?: number } })?.response?.status + if (status === 403 || status === 404) set(s => ({ bound: { ...s.bound, [key]: false } })) + } + }, +})) diff --git a/client/tests/unit/mobile/admin/MAdminAddonManager.test.tsx b/client/tests/unit/mobile/admin/MAdminAddonManager.test.tsx index 96bbb29fe..c369aacb8 100644 --- a/client/tests/unit/mobile/admin/MAdminAddonManager.test.tsx +++ b/client/tests/unit/mobile/admin/MAdminAddonManager.test.tsx @@ -1,4 +1,4 @@ -// FE-MOB-AADD-001 to FE-MOB-AADD-025 +// FE-MOB-AADD-001 to FE-MOB-AADD-031 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import userEvent from '@testing-library/user-event'; import { delay, http, HttpResponse } from 'msw'; @@ -356,6 +356,104 @@ describe('MAdminAddonManager', () => { expect(screen.getByRole('switch', { name: 'Todo List' })).toHaveAttribute('aria-checked', 'true'); }); + describe('document providers', () => { + const documents = (enabled = true) => + buildAddon({ id: 'documents', name: 'Documents', description: 'Store and manage travel documents', icon: 'FileText', enabled }); + const docProvider = (id: string, name: string, enabled: boolean) => + buildAddon({ id, name, description: `${name} archive`, icon: id, type: 'document_provider', enabled }); + + it('FE-MOB-AADD-028: they render as sub-rows under the Documents addon, not as addons of their own', async () => { + server.use( + addonsRoute([ + documents(), + docProvider('paperless', 'Paperless-ngx', false), + docProvider('nextcloud', 'Nextcloud', true), + ]), + ); + render(); + + await screen.findByText('Paperless-ngx'); + expect(screen.getByText('Nextcloud archive')).toBeInTheDocument(); + expect(screen.getByRole('switch', { name: 'Paperless-ngx' })).toHaveAttribute('aria-checked', 'false'); + expect(screen.getByRole('switch', { name: 'Nextcloud' })).toHaveAttribute('aria-checked', 'true'); + // Documents plus two providers, and neither provider carries a type badge of its own. + expect(screen.getAllByRole('switch')).toHaveLength(3); + expect(screen.getAllByText('Trip')).toHaveLength(1); + }); + + it('FE-MOB-AADD-029: the shelf stays hidden while Documents is off', async () => { + server.use(addonsRoute([documents(false), docProvider('paperless', 'Paperless-ngx', false)])); + render(); + + await screen.findByText('Documents'); + expect(screen.queryByText('Paperless-ngx')).not.toBeInTheDocument(); + expect(screen.getAllByRole('switch')).toHaveLength(1); + }); + + it('FE-MOB-AADD-030: toggling one persists it and rolls back on failure', async () => { + const user = userEvent.setup(); + let body: unknown = null; + server.use( + addonsRoute([ + documents(), + docProvider('paperless', 'Paperless-ngx', false), + docProvider('papra', 'Papra', false), + ]), + http.put('/api/admin/addons/paperless', async ({ request }) => { + body = await request.json(); + return HttpResponse.json({ success: true }); + }), + http.put('/api/admin/addons/papra', () => HttpResponse.error()), + ); + render(<>); + await screen.findByText('Paperless-ngx'); + + await user.click(screen.getByRole('switch', { name: 'Paperless-ngx' })); + await waitFor(() => expect(body).toEqual({ enabled: true })); + expect(screen.getByRole('switch', { name: 'Paperless-ngx' })).toHaveAttribute('aria-checked', 'true'); + await screen.findByText('Addon updated'); + expect(loadAddonsSpy).toHaveBeenCalled(); + + await user.click(screen.getByRole('switch', { name: 'Papra' })); + await screen.findByText('Failed to update addon'); + await waitFor(() => expect(screen.getByRole('switch', { name: 'Papra' })).toHaveAttribute('aria-checked', 'false')); + expect(screen.getByRole('switch', { name: 'Paperless-ngx' })).toHaveAttribute('aria-checked', 'true'); + }); + + it('FE-MOB-AADD-031: switching Documents off and on again shows the cascaded providers as off', async () => { + const user = userEvent.setup(); + let loads = 0; + let documentsOn = true; + let paperlessOn = true; + server.use( + http.get('/api/admin/addons', () => { + loads += 1; + return HttpResponse.json({ + addons: [documents(documentsOn), docProvider('paperless', 'Paperless-ngx', paperlessOn)], + }); + }), + http.put('/api/admin/addons/documents', async ({ request }) => { + documentsOn = (await request.json() as { enabled: boolean }).enabled; + // Documents off takes its providers with it, see admin.service.ts. + if (!documentsOn) paperlessOn = false; + return HttpResponse.json({ success: true }); + }), + ); + render(<>); + await screen.findByText('Paperless-ngx'); + + await user.click(screen.getByRole('switch', { name: 'Documents' })); + await screen.findByText('Addon updated'); + expect(loads).toBe(2); + expect(screen.queryByText('Paperless-ngx')).not.toBeInTheDocument(); + + await user.click(screen.getByRole('switch', { name: 'Documents' })); + + await screen.findByText('Paperless-ngx'); + expect(screen.getByRole('switch', { name: 'Paperless-ngx' })).toHaveAttribute('aria-checked', 'false'); + }); + }); + it('FE-MOB-AADD-016: a disabled AI-parsing addon renders the integration row without its config', async () => { server.use(addonsRoute([llmAddon({ provider: 'local' })].map(a => ({ ...a, enabled: false })))); render(); diff --git a/client/tests/unit/mobile/trip/MTripShell.test.tsx b/client/tests/unit/mobile/trip/MTripShell.test.tsx index 96b9893e0..6082b038b 100644 --- a/client/tests/unit/mobile/trip/MTripShell.test.tsx +++ b/client/tests/unit/mobile/trip/MTripShell.test.tsx @@ -1,10 +1,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { act, fireEvent, render, screen, within } from '../../../helpers/render' +import { http, HttpResponse } from 'msw' +import { act, fireEvent, render, screen, waitFor, within } from '../../../helpers/render' import { buildPlanner, buildShell } from '../../../helpers/mobileTrip' +import { server } from '../../../helpers/msw/server' +import { seedStore } from '../../../helpers/store' +import { buildUser } from '../../../helpers/factories' +import { useAuthStore } from '../../../../src/store/authStore' +import { useDocSyncOfferStore } from '../../../../src/store/docSyncOfferStore' import type { MTripShellApi, TripPlanner } from '../../../../src/mobile/screens/trip/MTripShell' import type { Day, PackingItem, TodoItem } from '../../../../src/types' -// FE-MOB-SHELL-001 to FE-MOB-SHELL-066 +// FE-MOB-SHELL-001 to FE-MOB-SHELL-068 const mocks = vi.hoisted(() => ({ planner: {} as TripPlanner })) @@ -120,6 +126,13 @@ const spy = (planner: TripPlanner, name: keyof TripPlanner) => describe('MTripShell', () => { beforeEach(() => { sessionStorage.clear() + // Document sync as a fresh install has it: every provider off, nothing bound. + useDocSyncOfferStore.setState({ bound: {}, providers: null }) + seedStore(useAuthStore, { user: null, isAuthenticated: false }) + server.use( + http.get('/api/trips/:tripId/docsync/providers', () => HttpResponse.json([])), + http.get('/api/trips/:tripId/docsync/links', () => HttpResponse.json([])), + ) }) it('FE-MOB-SHELL-001: shows the loading splash with the trip title while the planner loads', () => { @@ -455,6 +468,32 @@ describe('MTripShell', () => { expect(shellApi.openFilesTrashSignal).toBe(1) }) + it('FE-MOB-SHELL-067: the files header leaves the sync button out while no provider is on and nothing is bound', async () => { + seedStore(useAuthStore, { user: buildUser({ id: 1, role: 'user' }), isAuthenticated: true }) + const asked: string[] = [] + server.use( + http.get('/api/trips/:tripId/docsync/links', () => { + asked.push('links') + return HttpResponse.json([]) + }), + ) + renderShell({ activeTab: 'dateien' } as Partial) + + await waitFor(() => expect(asked).toContain('links')) + expect(screen.queryByRole('button', { name: 'docsync.title' })).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'common.upload' })).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'files.trash' })).toBeInTheDocument() + }) + + it('FE-MOB-SHELL-068: a bound trip shows the sync button to a member, and it raises the open signal', async () => { + seedStore(useAuthStore, { user: buildUser({ id: 999, role: 'user' }), isAuthenticated: true }) + server.use(http.get('/api/trips/:tripId/docsync/links', () => HttpResponse.json([{ id: 1, providerId: 'paperless' }]))) + renderShell({ activeTab: 'dateien' } as Partial) + + fireEvent.click(await screen.findByRole('button', { name: 'docsync.title' })) + expect(shellApi.openDocSyncSignal).toBe(1) + }) + it('FE-MOB-SHELL-034: the lists header shows packed/open counts and persists the sub-tab', () => { const packingItems = [ { id: 1, checked: true }, { id: 2, checked: false }, { id: 3, checked: true }, diff --git a/server/src/nest/doc-sync/doc-sync.controller.ts b/server/src/nest/doc-sync/doc-sync.controller.ts index fef66c4e0..3f34df84a 100644 --- a/server/src/nest/doc-sync/doc-sync.controller.ts +++ b/server/src/nest/doc-sync/doc-sync.controller.ts @@ -22,8 +22,9 @@ import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { TripAccessGuard } from '../permissions/trip-access.guard'; import { DatabaseService } from '../database/database.service'; +import { RealtimeService } from '../realtime/realtime.service'; import { docFailed } from './document-provider'; -import { DocSyncConfigService } from './doc-sync-config.service'; +import { DocSyncConfigService, type LinkRow } from './doc-sync-config.service'; import { DocSyncService } from './doc-sync.service'; import { DocumentProviderRegistry } from './document-provider.registry'; import { @@ -37,7 +38,7 @@ import { } from './doc-sync.dto'; /** - * `/api/trips/:tripId/docsync` — a trip's document provider binding. + * `/api/trips/:tripId/docsync`: a trip's document provider binding. * * Trip-scoped rather than user-scoped, and that is the whole point of the * feature: one connection per trip, which every member reads through. The @@ -53,7 +54,7 @@ import { * overrides trip permissions everywhere else too). It hands TREK a credential * that usually reaches the owner's entire document archive, so widening this to * every member would let any member point the trip at a folder the owner never - * meant to share. Reading the status is open to all members — they need to know + * meant to share. Reading the status is open to all members: they need to know * where their documents are going. */ @Controller('api/trips/:tripId/docsync') @@ -65,8 +66,23 @@ export class DocSyncController { private readonly sync: DocSyncService, private readonly registry: DocumentProviderRegistry, private readonly db: DatabaseService, + private readonly realtime: RealtimeService, ) {} + /** + * Tell the trip that a binding came or went. + * + * Whether a trip is bound decides whether its members see the sync button + * at all, and a run only pings when it moved a document. Binding two empty + * sides, or unbinding, moves nothing, so a member sitting on the Files tab + * kept the old answer until they left it. The whole room, the caller + * included: it is a ping to re-read, not a change to apply, and the owner's + * own button depends on it too once every provider is switched off. + */ + private announceBinding(link: Pick): void { + this.realtime.broadcast(link.trip_id, 'docsync:changed', { linkId: link.id, pulled: 0, pushed: 0 }); + } + /** * The trip owner or an instance admin, the same rule as `canManageDocSync` * on the client. Checked here rather than through @RequirePermission: the @@ -132,7 +148,7 @@ export class DocSyncController { } /** - * Probe form values. Always 200 — the result is in the body, because this is + * Probe form values. Always 200: the result is in the body, because this is * a form field, not a failure of the request. The photo providers pin the * same contract with a test their e2e suite marks CRITICAL. */ @@ -157,7 +173,7 @@ export class DocSyncController { .find((c) => c.provider_id === body.providerId); // Only for the address the credential was stored against. Merging it into a // probe of an arbitrary baseUrl turns this route into a way to have TREK - // post a stored API token at a server of the caller's choosing — which is + // post a stored API token at a server of the caller's choosing, which is // exactly what somebody who inherited a trip but not its credentials would // reach for. Same host, same scheme, same port, or the caller types it in. const sameTarget = !!existing && sameOrigin(existing.base_url, urlCheck.data.url); @@ -174,6 +190,7 @@ export class DocSyncController { const res = await provider.probe({ connectionId: existing?.id ?? 0, + createdAt: existing?.created_at ?? '', ownerId: Number(user.id), baseUrl: urlCheck.data.url, secrets, @@ -190,13 +207,15 @@ export class DocSyncController { deleteConnection( @Param('tripId') tripId: string, @Param('connectionId') connectionId: string, - createdAt: existing?.created_at ?? '', @CurrentUser() user: User, ) { this.assertCanManage(tripId, user); const conn = this.config.getConnection(Number(connectionId)); if (!conn || conn.trip_id !== Number(tripId)) throw new HttpException('Connection not found', 404); + // Its bindings go with it (ON DELETE CASCADE), so read them first. + const unbound = this.config.listLinks(conn.trip_id).filter((l) => l.connection_id === conn.id); this.config.deleteConnection(conn.id); + for (const link of unbound) this.announceBinding(link); return { success: true }; } @@ -256,9 +275,10 @@ export class DocSyncController { this.assertCanManage(tripId, user); const res = this.config.createLink(Number(tripId), Number(user.id), body); if (docFailed(res)) throw new HttpException(res.error.detail || res.error.code, 400); + this.announceBinding(res.data); // Subscribe where the provider lets TREK do it itself. A failure here is - // not a failure of the binding — polling still carries it — so it is logged + // not a failure of the binding (polling still carries it), so it is logged // into the link state rather than thrown at the user. const conn = this.config.getConnection(res.data.connection_id); const provider = conn ? this.registry.get(conn.provider_id) : undefined; @@ -318,6 +338,7 @@ export class DocSyncController { await provider.unregisterWebhook(this.config.toRef(conn), link.webhook_subscription_id); } this.config.deleteLink(link.id); + this.announceBinding(link); // Unbinding keeps both copies. Nothing is deleted anywhere. return { success: true, documentsKept: true }; } diff --git a/server/tests/unit/nest/doc-sync/doc-sync.controller.test.ts b/server/tests/unit/nest/doc-sync/doc-sync.controller.test.ts index 2f8e895b0..0ba1a3d4d 100644 --- a/server/tests/unit/nest/doc-sync/doc-sync.controller.test.ts +++ b/server/tests/unit/nest/doc-sync/doc-sync.controller.test.ts @@ -12,7 +12,7 @@ import type { Request } from 'express'; * form field that must not lose a secret it never saw, and a webhook whose * callback URL is derived from the request rather than from configuration. * - * The config service and the provider registry are the real ones — the merge + * The config service and the provider registry are the real ones: the merge * this file pins runs through actual encryption and actual provider-field rows, * and a stub of either would only be a restatement of the controller. The * reconciler is a spy: a run is a side effect here, not the subject. @@ -71,6 +71,7 @@ import type { } from '../../../../src/nest/doc-sync/document-provider'; import type { LinkRow } from '../../../../src/nest/doc-sync/doc-sync-config.service'; import type { DocSyncService } from '../../../../src/nest/doc-sync/doc-sync.service'; +import type { RealtimeService } from '../../../../src/nest/realtime/realtime.service'; import type { DocsyncConnectionDto, DocsyncConnectionTestDto, @@ -123,7 +124,8 @@ const sync = { const registry = new DocumentProviderRegistry([paperless, nextcloud] as unknown as DocumentProvider[]); const dbs = new DatabaseService(testDb); const config = new DocSyncConfigService(dbs, registry); -const controller = new DocSyncController(config, sync as unknown as DocSyncService, registry, dbs); +const realtime = { broadcast: vi.fn() }; +const controller = new DocSyncController(config, sync as unknown as DocSyncService, registry, dbs, realtime as unknown as RealtimeService); /** Only what publicOrigin reads: header lookup plus the connection's own scheme. */ function makeReq(headers: Record = {}, protocol = 'http'): Request { @@ -368,8 +370,6 @@ describe('probing form values', () => { }); }); -describe('reading the trip state', () => { - it('asks the reconciler with a number, because the path hands the handler a string', () => { describe('a secret the adapter earned itself', () => { const EARNED = 'a1b2c3:DEVICE-SECRET'; @@ -416,6 +416,8 @@ describe('a secret the adapter earned itself', () => { }); }); +describe('reading the trip state', () => { + it('asks the reconciler with a number, because the path hands the handler a string', () => { controller.status(String(tripId)); expect(sync.status).toHaveBeenCalledWith(tripId); }); @@ -439,6 +441,32 @@ describe('removing a connection', () => { const err = await thrown(() => controller.deleteConnection(String(otherTripId), String(conn.id), admin)); expect(err.getStatus()).toBe(404); expect(config.getConnection(conn.id)).toBeTruthy(); + expect(realtime.broadcast).not.toHaveBeenCalled(); + }); + + it('pings the trip once for every binding that went with it, and for no other', async () => { + const conn = await storedPaperless(); + const cloud = await config.upsertConnection(tripId, Number(owner.id), connBody({ + providerId: 'nextcloud', + baseUrl: 'https://cloud.example.com', + credentials: { login_name: 'alice', app_password: 'app-pw', base_path: '/TREK' }, + })); + if ('error' in cloud) throw new Error(`fixture failed: ${cloud.error.code}`); + const bind = (connectionId: number, scopeKey: string) => { + const res = config.createLink(tripId, Number(owner.id), linkBody(connectionId, { scopeKey })); + if ('error' in res) throw new Error(`fixture failed: ${res.error.code}`); + return res.data.id; + }; + const first = bind(conn.id, 'tag:1'); + const second = bind(conn.id, 'tag:2'); + bind(cloud.data.id, 'folder:1'); + + controller.deleteConnection(String(tripId), String(conn.id), owner); + + expect(realtime.broadcast.mock.calls).toEqual([ + [tripId, 'docsync:changed', { linkId: first, pulled: 0, pushed: 0 }], + [tripId, 'docsync:changed', { linkId: second, pulled: 0, pushed: 0 }], + ]); }); }); @@ -556,6 +584,14 @@ describe('creating a binding', () => { expect(sync.syncLink.mock.calls[0][1]).toEqual({ full: true }); }); + it('pings the whole trip, so a member already on the Files tab gets the sync button', async () => { + const conn = await storedPaperless(); + const link = (await controller.createLink(String(tripId), owner, linkBody(conn.id), makeReq())) as { id: number }; + + // Three arguments: no socket is skipped, the owner's own button reads it too. + expect(realtime.broadcast).toHaveBeenCalledWith(tripId, 'docsync:changed', { linkId: link.id, pulled: 0, pushed: 0 }); + }); + it('refuses a binding for a connection that belongs to a different trip', async () => { const conn = await storedPaperless(); const err = await thrown(() => @@ -563,6 +599,7 @@ describe('creating a binding', () => { ); expect(err.getStatus()).toBe(400); expect(sync.syncLink).not.toHaveBeenCalled(); + expect(realtime.broadcast).not.toHaveBeenCalled(); }); }); @@ -682,12 +719,26 @@ describe('removing a binding', () => { expect(testDb.prepare('SELECT id FROM trip_files WHERE id = ?').get(fileId)).toBeTruthy(); }); + it('pings the trip that the binding is gone', async () => { + const conn = await storedPaperless(); + const created = (await controller.createLink(String(tripId), owner, linkBody(conn.id), makeReq())) as { id: number }; + realtime.broadcast.mockClear(); + + await controller.deleteLink(String(tripId), String(created.id), owner); + + expect(realtime.broadcast.mock.calls).toEqual([ + [tripId, 'docsync:changed', { linkId: created.id, pulled: 0, pushed: 0 }], + ]); + }); + it('refuses a link that belongs to a different trip', async () => { const conn = await storedPaperless(); const created = (await controller.createLink(String(tripId), owner, linkBody(conn.id), makeReq())) as { id: number }; + realtime.broadcast.mockClear(); const err = await thrown(() => controller.deleteLink(String(otherTripId), String(created.id), admin)); expect(err.getStatus()).toBe(404); expect(config.getLink(created.id)).toBeTruthy(); + expect(realtime.broadcast).not.toHaveBeenCalled(); }); });