From 416f18dd251cac9d26a35d2c9e0b9625c87202b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Thu, 30 Sep 2021 12:23:16 -0700 Subject: [PATCH] Update the model when returning cached choices --- .../javarosa/core/model/ItemsetBinding.java | 37 ++++++++++++++++--- .../core/model/SelectOneChoiceFilterTest.java | 1 + 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/javarosa/core/model/ItemsetBinding.java b/src/main/java/org/javarosa/core/model/ItemsetBinding.java index 97daa5a2e..dbf2122f9 100644 --- a/src/main/java/org/javarosa/core/model/ItemsetBinding.java +++ b/src/main/java/org/javarosa/core/model/ItemsetBinding.java @@ -92,6 +92,8 @@ public List getChoices(FormDef formDef, TreeReference curQRef) { // Return cached list if possible if (cachedFilteredChoiceList != null && allTriggerRefsBound && Objects.equals(currentTriggerValues, cachedTriggerValues) && Objects.equals(currentRandomizeSeed, cachedRandomizeSeed)) { + updateQuestionAnswerInModel(formDef, curQRef); + return randomize && cachedRandomizeSeed == null ? shuffle(cachedFilteredChoiceList) : cachedFilteredChoiceList; } @@ -116,10 +118,10 @@ public List getChoices(FormDef formDef, TreeReference curQRef) { Map selectChoicesForAnswer = initializeAnswerMap(formDef, curQRef); - List choices = new ArrayList<>(); + List filteredChoiceList = new ArrayList<>(); for (int i = 0; i < filteredItemReferences.size(); i++) { SelectChoice choice = getChoiceForTreeReference(formDef, formInstance, i, filteredItemReferences.get(i)); - choices.add(choice); + filteredChoiceList.add(choice); if (selectChoicesForAnswer != null && selectChoicesForAnswer.containsKey(choice.getValue())) { // Keys with values that don't get set here will have null values and must be filtered out of the answer. selectChoicesForAnswer.put(choice.getValue(), choice); @@ -128,13 +130,13 @@ public List getChoices(FormDef formDef, TreeReference curQRef) { updateQuestionAnswerInModel(formDef, curQRef, selectChoicesForAnswer); - cachedFilteredChoiceList = randomize ? shuffle(choices, currentRandomizeSeed) : choices; + cachedFilteredChoiceList = randomize ? shuffle(filteredChoiceList, currentRandomizeSeed) : filteredChoiceList; // TODO: write a test that fails if this is removed. It looks like a no-op because it's not accessing the shuffled collection. if (randomize) { // Match indices to new positions - for (int i = 0; i < choices.size(); i++) - choices.get(i).setIndex(i); + for (int i = 0; i < filteredChoiceList.size(); i++) + filteredChoiceList.get(i).setIndex(i); } //init localization @@ -202,7 +204,7 @@ private SelectChoice getChoiceForTreeReference(FormDef formDef, DataInstance for } /** - * Build a map with keys for each value in the current answer, each mapped to null. + * Builds a map with keys for each value in the current answer, each mapped to null. * * When we iterate over the new filtered choice list, we will update the values in this map. Keys with null values * after this process will be removed from the answer. We will also use this map to bind selection(s) in the IAnswerData @@ -226,6 +228,12 @@ private Map initializeAnswerMap(FormDef formDef, TreeRefer return selectChoicesForAnswer; } + /** + * Removes any answers that aren't in the filtered choice list. Binds the answer to its choice(s) so that the answer + * label can be retrieved. + * + * SIDE EFFECTS: mutates the instance node's value (TreeElement.value, type {@link SelectOneData} or {@link MultipleItemsData}) + */ private void updateQuestionAnswerInModel(FormDef formDef, TreeReference curQRef, Map selectChoicesForAnswer) { IAnswerData originalValue = formDef.getMainInstance().resolveReference(curQRef).getValue(); @@ -244,6 +252,23 @@ private void updateQuestionAnswerInModel(FormDef formDef, TreeReference curQRef, } } + /** + * Updates the model using the previously-cached choice list. + * + * @see #updateQuestionAnswerInModel(FormDef, TreeReference, Map) for details and side-effects + */ + private void updateQuestionAnswerInModel(FormDef formDef, TreeReference curQRef) { + Map selectChoicesForAnswer = initializeAnswerMap(formDef, curQRef); + if (selectChoicesForAnswer != null) { + for (SelectChoice choice : cachedFilteredChoiceList) { + if (selectChoicesForAnswer.containsKey(choice.getValue())) { + selectChoicesForAnswer.put(choice.getValue(), choice); + } + } + } + updateQuestionAnswerInModel(formDef, curQRef, selectChoicesForAnswer); + } + /** * @param selections an answer to a multiple selection question * @param selectChoicesForAnswer maps each value that could be in @{code selections} to a SelectChoice if it should be bound diff --git a/src/test/java/org/javarosa/core/model/SelectOneChoiceFilterTest.java b/src/test/java/org/javarosa/core/model/SelectOneChoiceFilterTest.java index 817ff0deb..f6556d72c 100644 --- a/src/test/java/org/javarosa/core/model/SelectOneChoiceFilterTest.java +++ b/src/test/java/org/javarosa/core/model/SelectOneChoiceFilterTest.java @@ -131,6 +131,7 @@ public void clearingValueAtLevel1_ShouldClearValuesAtLevels2And3() { // If we set level2 to "aa", form validation passes. Currently, clearing a choice only updates filter expressions // that directly depend on it. With this form, we could force clearing the third level when the first level is cleared // by making the level3 filter expression in the form definition reference level1 AND level2. + scenario.answer("/data/level1", "b"); scenario.answer("/data/level2", "bb"); validate = scenario.getValidationOutcome();