From c33bb425bbe6e7e5b89ef4b3a348f77967592014 Mon Sep 17 00:00:00 2001 From: joshunrau Date: Mon, 5 Oct 2026 12:55:05 -0400 Subject: [PATCH 1/9] fix(api): stop the in-memory replica set when the app shuts down createMemoryConnection kept the replica set in a local, so onApplicationShutdown never saw it and every test run left a mongod and its temp dbPath behind until the process exited. Closes #1651 Co-Authored-By: Claude Opus 5.5 --- apps/api/src/core/__tests__/prisma.spec.ts | 18 +++++++++++++++--- apps/api/src/core/prisma.ts | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/apps/api/src/core/__tests__/prisma.spec.ts b/apps/api/src/core/__tests__/prisma.spec.ts index 82d3a316b..b89f44cee 100644 --- a/apps/api/src/core/__tests__/prisma.spec.ts +++ b/apps/api/src/core/__tests__/prisma.spec.ts @@ -25,8 +25,10 @@ vi.mock('@prisma/client', async (importOriginal) => ({ } })); -/** Only `getUri` is read from the replica set the factory starts. */ -const createReplSet = vi.hoisted(() => vi.fn<(options: unknown) => Promise<{ getUri: (db: string) => string }>>()); +/** Only `getUri` and `stop` are used on the replica set the factory starts. */ +const createReplSet = vi.hoisted(() => + vi.fn<(options: unknown) => Promise<{ getUri: (db: string) => string; stop: () => Promise }>>() +); vi.mock('mongodb-memory-server', () => ({ MongoMemoryReplSet: { create: createReplSet } @@ -238,7 +240,8 @@ describe('PrismaModuleOptionsFactory', () => { it('should connect to a fresh in-memory replica set under test, so tests never touch a real database', async () => { createReplSet.mockResolvedValueOnce({ - getUri: (db) => `mongodb://127.0.0.1:41234/${db}?replicaSet=rs0` + getUri: (db) => `mongodb://127.0.0.1:41234/${db}?replicaSet=rs0`, + stop: vi.fn() }); await (await createFactory({ NODE_ENV: 'test' })).create(); expect(createReplSet).toHaveBeenCalledExactlyOnceWith({ replSet: { count: 1, name: 'rs0' } }); @@ -269,6 +272,15 @@ describe('PrismaModuleOptionsFactory', () => { expect(options).toStrictEqual({ client: prismaClient }); }); + it('should stop the in-memory replica set on shutdown, so no mongod outlives the application', async () => { + const stop = vi.fn(() => Promise.resolve(true)); + createReplSet.mockResolvedValueOnce({ getUri: (db) => `mongodb://127.0.0.1:41234/${db}`, stop }); + const factory = await createFactory({ NODE_ENV: 'test' }); + await factory.create(); + await factory.onApplicationShutdown(); + expect(stop).toHaveBeenCalledOnce(); + }); + it('should shut down cleanly when it started no in-memory replica set', async () => { const factory = await createFactory(externalEnv); await factory.create(); diff --git a/apps/api/src/core/prisma.ts b/apps/api/src/core/prisma.ts index 27572f0e6..6cb190d8f 100644 --- a/apps/api/src/core/prisma.ts +++ b/apps/api/src/core/prisma.ts @@ -194,8 +194,8 @@ export class PrismaModuleOptionsFactory implements OnApplicationShutdown { private async createMemoryConnection(): Promise { // prevent mongodb-memory-server from being included in the production bundle const { MongoMemoryReplSet } = await import('mongodb-memory-server'); - const replSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } }); - return new URL(replSet.getUri('test')).href; + this.memoryReplSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } }); + return new URL(this.memoryReplSet.getUri('test')).href; } private getExternalConnection(): string { From 7d0f941445a125f27ee6c52fba28345943173a0c Mon Sep 17 00:00:00 2001 From: joshunrau Date: Mon, 5 Oct 2026 12:55:06 -0400 Subject: [PATCH 2/9] refactor(api): remove code no input can reach - export worker: expandData only builds successful entries, so the failure branch and its type go - users: GroupsService.findById throws on a miss, so the null check after it never ran - mail: passwordSource decides password inheritance once, which removes the impossible null-saved fallbacks and the empty-plaintext branch in encryptPassword - instrument records: the export worker only acknowledges INIT with success, so the check and the test pinning a failure it cannot send go Co-Authored-By: Claude Opus 5.5 --- .../instrument-records.service.spec.ts | 19 ---- .../src/instrument-records/export-worker.js | 21 +---- .../instrument-records.service.ts | 33 +++---- .../src/instrument-records/thread-types.ts | 2 - apps/api/src/mail/mail.service.ts | 90 +++++++++++-------- apps/api/src/users/users.service.ts | 5 +- 6 files changed, 73 insertions(+), 97 deletions(-) diff --git a/apps/api/src/instrument-records/__tests__/instrument-records.service.spec.ts b/apps/api/src/instrument-records/__tests__/instrument-records.service.spec.ts index a9265f13f..39778fae6 100644 --- a/apps/api/src/instrument-records/__tests__/instrument-records.service.spec.ts +++ b/apps/api/src/instrument-records/__tests__/instrument-records.service.spec.ts @@ -954,25 +954,6 @@ describe('InstrumentRecordsService', () => { await expect(result).rejects.toBe(error); expect(worker.terminate).toHaveBeenCalledOnce(); }); - - // Pins a reported defect: a failed INIT neither rejects nor terminates. Expected to change when the source is fixed. - it('should leave the export pending without sending a chunk when a worker fails to initialize', async () => { - instrumentsService.findById.mockResolvedValueOnce({ id: 'instrument-1', internal: { edition: 1, name: 'X' } }); - const worker = useFakeWorker(); - - const result = instrumentRecordsService.exportRecords({}, { ability }); - await vi.waitFor(() => expect(worker.postMessage).toHaveBeenCalledOnce()); - worker.emit('message', { success: false }); - - await expect(Promise.race([result, Promise.resolve('pending')])).resolves.toBe('pending'); - expect(worker.postMessage).toHaveBeenCalledTimes(1); - expect(worker.postMessage).toHaveBeenCalledWith({ - data: [{ edition: 1, id: 'instrument-1', name: 'X' }], - type: 'INIT' - }); - worker.emit('error', new Error('settle')); - await expect(result).rejects.toThrow('settle'); - }); }); }); diff --git a/apps/api/src/instrument-records/export-worker.js b/apps/api/src/instrument-records/export-worker.js index bd76f954b..0bf932a8c 100644 --- a/apps/api/src/instrument-records/export-worker.js +++ b/apps/api/src/instrument-records/export-worker.js @@ -16,28 +16,19 @@ try { } /** - * @typedef {Object} SuccessExpand + * @typedef {Object} ExpandedMeasure * @property {string} measure * @property {any} measureValue - * @property {true} success */ -/** - * @typedef {Object} FailureExpand - * @property {string} message - * @property {false} success - */ - -/** @typedef {SuccessExpand | FailureExpand} ExpandDataType */ - /** * Flattens nested record array data into a list of expandable data objects. * @param {any[]} listEntry - The array of records to expand. - * @returns {ExpandDataType[]} An array of expanded measure objects. + * @returns {ExpandedMeasure[]} An array of expanded measure objects. * @throws {Error} If the provided listEntry is empty. */ function expandData(listEntry) { - /** @type {SuccessExpand[]} */ + /** @type {ExpandedMeasure[]} */ const validRecordArrayList = []; if (listEntry.length < 1) { throw new Error('Record Array is Empty'); @@ -46,8 +37,7 @@ function expandData(listEntry) { for (const [dataKey, dataValue] of Object.entries(objectEntry)) { validRecordArrayList.push({ measure: dataKey, - measureValue: dataValue, - success: true + measureValue: dataValue }); } } @@ -123,9 +113,6 @@ function handleChunkComplete(_data) { const expanded = expandData(measureValue); for (const entry of expanded) { - if (!entry.success) { - throw new Error(`exportRecords: ${instrument.name}.${measureKey} — ${entry.message}`); - } rows.push({ groupId: record.groupId ?? DEFAULT_GROUP_NAME, instrumentEdition: instrument.edition, diff --git a/apps/api/src/instrument-records/instrument-records.service.ts b/apps/api/src/instrument-records/instrument-records.service.ts index 806acb3d2..5047187ba 100644 --- a/apps/api/src/instrument-records/instrument-records.service.ts +++ b/apps/api/src/instrument-records/instrument-records.service.ts @@ -40,14 +40,7 @@ import { UsersService } from '@/users/users.service'; import { InstrumentMeasuresService } from './instrument-measures.service'; -import type { - BeginChunkProcessingMessage, - InitData, - InitialMessage, - InitMessage, - RecordType, - WorkerMessage -} from './thread-types'; +import type { BeginChunkProcessingMessage, InitData, InitMessage, RecordType, WorkerMessage } from './thread-types'; @Injectable() export class InstrumentRecordsService { @@ -235,18 +228,18 @@ export class InstrumentRecordsService { const worker = new Worker(join(import.meta.dirname, 'export-worker.js')); worker.postMessage({ data: availableInstrumentArray, type: 'INIT' } satisfies InitMessage); - worker.on('message', (message: InitialMessage) => { - if (message.success) { - worker.postMessage({ data: chunk, type: 'BEGIN_CHUNK_PROCESSING' } satisfies BeginChunkProcessingMessage); - worker.on('message', (message: WorkerMessage) => { - if (message.success) { - resolve(message.data); - } else { - reject(new Error(message.error)); - } - void worker.terminate(); - }); - } + // The worker only ever acknowledges INIT with `{ success: true }`; a failure inside it arrives + // as an `error` event instead, handled below. + worker.on('message', () => { + worker.postMessage({ data: chunk, type: 'BEGIN_CHUNK_PROCESSING' } satisfies BeginChunkProcessingMessage); + worker.on('message', (message: WorkerMessage) => { + if (message.success) { + resolve(message.data); + } else { + reject(new Error(message.error)); + } + void worker.terminate(); + }); }); worker.on('error', (error) => { diff --git a/apps/api/src/instrument-records/thread-types.ts b/apps/api/src/instrument-records/thread-types.ts index 005896228..d4ddfb90e 100644 --- a/apps/api/src/instrument-records/thread-types.ts +++ b/apps/api/src/instrument-records/thread-types.ts @@ -43,5 +43,3 @@ export type BeginChunkProcessingMessage = { export type ParentMessage = BeginChunkProcessingMessage | InitMessage; export type WorkerMessage = { data: InstrumentRecordsExport; success: true } | { error: string; success: false }; - -export type InitialMessage = { success: true }; diff --git a/apps/api/src/mail/mail.service.ts b/apps/api/src/mail/mail.service.ts index db81b9cce..05db2fc7f 100644 --- a/apps/api/src/mail/mail.service.ts +++ b/apps/api/src/mail/mail.service.ts @@ -156,13 +156,14 @@ export class MailService { */ async test({ config, recipient }: $TestMailData): Promise { const { config: saved } = await this.readState(); - if (this.requiresNewPassword(config, saved)) { + const resolve = this.configResolver(config, saved); + if (resolve === 'PASSWORD_REQUIRED') { return { error: 'PASSWORD_REQUIRED', success: false }; } try { // Resolution decrypts an inherited stored password, so it belongs inside the collapse: an // undecryptable secret must report a code here exactly as it does on a real send. - const resolved = this.resolveConfig(config, saved); + const resolved = resolve(); if (!resolved) { return { error: 'UNKNOWN', success: false }; } @@ -192,17 +193,19 @@ export class MailService { throw new ServiceUnavailableException('Cannot update mail settings before setup'); } const { config: saved } = this.parseState(setupState); - if (this.requiresNewPassword(data.config, saved)) { - throw new BadRequestException('A password is required when changing the mail server'); + let nextConfig: MailConfig | undefined; + if (data.config) { + const password = this.passwordSource(data.config, saved); + if (!password) { + throw new BadRequestException('A password is required when changing the mail server'); + } + // A kept password is inherited as its stored ciphertext, so updating settings never has to + // decrypt the old secret — after a SECRET_KEY rotation the admin recovers by re-entering it. + nextConfig = this.mergeConfig( + data.config, + 'plaintext' in password ? this.encryptPassword(password.plaintext) : password.ciphertext + ); } - // A kept password is inherited as its stored ciphertext, so updating settings never has to - // decrypt the old secret — after a SECRET_KEY rotation the admin recovers by re-entering it. - const nextConfig = data.config - ? this.mergeConfig( - data.config, - data.config.password ? this.encryptPassword(data.config.password) : (saved?.password ?? '') - ) - : undefined; const updated = await this.setupStateModel.update({ data: { ...(nextConfig ? { mailConfig: { set: nextConfig } } : {}), @@ -213,6 +216,30 @@ export class MailService { return this.toSettings(this.parseState(updated)); } + /** + * How a (possibly partial) payload resolves into a complete config ready to authenticate with: + * a resolver to call, or `PASSWORD_REQUIRED` when the payload may not inherit the stored + * password. Calling the resolver decrypts an inherited password, so callers decide where that + * failure lands. + */ + private configResolver( + partial: undefined | UpdateMailConfigData, + saved: MailConfig | null + ): 'PASSWORD_REQUIRED' | (() => MailConfig | null) { + if (!partial) { + return () => saved && this.decryptConfig(saved); + } + const password = this.passwordSource(partial, saved); + if (!password) { + return 'PASSWORD_REQUIRED'; + } + return () => + this.mergeConfig( + partial, + 'plaintext' in password ? password.plaintext : this.decryptPassword(password.ciphertext) + ); + } + private createTransporter(config: MailConfig): Transporter { return createTransport({ auth: { pass: config.password, user: config.username }, @@ -269,7 +296,7 @@ export class MailService { /** Encrypt the SMTP password so a database dump yields no working mail credential. */ private encryptPassword(plaintext: string): string { - return plaintext ? encryptSecret(plaintext, this.configService.getOrThrow('SECRET_KEY')) : ''; + return encryptSecret(plaintext, this.configService.getOrThrow('SECRET_KEY')); } /** Combine an update payload with an already-resolved password into a complete config. */ @@ -315,33 +342,26 @@ export class MailService { }; } - private async readState(): Promise { - return this.parseState(await this.setupStateModel.findFirst()); - } - /** - * Whether the caller has to supply a password rather than inheriting the stored one. Inheritance - * is confined to the server the password belongs to — see {@link isSameMailServer}. + * Where a payload's password comes from: its own, or for a blank one the stored ciphertext. + * Inheritance is confined to the server the stored password belongs to — see + * {@link isSameMailServer} — and null means the payload has to supply one. */ - private requiresNewPassword(partial: undefined | UpdateMailConfigData, saved: MailConfig | null): boolean { - if (!partial || partial.password) { - return false; + private passwordSource( + partial: UpdateMailConfigData, + saved: MailConfig | null + ): null | { ciphertext: string } | { plaintext: string } { + if (partial.password) { + return { plaintext: partial.password }; + } + if (saved && isSameMailServer(saved, partial)) { + return { ciphertext: saved.password }; } - return !saved || !isSameMailServer(saved, partial); + return null; } - /** - * Resolve a (possibly partial) update payload into a complete config ready to authenticate - * with — an inherited stored password is decrypted here. Callers reject the payload with - * {@link requiresNewPassword} first, which is what confines inheritance to the server the - * stored password belongs to. - */ - private resolveConfig(partial: undefined | UpdateMailConfigData, saved: MailConfig | null): MailConfig | null { - if (!partial) { - return saved && this.decryptConfig(saved); - } - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- a blank password means "keep the stored one"; fall through intentionally - return this.mergeConfig(partial, partial.password || this.decryptPassword(saved?.password ?? '')); + private async readState(): Promise { + return this.parseState(await this.setupStateModel.findFirst()); } private async send(transporter: Transporter, config: MailConfig, { body, subject, to }: MailMessage): Promise { diff --git a/apps/api/src/users/users.service.ts b/apps/api/src/users/users.service.ts index b66e10c98..ccb94a575 100644 --- a/apps/api/src/users/users.service.ts +++ b/apps/api/src/users/users.service.ts @@ -98,10 +98,7 @@ export class UsersService { // Check that all group exist and are accessible to the user for (const id of groupIds) { - const group = await this.groupsService.findById(id, options); - if (!group) { - throw new NotFoundException(`Failed to resolve group with ID: ${id}`); - } + await this.groupsService.findById(id, options); } const hashedPassword = await this.cryptoService.hashPassword(password); From 2c1cb67891488f02d4688444181d22556de16f55 Mon Sep 17 00:00:00 2001 From: joshunrau Date: Mon, 5 Oct 2026 12:55:31 -0400 Subject: [PATCH 3/9] refactor(web): remove code no input can reach Each removal is backed by the types, every caller, or the library contract: - dialogs without a Dialog.Trigger: Radix only calls onOpenChange(false), so `if (!open)` goes - fallbacks on values the schema or the query makes non-null (`?? []`, `?? {}`, `?? ''`) - guards repeating an earlier guard: the bulk mapping resolve behind a disabled button, the login page submit after its colour check, the group reseed the per-group key already performs - branches the data cannot take: the PERSONAL_INFO `else if` on a two-value enum, an exhaustive switch default, zod 4's never-produced `int` type name, a trimmed value compared to '\n' - EmailTemplateEditor is read-only when given no onChange, so the default-template dialog no longer passes a no-op handler - mail settings read the flattened field errors of the four fields that can fail Co-Authored-By: Claude Opus 5.5 --- .../BulkRemoteAssignmentWizard/MapStep.tsx | 23 ++++----- .../EmailTemplateEditor.tsx | 29 ++++++----- .../__tests__/EmailTemplateEditor.test.tsx | 4 +- .../EditTemplateDialog.tsx | 4 +- .../ViewDefaultTemplateDialog.tsx | 7 +-- .../IdentificationForm/IdentificationForm.tsx | 2 +- .../InstrumentPreviewDialog.tsx | 7 +-- .../InstrumentShowcase/InstrumentShowcase.tsx | 5 +- .../LoginBranding/LoginBrandingPanel.tsx | 9 ++-- .../src/components/LoginPageEditor/hooks.ts | 2 +- .../LoginPageEditor/sections/SectionCards.tsx | 2 - .../components/MailSettings/MailSettings.tsx | 12 +++-- .../__tests__/MailSettings.test.tsx | 15 ++++++ .../StartSessionForm/StartSessionForm.tsx | 2 +- apps/web/src/hooks/useGraphData.ts | 40 ++++++++-------- .../_app/admin/instrument-repos/index.tsx | 14 +++--- .../web/src/routes/_app/admin/instruments.tsx | 13 ++--- .../src/routes/_app/admin/users/create.tsx | 10 ++-- .../web/src/routes/_app/admin/users/index.tsx | 7 +-- .../routes/_app/datahub/$subjectId/graph.tsx | 15 +++--- apps/web/src/routes/_app/group/manage.tsx | 48 +++++++------------ .../routes/_app/session/remote-assignment.tsx | 8 ++-- apps/web/src/utils/__tests__/upload.test.ts | 14 ++++-- apps/web/src/utils/bulk-assignments.ts | 4 +- apps/web/src/utils/upload.ts | 14 +----- 25 files changed, 135 insertions(+), 175 deletions(-) diff --git a/apps/web/src/components/BulkRemoteAssignmentWizard/MapStep.tsx b/apps/web/src/components/BulkRemoteAssignmentWizard/MapStep.tsx index f845881af..2dec22700 100644 --- a/apps/web/src/components/BulkRemoteAssignmentWizard/MapStep.tsx +++ b/apps/web/src/components/BulkRemoteAssignmentWizard/MapStep.tsx @@ -54,21 +54,9 @@ export const MapStep = ({ groupName, onBack, onResolved, onStepChange, parsed }: }); }; - const resolve = async () => { + const resolve = async (sourceMode: BulkSourceMode) => { setErrors([]); - if (!mode) { - setErrors([ - { - message: t({ - en: 'Map a subject ID column, or a complete set of first name, last name, date of birth and sex.', - es: 'Asocie una columna de identificador de sujeto, o un conjunto completo de nombre, apellido, fecha de nacimiento y sexo.', - fr: "Associez une colonne d'identifiant, ou un ensemble complet de prénom, nom, date de naissance et sexe." - }) - } - ]); - return; - } - const resolvedParsed: BulkParseResult = { ...parsed, mapping, mode }; + const resolvedParsed: BulkParseResult = { ...parsed, mapping, mode: sourceMode }; try { onResolved(await resolveSubjectIds(resolvedParsed, { groupName, maxSubjects: BULK_ASSIGNMENT_MAX_SUBJECTS })); } catch (err) { @@ -101,7 +89,12 @@ export const MapStep = ({ groupName, onBack, onResolved, onStepChange, parsed }: - diff --git a/apps/web/src/components/EmailTemplateEditor/EmailTemplateEditor.tsx b/apps/web/src/components/EmailTemplateEditor/EmailTemplateEditor.tsx index 675689e72..5b8b1b57b 100644 --- a/apps/web/src/components/EmailTemplateEditor/EmailTemplateEditor.tsx +++ b/apps/web/src/components/EmailTemplateEditor/EmailTemplateEditor.tsx @@ -11,8 +11,8 @@ export type EmailTemplateEditorProps = { error?: string; /** Namespaces the control ids and test ids, so two editors can coexist on one page. */ idPrefix: string; - onChange: (template: MailTemplate) => void; - readOnly?: boolean; + /** Omit to show the template read-only. */ + onChange?: (template: MailTemplate) => void; template: MailTemplate; /** Placeholder names offered as insert buttons above the body. */ variables?: readonly string[]; @@ -23,7 +23,6 @@ export const EmailTemplateEditor = ({ error, idPrefix, onChange, - readOnly = false, template, variables = [] }: EmailTemplateEditorProps) => { @@ -36,15 +35,15 @@ export const EmailTemplateEditor = ({ const body = template.body[language] ?? ''; const subject = template.subject[language] ?? ''; - const insertVariable = (variable: string) => { + const insertVariable = (edit: (template: MailTemplate) => void, variable: string) => { const tag = `{{${variable}}}`; const cursor = bodyCursorRef.current; if (cursor === null) { - onChange({ ...template, body: { ...template.body, [language]: body ? `${body} ${tag}` : tag } }); + edit({ ...template, body: { ...template.body, [language]: body ? `${body} ${tag}` : tag } }); return; } bodyCursorRef.current = { end: cursor.start + tag.length, start: cursor.start + tag.length }; - onChange({ + edit({ ...template, body: { ...template.body, [language]: body.slice(0, cursor.start) + tag + body.slice(cursor.end) } }); @@ -67,10 +66,11 @@ export const EmailTemplateEditor = ({ - onChange({ ...template, subject: { ...template.subject, [language]: event.target.value } }) + onChange={ + onChange && + ((event) => onChange({ ...template, subject: { ...template.subject, [language]: event.target.value } })) } /> @@ -78,7 +78,7 @@ export const EmailTemplateEditor = ({
- {!readOnly && variables.length > 0 && ( + {onChange && variables.length > 0 && (
{t({ en: 'Insert:', es: 'Insertar:', fr: 'Insérer :' })} @@ -94,7 +94,7 @@ export const EmailTemplateEditor = ({ size="sm" type="button" variant="outline" - onClick={() => insertVariable(variable)} + onClick={() => insertVariable(onChange, variable)} > {tag} @@ -107,7 +107,7 @@ export const EmailTemplateEditor = ({ className="min-h-[15rem]" data-testid={`${idPrefix}-body`} id={`${idPrefix}-body`} - readOnly={readOnly} + readOnly={!onChange} value={body} onBlur={(event: React.FocusEvent) => { bodyCursorRef.current = { @@ -115,7 +115,10 @@ export const EmailTemplateEditor = ({ start: event.currentTarget.selectionStart }; }} - onChange={(event) => onChange({ ...template, body: { ...template.body, [language]: event.target.value } })} + onChange={ + onChange && + ((event) => onChange({ ...template, body: { ...template.body, [language]: event.target.value } })) + } /> {error && (

diff --git a/apps/web/src/components/EmailTemplateEditor/__tests__/EmailTemplateEditor.test.tsx b/apps/web/src/components/EmailTemplateEditor/__tests__/EmailTemplateEditor.test.tsx index 016c5ec38..6d8f4b6c8 100644 --- a/apps/web/src/components/EmailTemplateEditor/__tests__/EmailTemplateEditor.test.tsx +++ b/apps/web/src/components/EmailTemplateEditor/__tests__/EmailTemplateEditor.test.tsx @@ -114,8 +114,8 @@ describe('EmailTemplateEditor', () => { expect(screen.queryByText('Insert:')).toBeNull(); }); - it('should lock the fields and hide the insert buttons when read-only', () => { - renderEditor({ readOnly: true, variables: ['name'] }); + it('should lock the fields and hide the insert buttons without a change handler, so a template can be shown read-only', () => { + renderEditor({ onChange: undefined, variables: ['name'] }); expect(subjectInput().readOnly).toBe(true); expect(bodyInput().readOnly).toBe(true); expect(screen.queryByTestId('invite-insert-name')).toBeNull(); diff --git a/apps/web/src/components/GroupEmailTemplates/EditTemplateDialog.tsx b/apps/web/src/components/GroupEmailTemplates/EditTemplateDialog.tsx index 3800b7dab..25956d4ee 100644 --- a/apps/web/src/components/GroupEmailTemplates/EditTemplateDialog.tsx +++ b/apps/web/src/components/GroupEmailTemplates/EditTemplateDialog.tsx @@ -29,8 +29,8 @@ const EditTemplateForm = ({ const addNotification = useNotificationsStore((store) => store.addNotification); const [name, setName] = React.useState(template.name); const [draft, setDraft] = React.useState({ - body: template.body ?? {}, - subject: template.subject ?? {} + body: template.body, + subject: template.subject }); const contentError = validateContent(draft); diff --git a/apps/web/src/components/GroupEmailTemplates/ViewDefaultTemplateDialog.tsx b/apps/web/src/components/GroupEmailTemplates/ViewDefaultTemplateDialog.tsx index a6d2a0e28..0343048eb 100644 --- a/apps/web/src/components/GroupEmailTemplates/ViewDefaultTemplateDialog.tsx +++ b/apps/web/src/components/GroupEmailTemplates/ViewDefaultTemplateDialog.tsx @@ -31,12 +31,7 @@ export const ViewDefaultTemplateDialog = ({ onOpenChange, open }: ViewDefaultTem })} - undefined} - /> +