From 4e7318c4ed3e16f175931db0aa2b5084fd8d98c9 Mon Sep 17 00:00:00 2001 From: zachary Date: Mon, 27 Jul 2026 13:49:59 +0100 Subject: [PATCH 1/5] Add custom validation logic for config --- .../postgres/TemplateDataSource.ts | 19 ++++++++++++++++++- .../models/questionTypes/InstrumentPicker.ts | 7 +++++++ .../models/questionTypes/QuestionRegistry.ts | 5 +++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/apps/backend/src/datasources/postgres/TemplateDataSource.ts b/apps/backend/src/datasources/postgres/TemplateDataSource.ts index 185b53b8a4..861c1449dd 100644 --- a/apps/backend/src/datasources/postgres/TemplateDataSource.ts +++ b/apps/backend/src/datasources/postgres/TemplateDataSource.ts @@ -1,7 +1,6 @@ import { logger } from '@user-office-software/duo-logger'; import { GraphQLError } from 'graphql'; -import { createConfig } from '../../models/questionTypes/QuestionRegistry'; import { ComparisonStatus, ConflictResolutionStrategy, @@ -59,6 +58,8 @@ import { TemplateRecord, TopicRecord, } from './records'; +import { createConfig } from '../../models/questionTypes/QuestionRegistry'; +import { getQuestionDefinition } from '../../models/questionTypes/QuestionRegistry'; const EXPORT_VERSION = '1.2.0'; const MIN_SUPPORTED_VERSION = '1.2.0'; @@ -682,6 +683,22 @@ export default class PostgresTemplateDataSource implements TemplateDataSource { dependenciesOperator, } = args; + const questionType = await database('questions') + .select('data_type') + .where('question_id', questionId) + .first(); + const questionDef = getQuestionDefinition( + questionType.data_type as DataType + ); + if (config) { + const isValidConfig = questionDef.validateConfig + ? questionDef.validateConfig(JSON.parse(config)) + : true; + if (!isValidConfig) { + throw new GraphQLError('Invalid config for question.'); + } + } + await database('templates_has_questions') .update({ config: config, diff --git a/apps/backend/src/models/questionTypes/InstrumentPicker.ts b/apps/backend/src/models/questionTypes/InstrumentPicker.ts index f4d5734a89..cb2982e376 100644 --- a/apps/backend/src/models/questionTypes/InstrumentPicker.ts +++ b/apps/backend/src/models/questionTypes/InstrumentPicker.ts @@ -125,4 +125,11 @@ export const instrumentPickerDefinition: Question = } ); }, + validateConfig: (config) => { + if (config?.variant === 'radio' && config?.isMultipleSelect) { + return false; + } + + return true; + }, }; diff --git a/apps/backend/src/models/questionTypes/QuestionRegistry.ts b/apps/backend/src/models/questionTypes/QuestionRegistry.ts index 52f5a80eb1..cd5fd12fdb 100644 --- a/apps/backend/src/models/questionTypes/QuestionRegistry.ts +++ b/apps/backend/src/models/questionTypes/QuestionRegistry.ts @@ -124,6 +124,11 @@ export interface Question { value: any ) => Promise; + /** + * Performs validation on the config submitted before persisting data into the database + */ + readonly validateConfig?: (config: any) => boolean; + /** * Question can contain configuration, e.g. isRequired, maxValue, etc, * This function returns configuration for newly created questions From 8882580a457ec595a83cec1f02c5e41f14f9c53d Mon Sep 17 00:00:00 2001 From: zachary Date: Mon, 27 Jul 2026 14:30:46 +0100 Subject: [PATCH 2/5] check keys match --- .../postgres/TemplateDataSource.ts | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/apps/backend/src/datasources/postgres/TemplateDataSource.ts b/apps/backend/src/datasources/postgres/TemplateDataSource.ts index 861c1449dd..525ed6d6e3 100644 --- a/apps/backend/src/datasources/postgres/TemplateDataSource.ts +++ b/apps/backend/src/datasources/postgres/TemplateDataSource.ts @@ -683,21 +683,7 @@ export default class PostgresTemplateDataSource implements TemplateDataSource { dependenciesOperator, } = args; - const questionType = await database('questions') - .select('data_type') - .where('question_id', questionId) - .first(); - const questionDef = getQuestionDefinition( - questionType.data_type as DataType - ); - if (config) { - const isValidConfig = questionDef.validateConfig - ? questionDef.validateConfig(JSON.parse(config)) - : true; - if (!isValidConfig) { - throw new GraphQLError('Invalid config for question.'); - } - } + await validateConfigBeforeWrite(config, questionId); await database('templates_has_questions') .update({ @@ -1300,3 +1286,38 @@ export default class PostgresTemplateDataSource implements TemplateDataSource { return newTemplate; } } + +async function validateConfigBeforeWrite(newConfig: any, questionId: string) { + let newConfigObject; + try { + newConfigObject = JSON.parse(newConfig); + } catch { + throw new GraphQLError('Invalid JSON for config.'); + } + + const questionType = await database('questions') + .select('data_type') + .where('question_id', questionId) + .first(); + + const questionDef = getQuestionDefinition(questionType.data_type as DataType); + + const newConfigKeys = JSON.stringify(Object.keys(newConfigObject).sort()); + const defaultQuestionKeys = JSON.stringify( + Object.keys(questionDef.createBlankConfig()).sort() + ); + if (newConfigKeys !== defaultQuestionKeys) { + throw new GraphQLError( + 'Keys of new config type do not match keys in database.' + ); + } + + const isValidConfig = questionDef.validateConfig + ? questionDef.validateConfig(newConfigObject) + : true; + if (!isValidConfig) { + throw new GraphQLError('Invalid config for question.'); + } + + return; +} From f29398b976a1785158538797e6c5fa5737bd194b Mon Sep 17 00:00:00 2001 From: zachary Date: Mon, 27 Jul 2026 16:08:27 +0100 Subject: [PATCH 3/5] add yup validation schemas --- .../postgres/TemplateDataSource.ts | 33 +++++++++++-------- .../models/questionTypes/InstrumentPicker.ts | 20 +++++++++-- .../models/questionTypes/QuestionRegistry.ts | 4 ++- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/apps/backend/src/datasources/postgres/TemplateDataSource.ts b/apps/backend/src/datasources/postgres/TemplateDataSource.ts index 525ed6d6e3..aad936d55a 100644 --- a/apps/backend/src/datasources/postgres/TemplateDataSource.ts +++ b/apps/backend/src/datasources/postgres/TemplateDataSource.ts @@ -1,5 +1,6 @@ import { logger } from '@user-office-software/duo-logger'; import { GraphQLError } from 'graphql'; +import * as Yup from 'yup'; import { ComparisonStatus, @@ -1302,21 +1303,27 @@ async function validateConfigBeforeWrite(newConfig: any, questionId: string) { const questionDef = getQuestionDefinition(questionType.data_type as DataType); - const newConfigKeys = JSON.stringify(Object.keys(newConfigObject).sort()); - const defaultQuestionKeys = JSON.stringify( - Object.keys(questionDef.createBlankConfig()).sort() - ); - if (newConfigKeys !== defaultQuestionKeys) { - throw new GraphQLError( - 'Keys of new config type do not match keys in database.' - ); + const configBaseYupSchema = Yup.object({ + small_label: Yup.string(), + required: Yup.boolean().required(), + tooltip: Yup.string(), + readPermissions: Yup.array().of(Yup.string().required()).required(), + }); + + try { + //When all configs have their own schemas written this will no longer be optional. + if (questionDef.customYupSchema) { + const combinedYupSchema = configBaseYupSchema + .concat(questionDef.customYupSchema) + .noUnknown(true, 'Unknown field'); + await combinedYupSchema.validate(newConfigObject); + } + } catch (error) { + throw new GraphQLError('Config schema not valid'); } - const isValidConfig = questionDef.validateConfig - ? questionDef.validateConfig(newConfigObject) - : true; - if (!isValidConfig) { - throw new GraphQLError('Invalid config for question.'); + if (questionDef.validateConfig) { + questionDef.validateConfig(newConfigObject); } return; diff --git a/apps/backend/src/models/questionTypes/InstrumentPicker.ts b/apps/backend/src/models/questionTypes/InstrumentPicker.ts index cb2982e376..fc00805ce2 100644 --- a/apps/backend/src/models/questionTypes/InstrumentPicker.ts +++ b/apps/backend/src/models/questionTypes/InstrumentPicker.ts @@ -2,6 +2,7 @@ import { logger } from '@user-office-software/duo-logger'; import { GraphQLError } from 'graphql'; import { container } from 'tsyringe'; +import * as Yup from 'yup'; import { Tokens } from '../../config/Tokens'; import { InstrumentDataSource } from '../../datasources/InstrumentDataSource'; @@ -127,9 +128,24 @@ export const instrumentPickerDefinition: Question = }, validateConfig: (config) => { if (config?.variant === 'radio' && config?.isMultipleSelect) { - return false; + throw new GraphQLError( + 'Radio instrument pickers cannot have isMultipleSelect as true.' + ); } - return true; + return; }, + customYupSchema: Yup.object({ + variant: Yup.string().required(), + instruments: Yup.array() + .of( + Yup.object({ + id: Yup.number().required(), + name: Yup.string().required(), + }) + ) + .required(), + isMultipleSelect: Yup.boolean().required(), + requestTime: Yup.boolean().required(), + }), }; diff --git a/apps/backend/src/models/questionTypes/QuestionRegistry.ts b/apps/backend/src/models/questionTypes/QuestionRegistry.ts index cd5fd12fdb..6fa12325fd 100644 --- a/apps/backend/src/models/questionTypes/QuestionRegistry.ts +++ b/apps/backend/src/models/questionTypes/QuestionRegistry.ts @@ -1,6 +1,7 @@ import { logger } from '@user-office-software/duo-logger'; import { GraphQLError } from 'graphql'; import { Knex } from 'knex'; +import * as Yup from 'yup'; import { AnswerInput } from '../../resolvers/mutations/AnswerTopicMutation'; import { QuestionFilterInput } from '../../resolvers/queries/ProposalsQuery'; @@ -127,8 +128,9 @@ export interface Question { /** * Performs validation on the config submitted before persisting data into the database */ - readonly validateConfig?: (config: any) => boolean; + readonly validateConfig?: (config: any) => void; + readonly customYupSchema?: Yup.ObjectSchema; /** * Question can contain configuration, e.g. isRequired, maxValue, etc, * This function returns configuration for newly created questions From aaa48f7e1a6bb1a97d45ad03aedecc0eafcb31f6 Mon Sep 17 00:00:00 2001 From: zachary Date: Mon, 27 Jul 2026 16:15:03 +0100 Subject: [PATCH 4/5] add comment --- apps/backend/src/models/questionTypes/QuestionRegistry.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/backend/src/models/questionTypes/QuestionRegistry.ts b/apps/backend/src/models/questionTypes/QuestionRegistry.ts index 6fa12325fd..bd6177eedb 100644 --- a/apps/backend/src/models/questionTypes/QuestionRegistry.ts +++ b/apps/backend/src/models/questionTypes/QuestionRegistry.ts @@ -130,6 +130,9 @@ export interface Question { */ readonly validateConfig?: (config: any) => void; + /** + * Defines the Yup schema used to validate the config before writing to database + */ readonly customYupSchema?: Yup.ObjectSchema; /** * Question can contain configuration, e.g. isRequired, maxValue, etc, From 8572604b64985e528dc819310ed75e4374163f2b Mon Sep 17 00:00:00 2001 From: zachary Date: Mon, 27 Jul 2026 16:52:10 +0100 Subject: [PATCH 5/5] add testing --- .../postgres/TemplateDataSource.spec.ts | 137 ++++++++++++++++++ .../postgres/TemplateDataSource.ts | 5 +- 2 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 apps/backend/src/datasources/postgres/TemplateDataSource.spec.ts diff --git a/apps/backend/src/datasources/postgres/TemplateDataSource.spec.ts b/apps/backend/src/datasources/postgres/TemplateDataSource.spec.ts new file mode 100644 index 0000000000..35c960a4ba --- /dev/null +++ b/apps/backend/src/datasources/postgres/TemplateDataSource.spec.ts @@ -0,0 +1,137 @@ +import * as Yup from 'yup'; + +import database from './database'; +import { validateConfigBeforeWrite } from './TemplateDataSource'; +import { getQuestionDefinition } from '../../models/questionTypes/QuestionRegistry'; + +jest.mock('../database'); +jest.mock('../QuestionRegistry'); + +describe('validateConfigBeforeWrite', () => { + it('should validate a valid config without throwing', async () => { + (database as any).mockReturnValue({ + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + first: jest.fn().mockResolvedValue({ + data_type: 'instrument_picker', + }), + }); + + (getQuestionDefinition as jest.Mock).mockReturnValue({ + customYupSchema: Yup.object({ + variant: Yup.string().required(), + instruments: Yup.array() + .of( + Yup.object({ + id: Yup.number().required(), + name: Yup.string().required(), + }) + ) + .required(), + isMultipleSelect: Yup.boolean().required(), + requestTime: Yup.boolean().required(), + }), + validateConfig: jest.fn(), + }); + + const config = JSON.stringify({ + small_label: '', + required: true, + tooltip: '', + readPermissions: [], + variant: 'radio', + instruments: [], + isMultipleSelect: false, + requestTime: false, + }); + + await expect( + validateConfigBeforeWrite(config, 'question-1') + ).resolves.toBeUndefined(); + }); + + it('Should throw an error when an extra field is supplied', async () => { + (database as any).mockReturnValue({ + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + first: jest.fn().mockResolvedValue({ + data_type: 'instrument_picker', + }), + }); + + (getQuestionDefinition as jest.Mock).mockReturnValue({ + customYupSchema: Yup.object({ + variant: Yup.string().required(), + instruments: Yup.array() + .of( + Yup.object({ + id: Yup.number().required(), + name: Yup.string().required(), + }) + ) + .required(), + isMultipleSelect: Yup.boolean().required(), + requestTime: Yup.boolean().required(), + }), + validateConfig: jest.fn(), + }); + + const config = JSON.stringify({ + small_label: '', + required: true, + tooltip: '', + readPermissions: [], + variant: 'radio', + instruments: [], + isMultipleSelect: false, + requestTime: false, + extraField: true, + }); + + await expect( + validateConfigBeforeWrite(config, 'question-1') + ).rejects.toThrow(); + }); + + it('Should throw an error when missing a field', async () => { + (database as any).mockReturnValue({ + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + first: jest.fn().mockResolvedValue({ + data_type: 'instrument_picker', + }), + }); + + (getQuestionDefinition as jest.Mock).mockReturnValue({ + customYupSchema: Yup.object({ + variant: Yup.string().required(), + instruments: Yup.array() + .of( + Yup.object({ + id: Yup.number().required(), + name: Yup.string().required(), + }) + ) + .required(), + isMultipleSelect: Yup.boolean().required(), + requestTime: Yup.boolean().required(), + }), + validateConfig: jest.fn(), + }); + + const config = JSON.stringify({ + small_label: '', + required: true, + tooltip: '', + readPermissions: [], + variant: 'radio', + instruments: [], + isMultipleSelect: false, + extraField: true, + }); + + await expect( + validateConfigBeforeWrite(config, 'question-1') + ).rejects.toThrow(); + }); +}); diff --git a/apps/backend/src/datasources/postgres/TemplateDataSource.ts b/apps/backend/src/datasources/postgres/TemplateDataSource.ts index aad936d55a..80944999ed 100644 --- a/apps/backend/src/datasources/postgres/TemplateDataSource.ts +++ b/apps/backend/src/datasources/postgres/TemplateDataSource.ts @@ -1288,7 +1288,10 @@ export default class PostgresTemplateDataSource implements TemplateDataSource { } } -async function validateConfigBeforeWrite(newConfig: any, questionId: string) { +export async function validateConfigBeforeWrite( + newConfig: any, + questionId: string +) { let newConfigObject; try { newConfigObject = JSON.parse(newConfig);