fix(settings): the desktop photo provider badge follows the live status check

On the desktop, the Immich (or Synology) card could show "Connected" for a
server that was down. The settings page loads two answers at once: the
saved settings, whose connected flag for Immich only says that a URL and an
API key are stored, and the status route, which actually reaches the
server. Whichever landed last set the badge.

usePhotoProviderConnections had a statusRouteOwnsBadge option that only the
phone turned on. The option is gone: whenever a provider declares a status
route, only that route sets the badge, on both views. A provider without a
status route still takes the badge from its settings read.

Regression test FE-COMP-PHOTOPROVIDERS-028 (PhotoProvidersSection.test.tsx)
was red on the old code: "Unable to find an element with the text: Not
connected" (the late settings read had turned the badge to Connected).

Changed assertions:
- usePhotoProviderConnections.test.ts FE-COMP-PHOTOCONN-005: it expected the
  later settings read (connected: false) to override the status route's
  connected: true; it now checks that a provider without a status route
  takes the badge from the settings read.
- usePhotoProviderConnections.test.ts: render() no longer takes the option,
  so every call drops its true or false argument.
This commit is contained in:
Maurice
2026-10-09 17:41:43 +02:00
parent 0b90fc8cf5
commit 76b1e67be8
5 changed files with 50 additions and 37 deletions
@@ -1,4 +1,4 @@
// FE-COMP-PHOTOPROVIDERS-001 to FE-COMP-PHOTOPROVIDERS-027
// FE-COMP-PHOTOPROVIDERS-001 to FE-COMP-PHOTOPROVIDERS-028
import { render, screen, waitFor } from '../../../tests/helpers/render';
import userEvent from '@testing-library/user-event';
import { http, HttpResponse } from 'msw';
@@ -280,6 +280,31 @@ describe('PhotoProvidersSection', () => {
expect(await screen.findByText('Could not connect to Immich: Auth failed')).toBeInTheDocument();
});
it('FE-COMP-PHOTOPROVIDERS-028: a settings read landing after the status probe does not flip the badge to connected', async () => {
let releaseSettings!: () => void;
const settingsGate = new Promise<void>(r => (releaseSettings = r));
let statusAnswered = false;
server.use(
// Stored credentials make the settings read say connected; the probe says the server is down.
http.get('/api/addons/immich/settings', async () => {
await settingsGate;
return HttpResponse.json({ url: 'https://photos.example.com', connected: true });
}),
http.get('/api/addons/immich/status', () => {
statusAnswered = true;
return HttpResponse.json({ connected: false });
}),
);
seedMemoriesEnabled();
render(<PhotoProvidersSection />);
await screen.findByText('Immich');
await waitFor(() => expect(statusAnswered).toBe(true));
releaseSettings();
await waitFor(() => expect(screen.getByDisplayValue('https://photos.example.com')).toBeInTheDocument());
expect(screen.getByText('Not connected')).toBeInTheDocument();
expect(screen.queryByText('Connected')).not.toBeInTheDocument();
});
it('FE-COMP-PHOTOPROVIDERS-016: Test button is disabled while test is in progress', async () => {
const user = userEvent.setup();
let resolveTest!: () => void;
@@ -27,7 +27,7 @@ export default function PhotoProvidersSection(): React.ReactElement {
isProviderSaveDisabled,
handleSaveProvider,
handleTestProvider,
} = usePhotoProviderConnections({ statusRouteOwnsBadge: false })
} = usePhotoProviderConnections()
const renderPhotoProviderSection = (provider: PhotoProviderAddon): React.ReactElement => {
const fields = getProviderFields(provider)
@@ -81,8 +81,8 @@ function stubGets(byPath: Record<string, Resp | (() => Promise<Resp>)>) {
});
}
function render(statusRouteOwnsBadge: boolean) {
return renderHook(() => usePhotoProviderConnections({ statusRouteOwnsBadge }));
function render() {
return renderHook(() => usePhotoProviderConnections());
}
beforeEach(() => {
@@ -111,7 +111,7 @@ describe('usePhotoProviderConnections', () => {
false
);
stubGets({});
const { result } = render(true);
const { result } = render();
expect(result.current.memoriesEnabled).toBe(false);
expect(result.current.activePhotoProviders.map((p) => p.id)).toEqual(['immich']);
});
@@ -122,43 +122,37 @@ describe('usePhotoProviderConnections', () => {
'/addons/immich/settings': { data: { url: 'https://p.example', api_key: 'secret', include_shared: true } },
'/addons/immich/status': { data: { connected: true } },
});
const { result } = render(true);
const { result } = render();
await waitFor(() => expect(result.current.providerValues.immich?.url).toBe('https://p.example'));
expect(result.current.providerValues.immich).toEqual({ url: 'https://p.example', shared: 'true' });
await waitFor(() => expect(result.current.providerConnected.immich).toBe(true));
});
it('FE-COMP-PHOTOCONN-004: with the status route owning the badge, a late settings read does not override it', async () => {
it('FE-COMP-PHOTOCONN-004: the status route owns the badge, so a late settings read does not override it', async () => {
seedProviders([PROVIDER]);
let resolveSettings!: (r: Resp) => void;
stubGets({
'/addons/immich/settings': () => new Promise<Resp>((r) => (resolveSettings = r)),
'/addons/immich/status': { data: { connected: true } },
});
const { result } = render(true);
const { result } = render();
await waitFor(() => expect(result.current.providerConnected.immich).toBe(true));
await act(async () => resolveSettings({ data: { url: 'u', connected: false } }));
expect(result.current.providerValues.immich?.url).toBe('u');
expect(result.current.providerConnected.immich).toBe(true);
});
it('FE-COMP-PHOTOCONN-005: without that, the later settings read sets the badge', async () => {
seedProviders([PROVIDER]);
let resolveSettings!: (r: Resp) => void;
stubGets({
'/addons/immich/settings': () => new Promise<Resp>((r) => (resolveSettings = r)),
'/addons/immich/status': { data: { connected: true } },
});
const { result } = render(false);
it('FE-COMP-PHOTOCONN-005: a provider without a status route takes the badge from the settings read', async () => {
seedProviders([{ ...PROVIDER, config: { settings_get: '/addons/immich/settings' } }]);
stubGets({ '/addons/immich/settings': { data: { url: 'u', connected: true } } });
const { result } = render();
await waitFor(() => expect(result.current.providerConnected.immich).toBe(true));
await act(async () => resolveSettings({ data: { url: 'u', connected: false } }));
expect(result.current.providerConnected.immich).toBe(false);
});
it('FE-COMP-PHOTOCONN-006: save is blocked while a required field is blank', async () => {
seedProviders([PROVIDER]);
stubGets({ '/addons/immich/settings': { data: {} }, '/addons/immich/status': { data: {} } });
const { result } = render(true);
const { result } = render();
await waitFor(() => expect(result.current.providerValues.immich?.url).toBe(''));
expect(result.current.isProviderSaveDisabled(PROVIDER)).toBe(true);
act(() => result.current.handleProviderFieldChange('immich', 'url', 'https://a'));
@@ -173,7 +167,7 @@ describe('usePhotoProviderConnections', () => {
'/addons/immich/status': { data: { connected: false } },
});
const put = vi.spyOn(apiClient, 'put').mockResolvedValue({ data: {} });
const { result } = render(true);
const { result } = render();
await waitFor(() => expect(result.current.providerValues.immich?.url).toBe('https://a'));
act(() => result.current.handleProviderFieldChange('immich', 'url', ' https://b '));
get.mockClear();
@@ -188,7 +182,7 @@ describe('usePhotoProviderConnections', () => {
seedProviders([PROVIDER]);
stubGets({ '/addons/immich/settings': { data: {} }, '/addons/immich/status': { data: {} } });
const put = vi.spyOn(apiClient, 'put').mockRejectedValue(new Error('x'));
const { result } = render(true);
const { result } = render();
await act(() => result.current.handleSaveProvider(PROVIDER));
expect(toast.error).toHaveBeenCalledWith('memories.saveError(Immich)');
put.mockClear();
@@ -203,7 +197,7 @@ describe('usePhotoProviderConnections', () => {
'/addons/immich/status': { data: {} },
});
const post = vi.spyOn(apiClient, 'post').mockResolvedValue({ data: { connected: true } });
const { result } = render(true);
const { result } = render();
await waitFor(() => expect(result.current.providerValues.immich?.url).toBe('https://a'));
await act(() => result.current.handleTestProvider(PROVIDER));
expect(post).toHaveBeenCalledWith('/addons/immich/test', { url: 'https://a', include_shared: false });
@@ -220,10 +214,10 @@ describe('usePhotoProviderConnections', () => {
seedProviders([PROVIDER]);
stubGets({ '/addons/immich/settings': { data: {} }, '/addons/immich/status': { data: {} } });
vi.spyOn(apiClient, 'post').mockResolvedValue({ data: { connected: false, error: 'bad key' } });
const phone = render(true);
const phone = render();
await act(() => phone.result.current.handleTestProvider(PROVIDER));
expect(toast.error).toHaveBeenLastCalledWith('memories.connectionError(Immich): bad key');
const desktop = render(false);
const desktop = render();
await act(() => desktop.result.current.handleTestProvider(PROVIDER));
expect(toast.error).toHaveBeenLastCalledWith('memories.connectionError(Immich): bad key');
vi.mocked(apiClient.post).mockResolvedValue({ data: { connected: false } });
@@ -49,22 +49,13 @@ export const getProviderFields = (provider: PhotoProviderAddon): ProviderField[]
return [...(provider.fields || [])].sort((a, b) => a.sort_order - b.sort_order);
};
export interface PhotoProviderConnectionOptions {
/**
* When the provider has a status route, let only that route set the badge and
* ignore the `connected` flag of the settings read, so the two responses cannot
* race. The phone does this; the desktop takes whichever answer lands last.
*/
statusRouteOwnsBadge: boolean;
}
/**
* The photo provider connection cards behind both settings shells: every enabled
* photo_provider addon with its declared fields, seeded and hydrated values
* (secrets never prefilled), the connection badge, and save and test. The desktop
* section and its phone twin render their own markup over this.
*/
export function usePhotoProviderConnections({ statusRouteOwnsBadge }: PhotoProviderConnectionOptions) {
export function usePhotoProviderConnections() {
const { t } = useTranslation();
const toast = useToast();
const { isEnabled: addonEnabled, addons } = useAddonStore();
@@ -156,7 +147,10 @@ export function usePhotoProviderConnections({ statusRouteOwnsBadge }: PhotoProvi
...prev,
[provider.id]: { ...(prev[provider.id] || {}), ...nextValues },
}));
if (!(statusRouteOwnsBadge && cfg.status_get) && typeof res.data?.connected === 'boolean') {
// A provider with a status route lets only that route set the badge: the settings
// read can only say credentials are stored, and taking whichever of the two answers
// lands last would show an unreachable server as connected.
if (!cfg.status_get && typeof res.data?.connected === 'boolean') {
setProviderConnected((prev) => ({ ...prev, [provider.id]: !!res.data.connected }));
}
})
@@ -169,7 +163,7 @@ export function usePhotoProviderConnections({ statusRouteOwnsBadge }: PhotoProvi
return () => {
isCancelled = true;
};
}, [activePhotoProviders, activeProviderSignature, statusRouteOwnsBadge]);
}, [activePhotoProviders, activeProviderSignature]);
const handleProviderFieldChange = (providerId: string, key: string, value: string) => {
setProviderValues((prev) => ({
@@ -30,7 +30,7 @@ export default function MPhotoProvidersSection(): React.ReactElement {
isProviderSaveDisabled,
handleSaveProvider,
handleTestProvider,
} = usePhotoProviderConnections({ statusRouteOwnsBadge: true })
} = usePhotoProviderConnections()
const renderPhotoProviderSection = (provider: PhotoProviderAddon): React.ReactElement => {
const fields = getProviderFields(provider)