disallow list slots in flows, add log, amend exception messages

rasa/validator.py

@@ -34,6 +34,7 @@
 from rasa.shared.core.domain import Domain
 from rasa.shared.core.generator import TrainingDataGenerator
 from rasa.shared.core.constants import SlotMappingType, MAPPING_TYPE
+from rasa.shared.core.slots import ListSlot
 from rasa.shared.core.training_data.structures import StoryGraph
 from rasa.shared.exceptions import RasaException
 from rasa.shared.importers.importer import TrainingDataImporter
@@ -495,34 +496,52 @@ def warn_if_config_mandatory_keys_are_not_set(self) -> None:
     def verify_flows_steps_against_domain(self, user_flows: List[Flow]) -> bool:
         """Checks flows steps' references against the domain file."""
         all_good = True
-        domain_slot_names = [slot.name for slot in self.domain.slots]
+        domain_slots = {slot.name: slot for slot in self.domain.slots}
         for flow in user_flows:
             for step in flow.steps:
                 if isinstance(step, CollectInformationFlowStep):
-                    if step.collect_information not in domain_slot_names:
+                    if step.collect_information not in domain_slots:
                         raise RasaException(
                             f"The slot '{step.collect_information}' is used in the "
-                            f"step '{step.id}' of flow '{flow.name}', but it "
+                            f"step '{step.id}' of flow id '{flow.id}', but it "
                             f"is not listed in the domain slots. "
                             f"You should add it to your domain file!",
                         )
+                    current_slot = domain_slots[step.collect_information]
+                    if isinstance(current_slot, ListSlot):
+                        raise RasaException(
+                            f"The slot '{step.collect_information}' is used in the "
+                            f"step '{step.id}' of flow id '{flow.id}', but it "
+                            f"is a list slot. List slots are currently not "
+                            f"supported in flows. You should change it to a "
+                            f"text, boolean or float slot in your domain file!",
+                        )
 
                 elif isinstance(step, SetSlotsFlowStep):
                     for slot in step.slots:
                         slot_name = slot["key"]
-                        if slot_name not in domain_slot_names:
+                        if slot_name not in domain_slots:
                             raise RasaException(
                                 f"The slot '{slot_name}' is used in the step "
-                                f"'{step.id}' of flow '{flow.name}', but it "
+                                f"'{step.id}' of flow id '{flow.id}', but it "
                                 f"is not listed in the domain slots. "
                                 f"You should add it to your domain file!",
                             )
+                        current_slot = domain_slots[slot_name]
+                        if isinstance(current_slot, ListSlot):
+                            raise RasaException(
+                                f"The slot '{slot_name}' is used in the "
+                                f"step '{step.id}' of flow id '{flow.id}', but it "
+                                f"is a list slot. List slots are currently not "
+                                f"supported in flows. You should change it to a "
+                                f"text, boolean or float slot in your domain file!",
+                            )
 
                 elif isinstance(step, ActionFlowStep):
                     if step.action not in self.domain.action_names_or_texts:
                         raise RasaException(
                             f"The action '{step.action}' is used in the step "
-                            f"'{step.id}' of flow '{flow.name}', but it "
+                            f"'{step.id}' of flow id '{flow.id}', but it "
                             f"is not listed in the domain file. "
                             f"You should add it to your domain file!",
                         )
@@ -541,15 +560,15 @@ def verify_unique_flows(user_flows: List[Flow]) -> bool:
             cleaned_description = flow_description.translate(punctuation_table)  # type: ignore[union-attr] # noqa: E501
             if cleaned_description in flows_mapping.values():
                 raise RasaException(
-                    f"Detected duplicate flow description for flow '{flow.name}'. "
+                    f"Detected duplicate flow description for flow id '{flow.id}'. "
                     f"Flow descriptions must be unique. "
                     f"Please make sure that all flows have different descriptions."
                 )
 
             if flow.name in flows_mapping:
                 raise RasaException(
-                    f"Detected duplicate flow name '{flow.name}'. "
-                    f"Flow names must be unique. "
+                    f"Detected duplicate flow name '{flow.name}' for flow "
+                    f"id '{flow.id}'. Flow names must be unique. "
                     f"Please make sure that all flows have different names."
                 )
 
@@ -579,7 +598,7 @@ def verify_predicates(user_flows: List[Flow]) -> bool:
                             if not is_valid:
                                 raise RasaException(
                                     f"Detected invalid condition '{link.condition}' "
-                                    f"at step '{step.id}' for flow '{flow.name}'. "
+                                    f"at step '{step.id}' for flow id '{flow.id}'. "
                                     f"Please make sure that all conditions are valid."
                                 )
                 elif isinstance(step, CollectInformationFlowStep):
@@ -599,13 +618,15 @@ def verify_predicates(user_flows: List[Flow]) -> bool:
                             raise RasaException(
                                 f"Detected invalid rejection '{predicate}' "
                                 f"at `collect_information` step '{step.id}' "
-                                f"for flow '{flow.name}'. "
+                                f"for flow id '{flow.id}'. "
                                 f"Please make sure that all conditions are valid."
                             )
         return all_good
 
     def verify_flows(self) -> bool:
         """Checks for inconsistencies across flows."""
+        logger.info("Validating flows...")
+
         if self.flows.is_empty():
             logger.warning(
                 "No flows were found in the data files. "

tests/test_validator.py

@@ -917,7 +917,7 @@ def test_verify_flow_steps_against_domain_missing_slot_in_domain(
 
     assert (
         f"The slot '{missing_slot_in_domain}' is used in the step 'ask_amount' of "
-        f"flow 'transfer money', but it is not listed in the domain slots."
+        f"flow id 'transfer_money', but it is not listed in the domain slots."
     ) in str(e.value)
 
 
@@ -980,7 +980,7 @@ def test_verify_flow_steps_against_domain_missing_action_in_domain(
 
     assert (
         f"The action '{missing_action_in_domain}' is used in the step "
-        f"'execute_transfer' of flow 'transfer money', but it "
+        f"'execute_transfer' of flow id 'transfer_money', but it "
         f"is not listed in the domain file."
     ) in str(e.value)
 
@@ -1050,11 +1050,71 @@ def test_verify_flow_steps_against_domain_missing_slot_from_set_slot_step(
 
     assert (
         f"The slot '{missing_slot_in_domain}' is used in the step "
-        f"'set_account_type' of flow 'transfer money', "
+        f"'set_account_type' of flow id 'transfer_money', "
         f"but it is not listed in the domain slots."
     ) in str(e.value)
 
 
+def test_verify_flow_steps_against_domain_disallowed_list_slot(
+    tmp_path: Path,
+    nlu_data_path: Path,
+) -> None:
+    flows_file = tmp_path / "flows.yml"
+    with open(flows_file, "w") as file:
+        file.write(
+            f"""
+                version: "{LATEST_TRAINING_DATA_FORMAT_VERSION}"
+                flows:
+                  order_pizza:
+                    description: This flow lets users order their favourite pizza.
+                    name: order pizza
+                    steps:
+                    - id: "ask_pizza_toppings"
+                      collect_information: pizza_toppings
+                      next: "ask_address"
+                    - id: "ask_address"
+                      collect_information: address
+                """
+        )
+    domain_file = tmp_path / "domain.yml"
+    with open(domain_file, "w") as file:
+        file.write(
+            f"""
+                version: "{LATEST_TRAINING_DATA_FORMAT_VERSION}"
+                intents:
+                  - greet
+                slots:
+                    pizza_toppings:
+                        type: list
+                        mappings: []
+                    address:
+                        type: text
+                        mappings: []
+                """
+        )
+    importer = RasaFileImporter(
+        config_file="data/test_moodbot/config.yml",
+        domain_path=str(domain_file),
+        training_data_paths=[str(flows_file), str(nlu_data_path)],
+    )
+
+    validator = Validator.from_importer(importer)
+    user_flows = [
+        flow
+        for flow in validator.flows.underlying_flows
+        if not flow.id.startswith("pattern_")
+    ]
+
+    with pytest.raises(RasaException) as e:
+        validator.verify_flows_steps_against_domain(user_flows)
+
+    assert (
+        "The slot 'pizza_toppings' is used in the step 'ask_pizza_toppings' "
+        "of flow id 'order_pizza', but it is a list slot. List slots are "
+        "currently not supported in flows." in str(e.value)
+    )
+
+
 def test_verify_unique_flows_duplicate_names(
     tmp_path: Path,
     nlu_data_path: Path,
@@ -1122,8 +1182,8 @@ def test_verify_unique_flows_duplicate_names(
         validator.verify_unique_flows(user_flows)
 
     assert (
-        f"Detected duplicate flow name '{duplicate_flow_name}'. "
-        f"Flow names must be unique. "
+        f"Detected duplicate flow name '{duplicate_flow_name}' for "
+        f"flow id 'recurrent_payment'. Flow names must be unique. "
         f"Please make sure that all flows have different names."
     ) in str(e.value)
 
@@ -1196,7 +1256,7 @@ def test_verify_unique_flows_duplicate_descriptions(
         validator.verify_unique_flows(user_flows)
 
     assert (
-        "Detected duplicate flow description for flow 'setup recurrent payment'. "
+        "Detected duplicate flow description for flow id 'recurrent_payment'. "
         "Flow descriptions must be unique. "
         "Please make sure that all flows have different descriptions."
     ) in str(e.value)
@@ -1210,7 +1270,7 @@ def test_verify_predicates_invalid_rejection_if(
     expected_exception = (
         f"Detected invalid rejection '{predicate}' "
         f"at `collect_information` step 'ask_account_type' "
-        f"for flow 'transfer money'. "
+        f"for flow id 'transfer_money'. "
         f"Please make sure that all conditions are valid."
     )
     flows_file = tmp_path / "flows.yml"