From cfd15c48d5e683530a59e5e28b4463fcd26cb01b Mon Sep 17 00:00:00 2001
From: Robbendebiene <robbendebiene@mailbox.org>
Date: Mon, 21 Nov 2022 11:08:13 +0100
Subject: [PATCH] Fix questionnaire entry removal (#114) & add test

---
 lib/models/questionnaire.dart | 14 ++++++-
 test/questionnaire_test.dart  | 79 +++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 test/questionnaire_test.dart

diff --git a/lib/models/questionnaire.dart b/lib/models/questionnaire.dart
index 2273024e..07d32c8a 100644
--- a/lib/models/questionnaire.dart
+++ b/lib/models/questionnaire.dart
@@ -75,8 +75,8 @@ class Questionnaire {
         _entries[_activeIndex].question,
         answer
       );
-      _updateWorkingElement();
       _removeObsoleteEntries();
+      _updateWorkingElement();
       _insertMatchingEntries();
     }
   }
@@ -118,8 +118,14 @@ class Questionnaire {
 
 
   void _removeObsoleteEntries() {
+    // Note: do not use reverse iteration here
+    // because removing a previous entry might result in the removal of a following entry (and so forth)
+    // which wouldn't be detected when iterating in reverse
+
     // only iterate over all elements after the current active index
-    for (var i = _entries.length - 1; i > _activeIndex; i--) {
+    var i = _activeIndex + 1;
+
+    while (i < _entries.length) {
       final entry = _entries[i];
       // create working element from all preceding entries excluding the current entry
       // this needs to be done, because otherwise an entry can get obsolete by its own answer or answers of succeeding questions
@@ -140,6 +146,10 @@ class Questionnaire {
         // remove the answer
         _entries.removeAt(i);
       }
+      else {
+        // only increment if no element was removed, because the removal will modify the indexes
+        i++;
+      }
     }
   }
 
diff --git a/test/questionnaire_test.dart b/test/questionnaire_test.dart
new file mode 100644
index 00000000..2787d4be
--- /dev/null
+++ b/test/questionnaire_test.dart
@@ -0,0 +1,79 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:open_stop/models/answer.dart';
+import 'package:open_stop/models/osm_condition.dart';
+import 'package:open_stop/models/question_catalog/answer_constructor.dart';
+import 'package:open_stop/models/question_catalog/answer_definition.dart';
+import 'package:open_stop/models/question_catalog/question_catalog.dart';
+import 'package:open_stop/models/question_catalog/question_definition.dart';
+import 'package:open_stop/models/questionnaire.dart';
+import 'package:osm_api/osm_api.dart';
+
+void main() async {
+
+  const stringAnswer = StringAnswerDefinition(
+    input: StringInputDefinition(),
+    constructor: AnswerConstructor({ 'tag_a': [r'$input'] }),
+  );
+
+  const dummyCatalog = QuestionCatalog([
+    QuestionDefinition(id: 0, name: 'q1', question: 'q1', isProfessional: false,
+      conditions: [
+        OsmCondition({ 'tag_a': false }, [])
+      ],
+      answer: stringAnswer,
+    ),
+    QuestionDefinition(id: 0, name: 'q2', question: 'q2', isProfessional: false,
+      conditions: [
+        OsmCondition({ 'tag_a': '1' }, [])
+      ],
+      answer: stringAnswer,
+    ),
+    QuestionDefinition(id: 0, name: 'q2', question: 'q2', isProfessional: false,
+      conditions: [
+        OsmCondition({ 'tag_a': '2' }, [])
+      ],
+      answer: stringAnswer,
+    ),
+  ]);
+
+
+  test('test if succeeding questions get correctly removed when pre-condition changes', () {
+    final element = OSMNode(0, 0, tags: {
+      'other_tag': 'val',
+    });
+
+    final questionnaire = Questionnaire(osmElement: element, questionCatalog: dummyCatalog);
+
+    expect(questionnaire.length, equals(1));
+    expect(questionnaire.activeIndex, equals(0));
+    expect(questionnaire.workingElement.tags, equals({ 'other_tag': 'val' }));
+
+    questionnaire.update(const StringAnswer(definition: stringAnswer, value: '1'));
+    questionnaire.next();
+
+    expect(questionnaire.length, equals(2));
+    expect(questionnaire.activeIndex, equals(1));
+    expect(questionnaire.workingElement.tags, equals({ 'other_tag': 'val', 'tag_a': '1' }));
+
+    questionnaire.update(const StringAnswer(definition: stringAnswer, value: '2'));
+    questionnaire.next();
+
+    expect(questionnaire.length, equals(3));
+    expect(questionnaire.activeIndex, equals(2));
+    expect(questionnaire.workingElement.tags, equals({ 'other_tag': 'val', 'tag_a': '2' }));
+
+    questionnaire.jumpTo(0);
+
+    expect(questionnaire.length, equals(3));
+    expect(questionnaire.activeIndex, equals(0));
+    expect(questionnaire.workingElement.tags, equals({ 'other_tag': 'val', 'tag_a': '2' }));
+
+    questionnaire.update(const StringAnswer(definition: stringAnswer, value: 'OTHER'));
+
+    expect(questionnaire.length, equals(1));
+    expect(questionnaire.activeIndex, equals(0));
+    expect(questionnaire.workingElement.tags, equals({ 'other_tag': 'val', 'tag_a': 'OTHER' }));
+
+    expect(questionnaire.next(), isFalse);
+  });
+}