feat: add validation logic to config of questions

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();
+  });
+});

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;
+}

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<DataType.INSTRUMENT_PICKER> =
         }
       );
     },
+    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(),
+    }),
   };

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<T extends DataType> {
     value: any
   ) => Promise<boolean>;
 
+  /**
+   * 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<any>;
   /**
    * Question can contain configuration, e.g. isRequired, maxValue, etc,
    * This function returns configuration for newly created questions