Skip to content

Commit

Permalink
Do not update choices on loading data from web if the data is the same
Browse files Browse the repository at this point in the history
…fix #7674 (#7675)
  • Loading branch information
andrewtelnov authored Jan 15, 2024
1 parent b56d772 commit 4169cdf
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 15 deletions.
10 changes: 10 additions & 0 deletions src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,16 @@ export class Base {
}
return Helpers.isValueEmpty(value);
}
public equals(obj: Base): boolean {
if(!obj) return false;
if (this.isDisposed || obj.isDisposed) return false;
if(this.getType() != obj.getType()) return false;
return this.equalsCore(obj);
}
protected equalsCore(obj: Base): boolean {
if((<any>this).name !== (<any>obj).name) return false;
return Helpers.isTwoValueEquals(this.toJSON(), obj.toJSON(), false, true, false);
}
protected trimValue(value: any): any {
if (!!value && (typeof value === "string" || value instanceof String))
return value.trim();
Expand Down
12 changes: 3 additions & 9 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,10 @@ export class Helpers {
}
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.isDisposed || y.isDisposed) return false;
if (x.getType() !== y.getType()) return false;
if (!!x.name && x.name !== y.name) return false;
return this.isTwoValueEquals(x.toJSON(), y.toJSON(), ignoreOrder, caseSensitive, trimStrings);
}
if (Array.isArray(x) && Array.isArray(y))
if (x["equals"] && y["equals"]) return x.equals(y);
if (Array.isArray(x) && Array.isArray(y)) {
return Helpers.isArraysEqual(x, y, ignoreOrder, caseSensitive, trimStrings);
if(!!x.equalsTo && y.equalsTo) return x.equalsTo(y);
}

for (var p in x) {
if (!x.hasOwnProperty(p)) continue;
Expand Down
14 changes: 12 additions & 2 deletions src/question_baseselect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,7 @@ export class QuestionSelectBase extends Question {
this.readOnly = true;
}
}
protected onLoadChoicesFromUrl(array: Array<ItemValue>) {
protected onLoadChoicesFromUrl(array: Array<ItemValue>): void {
if (this.enableOnLoadingChoices) {
this.readOnly = false;
}
Expand All @@ -1341,7 +1341,6 @@ export class QuestionSelectBase extends Question {
if (this.isValueEmpty(this.cachedValueForUrlRequests)) {
this.cachedValueForUrlRequests = this.value;
}
this.isFirstLoadChoicesFromUrl = false;
var cachedValues = this.createCachedValueForUrlRequests(
this.cachedValueForUrlRequests,
checkCachedValuesOnExisting
Expand All @@ -1355,6 +1354,17 @@ export class QuestionSelectBase extends Question {
newChoices[i].locOwner = this;
}
}
this.setChoicesFromUrl(newChoices, errors, cachedValues);
}
private canAvoidSettChoicesFromUrl(newChoices: Array<ItemValue>): boolean {
if(this.isFirstLoadChoicesFromUrl) return false;
const chocesAreEmpty = !newChoices || Array.isArray(newChoices) && newChoices.length === 0;
if(chocesAreEmpty && !this.isEmpty()) return false;
return Helpers.isTwoValueEquals(this.choicesFromUrl, newChoices);
}
private setChoicesFromUrl(newChoices: Array<ItemValue>, errors: Array<any>, cachedValues: any): void {
if(this.canAvoidSettChoicesFromUrl(newChoices)) return;
this.isFirstLoadChoicesFromUrl = false;
this.choicesFromUrl = newChoices;
this.filterItems();
this.onVisibleChoicesChanged();
Expand Down
2 changes: 1 addition & 1 deletion src/survey-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export class SurveyError {
public text: string = null,
protected errorOwner: ISurveyErrorOwner = null
) {}
public equalsTo(error: SurveyError): boolean {
public equals(error: SurveyError): boolean {
if(!error || !error.getErrorType) return false;
if(this.getErrorType() !== error.getErrorType()) return false;
return this.text === error.text && this.visible === error.visible;
Expand Down
4 changes: 4 additions & 0 deletions src/survey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,7 @@ export class SurveyModel extends SurveyElementCore
return this.locale;
}
public locStrsChanged(): void {
if(this.isClearingUnsedValues) return;
super.locStrsChanged();
if (!this.currentPage) return;
if (this.isDesignMode) {
Expand Down Expand Up @@ -6219,12 +6220,15 @@ export class SurveyModel extends SurveyElementCore
var pos = Math.max(pos1, pos2);
return name.substring(0, pos);
}
private isClearingUnsedValues: boolean;
private clearUnusedValues() {
this.isClearingUnsedValues = true;
var questions = this.getAllQuestions();
for (var i: number = 0; i < questions.length; i++) {
questions[i].clearUnusedValues();
}
this.clearInvisibleQuestionValues();
this.isClearingUnsedValues = false;
}
hasVisibleQuestionByValueName(valueName: string): boolean {
var questions = this.getQuestionsByValueName(valueName);
Expand Down
36 changes: 33 additions & 3 deletions tests/choicesRestfultests.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Base, ArrayChanges } from "../src/base";
import { ITextProcessor } from "../src/base-interfaces";
import { IQuestion, ITextProcessor } from "../src/base-interfaces";
import { SurveyModel } from "../src/survey";
import { Question } from "../src/question";
import { ChoicesRestful } from "../src/choicesRestful";
Expand Down Expand Up @@ -770,10 +770,40 @@ QUnit.test("Clear value on getting empty array, bug #6251", function(assert) {
question.choicesByUrl.error = new SurveyError("Empty request");
question["onLoadChoicesFromUrl"]([]);
assert.equal(question.isEmpty(), true, "value is empty");
assert.equal(question.selectedItem, null, "selectedItem is null");
const isSelected = !!question.selectedItem;
assert.equal(isSelected, false, "selectedItem is null");
assert.equal(question.errors.length, 1, "It shows error on empty result");
});

QUnit.test("Do not call loadedChoicesFromServer on setting same items", function(assert) {
var survey = new SurveyModel();
let counter = 0;
survey.loadedChoicesFromServer = (question: IQuestion) => {
counter ++;
};
survey.addNewPage("1");
var question = new QuestionDropdownModelTester("q1");
question.choicesByUrl.url = "{state}";
survey.pages[0].addQuestion(question);
question.hasItemsCallbackDelay = true;
question.onSurveyLoad();
question["onLoadChoicesFromUrl"]([]);
assert.equal(counter, 1, "#1");
question["onLoadChoicesFromUrl"]([]);
question["onLoadChoicesFromUrl"]([]);
assert.equal(counter, 1, "#2");
question["onLoadChoicesFromUrl"]([new ItemValue("CA"), new ItemValue("TX")]);
assert.equal(counter, 2, "#3");
question["onLoadChoicesFromUrl"]([new ItemValue("CA"), new ItemValue("TX")]);
question["onLoadChoicesFromUrl"]([new ItemValue("CA"), new ItemValue("TX")]);
assert.equal(counter, 2, "#4");
question["onLoadChoicesFromUrl"]([]);
assert.equal(counter, 3, "#5");
question["onLoadChoicesFromUrl"]([]);
assert.equal(counter, 3, "#6");
survey.setValue("q1", "CA");
question["onLoadChoicesFromUrl"]([]);
assert.equal(counter, 4, "#7");
});
QUnit.test(
"Set value before loading data + storeOthersAsComment, bug #1089",
function(assert) {
Expand Down
18 changes: 18 additions & 0 deletions tests/helperstests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { Base } from "../src/base";
import { property } from "../src/jsonobject";
import { settings } from "../src/settings";
import { SurveyError } from "../src/survey-error";
import { QuestionTextModel } from "../src/question_text";
import { QuestionCommentModel } from "../src/question_comment";

export default QUnit.module("Helpers");

Expand Down Expand Up @@ -509,4 +511,20 @@ QUnit.test("isValueObject", function(assert) {
assert.equal(Helpers.isValueObject([1], true), false, "[1], exclude array");
assert.equal(Helpers.isValueObject({ a: "abc" }), true, "{ a: 'abc' }");
assert.equal(Helpers.isValueObject({ a: "abc" }, true), true, "{ a: 'abc' }, exclude array");
});
QUnit.test("base.equals", function(assert) {
const q1 = new QuestionTextModel("q1");
q1.title = "title1";
const q2 = new QuestionTextModel("q1");
q2.title = "title2";
const q3 = new QuestionTextModel("q1");
q3.title = "title1";
const q4 = new QuestionTextModel("q2");
q4.title = "title1";
const q5 = new QuestionCommentModel("q1");
q5.title = "title1";
assert.equal(Helpers.isTwoValueEquals(q1, q2), false, "#1");
assert.equal(Helpers.isTwoValueEquals(q1, q3), true, "#2");
assert.equal(Helpers.isTwoValueEquals(q1, q4), false, "#3");
assert.equal(Helpers.isTwoValueEquals(q1, q5), false, "#4");
});

0 comments on commit 4169cdf

Please sign in to comment.