diff --git a/src/main/java/org/javarosa/core/model/FormDef.java b/src/main/java/org/javarosa/core/model/FormDef.java index e34485118..e1534bed4 100644 --- a/src/main/java/org/javarosa/core/model/FormDef.java +++ b/src/main/java/org/javarosa/core/model/FormDef.java @@ -532,7 +532,15 @@ public void processResultOfAction(TreeReference refSetByAction, String event) { public boolean isRepeatRelevant(TreeReference repeatRef) { TreeElement repeatNode = mainInstance.resolveReference(repeatRef); - return repeatNode == null || repeatNode.isRelevant(); + + if (repeatNode != null) { + return repeatNode.isRelevant(); + } else { + // We are dealing with a repeat that doesn't exist yet so get + // relevance from the template + TreeElement template = mainInstance.getTemplate(repeatRef); + return template.isRelevant(); + } } public boolean canCreateRepeat(TreeReference repeatRef, FormIndex repeatIndex) { diff --git a/src/main/java/org/javarosa/core/model/TriggerableDag.java b/src/main/java/org/javarosa/core/model/TriggerableDag.java index 2c4f3e76c..3e7fc3f70 100644 --- a/src/main/java/org/javarosa/core/model/TriggerableDag.java +++ b/src/main/java/org/javarosa/core/model/TriggerableDag.java @@ -528,19 +528,30 @@ private void evaluateTriggerable(FormInstance mainInstance, EvaluationContext ev // the repeat are only triggered for the changed instance. This is important for performance. TreeReference contextRef = affectsAllRepeatInstances ? toTrigger.getContext() : toTrigger.getContext().contextualize(changedRef); - List evaluationResults = new ArrayList<>(0); // In general, expansion will have no effect. It only makes a difference if affectsAllRepeatInstances is true in // which case the triggerable will be applied for every repeat instance. - for (TreeReference qualified : evalContext.expandReference(contextRef)) + List qualifiedReferences = evalContext.expandReference(contextRef); + + // We want to update the repeat's template as well + TreeElement template = mainInstance.getTemplate(contextRef); + if (template != null) { + TreeReference templateRef = template.getRef(); + qualifiedReferences.add(templateRef); + } + + List evaluationResults = new ArrayList<>(0); + for (TreeReference qualified : qualifiedReferences) { try { // apply evaluates the expression in the given context and saves the result in the contextualized target(s). evaluationResults.addAll(toTrigger.apply(mainInstance, new EvaluationContext(evalContext, qualified), qualified)); } catch (Exception e) { throw new RuntimeException("Error evaluating field '" + contextRef.getNameLast() + "' (" + qualified + "): " + e.getMessage(), e); } + } - if (evaluationResults.size() > 0) + if (evaluationResults.size() > 0) { accessor.getEventNotifier().publishEvent(new Event(toTrigger.isCondition() ? "Condition" : "Recalculate", evaluationResults)); + } } private void evaluateChildrenTriggerables(FormInstance mainInstance, EvaluationContext evalContext, TreeElement newNode, boolean createdOrDeleted, Set alreadyEvaluated) { diff --git a/src/main/java/org/javarosa/form/api/FormEntryModel.java b/src/main/java/org/javarosa/form/api/FormEntryModel.java index 9d426be7d..d34708c78 100644 --- a/src/main/java/org/javarosa/form/api/FormEntryModel.java +++ b/src/main/java/org/javarosa/form/api/FormEntryModel.java @@ -75,19 +75,19 @@ public FormEntryModel(FormDef form) { * * @param form * @param repeatStructure The structure of repeats (the repeat signals which should - * be sent during form entry) + * be sent during form entry) * @throws IllegalArgumentException If repeatStructure is not valid */ public FormEntryModel(FormDef form, int repeatStructure) { this.form = form; - if(repeatStructure != REPEAT_STRUCTURE_LINEAR && repeatStructure != REPEAT_STRUCTURE_NON_LINEAR) { - throw new IllegalArgumentException(repeatStructure +": does not correspond to a valid repeat structure"); + if (repeatStructure != REPEAT_STRUCTURE_LINEAR && repeatStructure != REPEAT_STRUCTURE_NON_LINEAR) { + throw new IllegalArgumentException(repeatStructure + ": does not correspond to a valid repeat structure"); } //We need to see if there are any guessed repeat counts in the form, which prevents //us from being able to use the new repeat style //Unfortunately this is probably (A) slow and (B) might overflow the stack. It's not the only //recursive walk of the form, though, so (B) isn't really relevant - if(repeatStructure == REPEAT_STRUCTURE_NON_LINEAR && containsRepeatGuesses(form)) { + if (repeatStructure == REPEAT_STRUCTURE_NON_LINEAR && containsRepeatGuesses(form)) { repeatStructure = REPEAT_STRUCTURE_LINEAR; } this.repeatStructure = repeatStructure; @@ -131,7 +131,6 @@ public int getEvent(FormIndex index) { } /** - * * @param index * @return */ @@ -158,25 +157,23 @@ public String getFormTitle() { /** - * * @param index * @return Returns the FormEntryPrompt for the specified FormIndex if the - * index represents a question. + * index represents a question. */ public FormEntryPrompt getQuestionPrompt(FormIndex index) { if (form.getChild(index) instanceof QuestionDef) { return new FormEntryPrompt(form, index); } else { throw new RuntimeException( - "Invalid query for Question prompt. Non-Question object at the form index"); + "Invalid query for Question prompt. Non-Question object at the form index"); } } /** - * * @return Returns the FormEntryPrompt for the current FormIndex if the - * index represents a question. + * index represents a question. */ public FormEntryPrompt getQuestionPrompt() { return getQuestionPrompt(currentFormIndex); @@ -189,20 +186,19 @@ public FormEntryPrompt getQuestionPrompt() { * * @param index * @return Returns the FormEntryCaption for the given FormIndex if is not a - * question. + * question. */ public FormEntryCaption getCaptionPrompt(FormIndex index) { return new FormEntryCaption(form, index); } - /** * When you have a non-question event, a CaptionPrompt will have all the * information needed to display to the user. * * @return Returns the FormEntryCaption for the current FormIndex if is not - * a question. + * a question. */ public FormEntryCaption getCaptionPrompt() { return getCaptionPrompt(currentFormIndex); @@ -210,9 +206,8 @@ public FormEntryCaption getCaptionPrompt() { /** - * * @return an array of Strings of the current langauges. Null if there are - * none. + * none. */ public String[] getLanguages() { if (form.getLocalizer() != null) { @@ -224,7 +219,7 @@ public String[] getLanguages() { /** * Not yet implemented - * + *

* Should get the number of completed questions to this point. */ public int getCompletedRelevantQuestionCount() { @@ -235,7 +230,7 @@ public int getCompletedRelevantQuestionCount() { /** * Not yet implemented - * + *

* Should get the total possible questions given the current path through the form. */ public int getTotalRelevantQuestionCount() { @@ -252,7 +247,6 @@ public int getNumQuestions() { /** - * * @return Returns the current FormIndex referenced by the FormEntryModel. */ public FormIndex getFormIndex() { @@ -268,7 +262,6 @@ protected void setLanguage(String language) { /** - * * @return Returns the currently selected language. */ public String getLanguage() { @@ -292,7 +285,6 @@ public void setQuestionIndex(FormIndex index) { /** - * * @return */ public FormDef getForm() { @@ -352,7 +344,7 @@ public boolean isIndexReadonly(FormIndex index) { TreeReference ref = form.getChildInstanceRef(index); boolean isAskNewRepeat = (getEvent(index) == FormEntryController.EVENT_PROMPT_NEW_REPEAT || - getEvent(index) == FormEntryController.EVENT_REPEAT_JUNCTURE); + getEvent(index) == FormEntryController.EVENT_REPEAT_JUNCTURE); if (isAskNewRepeat) { return false; @@ -383,19 +375,20 @@ public boolean isIndexRelevant(FormIndex index) { boolean isAskNewRepeat = (getEvent(index) == FormEntryController.EVENT_PROMPT_NEW_REPEAT); boolean isRepeatJuncture = (getEvent(index) == FormEntryController.EVENT_REPEAT_JUNCTURE); - boolean relevant; if (isAskNewRepeat) { - relevant = form.isRepeatRelevant(ref) && form.canCreateRepeat(ref, index); + if (!form.canCreateRepeat(ref, index)) { + return false; + } + + return form.isRepeatRelevant(ref); + } else if (isRepeatJuncture) { //repeat junctures are still relevant if no new repeat can be created; that option //is simply missing from the menu - } else if (isRepeatJuncture) { - relevant = form.isRepeatRelevant(ref); + return form.isRepeatRelevant(ref); } else { TreeElement node = form.getMainInstance().resolveReference(ref); - relevant = node != null && node.isRelevant(); + return node != null && node.isRelevant(); } - - return relevant; } @@ -414,17 +407,17 @@ public boolean isIndexRelevant() { * For the current index: Checks whether the index represents a node which * should exist given a non-interactive repeat, along with a count for that * repeat which is beneath the dynamic level specified. - * + *

* If this index does represent such a node, the new model for the repeat is * created behind the scenes and the index for the initial question is * returned. - * + *

* Note: This method will not prevent the addition of new repeat elements in * the interface, it will merely use the xforms repeat hint to create new * nodes that are assumed to exist * * @param index The index to be evaluated as to whether the underlying model is - * hinted to exist + * hinted to exist */ private void createModelIfNecessary(FormIndex index) { if (index.isInForm()) { @@ -443,12 +436,12 @@ private void createModelIfNecessary(FormIndex index) { if (element == null) { if (index.getTerminal().getInstanceIndex() < fullcount) { - try { - getForm().createNewRepeat(index); - } catch (InvalidReferenceException ire) { - logger.error("Error", ire); - throw new RuntimeException("Invalid Reference while creating new repeat!" + ire.getMessage()); - } + try { + getForm().createNewRepeat(index); + } catch (InvalidReferenceException ire) { + logger.error("Error", ire); + throw new RuntimeException("Invalid Reference while creating new repeat!" + ire.getMessage()); + } } } } @@ -465,8 +458,8 @@ public boolean isIndexCompoundContainer() { public boolean isIndexCompoundContainer(FormIndex index) { FormEntryCaption caption = getCaptionPrompt(index); return getEvent(index) == FormEntryController.EVENT_GROUP && - caption.getAppearanceHint() != null && - caption.getAppearanceHint().toLowerCase(Locale.ENGLISH).equals("full"); + caption.getAppearanceHint() != null && + caption.getAppearanceHint().toLowerCase(Locale.ENGLISH).equals("full"); } public boolean isIndexCompoundElement() { @@ -475,16 +468,16 @@ public boolean isIndexCompoundElement() { public boolean isIndexCompoundElement(FormIndex index) { //Can't be a subquestion if it's not even a question! - if(getEvent(index) != FormEntryController.EVENT_QUESTION) { + if (getEvent(index) != FormEntryController.EVENT_QUESTION) { return false; } //get the set of nested groups that this question is in. FormEntryCaption[] captions = getCaptionHierarchy(index); - for(FormEntryCaption caption : captions) { + for (FormEntryCaption caption : captions) { //If one of this question's parents is a group, this question is inside of it. - if(isIndexCompoundContainer(caption.getIndex())) { + if (isIndexCompoundContainer(caption.getIndex())) { return true; } } @@ -497,16 +490,16 @@ public FormIndex[] getCompoundIndices() { public FormIndex[] getCompoundIndices(FormIndex container) { //ArrayLists are a no-go for J2ME - List indices = new ArrayList(); + List indices = new ArrayList(); FormIndex walker = incrementIndex(container); - while(FormIndex.isSubElement(container, walker)) { - if(isIndexRelevant(walker)) { + while (FormIndex.isSubElement(container, walker)) { + if (isIndexRelevant(walker)) { indices.add(walker); } walker = incrementIndex(walker); } FormIndex[] array = new FormIndex[indices.size()]; - for(int i = 0 ; i < indices.size() ; ++i) { + for (int i = 0; i < indices.size(); ++i) { array[i] = indices.get(i); } return array; @@ -548,7 +541,7 @@ public FormIndex incrementIndex(FormIndex index, boolean descend) { } } - private void incrementHelper(List indexes, List multiplicities, List elements, boolean descend) { + private void incrementHelper(List indexes, List multiplicities, List elements, boolean descend) { int i = indexes.size() - 1; boolean exitRepeat = false; //if exiting a repetition? (i.e., go to next repetition instead of one level up) @@ -571,7 +564,7 @@ private void incrementHelper(List indexes, List multiplicities } else { - if (form.getMainInstance().resolveReference(form.getChildInstanceRef(elements, multiplicities)) == null) { + if (form.getMainInstance().resolveReference(form.getChildInstanceRef(elements, multiplicities)) == null) { descend = false; // repeat instance does not exist; do not descend into it exitRepeat = true; } @@ -588,7 +581,7 @@ private void incrementHelper(List indexes, List multiplicities elements.add((i == -1 ? form : elements.get(i)).getChild(0)); if (repeatStructure == REPEAT_STRUCTURE_NON_LINEAR) { - if (elements.get(elements.size() - 1) instanceof GroupDef && ((GroupDef)elements.get(elements.size() - 1)).getRepeat()) { + if (elements.get(elements.size() - 1) instanceof GroupDef && ((GroupDef) elements.get(elements.size() - 1)).getRepeat()) { multiplicities.set(multiplicities.size() - 1, TreeReference.INDEX_REPEAT_JUNCTURE); } } @@ -634,7 +627,7 @@ private void incrementHelper(List indexes, List multiplicities elements.set(i, parent.getChild(curIndex + 1)); if (repeatStructure == REPEAT_STRUCTURE_NON_LINEAR) { - if (elements.get(elements.size() - 1) instanceof GroupDef && ((GroupDef)elements.get(elements.size() - 1)).getRepeat()) { + if (elements.get(elements.size() - 1) instanceof GroupDef && ((GroupDef) elements.get(elements.size() - 1)).getRepeat()) { multiplicities.set(multiplicities.size() - 1, TreeReference.INDEX_REPEAT_JUNCTURE); } } @@ -676,8 +669,8 @@ private void decrementHelper(List indexes, List multiplicities int curMult = multiplicities.get(i); if (repeatStructure == REPEAT_STRUCTURE_NON_LINEAR && - elements.get(elements.size() - 1) instanceof GroupDef && ((GroupDef) elements.get(elements.size() - 1)).getRepeat() && - multiplicities.get(multiplicities.size() - 1) != TreeReference.INDEX_REPEAT_JUNCTURE) { + elements.get(elements.size() - 1) instanceof GroupDef && ((GroupDef) elements.get(elements.size() - 1)).getRepeat() && + multiplicities.get(multiplicities.size() - 1) != TreeReference.INDEX_REPEAT_JUNCTURE) { multiplicities.set(i, TreeReference.INDEX_REPEAT_JUNCTURE); return; } else if (repeatStructure != REPEAT_STRUCTURE_NON_LINEAR && curMult > 0) { @@ -701,7 +694,7 @@ private void decrementHelper(List indexes, List multiplicities IFormElement element = (i < 0 ? form : elements.get(i)); while (!(element instanceof QuestionDef)) { - if(element.getChildren() == null || element.getChildren().size() == 0) { + if (element.getChildren() == null || element.getChildren().size() == 0) { //if there are no children we just return the current index (the group itself) return; } @@ -751,17 +744,21 @@ private boolean setRepeatNextMultiplicity(List elements, List children = parent.getChildren(); - if(children == null) { return false; } - for (IFormElement child : children) { - if(containsRepeatGuesses(child)) {return true;} + if (children == null) { + return false; + } + for (IFormElement child : children) { + if (containsRepeatGuesses(child)) { + return true; + } } return false; } diff --git a/src/test/java/org/javarosa/core/model/RepeatTest.java b/src/test/java/org/javarosa/core/model/RepeatTest.java new file mode 100644 index 000000000..0f36c8485 --- /dev/null +++ b/src/test/java/org/javarosa/core/model/RepeatTest.java @@ -0,0 +1,76 @@ +package org.javarosa.core.model; + +import org.javarosa.core.test.Scenario; +import org.javarosa.form.api.FormEntryController; +import org.junit.Test; + +import static org.hamcrest.Matchers.is; +import static org.javarosa.core.util.BindBuilderXFormsElement.bind; +import static org.javarosa.core.util.XFormsElement.body; +import static org.javarosa.core.util.XFormsElement.head; +import static org.javarosa.core.util.XFormsElement.html; +import static org.javarosa.core.util.XFormsElement.input; +import static org.javarosa.core.util.XFormsElement.item; +import static org.javarosa.core.util.XFormsElement.mainInstance; +import static org.javarosa.core.util.XFormsElement.model; +import static org.javarosa.core.util.XFormsElement.repeat; +import static org.javarosa.core.util.XFormsElement.select1; +import static org.javarosa.core.util.XFormsElement.t; +import static org.javarosa.core.util.XFormsElement.title; +import static org.junit.Assert.assertThat; + +public class RepeatTest { + + @Test + public void whenRepeatIsNotRelevant_repeatPromptIsSkipped() throws Exception { + Scenario scenario = Scenario.init("Non relevant repeat", html( + head( + title("Non relevant repeat"), + model( + mainInstance(t("data id=\"non_relevant_repeat\"", + t("repeat1", + t("q1")) + )), + bind("/data/repeat1").relevant("false()") + ), + body( + repeat("/data/repeat1", + input("/data/repeat1/q1") + ) + )))); + + scenario.jumpToBeginningOfForm(); + + int event = scenario.next(); + assertThat(event, is(FormEntryController.EVENT_END_OF_FORM)); + } + + @Test + public void whenRepeatRelevanceIsDynamic_andNotRelevant_repeatPromptIsSkipped() throws Exception { + Scenario scenario = Scenario.init("Repeat relevance - dynamic expression", html( + head( + title("Repeat relevance - dynamic expression"), + model( + mainInstance(t("data id=\"repeat_relevance_dynamic\"", + t("selectYesNo"), + t("repeat1", + t("q1")) + )), + bind("/data/repeat1").relevant("/data/selectYesNo = 'yes'") + ), + body( + select1("/data/selectYesNo", + item("yes", "Yes"), + item("no", "No")), + repeat("/data/repeat1", + input("/data/repeat1/q1") + ) + )))); + + scenario.jumpToBeginningOfForm(); + scenario.answer("/data/selectYesNo", "no"); + + int event = scenario.next(); + assertThat(event, is(FormEntryController.EVENT_END_OF_FORM)); + } +} diff --git a/src/test/java/org/javarosa/core/model/test/FormDefTest.java b/src/test/java/org/javarosa/core/model/test/FormDefTest.java index a3d6143bd..52b7d9bff 100644 --- a/src/test/java/org/javarosa/core/model/test/FormDefTest.java +++ b/src/test/java/org/javarosa/core/model/test/FormDefTest.java @@ -16,6 +16,15 @@ package org.javarosa.core.model.test; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.javarosa.core.model.FormDef; +import org.javarosa.core.test.Scenario; +import org.javarosa.form.api.FormEntryCaption; +import org.junit.Test; + +import java.io.IOException; + import static org.hamcrest.Matchers.is; import static org.javarosa.core.test.Scenario.AnswerResult.CONSTRAINT_VIOLATED; import static org.javarosa.core.test.Scenario.AnswerResult.OK; @@ -35,14 +44,6 @@ import static org.javarosa.core.util.XFormsElement.title; import static org.javarosa.test.utils.ResourcePathHelper.r; import static org.junit.Assert.assertThat; - -import java.io.IOException; -import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; -import org.javarosa.core.model.FormDef; -import org.javarosa.core.test.Scenario; -import org.javarosa.form.api.FormEntryCaption; -import org.junit.Test; /** * See testAnswerConstraint() for an example of how to write the * constraint unit type tests. @@ -179,6 +180,14 @@ public void nestedRepeatRelevance_updatesBasedOnParentPosition() throws IOExcept title("Nested repeat relevance"), model( mainInstance(t("data id=\"nested-repeat-relevance\"", + t("outer", + t("inner", + t("q1") + ), + t("inner", + t("q1") + ) + ), t("outer", t("inner", t("q1") @@ -198,17 +207,28 @@ public void nestedRepeatRelevance_updatesBasedOnParentPosition() throws IOExcept input("/data/relevance-condition") )))); - FormDef formDef = scenario.getFormDef(); - assertThat(formDef.isRepeatRelevant(getRef("/data/outer[0]/inner[0]")), is(false)); scenario.next(); + + // For ref /data/outer[0]/inner[0], the parent position is 1 so the boolean expression is false. That means + // none of the inner groups in /data/outer[0] can be relevant. + assertThat(scenario.refAtIndex(), is(getRef("/data/outer[0]"))); + + scenario.next(); + assertThat(scenario.refAtIndex(), is(getRef("/data/outer[1]"))); + + scenario.next(); + assertThat(scenario.refAtIndex(), is(getRef("/data/outer[1]/inner[0]"))); + scenario.next(); - assertThat(scenario.refAtIndex(), is(getRef("/data/outer[0]/inner[1]"))); + assertThat(scenario.refAtIndex(), is(getRef("/data/outer[1]/inner[0]/q1[0]"))); scenario.answer("/data/relevance-condition", "1"); - scenario.jumpToBeginningOfForm(); + scenario.jumpToBeginningOfForm(); scenario.next(); + assertThat(scenario.refAtIndex(), is(getRef("/data/outer[0]"))); + scenario.next(); assertThat(scenario.refAtIndex(), is(getRef("/data/outer[0]/inner[0]"))); } diff --git a/src/test/resources/repeat_groups_relevance.xml b/src/test/resources/repeat_groups_relevance.xml deleted file mode 100644 index 65e7f91de..000000000 --- a/src/test/resources/repeat_groups_relevance.xml +++ /dev/null @@ -1,69 +0,0 @@ - - - - repeatGroupsRelevance - - - - no - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - yes - - - - no - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file