Merge pull request #6105 from grzesiek2010/COLLECT-6015

Fixed handling repeatable groups wrapped with regular ones
getodk · May 25, 2024 · fcbddb6 · fcbddb6
2 parents 70a1968 + 1193beb
commit fcbddb6
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 16 deletions.
diff --git a/...app/src/androidTest/java/org/odk/collect/android/feature/formentry/FormHierarchyTest.java b/...app/src/androidTest/java/org/odk/collect/android/feature/formentry/FormHierarchyTest.java
@@ -187,4 +187,17 @@ public void showRepeatsPickerWhenFirstRepeatIsEmpty() {
                 .clickGoUpIcon()
                 .assertTexts("Repeat", "Repeatable Group");
     }
+
+    @Test
+    //https://github.com/getodk/collect/issues/6015
+    public void regularGroupThatWrapsARepeatableGroupShouldBeTreatedAsARegularOne() {
+        rule.startAtMainMenu()
+                .copyForm("repeat_group_wrapped_with_a_regular_group.xml")
+                .startBlankForm("Repeat group wrapped with a regular group")
+                .clickGoToArrow()
+                .clickGoUpIcon()
+                .clickGoUpIcon()
+                .assertPath("Outer")
+                .assertNotRemovableGroup();
+    }
 }
diff --git a/...ect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/FormStylingTest.kt b/...ect_app/src/androidTest/java/org/odk/collect/android/feature/formentry/FormStylingTest.kt
@@ -223,7 +223,7 @@ class FormStylingTest {
             .startBlankForm(FORM_NAME)
             .clickGoToArrow()
             .clickOnGroup("selectOneQuestions")
-            .assertText("selectOneQuestions")
+            .assertPath("selectOneQuestions")
             .clickOnQuestion("Select one widget")
             .assertText("selectOneQuestions")
     }

diff --git a/...androidTest/java/org/odk/collect/android/regression/FillBlankFormWithRepeatGroupTest.java b/...androidTest/java/org/odk/collect/android/regression/FillBlankFormWithRepeatGroupTest.java
@@ -62,7 +62,7 @@ public void dynamicGroupLabel_should_beCalculatedProperly() {
                 .swipeToNextQuestion("Photo")
                 .assertText("gr1 > 1 > Person: 25")
                 .clickGoToArrow()
-                .assertText("gr1 > 1 > Person: 25")
+                .assertPath("gr1 > 1 > Person: 25")
                 .clickOnQuestion("Photo")
                 .swipeToNextQuestionWithRepeatGroup("gr1")
                 .clickOnDoNotAdd(new FormEntryPage("Repeat titles 1648"))
@@ -71,7 +71,7 @@ public void dynamicGroupLabel_should_beCalculatedProperly() {
                 .swipeToNextQuestion("Date")
                 .assertText("Part1 > 1 > Xxx: SecondPart")
                 .clickGoToArrow()
-                .assertText("Part1 > 1 > Xxx: SecondPart")
+                .assertPath("Part1 > 1 > Xxx: SecondPart")
                 .clickOnQuestion("Date")
                 .swipeToNextQuestion("Multi Select")
                 .swipeToNextQuestionWithRepeatGroup("Part1")
@@ -254,7 +254,7 @@ public void when_navigateOnHierarchyView_should_breadcrumbPathBeVisible() {
                 .swipeToNextQuestion("Age")
                 .inputText("3")
                 .clickGoToArrow()
-                .assertText("People > 3 > Person: C")
+                .assertPath("People > 3 > Person: C")
                 .clickGoUpIcon()
                 .assertText("3.\u200E Person: C")
                 .clickJumpEndButton()

diff --git a/...ect_app/src/androidTest/java/org/odk/collect/android/support/pages/FormHierarchyPage.java b/...ect_app/src/androidTest/java/org/odk/collect/android/support/pages/FormHierarchyPage.java
@@ -40,6 +40,11 @@ public FormHierarchyPage assertOnPage() {
         return this;
     }
 
+    public FormHierarchyPage assertNotRemovableGroup() {
+        onView(withId(R.id.menu_delete_child)).check(doesNotExist());
+        return this;
+    }
+
     public FormHierarchyPage clickGoUpIcon() {
         onView(withId(R.id.menu_go_up)).perform(click());
         return this;
@@ -83,6 +88,11 @@ public FormHierarchyPage assertHierarchyItem(int position, String primaryText, S
         return this;
     }
 
+    public FormHierarchyPage assertPath(String text) {
+        onView(withId(R.id.pathtext)).check(matches(withText(text)));
+        return this;
+    }
+
     public FormEntryPage clickOnQuestion(String questionLabel) {
         return clickOnQuestion(questionLabel, false);
     }

diff --git a/collect_app/src/main/java/org/odk/collect/android/formhierarchy/FormHierarchyActivity.java b/collect_app/src/main/java/org/odk/collect/android/formhierarchy/FormHierarchyActivity.java
@@ -333,7 +333,7 @@ private void updateOptionsMenu() {
 
         boolean isAtBeginning = screenIndex.isBeginningOfFormIndex() && !shouldShowRepeatGroupPicker();
         boolean shouldShowPicker = shouldShowRepeatGroupPicker();
-        boolean isInRepeat = formController.indexContainsRepeatableGroup();
+        boolean isInRepeat = formController.indexContainsRepeatableGroup(screenIndex);
         boolean isGroupSizeLocked = shouldShowPicker
                 ? isGroupSizeLocked(repeatGroupPickerIndex) : isGroupSizeLocked(screenIndex);
 
@@ -494,15 +494,7 @@ protected void goUpLevel() {
      */
     private CharSequence getCurrentPath() {
         FormController formController = formEntryViewModel.getFormController();
-        FormIndex index = formController.getFormIndex();
-
-        // Step out to the enclosing group if the current index is something
-        // we don't want to display in the path (e.g. a question name or the
-        // very first group in a form which is auto-entered).
-        if (formController.getEvent(index) == FormEntryController.EVENT_QUESTION
-                || getPreviousLevel(index) == null) {
-            index = getPreviousLevel(index);
-        }
+        FormIndex index = screenIndex;
 
         List<FormEntryCaption> groups = new ArrayList<>();
 
@@ -622,7 +614,7 @@ private void refreshView(boolean isGoingUp) {
                 groupPathTextView.setVisibility(View.VISIBLE);
                 groupPathTextView.setText(getCurrentPath());
 
-                if (formController.indexContainsRepeatableGroup() || shouldShowRepeatGroupPicker()) {
+                if (formController.indexContainsRepeatableGroup(screenIndex) || shouldShowRepeatGroupPicker()) {
                     groupIcon.setImageDrawable(ContextCompat.getDrawable(this, R.drawable.ic_repeat));
                 } else {
                     groupIcon.setImageDrawable(ContextCompat.getDrawable(this, R.drawable.ic_folder_open));

diff --git a/collect_app/src/main/java/org/odk/collect/android/javarosawrapper/FormController.kt b/collect_app/src/main/java/org/odk/collect/android/javarosawrapper/FormController.kt
@@ -275,6 +275,8 @@ interface FormController {
      */
     fun indexContainsRepeatableGroup(): Boolean
 
+    fun indexContainsRepeatableGroup(formIndex: FormIndex?): Boolean
+
     /**
      * The count of the closest group that repeats or -1.
      */

diff --git a/...ect_app/src/main/java/org/odk/collect/android/javarosawrapper/JavaRosaFormController.java b/...ect_app/src/main/java/org/odk/collect/android/javarosawrapper/JavaRosaFormController.java
@@ -904,7 +904,11 @@ public FormEntryCaption[] getGroupsForCurrentIndex() {
     }
 
     public boolean indexContainsRepeatableGroup() {
-        FormEntryCaption[] groups = getCaptionHierarchy();
+        return indexContainsRepeatableGroup(getFormIndex());
+    }
+
+    public boolean indexContainsRepeatableGroup(FormIndex formIndex) {
+        FormEntryCaption[] groups = getCaptionHierarchy(formIndex);
         if (groups.length == 0) {
             return false;
         }

diff --git a/collect_app/src/test/java/org/odk/collect/android/utilities/StubFormController.kt b/collect_app/src/test/java/org/odk/collect/android/utilities/StubFormController.kt
@@ -135,6 +135,8 @@ open class StubFormController : FormController {
 
     override fun indexContainsRepeatableGroup(): Boolean = false
 
+    override fun indexContainsRepeatableGroup(formIndex: FormIndex?): Boolean = false
+
     override fun getLastRepeatedGroupRepeatCount(): Int = -1
 
     override fun getLastRepeatedGroupName(): String? = null

diff --git a/test-forms/src/main/resources/forms/repeat_group_wrapped_with_a_regular_group.xml b/test-forms/src/main/resources/forms/repeat_group_wrapped_with_a_regular_group.xml
@@ -0,0 +1,45 @@
+<?xml version="1.0"?>
+<h:html
+    xmlns="http://www.w3.org/2002/xforms"
+    xmlns:h="http://www.w3.org/1999/xhtml"
+    xmlns:ev="http://www.w3.org/2001/xml-events"
+    xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+    xmlns:jr="http://openrosa.org/javarosa"
+    xmlns:orx="http://openrosa.org/xforms"
+    xmlns:odk="http://www.opendatakit.org/xforms">
+    <h:head>
+        <h:title>Repeat group wrapped with a regular group</h:title>
+        <model odk:xforms-version="1.0.0">
+            <instance>
+                <data id="groups_crash">
+                    <outer>
+                        <inner jr:template="">
+                            <name/>
+                        </inner>
+                        <inner>
+                            <name/>
+                        </inner>
+                    </outer>
+                    <meta>
+                        <instanceID/>
+                    </meta>
+                </data>
+            </instance>
+            <bind nodeset="/data/outer/inner/name" type="string"/>
+            <bind nodeset="/data/meta/instanceID" type="string" readonly="true()" jr:preload="uid"/>
+        </model>
+    </h:head>
+    <h:body>
+        <group ref="/data/outer">
+            <label>Outer</label>
+            <group ref="/data/outer/inner">
+                <label>Inner</label>
+                <repeat nodeset="/data/outer/inner">
+                    <input ref="/data/outer/inner/name">
+                        <label>Name</label>
+                    </input>
+                </repeat>
+            </group>
+        </group>
+    </h:body>
+</h:html>