From 95b6297c62dbec6b656a38bda9da098c6e901343 Mon Sep 17 00:00:00 2001 From: tshuli <63710093+tshuli@users.noreply.github.com> Date: Wed, 9 Jun 2021 10:35:59 +0800 Subject: [PATCH] chore: remove redundant ValidationOption object properties for short text, long text and number fields (#2040) * chore: script to verify that db is consistent * chore: remove references to customMin, customMax, Range for short text, long text and number fields in backend * chore: remove references to customMin, customMax for short text, long text and number fields in frontend * chore: add customMin and customMax virtuals for backward compatibility * chore: script to unset customMin and customMax for text and number fields * chore: remove Number casting for customVal * chore: add todos for virtuals * chore: correct types for virtuals * refactor: combine TextValidationOptionsSchema * chore: add elemMatch for field type at start of aggregation * chore: add check for non-text/number fields * style: lint * chore: refine filter for update (doesn't affect outcome) * chore: improve query syntax * chore: rename folder to common --- scripts/20210517_validation-object/script.js | 369 ++++++++++++++++++ .../common/textValidationOptionsSchema.ts | 39 ++ src/app/models/field/longTextField.ts | 28 +- src/app/models/field/numberField.ts | 58 ++- src/app/models/field/shortTextField.ts | 27 +- .../__tests__/number-validation.spec.ts | 42 +- .../__tests__/text-validator.spec.ts | 136 +------ .../validators/numberValidator.ts | 8 +- .../validators/textValidator.ts | 14 +- .../modules/forms/admin/constants/covid19.js | 26 -- .../edit-fields-modal.client.controller.js | 13 - .../admin/views/edit-fields.client.modal.html | 1 - .../field-number.client.view.html | 12 +- .../field-textarea.client.view.html | 16 +- .../field-textfield.client.view.html | 12 +- .../modules/forms/viewmodels/Fields/MixIns.js | 2 - src/types/field/baseField.ts | 1 - src/types/field/longTextField.ts | 14 +- src/types/field/numberField.ts | 3 - src/types/field/shortTextField.ts | 14 +- src/types/field/utils/textField.ts | 15 + src/types/field/utils/virtuals.ts | 9 + .../backend/helpers/generate-form-data.ts | 4 - 23 files changed, 549 insertions(+), 314 deletions(-) create mode 100644 scripts/20210517_validation-object/script.js create mode 100644 src/app/models/field/common/textValidationOptionsSchema.ts create mode 100644 src/types/field/utils/textField.ts create mode 100644 src/types/field/utils/virtuals.ts diff --git a/scripts/20210517_validation-object/script.js b/scripts/20210517_validation-object/script.js new file mode 100644 index 0000000000..5f1483bffc --- /dev/null +++ b/scripts/20210517_validation-object/script.js @@ -0,0 +1,369 @@ +/* eslint-disable */ +// Scripts for #408 + +/** + * Part 1: Verify that the database is internally consistent today + * for all short text, long text and number fields. + * Expect 0 records for all scripts + */ + +// a. customVal === customMin when selectedValidation === 'Minimum' +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + customVal: '$form_fields.ValidationOptions.customVal', + customMin: '$form_fields.ValidationOptions.customMin', + customMax: '$form_fields.ValidationOptions.customMax', + }, + }, + { + $match: { + $and: [ + { + selectedValidation: 'Minimum', + }, + { + $expr: { $ne: ['$customMin', '$customVal'] }, + }, + ], + }, + }, +]) + +// b. customVal === customMax when selectedValidation === 'Maximum' +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + customVal: '$form_fields.ValidationOptions.customVal', + customMin: '$form_fields.ValidationOptions.customMin', + customMax: '$form_fields.ValidationOptions.customMax', + }, + }, + { + $match: { + $and: [ + { + selectedValidation: 'Maximum', + }, + { + $expr: { $ne: ['$customMax', '$customVal'] }, + }, + ], + }, + }, +]) + +// c. customVal === customMin === customMax when selectedValidation === 'Exact' +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + customVal: '$form_fields.ValidationOptions.customVal', + customMin: '$form_fields.ValidationOptions.customMin', + customMax: '$form_fields.ValidationOptions.customMax', + }, + }, + { + $match: { + $and: [ + { + selectedValidation: 'Exact', + }, + { + $or: [ + { + $expr: { $ne: ['$customMax', '$customVal'] }, + }, + { + $expr: { $ne: ['$customMin', '$customVal'] }, + }, + ], + }, + ], + }, + }, +]) + +// d. selectedValidation === 'Range' should not exist +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + }, + }, + { + $match: { + selectedValidation: 'Range', + }, + }, +]) + +/** + * Part 2: Unset form_fields.ValidationOptions.customMin and + * form_fields.ValidationOptions.customMax + * for all short text, long text and number fields. + */ + +// ********** BEFORE ********** + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMin +// Should match number of updated records later + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMin': { $exists: true }, + }, + }, +}) + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMax +// Should match number of updated records later + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMax': { $exists: true }, + }, + }, +}) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMin': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMax': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// ********** UPDATE ********** + +// Remove customMin + +db.forms.update( + { + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMin': { $exists: true }, + }, + }, + }, + { $unset: { 'form_fields.$[elem].ValidationOptions.customMin': '' } }, + { + arrayFilters: [ + { + 'elem.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + 'elem.ValidationOptions.customMin': { $exists: true }, + }, + ], + multi: true, + }, +) + +// Remove customMax + +db.forms.update( + { + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMax': { $exists: true }, + }, + }, + }, + { $unset: { 'form_fields.$[elem].ValidationOptions.customMax': '' } }, + { + arrayFilters: [ + { + 'elem.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + 'elem.ValidationOptions.customMax': { $exists: true }, + }, + ], + multi: true, + }, +) + +// ********** AFTER ********** + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect 0 after running update + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMin': { $exists: true }, + }, + }, +}) + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect 0 after running update + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMax': { $exists: true }, + }, + }, +}) + +// Count number of form FIELDS with (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect 0 after running update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMin': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS with (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect 0 after running update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMax': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMin': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMax': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) diff --git a/src/app/models/field/common/textValidationOptionsSchema.ts b/src/app/models/field/common/textValidationOptionsSchema.ts new file mode 100644 index 0000000000..582da4d87c --- /dev/null +++ b/src/app/models/field/common/textValidationOptionsSchema.ts @@ -0,0 +1,39 @@ +import { Schema } from 'mongoose' + +import { TextSelectedValidation } from '../../../../types/field/baseField' +import { TextValidationOptions } from '../../../../types/field/utils/textField' +import { WithCustomMinMax } from '../../../../types/field/utils/virtuals' + +export const TextValidationOptionsSchema = new Schema< + WithCustomMinMax +>( + { + customVal: { + type: Number, + }, + selectedValidation: { + type: String, + enum: [...Object.values(TextSelectedValidation), null], + }, + }, + { + // TODO: Remove virtuals (#2039) + toJSON: { + virtuals: true, + }, + }, +) + +// Virtuals to allow for backwards compatibility after customMin and customMax were removed as part of #408 +// TODO: Remove virtuals (#2039) +TextValidationOptionsSchema.virtual('customMin').get(function ( + this: TextValidationOptions, +) { + return this.customVal +}) + +TextValidationOptionsSchema.virtual('customMax').get(function ( + this: TextValidationOptions, +) { + return this.customVal +}) diff --git a/src/app/models/field/longTextField.ts b/src/app/models/field/longTextField.ts index a893fe5861..ff289ab134 100644 --- a/src/app/models/field/longTextField.ts +++ b/src/app/models/field/longTextField.ts @@ -1,29 +1,21 @@ import { Schema } from 'mongoose' import { ILongTextFieldSchema } from '../../../types' -import { TextSelectedValidation } from '../../../types/field/baseField' + +import { TextValidationOptionsSchema } from './common/textValidationOptionsSchema' const createLongTextFieldSchema = () => { - return new Schema({ + const LongTextFieldSchema = new Schema({ ValidationOptions: { - customMax: { - type: Number, - default: null, - }, - customMin: { - type: Number, - default: null, - }, - customVal: { - type: Number, - default: null, - }, - selectedValidation: { - type: String, - enum: [...Object.values(TextSelectedValidation), null], - default: null, + type: TextValidationOptionsSchema, + default: { + // Defaults are defined here because subdocument paths are undefined by default, and Mongoose does not apply subdocument defaults unless you set the subdocument path to a non-nullish value (see https://mongoosejs.com/docs/subdocs.html) + customVal: null, + selectedValidation: null, }, }, }) + + return LongTextFieldSchema } export default createLongTextFieldSchema diff --git a/src/app/models/field/numberField.ts b/src/app/models/field/numberField.ts index 759fb45ebe..8ed27f99d1 100644 --- a/src/app/models/field/numberField.ts +++ b/src/app/models/field/numberField.ts @@ -1,31 +1,61 @@ import { Schema } from 'mongoose' -import { INumberFieldSchema, NumberSelectedValidation } from '../../../types' +import { + INumberFieldSchema, + NumberSelectedValidation, + NumberValidationOptions, +} from '../../../types' +import { WithCustomMinMax } from '../../../types/field/utils/virtuals' import { MyInfoSchema } from './baseField' const createNumberFieldSchema = () => { - return new Schema({ - myInfo: MyInfoSchema, - ValidationOptions: { - customMax: { - type: Number, - default: null, - }, - customMin: { - type: Number, - default: null, - }, + const ValidationOptionsSchema = new Schema< + WithCustomMinMax + >( + { customVal: { type: Number, - default: null, }, selectedValidation: { type: String, enum: [...Object.values(NumberSelectedValidation), null], - default: null, + }, + }, + { + // TODO: Remove virtuals (#2039) + toJSON: { + virtuals: true, + }, + }, + ) + + // Virtuals to allow for backwards compatibility after customMin and customMax were removed as part of #408 + // TODO: Remove virtuals (#2039) + ValidationOptionsSchema.virtual('customMin').get(function ( + this: NumberValidationOptions, + ) { + return this.customVal + }) + + ValidationOptionsSchema.virtual('customMax').get(function ( + this: NumberValidationOptions, + ) { + return this.customVal + }) + + const NumberFieldSchema = new Schema({ + myInfo: MyInfoSchema, + ValidationOptions: { + type: ValidationOptionsSchema, + default: { + // Defaults are defined here because subdocument paths are undefined by default, and Mongoose does not apply subdocument defaults unless you set the subdocument path to a non-nullish value (see https://mongoosejs.com/docs/subdocs.html) + customVal: null, + selectedValidation: null, }, }, }) + + return NumberFieldSchema } export default createNumberFieldSchema diff --git a/src/app/models/field/shortTextField.ts b/src/app/models/field/shortTextField.ts index 73eb99cdd8..3d46ff939d 100644 --- a/src/app/models/field/shortTextField.ts +++ b/src/app/models/field/shortTextField.ts @@ -1,30 +1,19 @@ import { Schema } from 'mongoose' import { IShortTextFieldSchema } from '../../../types' -import { TextSelectedValidation } from '../../../types/field/baseField' +import { TextValidationOptionsSchema } from './common/textValidationOptionsSchema' import { MyInfoSchema } from './baseField' const createShortTextFieldSchema = () => { - return new Schema({ + const ShortTextFieldSchema = new Schema({ myInfo: MyInfoSchema, ValidationOptions: { - customMax: { - type: Number, - default: null, - }, - customMin: { - type: Number, - default: null, - }, - customVal: { - type: Number, - default: null, - }, - selectedValidation: { - type: String, - enum: [...Object.values(TextSelectedValidation), null], - default: null, + type: TextValidationOptionsSchema, + default: { + // Defaults are defined here because subdocument paths are undefined by default, and Mongoose does not apply subdocument defaults unless you set the subdocument path to a non-nullish value (see https://mongoosejs.com/docs/subdocs.html) + customVal: null, + selectedValidation: null, }, }, allowPrefill: { @@ -33,6 +22,8 @@ const createShortTextFieldSchema = () => { default: false, }, }) + + return ShortTextFieldSchema } export default createShortTextFieldSchema diff --git a/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts b/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts index 5df3bd3bec..be26319617 100644 --- a/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts +++ b/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts @@ -12,9 +12,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: 2, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -30,9 +28,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: 2, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -47,9 +43,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: 2, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -66,9 +60,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Min, - customMin: 2, - customMax: null, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -83,9 +75,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Min, - customMin: 2, - customMax: null, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -100,8 +90,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Exact, - customMin: null, - customMax: null, customVal: 2, }, }) @@ -117,8 +105,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Exact, - customMin: null, - customMax: null, customVal: 2, }, }) @@ -136,8 +122,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: null, customVal: null, }, }) @@ -153,8 +137,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Min, - customMin: null, - customMax: null, customVal: null, }, }) @@ -170,8 +152,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Exact, - customMin: null, - customMax: null, customVal: null, }, }) @@ -187,8 +167,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -204,8 +182,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, required: false, @@ -222,8 +198,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -239,8 +213,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -258,8 +230,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -274,8 +244,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) diff --git a/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts b/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts index ab60ce4a27..6d8894e91a 100644 --- a/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts +++ b/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts @@ -56,13 +56,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMin if selectedValidation is Exact', () => { + it('should disallow fewer characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -75,13 +73,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMin if selectedValidation is Exact', () => { + it('should disallow more characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -95,51 +91,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMax if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.ShortText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.ShortText, { - answer: 'fewer', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow more characters than customMax if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.ShortText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.ShortText, { - answer: 'more than 10 chars', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow fewer characters than customMin if selectedValidation is Minimum', () => { + it('should disallow fewer characters than customVal if selectedValidation is Minimum', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Minimum, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -152,13 +108,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMax if selectedValidation is Maximum', () => { + it('should disallow more characters than customVal if selectedValidation is Maximum', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Maximum, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -174,8 +128,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -198,8 +150,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -217,8 +167,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, required: false, @@ -235,8 +183,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -252,8 +198,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -267,51 +211,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMin if selectedValidation is Exact', () => { + it('should disallow fewer characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.LongText, { - answer: 'less', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow more characters than customMin if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.LongText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.LongText, { - answer: 'more than 10 chars', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow fewer characters than customMax if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.LongText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -324,13 +228,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMax if selectedValidation is Exact', () => { + it('should disallow more characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -343,13 +245,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMin if selectedValidation is Minimum', () => { + it('should disallow fewer characters than customVal if selectedValidation is Minimum', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Minimum, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -362,13 +262,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMax if selectedValidation is Maximum', () => { + it('should disallow more characters than customVal if selectedValidation is Maximum', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Maximum, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -384,8 +282,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) diff --git a/src/app/utils/field-validation/validators/numberValidator.ts b/src/app/utils/field-validation/validators/numberValidator.ts index 23307d9fa1..6568e5f0f9 100644 --- a/src/app/utils/field-validation/validators/numberValidator.ts +++ b/src/app/utils/field-validation/validators/numberValidator.ts @@ -29,8 +29,8 @@ const numberFormatValidator: NumberValidator = (response) => { const minLengthValidator: NumberValidatorConstructor = (numberField) => (response) => { const { answer } = response - const { customMin } = numberField.ValidationOptions - return !customMin || answer.length >= customMin + const { customVal } = numberField.ValidationOptions + return !customVal || answer.length >= customVal ? right(response) : left(`NumberValidator:\t answer is shorter than custom minimum length`) } @@ -42,8 +42,8 @@ const minLengthValidator: NumberValidatorConstructor = const maxLengthValidator: NumberValidatorConstructor = (numberField) => (response) => { const { answer } = response - const { customMax } = numberField.ValidationOptions - return !customMax || answer.length <= customMax + const { customVal } = numberField.ValidationOptions + return !customVal || answer.length <= customVal ? right(response) : left(`NumberValidator:\t answer is longer than custom maximum length`) } diff --git a/src/app/utils/field-validation/validators/textValidator.ts b/src/app/utils/field-validation/validators/textValidator.ts index b58aea807c..83d5ef8dcb 100644 --- a/src/app/utils/field-validation/validators/textValidator.ts +++ b/src/app/utils/field-validation/validators/textValidator.ts @@ -19,8 +19,7 @@ type TextFieldValidatorConstructor = ( */ const minLengthValidator: TextFieldValidatorConstructor = (textField) => (response) => { - const { customMin } = textField.ValidationOptions - const min = customMin !== null ? Number(customMin) : null + const { customVal: min } = textField.ValidationOptions if (min === null) return right(response) return response.answer.length >= min ? right(response) @@ -33,8 +32,7 @@ const minLengthValidator: TextFieldValidatorConstructor = */ const maxLengthValidator: TextFieldValidatorConstructor = (textField) => (response) => { - const { customMax } = textField.ValidationOptions - const max = customMax !== null ? Number(customMax) : null + const { customVal: max } = textField.ValidationOptions if (max === null) return right(response) return response.answer.length <= max ? right(response) @@ -49,13 +47,7 @@ const maxLengthValidator: TextFieldValidatorConstructor = */ const exactLengthValidator: TextFieldValidatorConstructor = (textField) => (response) => { - const { customMin, customMax } = textField.ValidationOptions - const exact = - customMin !== null - ? Number(customMin) - : customMax !== null - ? Number(customMax) - : null + const { customVal: exact } = textField.ValidationOptions if (exact === null) return right(response) return response.answer.length === exact ? right(response) diff --git a/src/public/modules/forms/admin/constants/covid19.js b/src/public/modules/forms/admin/constants/covid19.js index 29a39dca5c..e3a6c39fa6 100644 --- a/src/public/modules/forms/admin/constants/covid19.js +++ b/src/public/modules/forms/admin/constants/covid19.js @@ -99,8 +99,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -115,8 +113,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -184,8 +180,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -246,8 +240,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -261,8 +253,6 @@ const templates = [ }, { ValidationOptions: { - customMax: 6, - customMin: 6, customVal: 6, selectedValidation: 'Exact', }, @@ -323,8 +313,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -357,8 +345,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -543,8 +529,6 @@ const templates = [ form_fields: [ { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -624,8 +608,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -692,8 +674,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -735,8 +715,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -750,8 +728,6 @@ const templates = [ }, { ValidationOptions: { - customMax: 6, - customMin: 6, customVal: 6, selectedValidation: 'Exact', }, @@ -897,8 +873,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, diff --git a/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js b/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js index a7e076bbf7..997c3ed742 100644 --- a/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js @@ -227,8 +227,6 @@ function EditFieldsModalController( vm.clearCustomValidation = function (field) { field.ValidationOptions = { selectedValidation: null, - customMax: null, - customMin: null, customVal: null, } } @@ -236,22 +234,11 @@ function EditFieldsModalController( let temp = field.ValidationOptions.selectedValidation // Reset all custom validation params to null, keep selected validation option field.ValidationOptions = { - customMax: null, - customMin: null, customVal: null, selectedValidation: temp, } } - vm.changeFxn = function (field) { - let selected = field.ValidationOptions.selectedValidation - let val = field.ValidationOptions.customVal - field.ValidationOptions.customMax = - selected === 'Maximum' || selected === 'Exact' ? val : null - field.ValidationOptions.customMin = - selected === 'Minimum' || selected === 'Exact' ? val : null - } - vm.setFocus = function () { angular.element('#customValInputBox').focus() } diff --git a/src/public/modules/forms/admin/views/edit-fields.client.modal.html b/src/public/modules/forms/admin/views/edit-fields.client.modal.html index 1d0472b426..70aadb680b 100644 --- a/src/public/modules/forms/admin/views/edit-fields.client.modal.html +++ b/src/public/modules/forms/admin/views/edit-fields.client.modal.html @@ -1081,7 +1081,6 @@

class="input-custom input-medium" placeholder="No. of Characters" ng-disabled="!vm.field.ValidationOptions.selectedValidation" - ng-change="vm.changeFxn(vm.field)" /> diff --git a/src/public/modules/forms/base/componentViews/field-number.client.view.html b/src/public/modules/forms/base/componentViews/field-number.client.view.html index a6ba22465b..27abe19ed0 100644 --- a/src/public/modules/forms/base/componentViews/field-number.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-number.client.view.html @@ -31,8 +31,8 @@ ng-required="vm.field.required" ng-disabled="vm.field.disabled" ng-pattern="/^\d*$/" - ng-maxlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : '' }}" - ng-minlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" ng-attr-placeholder="{{ vm.placeholder }}" @@ -62,17 +62,17 @@
- Minimum {{vm.field.ValidationOptions.customMin}} characters + Minimum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMin}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Maximum {{vm.field.ValidationOptions.customMax}} characters + Maximum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMax}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
diff --git a/src/public/modules/forms/base/componentViews/field-textarea.client.view.html b/src/public/modules/forms/base/componentViews/field-textarea.client.view.html index 59d7e05f2d..118a0ec194 100644 --- a/src/public/modules/forms/base/componentViews/field-textarea.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-textarea.client.view.html @@ -28,8 +28,8 @@ name="{{ vm.field._id || 'defaultID' }}" class="input-custom input-large" ng-model="vm.field.fieldValue" - ng-maxlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : ''}}" - ng-minlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" ng-model-options="{ allowInvalid: true }" ng-required="vm.field.required" ng-disabled="vm.field.disabled" @@ -58,18 +58,18 @@
- Minimum {{vm.field.ValidationOptions.customMin}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMin}}) + Minimum {{vm.field.ValidationOptions.customVal}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Maximum {{vm.field.ValidationOptions.customMax}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMax}}) + Maximum {{vm.field.ValidationOptions.customVal}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Must be exactly {{vm.field.ValidationOptions.customMax}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMax}}) + Must be exactly {{vm.field.ValidationOptions.customVal}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}})
diff --git a/src/public/modules/forms/base/componentViews/field-textfield.client.view.html b/src/public/modules/forms/base/componentViews/field-textfield.client.view.html index da39ac5557..6d898ff599 100644 --- a/src/public/modules/forms/base/componentViews/field-textfield.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-textfield.client.view.html @@ -30,8 +30,8 @@ ng-disabled="vm.field.disabled" ng-pattern="vm.pattern" ng-class="vm.field.allowPrefill && vm.field.isPrefilled && 'prefill-highlight'" - ng-maxlength="{{vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : ''}}" - ng-minlength="{{vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" ng-attr-placeholder="{{ vm.placeholder }}" @@ -54,17 +54,17 @@
- Minimum {{vm.field.ValidationOptions.customMin}} characters + Minimum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMin}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Maximum {{vm.field.ValidationOptions.customMax}} characters + Maximum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMax}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
diff --git a/src/public/modules/forms/viewmodels/Fields/MixIns.js b/src/public/modules/forms/viewmodels/Fields/MixIns.js index 3b2e0ffd0f..aba9eb7554 100644 --- a/src/public/modules/forms/viewmodels/Fields/MixIns.js +++ b/src/public/modules/forms/viewmodels/Fields/MixIns.js @@ -32,8 +32,6 @@ const CustomValidation = (Base) => getDefaultBasicData() { const fieldData = super.getDefaultBasicData() fieldData.ValidationOptions = { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, } diff --git a/src/types/field/baseField.ts b/src/types/field/baseField.ts index 7301e622c4..a66bfe4dcf 100644 --- a/src/types/field/baseField.ts +++ b/src/types/field/baseField.ts @@ -54,5 +54,4 @@ export enum TextSelectedValidation { Maximum = 'Maximum', Minimum = 'Minimum', Exact = 'Exact', - Range = 'Range', // TODO(#408) - questionable value } diff --git a/src/types/field/longTextField.ts b/src/types/field/longTextField.ts index 6957939890..59b8eac9f5 100644 --- a/src/types/field/longTextField.ts +++ b/src/types/field/longTextField.ts @@ -1,14 +1,6 @@ -import { IField, IFieldSchema, TextSelectedValidation } from './baseField' +import { ITextField } from './utils/textField' +import { IFieldSchema } from './baseField' -export type LongTextValidationOptions = { - customMax: number | null - customMin: number | null - customVal: number | null - selectedValidation: TextSelectedValidation | null -} - -export interface ILongTextField extends IField { - ValidationOptions: LongTextValidationOptions -} +export type ILongTextField = ITextField export interface ILongTextFieldSchema extends ILongTextField, IFieldSchema {} diff --git a/src/types/field/numberField.ts b/src/types/field/numberField.ts index 1fe8cf7f1f..f8c4ca908c 100644 --- a/src/types/field/numberField.ts +++ b/src/types/field/numberField.ts @@ -4,12 +4,9 @@ export enum NumberSelectedValidation { Max = 'Maximum', Min = 'Minimum', Exact = 'Exact', - Range = 'Range', } export type NumberValidationOptions = { - customMax: number | null - customMin: number | null customVal: number | null selectedValidation: NumberSelectedValidation | null } diff --git a/src/types/field/shortTextField.ts b/src/types/field/shortTextField.ts index a1d5e4dcfb..b2ab928a3a 100644 --- a/src/types/field/shortTextField.ts +++ b/src/types/field/shortTextField.ts @@ -1,15 +1,7 @@ -import { IField, IFieldSchema, TextSelectedValidation } from './baseField' +import { ITextField } from './utils/textField' +import { IFieldSchema } from './baseField' -export type ShortTextValidationOptions = { - customMax: number | null - customMin: number | null - customVal: number | null - selectedValidation: TextSelectedValidation | null -} - -export interface IShortTextField extends IField { - ValidationOptions: ShortTextValidationOptions -} +export type IShortTextField = ITextField export interface IShortTextFieldSchema extends IShortTextField, IFieldSchema { // Prefill flag diff --git a/src/types/field/utils/textField.ts b/src/types/field/utils/textField.ts new file mode 100644 index 0000000000..8678c70d1f --- /dev/null +++ b/src/types/field/utils/textField.ts @@ -0,0 +1,15 @@ +import { IField, IFieldSchema, TextSelectedValidation } from '../baseField' + +/** + * Utility types for short and long text field + */ +export type TextValidationOptions = { + customVal: number | null + selectedValidation: TextSelectedValidation | null +} + +export interface ITextField extends IField { + ValidationOptions: TextValidationOptions +} + +export interface ITextFieldSchema extends ITextField, IFieldSchema {} diff --git a/src/types/field/utils/virtuals.ts b/src/types/field/utils/virtuals.ts new file mode 100644 index 0000000000..fe670d2485 --- /dev/null +++ b/src/types/field/utils/virtuals.ts @@ -0,0 +1,9 @@ +import { Document } from 'mongoose' + +// Types for virtuals for backwards compatibility after customMin and customMax were removed as part of #408 +// TODO: Remove virtual type (#2039) +export type WithCustomMinMax = T & + Document & { + customMin: number | null + customMax: number | null + } diff --git a/tests/unit/backend/helpers/generate-form-data.ts b/tests/unit/backend/helpers/generate-form-data.ts index 2fcc97e109..fc3f4e2649 100644 --- a/tests/unit/backend/helpers/generate-form-data.ts +++ b/tests/unit/backend/helpers/generate-form-data.ts @@ -376,8 +376,6 @@ export const generateTableShortTextColumn = ( required: true, _id: new ObjectId().toHexString(), ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -390,8 +388,6 @@ export const generateTableShortTextColumn = ( required: true, _id: new ObjectId().toHexString(), ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, },