fix: controls in loops and filters (#766)

* fix(controls): simplify and fix loop and filter cases

* refactor: remove control property from sequences

Sequence and subsequence doesn't have controls.

* test(controls): controls in loops

* chore: bump version

* ci: tests for pull requests to v3-next

.github/workflows/gandalf.yml

@@ -4,6 +4,7 @@ on:
   pull_request:
     branches:
       - 'v3-main'
+      - 'v3-next'
     types: [opened, synchronize, reopened, ready_for_review]
     paths-ignore: ['docs/**', 'logo/**', 'Dockerfile', 'README**.md']
 

build.gradle

@@ -7,7 +7,7 @@ plugins {
 
 allprojects {
     group = 'fr.insee.eno'
-    version = '3.11.2-SNAPSHOT'
+    version = '3.11.3-SNAPSHOT'
     sourceCompatibility = '17'
 }
 

eno-core/src/main/java/fr/insee/eno/core/model/EnoComponent.java

@@ -13,7 +13,6 @@ public interface EnoComponent {
     String getId();
     List<Declaration> getDeclarations();
     List<Instruction> getInstructions();
-    List<Control> getControls();
     ComponentFilter getComponentFilter();
 
 }

eno-core/src/main/java/fr/insee/eno/core/model/question/Question.java

@@ -42,6 +42,9 @@ public abstract class Question extends EnoIdentifiableObject implements EnoCompo
     @Lunatic("getDeclarations()")
     List<Instruction> instructions = new ArrayList<>();
 
+    /** Controls applied to the question.
+     * In DDI, the controls are mapped in the questionnaire object, and are put here through a processing.
+     * In Lunatic, the question components have a list of controls. */
     @Lunatic("getControls()")
     private final List<Control> controls = new ArrayList<>();
 

eno-core/src/main/java/fr/insee/eno/core/model/sequence/AbstractSequence.java

@@ -48,12 +48,6 @@ public abstract class AbstractSequence extends EnoIdentifiableObject implements
     @Lunatic("getDeclarations()")
     private final List<Declaration> declarations = new ArrayList<>();
 
-    /** Sequence / subsequence controls.
-     * In DDI, the controls are mapped in the questionnaire object, and are put here through a 'processing' class.
-     * In Lunatic, the sequence / subsequence object has a list of controls. */
-    @Lunatic("getControls()")
-    private final List<Control> controls = new ArrayList<>();
-
     /** Sequence / subsequence filter.
      * In DDI, the filters are mapped in the questionnaire object.
      * If there is a declared filter for this sequence / subsequence, it is put here through a 'processing' class.

eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertControls.java

@@ -2,6 +2,8 @@
 
 import fr.insee.eno.core.model.EnoQuestionnaire;
 import fr.insee.eno.core.model.navigation.Control;
+import fr.insee.eno.core.model.navigation.Filter;
+import fr.insee.eno.core.model.navigation.Loop;
 import fr.insee.eno.core.model.question.Question;
 import fr.insee.eno.core.model.sequence.ItemReference;
 import fr.insee.eno.core.model.sequence.Sequence;
@@ -16,70 +18,60 @@ public class DDIInsertControls implements ProcessingStep<EnoQuestionnaire> {
      * This processing is intended to insert them into the objects to which they belong.
      * (Controls are placed after the object they belong to in the sequence items lists.)
      * Concerned objects : sequences, subsequences and questions. */
-    public void apply(EnoQuestionnaire enoQuestionnaire) { // Note: code is a bit clumsy but works
+    public void apply(EnoQuestionnaire enoQuestionnaire) {
         //
         assert enoQuestionnaire.getIndex() != null;
         //
         for (Sequence sequence : enoQuestionnaire.getSequences()) {
             List<ItemReference> sequenceItems = sequence.getSequenceItems();
-            if (! sequenceItems.isEmpty()) {
-                int bound = sequenceItems.size();
-                // Sequence controls
-                int i = 0;
-                while (i<bound && sequenceItems.get(i).getType() == ItemReference.ItemType.CONTROL) {
-                    sequence.getControls().add(
-                            (Control) enoQuestionnaire.get(sequenceItems.get(i).getId()));
-                    i ++;
-                }
-                // Elements (questions, subsequences) in sequence
-                while (i < bound) {
-                    ItemReference sequenceItem = sequenceItems.get(i);
-                    if (sequenceItem.getType() == ItemReference.ItemType.QUESTION) {
-                        Question question = (Question) enoQuestionnaire.get(sequenceItem.getId());
-                        i ++;
-                        while (i<bound && sequenceItems.get(i).getType() == ItemReference.ItemType.CONTROL) {
-                            question.getControls().add(
-                                    (Control) enoQuestionnaire.get(sequenceItems.get(i).getId()));
-                            i ++;
-                        }
-                    }
-                    else if (sequenceItem.getType() == ItemReference.ItemType.SUBSEQUENCE) {
-                        Subsequence subsequence = (Subsequence) enoQuestionnaire.get(sequenceItem.getId());
-                        List<ItemReference> subsequenceItems = subsequence.getSequenceItems();
-                        if (! subsequenceItems.isEmpty()) {
-                            int bound2 = subsequenceItems.size();
-                            // Subsequence controls
-                            int j = 0;
-                            while (j<bound2 && subsequenceItems.get(j).getType() == ItemReference.ItemType.CONTROL) {
-                                subsequence.getControls().add(
-                                        (Control) enoQuestionnaire.get(subsequenceItems.get(j).getId()));
-                                j ++;
-                            }
-                            // Elements (questions) in subsequence
-                            while (j < bound2) {
-                                ItemReference subsequenceItem = subsequenceItems.get(j);
-                                if (subsequenceItem.getType() == ItemReference.ItemType.QUESTION) {
-                                    Question question = (Question) enoQuestionnaire.get(subsequenceItem.getId());
-                                    j ++;
-                                    while (j<bound2 && subsequenceItems.get(j).getType() == ItemReference.ItemType.CONTROL) {
-                                        question.getControls().add(
-                                                (Control) enoQuestionnaire.get(subsequenceItems.get(j).getId()));
-                                        j ++;
-                                    }
-                                }
-                                else { // skip other elements
-                                    j ++;
-                                }
-                            }
-                        }
-                        i ++;
-                    }
-                    else { // skip other elements
-                        i ++;
-                    }
-                }
+            insertControlsInReferencedItems(enoQuestionnaire, sequenceItems);
+        }
+    }
+
+    private static void insertControlsInReferencedItems(EnoQuestionnaire enoQuestionnaire, List<ItemReference> itemReferences) {
+        if (itemReferences.isEmpty())
+            return;
+        int bound = itemReferences.size();
+        int i = 0;
+        while (i < bound) {
+            ItemReference sequenceItem = itemReferences.get(i);
+            if (sequenceItem.getType() == ItemReference.ItemType.QUESTION) {
+                i = insertControlInQuestion(enoQuestionnaire, itemReferences, bound, i, sequenceItem);
+            }
+            else if (sequenceItem.getType() == ItemReference.ItemType.SUBSEQUENCE) {
+                Subsequence subsequence = (Subsequence) enoQuestionnaire.get(sequenceItem.getId());
+                List<ItemReference> subsequenceItems = subsequence.getSequenceItems();
+                insertControlsInReferencedItems(enoQuestionnaire, subsequenceItems);
+                i ++;
+            }
+            else if (sequenceItem.getType() == ItemReference.ItemType.LOOP) {
+                Loop loop = (Loop) enoQuestionnaire.get(sequenceItem.getId());
+                List<ItemReference> loopItems = loop.getLoopItems();
+                insertControlsInReferencedItems(enoQuestionnaire, loopItems);
+                i ++;
             }
+            else if (sequenceItem.getType() == ItemReference.ItemType.FILTER) {
+                Filter filter = (Filter) enoQuestionnaire.get(sequenceItem.getId());
+                List<ItemReference> filterItems = filter.getFilterItems();
+                insertControlsInReferencedItems(enoQuestionnaire, filterItems);
+                i ++;
+            }
+            else { // skip other elements
+                i ++;
+            }
+        }
+    }
+
+    private static int insertControlInQuestion(EnoQuestionnaire enoQuestionnaire, List<ItemReference> itemReferences,
+                                               int bound, int i, ItemReference questionReferenceItem) {
+        Question question = (Question) enoQuestionnaire.get(questionReferenceItem.getId());
+        i++;
+        while (i < bound && itemReferences.get(i).getType() == ItemReference.ItemType.CONTROL) {
+            question.getControls().add(
+                    (Control) enoQuestionnaire.get(itemReferences.get(i).getId()));
+            i++;
         }
+        return i;
     }
 
 }

eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIResolveVariableReferencesInLabels.java

@@ -46,8 +46,9 @@ public void apply(EnoQuestionnaire enoQuestionnaire) {
         enoCatalog.getComponents().forEach(enoComponent -> {
                 enoComponent.getDeclarations().stream().map(Declaration::getLabel).forEach(this::resolveLabel);
                 enoComponent.getInstructions().stream().map(Instruction::getLabel).forEach(this::resolveLabel);
-                enoComponent.getControls().stream().map(Control::getMessage).forEach(this::resolveLabel);
         });
+        enoCatalog.getQuestions().forEach(enoQuestion ->
+                enoQuestion.getControls().stream().map(Control::getMessage).forEach(this::resolveLabel));
         // Code lists
         enoQuestionnaire.getCodeLists().stream().map(CodeList::getCodeItems).forEach(this::resolveCodeItemsLabel);
         // Code lists in multiple response questions (might be refactored afterward)

eno-core/src/test/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertControlsTest.java

@@ -0,0 +1,59 @@
+package fr.insee.eno.core.processing.in.steps.ddi;
+
+import fr.insee.eno.core.exceptions.business.DDIParsingException;
+import fr.insee.eno.core.mappers.DDIMapper;
+import fr.insee.eno.core.model.EnoQuestionnaire;
+import fr.insee.eno.core.model.navigation.Control;
+import fr.insee.eno.core.model.question.Question;
+import fr.insee.eno.core.serialize.DDIDeserializer;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class DDIInsertControlsTest {
+
+    @Test
+    void questionnaireWithLoops() throws DDIParsingException {
+        //
+        EnoQuestionnaire enoQuestionnaire = new EnoQuestionnaire();
+        DDIMapper ddiMapper = new DDIMapper();
+        ddiMapper.mapDDI(
+                DDIDeserializer.deserialize(this.getClass().getClassLoader().getResourceAsStream(
+                        "functional/ddi/ddi-l7j0wwqx.xml")),
+                enoQuestionnaire);
+
+        //
+        new DDIInsertControls().apply(enoQuestionnaire);
+
+        //
+        List<Question> questions = new ArrayList<>(enoQuestionnaire.getSingleResponseQuestions());
+        //
+        assertTrue(questions.get(0).getControls().isEmpty());
+        //
+        assertEquals(1, questions.get(1).getControls().size());
+        assertEquals(Control.Criticality.INFO, questions.get(1).getControls().get(0).getCriticality());
+        //
+        assertEquals(1, questions.get(2).getControls().size());
+        assertEquals(Control.Criticality.INFO, questions.get(2).getControls().get(0).getCriticality());
+        //
+        assertEquals(2, questions.get(3).getControls().size());
+        assertEquals(Control.Criticality.INFO, questions.get(3).getControls().get(0).getCriticality());
+        assertEquals(Control.Criticality.INFO, questions.get(3).getControls().get(1).getCriticality());
+        //
+        assertEquals(1, questions.get(4).getControls().size());
+        assertEquals(Control.Criticality.INFO, questions.get(4).getControls().get(0).getCriticality());
+        //
+        assertEquals(1, questions.get(5).getControls().size());
+        assertEquals(Control.Criticality.INFO, questions.get(5).getControls().get(0).getCriticality());
+        //
+        assertEquals(1, questions.get(6).getControls().size());
+        assertEquals(Control.Criticality.INFO, questions.get(6).getControls().get(0).getCriticality());
+        //
+        assertTrue(questions.get(7).getControls().isEmpty());
+    }
+
+}