diff --git a/apps/gateway/src/routers/root.router.ts b/apps/gateway/src/routers/root.router.ts index 1ede9cb45..09800cd07 100644 --- a/apps/gateway/src/routers/root.router.ts +++ b/apps/gateway/src/routers/root.router.ts @@ -1,4 +1,4 @@ -import { $Language } from '@opendatacapture/schemas/core'; +import { $Language, resolveActiveLanguage } from '@opendatacapture/schemas/core'; import { $InstrumentBundleContainer } from '@opendatacapture/schemas/instrument'; import { Router } from 'express'; @@ -45,10 +45,9 @@ router.get( // resolve the same language; anything else renders the page in English and then swaps it. const { activeLanguages } = assignment; const requestedLanguage = $Language.safeParse(req.query.lang); - const language = - requestedLanguage.success && activeLanguages.includes(requestedLanguage.data) - ? requestedLanguage.data - : activeLanguages[0]; + const language = requestedLanguage.success + ? resolveActiveLanguage(requestedLanguage.data, activeLanguages) + : activeLanguages[0]; const token = generateToken(assignment.id); const html = res.locals.loadRoot({ diff --git a/apps/web/src/__tests__/language-toggle.test.tsx b/apps/web/src/__tests__/language-toggle.test.tsx index 66819b47f..57ea40e9b 100644 --- a/apps/web/src/__tests__/language-toggle.test.tsx +++ b/apps/web/src/__tests__/language-toggle.test.tsx @@ -1,7 +1,8 @@ +import { useTranslation } from '@douglasneuroinformatics/libui/hooks'; import { i18n } from '@douglasneuroinformatics/libui/i18n'; import { LanguageToggle } from '@opendatacapture/react-core'; import type { ActiveLanguages } from '@opendatacapture/schemas/core'; -import { cleanup, render, screen } from '@testing-library/react'; +import { act, cleanup, render, screen } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import '@/services/i18n'; @@ -43,4 +44,36 @@ describe('LanguageToggle', () => { renderToggle(['es']); expect(i18n.resolvedLanguage).toBe('es'); }); + + // The sidebar renders the toggle as a descendant while translating its own strings, so it is the + // ancestor that has to re-render for the fix to be worth anything — asserting `resolvedLanguage` + // alone passed while the sidebar stayed in the deactivated language. + const Ancestor = ({ activeLanguages }: { activeLanguages: ActiveLanguages }) => { + const { t } = useTranslation(); + return ( +
+ {t({ en: 'Dashboard', es: 'Panel de control', fr: 'Tableau' })} + +
+ ); + }; + + it('should re-render an ancestor when a language is deactivated mid-session', () => { + const { rerender } = render(); + act(() => i18n.changeLanguage('es')); + expect(screen.getByTestId('ancestor-label').textContent).toBe('Panel de control'); + + rerender(); + expect(i18n.resolvedLanguage).toBe('en'); + expect(screen.getByTestId('ancestor-label').textContent).toBe('Dashboard'); + }); + + it('should leave a reader alone when the deactivated language was not theirs', () => { + const { rerender } = render(); + act(() => i18n.changeLanguage('fr')); + + rerender(); + expect(i18n.resolvedLanguage).toBe('fr'); + expect(screen.getByTestId('ancestor-label').textContent).toBe('Tableau'); + }); }); diff --git a/apps/web/src/__tests__/reconcile-interface-language.test.ts b/apps/web/src/__tests__/reconcile-interface-language.test.ts new file mode 100644 index 000000000..c899fb0b9 --- /dev/null +++ b/apps/web/src/__tests__/reconcile-interface-language.test.ts @@ -0,0 +1,28 @@ +import { i18n } from '@douglasneuroinformatics/libui/i18n'; +import { beforeEach, describe, expect, it } from 'vitest'; + +import { reconcileInterfaceLanguage } from '@/services/i18n'; + +import '@/services/i18n'; + +describe('reconcileInterfaceLanguage', () => { + beforeEach(() => { + i18n.changeLanguage('en'); + }); + + it('should move a reader off a language the instance no longer offers', () => { + i18n.changeLanguage('es'); + expect(reconcileInterfaceLanguage(['en', 'fr'])).toBe(true); + expect(i18n.resolvedLanguage).toBe('en'); + }); + + it('should leave a reader on a language the instance still offers', () => { + i18n.changeLanguage('fr'); + expect(reconcileInterfaceLanguage(['en', 'fr'])).toBe(false); + expect(i18n.resolvedLanguage).toBe('fr'); + }); + + it('should report no change when nothing moved, so it does not notify every translated component', () => { + expect(reconcileInterfaceLanguage(['en', 'es', 'fr'])).toBe(false); + }); +}); diff --git a/apps/web/src/routes/_app/route.tsx b/apps/web/src/routes/_app/route.tsx index 567493151..2e2d635bc 100644 --- a/apps/web/src/routes/_app/route.tsx +++ b/apps/web/src/routes/_app/route.tsx @@ -6,6 +6,7 @@ import { DisclaimerProvider } from '@/providers/DisclaimerProvider'; import { ForceClearQueryCacheProvider } from '@/providers/ForceClearQueryCacheProvider'; import { WalkthroughProvider } from '@/providers/WalkthroughProvider'; import { useAppStore } from '@/store'; +import { reconcileInterfaceLanguage } from '@/services/i18n'; export const Route = createFileRoute('/_app')({ beforeLoad: async ({ context }) => { @@ -17,6 +18,8 @@ export const Route = createFileRoute('/_app')({ if (!accessToken) { throw redirect({ to: '/auth/login' }); } + // Before the tree renders, so no component has to be told after the fact. + reconcileInterfaceLanguage(setupState.activeLanguages); }, component: () => { return ( diff --git a/apps/web/src/services/i18n.ts b/apps/web/src/services/i18n.ts index 808f6e96d..365f93dfb 100644 --- a/apps/web/src/services/i18n.ts +++ b/apps/web/src/services/i18n.ts @@ -2,6 +2,8 @@ /* eslint-disable @typescript-eslint/no-namespace */ import { i18n } from '@douglasneuroinformatics/libui/i18n'; +import { resolveActiveLanguage } from '@opendatacapture/schemas/core'; +import type { ActiveLanguages } from '@opendatacapture/schemas/core'; import auth from '../translations/auth.json'; import common from '../translations/common.json'; @@ -49,4 +51,21 @@ i18n.init({ } }); +/** + * Move a reader off a language their instance no longer offers, and report whether it moved them. + * + * Called before the app renders rather than from a component: `changeLanguage` notifies only the + * components already subscribed, libui's `useTranslation` subscribes in an effect, and effects run + * child-first — so a correction made after mount never reaches the ancestors of whatever made it. + * The sidebar renders the language toggle, so the sidebar is what a late correction leaves behind. + */ +export const reconcileInterfaceLanguage = (activeLanguages: ActiveLanguages): boolean => { + const language = resolveActiveLanguage(i18n.resolvedLanguage, activeLanguages); + if (language === i18n.resolvedLanguage) { + return false; + } + i18n.changeLanguage(language); + return true; +}; + export default i18n; diff --git a/packages/react-core/src/hooks/useLanguageOptions.ts b/packages/react-core/src/hooks/useLanguageOptions.ts index 525122cb8..29b224f64 100644 --- a/packages/react-core/src/hooks/useLanguageOptions.ts +++ b/packages/react-core/src/hooks/useLanguageOptions.ts @@ -1,6 +1,7 @@ import { useEffect } from 'react'; import { useTranslation } from '@douglasneuroinformatics/libui/hooks'; +import { resolveActiveLanguage } from '@opendatacapture/schemas/core'; import type { ActiveLanguages } from '@opendatacapture/schemas/core'; import { toLanguageToggleOptions } from '../utils/language'; @@ -8,21 +9,23 @@ import { toLanguageToggleOptions } from '../utils/language'; /** * The languages an instance offers, as options for libui's `LanguageToggle`. * - * Deactivating a language would otherwise strand every user already reading in it: their strings - * still resolve, but the toggle no longer lists it, so they have no way back. Reconciling here — - * rather than where an admin flips the setting — moves whoever is affected on their next load, - * not just the admin who made the change. + * The effect covers an admin deactivating a language **during** a session: every component is + * subscribed to `languageChange` by then, so they all re-render. It cannot cover a tree that + * mounts already stranded — effects run child-first, so this fires before the ancestors rendering + * the toggle have subscribed, and they would keep the deactivated language. A host resolves that + * case before it renders (`apps/web` in the `_app` route's `beforeLoad`; `apps/gateway` picks the + * language server-side from the same set), which is why this only has to handle the live change. */ export const useLanguageOptions = (activeLanguages: ActiveLanguages) => { const { changeLanguage, resolvedLanguage } = useTranslation(); - const isActive = activeLanguages.includes(resolvedLanguage); + const reconciled = resolveActiveLanguage(resolvedLanguage, activeLanguages); useEffect(() => { - if (!isActive) { - changeLanguage(activeLanguages[0]); + if (reconciled !== resolvedLanguage) { + changeLanguage(reconciled); } - }, [isActive, activeLanguages]); + }, [reconciled, resolvedLanguage]); return toLanguageToggleOptions(activeLanguages); }; diff --git a/packages/schemas/src/core/core.test.ts b/packages/schemas/src/core/core.test.ts index 3520c3a1a..953a80fc4 100644 --- a/packages/schemas/src/core/core.test.ts +++ b/packages/schemas/src/core/core.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { $ActiveLanguages, $LocalizedString, LANGUAGES, toInstrumentAuthoringLanguage } from './core.js'; +import { + $ActiveLanguages, + $LocalizedString, + LANGUAGES, + resolveActiveLanguage, + toInstrumentAuthoringLanguage +} from './core.js'; describe('$ActiveLanguages', () => { it.each([[['en']], [['es']], [['en', 'fr']], [['en', 'es', 'fr']]])( @@ -27,6 +33,20 @@ describe('$ActiveLanguages', () => { }); }); +describe('resolveActiveLanguage', () => { + it('should keep a reader on their language while the instance still offers it', () => { + expect(resolveActiveLanguage('fr', ['en', 'fr'])).toBe('fr'); + }); + + it('should move a reader off a deactivated language, which the toggle no longer offers a way out of', () => { + expect(resolveActiveLanguage('es', ['en', 'fr'])).toBe('en'); + }); + + it('should fall back to the first offered language, so the result does not depend on click order', () => { + expect(resolveActiveLanguage('en', ['fr', 'es'])).toBe('fr'); + }); +}); + describe('$LocalizedString', () => { it('should accept an entry for every interface language', () => { expect($LocalizedString.safeParse({ en: 'Hello', es: 'Hola', fr: 'Bonjour' }).success).toBe(true); diff --git a/packages/schemas/src/core/core.ts b/packages/schemas/src/core/core.ts index d0a2ea1ac..6e4d8b15c 100644 --- a/packages/schemas/src/core/core.ts +++ b/packages/schemas/src/core/core.ts @@ -82,6 +82,17 @@ export const $ActiveLanguages = z.tuple([$Language], $Language); /** The languages an instance offers before an admin has chosen, and the fallback for one saved before this setting existed. */ export const DEFAULT_ACTIVE_LANGUAGES: ActiveLanguages = ['en', 'fr']; +/** + * The language a reader should end up in, given the set their instance offers. A reader on a + * language that has since been deactivated falls back to the first active one — the toggle no + * longer lists theirs, so leaving them on it strands them with no way out. + * + * This is the one place that policy is decided; both the moment it is applied — before the app + * renders, and again whenever an admin changes the set mid-session — resolve through here. + */ +export const resolveActiveLanguage = (language: Language, activeLanguages: ActiveLanguages): Language => + activeLanguages.includes(language) ? language : activeLanguages[0]; + /** * A string authored in each of the application's languages. Every field is nullish so content * may target a single language, and nullish rather than optional to match Prisma's diff --git a/testing/src/specs/admin-settings.spec.ts b/testing/src/specs/admin-settings.spec.ts index e74430e62..903d3240b 100644 --- a/testing/src/specs/admin-settings.spec.ts +++ b/testing/src/specs/admin-settings.spec.ts @@ -65,6 +65,29 @@ test.describe('admin settings', () => { await settingsPage.activeLanguageCheckbox('fr').click(); expect((await restored).ok()).toBe(true); await expect(page.getByTestId('sidebar').getByTestId('language-toggle')).toBeVisible(); + + // Deactivating the language the reader is currently in. The sidebar is the casualty when this + // goes wrong: it renders the toggle, so it is the ancestor a correction made from the toggle + // cannot reach. `Iniciar una sesión` is a namespace string, translated on any branch. + const sidebar = page.getByTestId('sidebar'); + const activated = waitForSetupPatch(page); + await settingsPage.activeLanguageCheckbox('es').click(); + expect((await activated).ok()).toBe(true); + + await sidebar.getByTestId('language-toggle').getByRole('button').click(); + await page.getByRole('menuitem', { name: 'Español' }).click(); + await expect(sidebar).toContainText('Iniciar una sesión'); + + const deactivatedSpanish = waitForSetupPatch(page); + await settingsPage.activeLanguageCheckbox('es').click(); + expect((await deactivatedSpanish).ok()).toBe(true); + + await expect(sidebar).toContainText('Start Session'); + await expect(sidebar).not.toContainText('Iniciar una sesión'); + + // And on a fresh mount, where the correction has to already have happened before render. + await page.reload(); + await expect(sidebar).toContainText('Start Session'); }); test('should apply the group switcher position preference immediately', async ({ getPageModel }) => {