Skip to content

Commit

Permalink
Do not allow to set incorrect value into our complex questions fix #6629
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewtelnov committed Aug 4, 2023
1 parent ae71220 commit 184a697
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { ItemValue } from "./itemvalue";
import { IElement, IFindElement, IProgressInfo, ISurvey } from "./base-interfaces";
import { ExpressionRunner } from "./conditions";
import { surveyLocalization } from "./surveyStrings";
import { ConsoleWarnings } from "./console-warnings";

interface IExpressionRunnerInfo {
onExecute: (obj: Base, res: any) => void;
Expand Down Expand Up @@ -504,8 +505,7 @@ export class Base {
if (!this.isDisposedValue) {
this.setPropertyValueCoreHandler(propertiesHash, name, val);
} else {
// eslint-disable-next-line no-console
console.warn("Attempt to set property '" + name + "' of a disposed object '" + this.getType() + "'");
ConsoleWarnings.disposedObjectChangedProperty(name, this.getType());
}
}
else propertiesHash[name] = val;
Expand Down
13 changes: 13 additions & 0 deletions src/console-warnings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export class ConsoleWarnings {
public static disposedObjectChangedProperty(propName: string, objType: string): void {
ConsoleWarnings.warn("Attempt to set property '" + propName + "' of a disposed object '" + objType + "'");
}
public static inCorrectQuestionValue(questionName: string, val: any): void {
const valStr = JSON.stringify(val, null, 3);
ConsoleWarnings.warn("Try to set incorrect value into question. Question name: '" + questionName + "', value: " + valStr);
}
public static warn(text: string): void {
// eslint-disable-next-line no-console
console.warn(text);
}
}
11 changes: 7 additions & 4 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class Helpers {
*/
public static isValueEmpty(value: any) {
if (Array.isArray(value) && value.length === 0) return true;
if (!!value && typeof value === "object" && value.constructor === Object) {
if (!!value && Helpers.isValueObject(value) && value.constructor === Object) {
for (var key in value) {
if (!Helpers.isValueEmpty(value[key])) return false;
}
Expand Down Expand Up @@ -133,8 +133,8 @@ export class Helpers {
if ((y === true || y === false) && typeof x == "string") {
return y.toString() === x.toLocaleLowerCase();
}
if (!(x instanceof Object) && !(y instanceof Object)) return x == y;
if (!(x instanceof Object) || !(y instanceof Object)) return false;
if (!Helpers.isValueObject(x) && !Helpers.isValueObject(y)) return x == y;
if (!Helpers.isValueObject(x) || !Helpers.isValueObject(y)) return false;
if (x["equals"]) return x.equals(y);
if (!!x.toJSON && !!y.toJSON && !!x.getType && !!y.getType) {
if (x.isDiposed || y.isDiposed) return false;
Expand Down Expand Up @@ -173,7 +173,7 @@ export class Helpers {
}
return res;
}
if (!!value && value instanceof Object && !(value instanceof Date)) {
if (!!value && Helpers.isValueObject(value) && !(value instanceof Date)) {
return JSON.parse(JSON.stringify(value));
}
return value;
Expand All @@ -194,6 +194,9 @@ export class Helpers {
!isNaN(value)
);
}
public static isValueObject(val: any): boolean {
return val instanceof Object;
}
public static isNumber(value: any): boolean {
return !isNaN(this.getNumber(value));
}
Expand Down
8 changes: 8 additions & 0 deletions src/question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { SurveyError } from "./survey-error";
import { CssClassBuilder } from "./utils/cssClassBuilder";
import { getElementWidth, increaseHeightByContent, isContainerVisible } from "./utils/utils";
import { PopupModel } from "./popup";
import { ConsoleWarnings } from "./console-warnings";

export interface IConditionObject {
name: string;
Expand Down Expand Up @@ -1905,13 +1906,20 @@ export class Question extends SurveyElement<Question>
protected allowNotifyValueChanged = true;
protected setNewValue(newValue: any): void {
if(this.isNewValueEqualsToValue(newValue)) return;
if(!this.isValueEmpty(newValue) && !this.isNewValueCorrect(newValue)) {
ConsoleWarnings.inCorrectQuestionValue(this.name, newValue);
return;
}
var oldAnswered = this.isAnswered;
this.setNewValueInData(newValue);
this.allowNotifyValueChanged && this.onValueChanged();
if (this.isAnswered != oldAnswered) {
this.updateQuestionCss();
}
}
protected isNewValueCorrect(val: any): boolean {
return true;
}
protected isNewValueEqualsToValue(newValue: any): boolean {
const val = this.value;
if(!this.isTwoValueEquals(newValue, val)) return false;
Expand Down
3 changes: 3 additions & 0 deletions src/question_matrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@ export class QuestionMatrixModel
super.endLoadingFromJson();
this.rows = this.sortVisibleRows(this.rows);
}
protected isNewValueCorrect(val: any): boolean {
return Helpers.isValueObject(val);
}
protected processRowsOnSet(newRows: Array<any>) {
return this.sortVisibleRows(newRows);
}
Expand Down
6 changes: 3 additions & 3 deletions src/question_matrixdropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ItemValue } from "./itemvalue";
import { QuestionFactory } from "./questionfactory";
import { LocalizableString } from "./localizablestring";
import { IProgressInfo } from "./base-interfaces";
import { Helpers } from "./helpers";

export class MatrixDropdownRowModel extends MatrixDropdownRowModelBase {
private item: ItemValue;
Expand Down Expand Up @@ -119,9 +120,8 @@ export class QuestionMatrixDropdownModel extends QuestionMatrixDropdownModelBase
for (var i = 0; i < this.rows.length; i++) res.push(i);
return res;
}
protected setNewValue(newValue: any): void {
if(!!newValue && typeof newValue !== "object") return;
super.setNewValue(newValue);
protected isNewValueCorrect(val: any): boolean {
return Helpers.isValueObject(val);
}
public clearIncorrectValues() {
var val = this.value;
Expand Down
3 changes: 3 additions & 0 deletions src/question_matrixdynamic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ export class QuestionMatrixDynamicModel extends QuestionMatrixDropdownModelBase
for (var i = val.length; i < this.minRowCount; i++) val.push({});
return val;
}
protected isNewValueCorrect(val: any): boolean {
return Array.isArray(val);
}
protected setDefaultValue() {
if (
this.isValueEmpty(this.defaultRowValue) ||
Expand Down
12 changes: 6 additions & 6 deletions src/question_multipletext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,7 @@ export class QuestionMultipleTextModel extends Question
}
return null;
}
public addConditionObjectsByContext(
objects: Array<IConditionObject>,
context: any
) {
public addConditionObjectsByContext(objects: Array<IConditionObject>, context: any): void {
for (var i = 0; i < this.items.length; i++) {
var item = this.items[i];
objects.push({
Expand Down Expand Up @@ -451,6 +448,9 @@ export class QuestionMultipleTextModel extends Question
this.items[i].localeChanged();
}
}
protected isNewValueCorrect(val: any): boolean {
return Helpers.isValueObject(val);
}
supportGoNextPageAutomatic(): boolean {
for (var i = 0; i < this.items.length; i++) {
if (this.items[i].isEmpty()) return false;
Expand Down Expand Up @@ -496,14 +496,14 @@ export class QuestionMultipleTextModel extends Question
return rows;
}
private isMultipleItemValueChanging = false;
protected onValueChanged() {
protected onValueChanged(): void {
super.onValueChanged();
this.onItemValueChanged();
}
protected createTextItem(name: string, title: string): MultipleTextItemModel {
return new MultipleTextItemModel(name, title);
}
protected onItemValueChanged() {
protected onItemValueChanged(): void {
if (this.isMultipleItemValueChanging) return;
for (var i = 0; i < this.items.length; i++) {
var itemValue = null;
Expand Down
3 changes: 3 additions & 0 deletions src/question_paneldynamic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1861,6 +1861,9 @@ export class QuestionPanelDynamicModel extends Question
this.rebuildPanels();
}
}
protected isNewValueCorrect(val: any): boolean {
return Array.isArray(val);
}
//IQuestionPanelDynamicData
getItemIndex(item: ISurveyData): number {
var res = this.items.indexOf(item);
Expand Down
7 changes: 3 additions & 4 deletions src/question_signaturepad.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import SignaturePad from "signature_pad";
import { CssClassBuilder } from "./utils/cssClassBuilder";
import { SurveyModel } from "./survey";
import { ISurveyImpl } from "./base-interfaces";
import { ConsoleWarnings } from "./console-warnings";

var defaultWidth = 300;
var defaultHeight = 200;
Expand Down Expand Up @@ -250,14 +251,12 @@ export class QuestionSignaturePadModel extends Question {
super.endLoadingFromJson();
//todo: need to remove this code
if(this.signatureWidth === 300 && !!this.width && typeof this.width === "number" && this.width) {
// eslint-disable-next-line no-console
console.warn("Use signatureWidth property to set width for the signature pad");
ConsoleWarnings.warn("Use signatureWidth property to set width for the signature pad");
this.signatureWidth = this.width;
this.width = undefined;
}
if(this.signatureHeight === 200 && !!this.height) {
// eslint-disable-next-line no-console
console.warn("Use signatureHeight property to set width for the signature pad");
ConsoleWarnings.warn("Use signatureHeight property to set width for the signature pad");
this.signatureHeight = this.height;
this.height = undefined;
}
Expand Down
71 changes: 68 additions & 3 deletions tests/surveyquestiontests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { QuestionMatrixDropdownModelBase } from "../src/question_matrixdropdownb
import { PanelModel } from "../src/panel";
import { Helpers } from "../src/helpers";
import { CustomWidgetCollection } from "../src/questionCustomWidgets";
import { PageModel } from "../src/page";
import { ConsoleWarnings } from "../src/console-warnings";
import { StylesManager } from "../src/stylesmanager";

export default QUnit.module("Survey_Questions");
Expand Down Expand Up @@ -445,10 +445,11 @@ QUnit.test("Matrix Question: visible rows", function (assert) {
});
QUnit.test("Matrix Question: get/set values for empty rows", function (assert) {
var matrix = new QuestionMatrixModel("q1");
matrix.rows = ["row1"];
matrix.columns = ["col1", "col2"];
assert.equal(matrix.value, undefined, "the matrix initial value");
matrix.value = "col1";
assert.equal(matrix.value, "col1", "the matrix value changed correctly");
matrix.value = { row1: "col1" };
assert.deepEqual(matrix.value, { row1: "col1" }, "the matrix value changed correctly");
});
QUnit.test("Matrix Question: get/set values for two rows", function (assert) {
var matrix = new QuestionMatrixModel("q1");
Expand Down Expand Up @@ -6989,4 +6990,68 @@ QUnit.test("numeric validator, use custom text, bug#6588", function (assert) {
assert.equal(q1.errors[0].getText(), "Enter only numbers", "Customer error");
assert.equal(q2.errors.length, 1, "One error, #2");
assert.equal(q2.errors[0].getText(), "The value should be numeric.", "Default error");
});
QUnit.test("Try to set incorrect values, bug#6629", function (assert) {
const oldFunc = ConsoleWarnings.inCorrectQuestionValue;
const incorrectCalls: Array<string> = [];
ConsoleWarnings.inCorrectQuestionValue = (questionName: string, val: any): void => {
incorrectCalls.push(questionName);
};
const survey = new SurveyModel({
elements: [
{
type: "matrix",
name: "q1",
columns: [1, 2],
rows: [1, 2],
defaultValue: "a"
},
{
type: "matrixdropdown",
name: "q2",
columns: [{ cellType: "text", name: "col1" }],
rows: [1, 2],
defaultValue: "b"
},
{
type: "matrixdynamic",
name: "q3",
columns: [{ cellType: "text", name: "col1" }],
panelCount: 1,
defaultValue: "c"
},
{
type: "paneldynamic",
name: "q4",
templateElements: [{ type: "text", name: "q4_q1" }],
defaultValue: "d"
},
{
type: "multipletext",
name: "q5",
items: [{ name: "item1" }],
defaultValue: "e"
},
{
type: "checkbox",
name: "q6",
choices: [1, 2],
defaultValue: "f"
}
] });
const q1 = survey.getQuestionByName("q1");
const q2 = survey.getQuestionByName("q2");
const q3 = survey.getQuestionByName("q3");
const q4 = survey.getQuestionByName("q4");
const q5 = survey.getQuestionByName("q5");
const q6 = survey.getQuestionByName("q6");
assert.equal(incorrectCalls.length, 5, "Incorrect calls 5 times");
assert.deepEqual(incorrectCalls, ["q1", "q2", "q3", "q4", "q5"]);
assert.equal(q1.isEmpty(), true, "Can't set 'a' to matrix");
assert.equal(q2.isEmpty(), true, "Can't set 'b' to matrixdropdown");
assert.equal(q3.isEmpty(), true, "Can't set 'c' to matrixdynamic");
assert.equal(q4.isEmpty(), true, "Can't set 'd' to paneldynamic");
assert.equal(q5.isEmpty(), true, "Can't set 'e' to multipletext");
assert.deepEqual(q6.value, ["f"], "Convert to array");
ConsoleWarnings.inCorrectQuestionValue = oldFunc;
});

0 comments on commit 184a697

Please sign in to comment.