Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed handling repeatable groups wrapped with regular ones #6105

Merged
merged 5 commits into from
May 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ class FormStylingTest {
.startBlankForm(FORM_NAME)
.clickGoToArrow()
.clickOnGroup("selectOneQuestions")
.assertText("selectOneQuestions")
.assertPath("selectOneQuestions")
.clickOnQuestion("Select one widget")
.assertText("selectOneQuestions")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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")
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ interface FormController {
*/
fun indexContainsRepeatableGroup(): Boolean

fun indexContainsRepeatableGroup(formIndex: FormIndex?): Boolean

/**
* The count of the closest group that repeats or -1.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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>