Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/api/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ model SetupState {
createdAt DateTime @default(now()) @db.Date
updatedAt DateTime @updatedAt @db.Date
id String @id @default(auto()) @map("_id") @db.ObjectId
activeLanguages String[]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A required scalar list added to a model with existing documents. #1441 adds its SetupState field as Int?; this one has neither ? (not permitted on lists) nor @default([]).

Prisma's MongoDB connector should return [] for a list field absent from a stored document, so this probably works — but "probably" is doing real work in a field that gates the language toggle for the whole instance. @default([]) costs nothing and removes the question for every instance created before this deploys.

Also worth noting initApp creates SetupState without this field, so the very first document an instance writes exercises exactly that path.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomasbeaudry I agree, add the default

branding BrandingConfig?
isDemo Boolean
isExperimentalFeaturesEnabled Boolean?
Expand Down
3 changes: 3 additions & 0 deletions apps/api/src/setup/dto/update-setup-state.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import type { BrandingConfig, UpdateSetupStateData } from '@opendatacapture/sche

@ValidationSchema($UpdateSetupStateData)
export class UpdateSetupStateDto implements UpdateSetupStateData {
@ApiProperty({ required: false })
activeLanguages?: string[];

@ApiProperty({ required: false })
branding?: BrandingConfig | null;

Expand Down
1 change: 1 addition & 0 deletions apps/api/src/setup/setup.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class SetupService {
// older $BrandingConfig will silently drop newer branding fields on read.
const branding = $BrandingConfig.nullable().safeParse(savedOptions?.branding ?? null);
return {
activeLanguages: savedOptions?.activeLanguages?.length ? savedOptions.activeLanguages : ['en', 'fr'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first of four copies of ['en', 'fr'] in this PR (also Navbar, Sidebar, settings.tsx) and #1439 adds five more. AGENTS.md: "One source of truth; everything derived."

Export DEFAULT_ACTIVE_LANGUAGES from schemas/setup and use it here. Then, since this method already guarantees a non-empty array, $SetupState.activeLanguages can drop .optional() and every client-side ?? ['en', 'fr'] disappears — the clients stop needing to know the default exists.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomasbeaudry I agree

branding: branding.success ? branding.data : null,
isDemo: Boolean(savedOptions?.isDemo),
isExperimentalFeaturesEnabled: Boolean(savedOptions?.isExperimentalFeaturesEnabled),
Expand Down
27 changes: 27 additions & 0 deletions apps/gateway/src/services/i18n.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,32 @@
/* eslint-disable @typescript-eslint/consistent-type-definitions */
/* eslint-disable @typescript-eslint/no-namespace */

import { i18n } from '@douglasneuroinformatics/libui/i18n';
import type { Language } from '@douglasneuroinformatics/libui/i18n';

declare module '@douglasneuroinformatics/libui/i18n' {
export namespace UserConfig {
export interface LanguageOptions {
en: true;
es: true;
fr: true;
}
}
}

const VALID_LANGUAGES = ['en', 'es', 'fr'];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third hardcoded language list in this PR (after ALL_LANGUAGES and LanguageOptions), and #1439 adds a fourth (MAIL_LANGUAGE). Import the tuple from schemas and derive.

Two behavioural notes:

  1. The gateway ignores activeLanguages. ?lang=es renders Spanish even on an instance where Spanish has been deactivated. Since the emailed assignment links in feat(mail): add outgoing email, group email templates, and assignment emails #1439 are built as ${assignment.url}?lang=${language}, that's reachable in practice.

  2. SSR/hydration. detectLanguage() reads window.location.search at module scope; the guard makes the server always resolve 'en' while the client can resolve 'es'. Worth confirming the first paint doesn't flash English or trip a hydration mismatch — a ?lang= link is precisely the case where the two disagree.

Minor: the two /* eslint-disable */ lines at the top disable those rules for the whole file rather than the declare module block that needs them.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomasbeaudry I agree

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not change the eslint-disable though


function detectLanguage(): Language {
if (typeof window !== 'undefined') {
const param = new URLSearchParams(window.location.search).get('lang');
if (param && VALID_LANGUAGES.includes(param)) {
return param as Language;
}
}
return 'en';
}

i18n.init({
defaultLanguage: detectLanguage(),
translations: {}
});
29 changes: 18 additions & 11 deletions apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,11 @@ const LogoSection = ({ alignment, branding, instanceName, preview }: LogoSection
type ResourcesSectionProps = {
boldResourceLinks: boolean;
fontSize: null | number | undefined;
lang: 'en' | 'fr';
lang: string;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unacceptable. We do not use magic strings in this codebase. Language needs to be adjusted and used here.

links: ResourceLink[];
preview: boolean;
tc: (slateClass: string) => null | string;
tl: (obj: { en: string; fr: string }) => string;
tl: (obj: { [key: string]: string }) => string;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use mapped type

};

const ResourcesSection = ({ boldResourceLinks, fontSize, lang, links, preview, tc, tl }: ResourcesSectionProps) => (
Expand All @@ -144,8 +144,13 @@ const ResourcesSection = ({ boldResourceLinks, fontSize, lang, links, preview, t
style={fontStyle(fontSize, preview)}
>
{links.map((link, index) => {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const linkLabel = link.label?.[lang]?.trim() || link.label?.en?.trim() || link.label?.fr?.trim() || '';
/* eslint-disable @typescript-eslint/prefer-nullish-coalescing -- blank strings must fall back, so `||` not `??` */
const linkLabel =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is very messy. Add a getValueForLanguage helper somewhere (shared) that will take in the preferred and default language.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be typed properly with generics

(link.label as undefined | { [key: string]: string })?.[lang]?.trim() ||
link.label?.en?.trim() ||
link.label?.fr?.trim() ||
'';
/* eslint-enable @typescript-eslint/prefer-nullish-coalescing */
if (!linkLabel) return null;
return (
<a
Expand Down Expand Up @@ -224,15 +229,17 @@ export const LoginBrandingPanel = ({
const { resolvedLanguage } = useTranslation();
const lang = langOverride ?? resolvedLanguage;

const tl = (obj: { en: string; fr: string }): string => obj[lang];
const tl = (obj: { [key: string]: string }): string => obj[lang] ?? obj.en ?? '';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapped types


const derived = useMemo(() => {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const instanceName = branding?.instanceName?.[lang]?.trim() || DEFAULT_INSTANCE_NAME;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const instanceTagline = branding?.instanceTagline?.[lang]?.trim() || null;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const instanceDetails = branding?.instanceDetails?.[lang]?.trim() || null;
/* eslint-disable @typescript-eslint/prefer-nullish-coalescing -- blank strings must fall back, so `||` not `??` */
const instanceName =
(branding?.instanceName as undefined | { [key: string]: string })?.[lang]?.trim() || DEFAULT_INSTANCE_NAME;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These casts (four in this file, plus login.tsx and StartSessionForm.tsx) are the symptom, not the fix. AGENTS.md: "no casting at call sites" and "Concentrate the cost: complex type machinery belongs in a small number of utilities."

The root cause is that $BrandingConfig types instanceName/instanceTagline/instanceDetails and resourceLinks[].label as literal { en, fr } objects, so they can't be indexed once Language widens. Keying them by Language in packages/schemas fixes all six sites without touching them:

const $LocalizedString = z.partialRecord($Language, z.string().nullish());

That also makes tl = (obj) => obj[lang] ?? obj.en ?? '' unnecessary and — importantly — makes adding a fourth language a compile-time task rather than a runtime fallback to ''. Today the cast means a missing language silently renders as an empty instance name.

Note #1439 adds a $LocalizedString in schemas/mail for the same reason; one shared definition in schemas/core would serve both PRs.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

const instanceTagline =
(branding?.instanceTagline as undefined | { [key: string]: string })?.[lang]?.trim() || null;
const instanceDetails =
(branding?.instanceDetails as undefined | { [key: string]: string })?.[lang]?.trim() || null;
/* eslint-enable @typescript-eslint/prefer-nullish-coalescing */
const panelTextColor = branding?.panelTextColor ?? null;
return {
boldDetails: branding?.boldDetails === true,
Expand Down
18 changes: 11 additions & 7 deletions apps/web/src/components/Navbar/Navbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import { MenuIcon, StopCircle } from 'lucide-react';

import { useIsDesktop } from '@/hooks/useIsDesktop';
import { useNavItems } from '@/hooks/useNavItems';
import { useSetupStateQuery } from '@/hooks/useSetupStateQuery';
import { useAppStore } from '@/store';
import { ALL_LANGUAGES } from '@/utils/languages';

import { GroupSwitcher, useIsGroupSwitcherVisible } from '../GroupSwitcher';
import { NavButton } from '../NavButton';
Expand All @@ -23,6 +25,12 @@ export const Navbar = () => {
const { t } = useTranslation('layout');
const navigate = useNavigate();
const endSession = useAppStore((store) => store.endSession);
const setupStateQuery = useSetupStateQuery();

const activeLanguages = setupStateQuery.data.activeLanguages ?? ['en', 'fr'];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments above

const languageOptions = Object.fromEntries(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be a shared hook useLanguageOptions

Object.entries(ALL_LANGUAGES).filter(([key]) => activeLanguages.includes(key))
);

// This is to prevent ugly styling when resizing the viewport
const isDesktop = useIsDesktop();
Expand Down Expand Up @@ -124,13 +132,9 @@ export const Navbar = () => {
<div className="flex w-full justify-between gap-2 md:justify-end">
<UserDropup />
<div className="flex gap-2">
<LanguageToggle
options={{
en: 'English',
fr: 'Français'
}}
variant="outline"
/>
{Object.keys(languageOptions).length > 1 && (
<LanguageToggle options={languageOptions} variant="outline" />
)}
<ThemeToggle variant="outline" />
</div>
</div>
Expand Down
28 changes: 28 additions & 0 deletions apps/web/src/components/SaveStatus.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React from 'react';

import { useTranslation } from '@douglasneuroinformatics/libui/hooks';
import { CheckIcon, Loader2Icon } from 'lucide-react';

export const SaveStatus = ({ state }: { state: 'idle' | 'saved' | 'saving' }) => {
const { t } = useTranslation();
if (state === 'idle') {
return null;
}
return (
<div className="fixed bottom-4 right-4 z-50 flex items-center gap-1.5 rounded-full border border-slate-200/70 bg-white/95 px-3 py-1.5 text-xs font-medium shadow-md backdrop-blur dark:border-slate-700/70 dark:bg-slate-800/95">
{state === 'saving' ? (
<React.Fragment>
<Loader2Icon className="text-muted-foreground h-3.5 w-3.5 animate-spin" />
<span className="text-muted-foreground">{t({ en: 'Saving…', es: 'Guardando…', fr: 'Enregistrement…' })}</span>
</React.Fragment>
) : (
<React.Fragment>
<CheckIcon className="h-3.5 w-3.5 text-green-600" />
<span>
{t({ en: 'All changes saved', es: 'Todos los cambios guardados', fr: 'Modifications enregistrées' })}
</span>
</React.Fragment>
)}
</div>
);
};
27 changes: 17 additions & 10 deletions apps/web/src/components/Sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { StopCircle } from 'lucide-react';
import { AnimatePresence, motion } from 'motion/react';

import { useNavItems } from '@/hooks/useNavItems';
import { useSetupStateQuery } from '@/hooks/useSetupStateQuery';
import { useAppStore } from '@/store';
import { ALL_LANGUAGES } from '@/utils/languages';

import { GroupSwitcher, useIsGroupSwitcherVisible } from '../GroupSwitcher';
import { NavButton } from '../NavButton';
Expand All @@ -27,9 +29,15 @@ export const Sidebar = () => {
const endSession = useAppStore((store) => store.endSession);
const location = useLocation();
const navigate = useNavigate();
const setupStateQuery = useSetupStateQuery();

const { t } = useTranslation();

const activeLanguages = setupStateQuery.data.activeLanguages ?? ['en', 'fr'];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments above and shared hook please

const languageOptions = Object.fromEntries(
Object.entries(ALL_LANGUAGES).filter(([key]) => activeLanguages.includes(key))
);

return (
<div
className="flex h-screen w-[19rem] flex-col bg-slate-900 px-3 py-2 text-slate-100 shadow-lg dark:border-r dark:border-slate-700"
Expand Down Expand Up @@ -137,16 +145,15 @@ export const Sidebar = () => {
<div className="flex items-center justify-between py-2">
<UserDropup />
<div className="flex h-full items-center gap-2">
<LanguageToggle
contentClassName="bg-slate-800 border-slate-700 text-slate-300"
itemClassName="bg-slate-800 hover:bg-slate-700 focus:bg-slate-700 focus:text-slate-100"
options={{
en: 'English',
fr: 'Français'
}}
triggerClassName="hover:bg-slate-800 hover:text-slate-300 focus-visible:ring-0"
variant="ghost"
/>
{Object.keys(languageOptions).length > 1 && (
<LanguageToggle
contentClassName="bg-slate-800 border-slate-700 text-slate-300"
itemClassName="bg-slate-800 hover:bg-slate-700 focus:bg-slate-700 focus:text-slate-100"
options={languageOptions}
triggerClassName="hover:bg-slate-800 hover:text-slate-300 focus-visible:ring-0"
variant="ghost"
/>
)}
<ThemeToggle className="hover:bg-slate-800 hover:text-slate-300" variant="ghost" />
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,9 @@ export const StartSessionForm = ({
ctx.addIssue({
code: z.ZodIssueCode.custom,
message:
currentGroup.settings.idValidationRegexErrorMessage?.[resolvedLanguage] ??
(currentGroup.settings.idValidationRegexErrorMessage as undefined | { [key: string]: string })?.[

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no casting at call sites

resolvedLanguage
] ??
t({
en: `Must match regular expression: ${regex.source}`,
fr: `Doit correspondre à l'expression régulière : ${regex.source}`
Expand Down
9 changes: 8 additions & 1 deletion apps/web/src/hooks/useInstrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@ export function useInstrument(id: null | string) {
const { bundle, id } = instrumentBundleQuery.data;
interpreter
.interpret(bundle, { id, validate: import.meta.env.DEV })
.then((instrument) => setInstrument(translateInstrument(instrument, resolvedLanguage)))
.then((instrument) =>
setInstrument(
translateInstrument(
instrument,
resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en' appears five times in this PR — here, useInstrumentInfoQuery, group/manage.tsx, InteractiveContent, and useInterpretedInstrument. AGENTS.md: "Shape is never repeated."

One named helper says what it means and gives you a single place to change the policy:

/** Instruments are authored in en/fr only; other UI languages fall back to English. */
export const toInstrumentLanguage = (language: Language): 'en' | 'fr' =>
  language === 'fr' ? 'fr' : 'en';

The policy itself is also worth surfacing: a user who has selected Español gets instruments silently rendered in English with no indication that a translation is missing. That's defensible, but right now it's an implicit consequence of five scattered ternaries rather than a stated decision.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Translate instrument should handle this already downstream. I think it already does

)
)
)
.catch((err) => {
console.error(err);
setInstrument(null);
Expand Down
3 changes: 2 additions & 1 deletion apps/web/src/hooks/useInstrumentInfoQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export function useInstrumentInfoQuery<TKind extends InstrumentKind>({
params: { ...params, groupId: currentGroupId }
});
const infos = await $InstrumentInfo.array().parseAsync(response.data);
return infos.map((instrument) => translateInstrumentInfo(instrument, resolvedLanguage ?? 'en'));
const instrumentLang = resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be handled downstream

return infos.map((instrument) => translateInstrumentInfo(instrument, instrumentLang));
},
queryKey: [
'instrument-info',
Expand Down
Loading