UI: generalize autocompletion to work for multi-choice fields (code review)

thseiler · thseiler · commit bbc25cb10af7 · 2025-05-19T15:12:48.000+02:00
diff --git a/strictdoc/backend/sdoc/models/document.py b/strictdoc/backend/sdoc/models/document.py
@@ -237,4 +237,4 @@ def get_options_for_choice(
             field, GrammarElementFieldMultipleChoice
         ):
             return field.options
-        return []
+        raise AssertionError(f"Must not reach here: {field}")
diff --git a/strictdoc/export/html/_static/controllers/autocompletable_field_controller.js b/strictdoc/export/html/_static/controllers/autocompletable_field_controller.js
@@ -15,7 +15,7 @@
       ready: Boolean,
       url: String,
       minLength: Number,
-      delay: { type: Number, default: 300 },
+      delay: { type: Number, default: 10 },
       queryParam: { type: String, default: "q" },
       multipleChoice: Boolean,
     }
@@ -38,6 +38,8 @@
 
       this.onInputChange = debounce(this.onInputChange, this.delayValue)
 
+      this.autocompletable.addEventListener("input", this.onInputChange)
+
       autocompletable.addEventListener("keydown", (event) => {
         const handler = this[`on${event.key}Keydown`]
         if (handler) handler(event)
@@ -61,18 +63,6 @@
         }
       });
 
-      autocompletable.addEventListener("input", (event) => {
-        const query = autocompletable.innerText.trim()
-        if (query && query.length >= this.minLengthValue) {
-          this.fetchResults(query)
-        } else {
-          this.hideAndRemoveOptions()
-        }
-
-        const text = filterSingleLine(this.autocompletable.innerText)
-        this.hidden.value = text
-      });
-
       this.results.addEventListener("mousedown", this.onResultsMouseDown)
       this.results.addEventListener("click", this.onResultsClick)
 
@@ -193,7 +183,7 @@
       this.autocompletable.innerText = suggestion
       this.hidden.value = suggestion
 
-      // some shenanigans to move the cursor to the end
+      // Move the cursor to the end of the input. 
       this.autocompletable.focus();
       const range = document.createRange();
       range.selectNodeContents(this.autocompletable);
@@ -234,6 +224,18 @@
       }, { once: true })
     }
 
+    onInputChange = ()  => {
+      const query = this.autocompletable.innerText.trim()
+      if (query && query.length >= this.minLengthValue) {
+        this.fetchResults(query)
+      } else {
+        this.hideAndRemoveOptions()
+      }
+
+      const text = filterSingleLine(this.autocompletable.innerText)
+      this.hidden.value = text
+    }
+
     identifyOptions() {
       const prefix = this.results.id || "autocompletable"
       const optionsWithoutId = this.results.querySelectorAll(`${optionSelector}:not([id])`)
diff --git a/strictdoc/server/routers/main_router.py b/strictdoc/server/routers/main_router.py
@@ -2793,14 +2793,26 @@ def get_autocomplete_field_results(
                 )
 
                 if field.gef_type == RequirementFieldType.MULTIPLE_CHOICE:
-                    # MultipleChoice: Split the query into parts.
+                    # MultipleChoice: We split the query into its parts:
+                    #
+                    # Example User input: "Some Value, Another Value, Yet ano|".
+                    # parts = ['some value', 'another value', 'yet ano']                # noqa: ERA001
                     parts = q.lower().split(",")
 
-                    # only use the last_part for lookup
+                    # For the lookup, we want to use the only the last, still
+                    # incomplete part, not the full query:
+                    #
+                    # last_part = "yet ano"                                             # noqa: ERA001
+                    # query_words = ['yet', 'ano']                                      # noqa: ERA001
                     last_part = parts[-1].strip()
                     query_words = last_part.split()
 
-                    # Pre-filter: don't suggest choices that were already selected / present in the query.
+                    # We also filter the already selected choices from the
+                    # options we are going to be send to the user,
+                    # as MultipleChoices is a Set, so options shall be
+                    # selectable at most once.
+                    #
+                    # In the example, we would remove 'some value' and 'another value'.
                     already_selected = [
                         p.strip() for p in parts[:-1] if p.strip()
                     ]
@@ -2810,7 +2822,7 @@ def get_autocomplete_field_results(
                         if choice.lower() not in already_selected
                     ]
                 else:
-                    # SingleChoice: use all available options.
+                    # SingleChoice: we use the full query and all available options.
                     query_words = q.lower().split()
                     filtered_options = all_options
 
diff --git a/tests/end2end/helpers/form/form.py b/tests/end2end/helpers/form/form.py
@@ -1,7 +1,6 @@
 # pylint: disable=invalid-name
-
-
 from selenium.webdriver import Keys
+from selenium.webdriver.common.action_chains import ActionChains
 from selenium.webdriver.common.by import By
 from selenium.webdriver.support.ui import WebDriverWait
 
@@ -153,25 +152,36 @@ def do_use_first_autocomplete_result(
 
         field_xpath = f"(//*[@data-testid='{test_id}'])"
         element = self.test_case.find_element(field_xpath)
+        hidden_input = element.find_element(
+            By.XPATH, 'following-sibling::input[@type="hidden"]'
+        )
+        results_ul = hidden_input.find_element(
+            By.XPATH, "following-sibling::ul[1]"
+        )
 
-        for _ in range(3):
-            self.test_case.type(field_xpath, f"{field_value}", by=By.XPATH)
-            element.send_keys(Keys.ARROW_DOWN)
-            element.send_keys(Keys.RETURN)
+        # We simulate a user typing the supplied field_value.
+        self.test_case.type(field_xpath, f"{field_value}", by=By.XPATH)
 
-            try:
-                WebDriverWait(self.test_case.driver, timeout=3).until(
-                    lambda _: field_value.lower() in element.text.lower()
-                )
-                break
-            except Exception:
-                pass
+        # We wait until the results <ul> is displayed.
+        WebDriverWait(self.test_case.driver, 3).until(
+            lambda _: results_ul.is_displayed()
+        )
 
-        else:
-            raise AssertionError(
-                f"The text field could not be filled with the value: "
-                f"'{field_value}'."
-            )
+        # We send Arrow-Down and Enter select the first match.
+        # The stimulus.js controller uses a debounce of 10ms, we are
+        # careful to use an larger interval inbetween key presses.
+        len_before_autocomplete = len(element.text.lower().strip())
+        select_first_match_action = ActionChains(self.test_case.driver)
+        select_first_match_action.send_keys(Keys.ARROW_DOWN).pause(
+            0.1
+        ).send_keys(Keys.RETURN).perform()
+
+        # Now wait for the length of the field to increase, this means
+        # the autocomplete did happen.
+        WebDriverWait(self.test_case.driver, timeout=3).until(
+            lambda _: len(element.text.lower().strip())
+            > len_before_autocomplete
+        )
 
     def do_append_command_and_use_autocomplete_result_again(
         self, test_id: str, field_value: str
@@ -181,26 +191,36 @@ def do_append_command_and_use_autocomplete_result_again(
 
         field_xpath = f"(//*[@data-testid='{test_id}'])"
         element = self.test_case.find_element(field_xpath)
+        hidden_input = element.find_element(
+            By.XPATH, 'following-sibling::input[@type="hidden"]'
+        )
+        results_ul = hidden_input.find_element(
+            By.XPATH, "following-sibling::ul[1]"
+        )
 
-        for _ in range(3):
-            element.send_keys(f",{field_value}")
-            element.send_keys(Keys.ARROW_DOWN)
-            element.send_keys(Keys.RETURN)
+        # We simulate a user typing a comma, and the supplied field_value.
+        element.send_keys(f",{field_value}")
 
-            try:
-                WebDriverWait(self.test_case.driver, timeout=3).until(
-                    lambda _: field_value.lower()
-                    in element.text.lower().split()[-1]
-                )
-                break
-            except Exception:
-                pass
+        # We wait until the results <ul> is displayed.
+        WebDriverWait(self.test_case.driver, 3).until(
+            lambda _: results_ul.is_displayed()
+        )
 
-        else:
-            raise AssertionError(
-                f"The text field could not be filled with the value: "
-                f"'{field_value}'."
-            )
+        # We send Arrow-Down and Enter select the first match.
+        # The stimulus.js controller uses a debounce of 50ms, we are
+        # careful to use an larger interval inbetween key presses.
+        len_before_autocomplete = len(element.text.lower().strip())
+        select_first_match_action = ActionChains(self.test_case.driver)
+        select_first_match_action.send_keys(Keys.ARROW_DOWN).pause(
+            0.1
+        ).send_keys(Keys.RETURN).perform()
+
+        # Now wait for the length of the field to increase, this means
+        # the autocomplete did happen.
+        WebDriverWait(self.test_case.driver, timeout=3).until(
+            lambda _: len(element.text.lower().strip())
+            > len_before_autocomplete
+        )
 
     def do_use_first_autocomplete_result_mid(
         self, mid: MID, test_id: str, field_value: str
diff --git a/tests/end2end/screens/document/create_requirement/_MultipleChoice/create_requirement_MultipleChoice_field_using_autocomplete/expected_output/document.sdoc b/tests/end2end/screens/document/create_requirement/_MultipleChoice/create_requirement_MultipleChoice_field_using_autocomplete/expected_output/document.sdoc
@@ -25,12 +25,12 @@ ELEMENTS:
   - TITLE: TAGS
     TYPE: String
     REQUIRED: False
-  - TITLE: TITLE
-    TYPE: String
-    REQUIRED: False
   - TITLE: OWNER
     TYPE: MultipleChoice(Abigail ACCURACY, Ace ACCREDITATION, Basil BOUNDARY, Clarence COMPLIANCE, Daphne DESIGN, Fiona FRAMEWORK, Felix FUNCTIONAL, Gloria GOVERNANCE, Gertrude GUIDELINE, Hugo HARMONY, Harvey HIERARCHY, Lydia LOGIC, Olivia OBJECTIVE, Mildred METADATA, Nigel NORMATIVE, Penelope PROCEDURE, Rachel RATIONALE, Eleanor RIGOR, Samuel SPECIFICATION, Sylvia STANDARD, Theodore TRACEABILITY, Victor VALIDATION, Walter WORKFLOW)
     REQUIRED: False
+  - TITLE: TITLE
+    TYPE: String
+    REQUIRED: False
   - TITLE: STATEMENT
     TYPE: String
     REQUIRED: False
@@ -68,8 +68,8 @@ Shall test foo.
 [REQUIREMENT]
 UID: REQ-2
 STATUS: accepted
-TITLE: Requirement 2 XYZ
 OWNER: Abigail ACCURACY, Ace ACCREDITATION
+TITLE: Requirement 2 XYZ
 STATEMENT: >>>
 Shall test foo 2.
 <<<
diff --git a/tests/end2end/screens/document/create_requirement/_MultipleChoice/create_requirement_MultipleChoice_field_using_autocomplete/input/document.sdoc b/tests/end2end/screens/document/create_requirement/_MultipleChoice/create_requirement_MultipleChoice_field_using_autocomplete/input/document.sdoc
@@ -25,12 +25,12 @@ ELEMENTS:
   - TITLE: TAGS
     TYPE: String
     REQUIRED: False
-  - TITLE: TITLE
-    TYPE: String
-    REQUIRED: False
   - TITLE: OWNER
     TYPE: MultipleChoice(Abigail ACCURACY, Ace ACCREDITATION, Basil BOUNDARY, Clarence COMPLIANCE, Daphne DESIGN, Fiona FRAMEWORK, Felix FUNCTIONAL, Gloria GOVERNANCE, Gertrude GUIDELINE, Hugo HARMONY, Harvey HIERARCHY, Lydia LOGIC, Olivia OBJECTIVE, Mildred METADATA, Nigel NORMATIVE, Penelope PROCEDURE, Rachel RATIONALE, Eleanor RIGOR, Samuel SPECIFICATION, Sylvia STANDARD, Theodore TRACEABILITY, Victor VALIDATION, Walter WORKFLOW)
     REQUIRED: False
+  - TITLE: TITLE
+    TYPE: String
+    REQUIRED: False
   - TITLE: STATEMENT
     TYPE: String
     REQUIRED: False
diff --git a/tests/end2end/screens/document/update_requirement/_MultipleChoice/update_requirement_MultipleChoice_field_using_autocomplete/expected_output/document.sdoc b/tests/end2end/screens/document/update_requirement/_MultipleChoice/update_requirement_MultipleChoice_field_using_autocomplete/expected_output/document.sdoc
@@ -25,12 +25,12 @@ ELEMENTS:
   - TITLE: TAGS
     TYPE: String
     REQUIRED: False
-  - TITLE: TITLE
-    TYPE: String
-    REQUIRED: False
   - TITLE: OWNER
     TYPE: MultipleChoice(Abigail ACCURACY, Ace ACCREDITATION, Basil BOUNDARY, Clarence COMPLIANCE, Daphne DESIGN, Fiona FRAMEWORK, Felix FUNCTIONAL, Gloria GOVERNANCE, Gertrude GUIDELINE, Hugo HARMONY, Harvey HIERARCHY, Lydia LOGIC, Olivia OBJECTIVE, Mildred METADATA, Nigel NORMATIVE, Penelope PROCEDURE, Rachel RATIONALE, Eleanor RIGOR, Samuel SPECIFICATION, Sylvia STANDARD, Theodore TRACEABILITY, Victor VALIDATION, Walter WORKFLOW)
     REQUIRED: False
+  - TITLE: TITLE
+    TYPE: String
+    REQUIRED: False
   - TITLE: STATEMENT
     TYPE: String
     REQUIRED: False
@@ -61,8 +61,8 @@ ELEMENTS:
 [REQUIREMENT]
 UID: TEST-1
 STATUS: accepted
-TITLE: Unit test ABC
 OWNER: Abigail ACCURACY, Ace ACCREDITATION
+TITLE: Unit test ABC
 STATEMENT: >>>
 Shall test foo.
 <<<
diff --git a/tests/end2end/screens/document/update_requirement/_MultipleChoice/update_requirement_MultipleChoice_field_using_autocomplete/input/document.sdoc b/tests/end2end/screens/document/update_requirement/_MultipleChoice/update_requirement_MultipleChoice_field_using_autocomplete/input/document.sdoc
@@ -17,12 +17,12 @@ ELEMENTS:
   - TITLE: TAGS
     TYPE: String
     REQUIRED: False
-  - TITLE: TITLE
-    TYPE: String
-    REQUIRED: False
   - TITLE: OWNER
     TYPE: MultipleChoice(Abigail ACCURACY, Ace ACCREDITATION, Basil BOUNDARY, Clarence COMPLIANCE, Daphne DESIGN, Fiona FRAMEWORK, Felix FUNCTIONAL, Gloria GOVERNANCE, Gertrude GUIDELINE, Hugo HARMONY, Harvey HIERARCHY, Lydia LOGIC, Olivia OBJECTIVE, Mildred METADATA, Nigel NORMATIVE, Penelope PROCEDURE, Rachel RATIONALE, Eleanor RIGOR, Samuel SPECIFICATION, Sylvia STANDARD, Theodore TRACEABILITY, Victor VALIDATION, Walter WORKFLOW)
     REQUIRED: False
+  - TITLE: TITLE
+    TYPE: String
+    REQUIRED: False
   - TITLE: STATEMENT
     TYPE: String
     REQUIRED: False
