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 185b53b8a4..80944999ed 100644 --- a/apps/backend/src/datasources/postgres/TemplateDataSource.ts +++ b/apps/backend/src/datasources/postgres/TemplateDataSource.ts @@ -1,7 +1,7 @@ import { logger } from '@user-office-software/duo-logger'; import { GraphQLError } from 'graphql'; +import * as Yup from 'yup'; -import { createConfig } from '../../models/questionTypes/QuestionRegistry'; import { ComparisonStatus, ConflictResolutionStrategy, @@ -59,6 +59,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 +684,8 @@ export default class PostgresTemplateDataSource implements TemplateDataSource { dependenciesOperator, } = args; + await validateConfigBeforeWrite(config, questionId); + await database('templates_has_questions') .update({ config: config, @@ -1283,3 +1287,47 @@ export default class PostgresTemplateDataSource implements TemplateDataSource { return newTemplate; } } + +export 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 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'); + } + + 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 f4d5734a89..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'; @@ -125,4 +126,26 @@ export const instrumentPickerDefinition: Question = } ); }, + validateConfig: (config) => { + if (config?.variant === 'radio' && config?.isMultipleSelect) { + throw new GraphQLError( + 'Radio instrument pickers cannot have isMultipleSelect as 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 52f5a80eb1..bd6177eedb 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'; @@ -124,6 +125,15 @@ export interface Question { value: any ) => Promise; + /** + * Performs validation on the config submitted before persisting data into the database + */ + 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, * This function returns configuration for newly created questions