From 843582863c378fa19fad304397d54fab666f4336 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 19 Sep 2026 13:36:33 +0200 Subject: [PATCH] fix(docsync): offer the sync button only where something is behind it The Documents addon put a "Document sync" button on every trip's Files tab, and the seed ships every provider switched off, so after the upgrade each of those buttons led into "No document providers are available". The button now shows on a trip that is already bound, to every member, and otherwise only to somebody who may bind the trip on an instance that offers a provider. Without it the Files toolbar is the one from before the feature, trash flush right, on both shells. /api/addons carries no document providers for a normal user, so the answer comes from the docsync routes through a new online-only docSyncRepo and is kept per session in docSyncOfferStore. A revisit renders the last answer at once and checks it again; offline nothing is asked and the last answer stands. Creating a binding, removing one and removing a connection now ping the trip with docsync:changed and zero counts, so a member already on the Files tab sees the button come and go without leaving it. Both shells keep listing a binding whose provider the admin has since switched off instead of claiming there are no providers, and name its store through one storeName() helper. A load that answers 403 or 404 clears the bindings as well, so they do not linger once the Documents addon is gone. The phone admin screen gets the Documents shelf the desktop one has: the document providers as sub rows under the addon, toggled through the same admin route and re-read after Documents itself is toggled, because the server switches the providers off with it. isTripOwner and the isOwner prop become canManageSync and canManage, which is what canManageDocSync answers. Both folder pickers take loadScopes out of the sync object before their effect, so exhaustive-deps is satisfied without the effect running on every render. --- .../src/components/Files/FileManager.test.tsx | 68 ++++++- client/src/components/Files/FileManager.tsx | 2 +- .../components/Files/FileManagerToolbar.tsx | 27 ++- .../Files/docsync/DocSyncBinding.test.tsx | 43 ++-- .../Files/docsync/DocSyncBinding.tsx | 32 +-- .../Files/docsync/DocSyncPanel.test.tsx | 88 +++++--- .../components/Files/docsync/DocSyncPanel.tsx | 34 ++-- .../Files/docsync/DocSyncScopeModal.tsx | 16 +- .../Files/docsync/useDocSync.test.ts | 31 ++- .../components/Files/docsync/useDocSync.ts | 51 +++-- .../Files/docsync/useDocSyncOffered.test.ts | 190 ++++++++++++++++++ .../Files/docsync/useDocSyncOffered.ts | 34 ++++ client/src/components/Files/useFileManager.ts | 6 +- .../screens/admin/MAdminAddonManager.tsx | 55 +++-- client/src/mobile/screens/trip/MTripShell.tsx | 26 ++- .../screens/trip/tabs/MDocSyncSheet.test.tsx | 44 ++-- .../screens/trip/tabs/MDocSyncSheet.tsx | 63 +++--- .../mobile/screens/trip/tabs/MFilesTab.tsx | 2 +- client/src/repo/docSyncRepo.ts | 44 ++++ client/src/store/docSyncOfferStore.ts | 48 +++++ .../mobile/admin/MAdminAddonManager.test.tsx | 100 ++++++++- .../unit/mobile/trip/MTripShell.test.tsx | 43 +++- .../src/nest/doc-sync/doc-sync.controller.ts | 35 +++- .../nest/doc-sync/doc-sync.controller.test.ts | 59 +++++- 24 files changed, 938 insertions(+), 203 deletions(-) create mode 100644 client/src/components/Files/docsync/useDocSyncOffered.test.ts create mode 100644 client/src/components/Files/docsync/useDocSyncOffered.ts create mode 100644 client/src/repo/docSyncRepo.ts create mode 100644 client/src/store/docSyncOfferStore.ts 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(); }); });