702: nicer error when entities update specified but entity list not attached

.github/workflows/release.yml

@@ -35,7 +35,7 @@ jobs:
       - name: Publish release to PyPI
         if: success()
         run: |
-          pip install flit==3.12.0
+          pip install flit==3.12.0 "flit_core >=3.2,<4"
           flit --debug publish --no-use-vcs
         env:
           FLIT_USERNAME: __token__

.github/workflows/verify.yml

@@ -78,7 +78,7 @@ jobs:
       - name: Build sdist and wheel.
         if: success() && matrix.PYXFORM_TESTS_RUN_ODK_VALIDATE == 'false'
         run: |
-          pip install flit==3.12.0
+          pip install flit==3.12.0 "flit_core >=3.2,<4"
           flit --debug build --no-use-vcs
       - name: Upload sdist and wheel.
         if: success() && matrix.PYXFORM_TESTS_RUN_ODK_VALIDATE == 'false'

README.md

@@ -157,7 +157,7 @@ Releases are now automatic. These instructions are provided for forks or for a f
 3.  Install the production and packaging requirements:
 
         pip install -e .
-        pip install flit==3.12.0
+        pip install flit==3.12.0 "flit_core >=3.2,<4"
 
 4.  Clean up build and dist folders:
 

pyxform/entities/entities_parsing.py

@@ -433,6 +433,16 @@ def get_entity_declaration(row: dict, row_number: int) -> dict[str, Any]:
     return entity
 
 
+def get_entity_property(entity: dict[str, Any], name: str) -> dict[str, Any] | None:
+    """
+    From the entity "children" list, lookup a property item by the "name" key.
+
+    :param entity: The entity declaration to search.
+    :param name: The property name to look for.
+    """
+    return next(iter(c for c in entity[const.CHILDREN] if c[const.NAME] == name), None)
+
+
 def validate_dataset_name(dataset_name: str | None, row_number: int) -> None:
     """
     Check the dataset_name passes all naming rules.
@@ -462,6 +472,29 @@ def validate_dataset_name(dataset_name: str | None, row_number: int) -> None:
         )
 
 
+def validate_update_dataset_references(
+    entity_declarations: dict[str, dict[str, Any]],
+    secondary_instances: set[tuple[str, str]],
+) -> None:
+    """
+    Check that the entities in update mode refer to a secondary instance.
+
+    :param entity_declarations: The entities data `{list_name: declaration]}`.
+    :param secondary_instances:
+    :return:
+    """
+    secondary_instance_csvs = {n for n, t in secondary_instances if t.lower() == ".csv"}
+    for ed in entity_declarations.values():
+        update = get_entity_property(entity=ed, name="update")
+        if update is not None:
+            dataset = get_entity_property(entity=ed, name="dataset")
+            if dataset is not None and dataset["value"] not in secondary_instance_csvs:
+                raise PyXFormError(
+                    code=ErrorCode.ENTITY_014,
+                    context={"row": ed["__row_number"], "dataset": dataset["value"]},
+                )
+
+
 def validate_saveto(
     saveto: str | None,
     row_number: int,
@@ -726,10 +759,7 @@ def inject_entities_into_json(
     if dataset_name and dataset_name not in entities_allocated:
         entity_decl = entity_declarations[dataset_name]
         if has_repeat_ancestor:
-            id_attr = next(
-                iter(c for c in entity_decl[const.CHILDREN] if c[const.NAME] == "id"),
-                None,
-            )
+            id_attr = get_entity_property(entity=entity_decl, name="id")
             if id_attr and len(id_attr["actions"]) == 1:
                 new_repeat = action.ActionLibrary.setvalue_new_repeat.value.to_dict()
                 new_repeat["value"] = id_attr["actions"][0]["value"]

pyxform/errors.py

@@ -176,6 +176,17 @@ class ErrorCode(Enum):
             "Please check the spelling of this 'save_to' value."
         ),
     )
+    ENTITY_014 = Detail(
+        name="Entities - missing secondary instance for update",
+        msg=(
+            "[row : {row}] On the 'entities' sheet, the entity declaration is invalid. "
+            "The entity list name '{dataset}' does not match the name of a secondary instance, "
+            "which is required when updating entities. "
+            "Please either: add a question on the 'survey' sheet with the type "
+            "'select_*_from_file' or 'csv-external', or check the spelling of existing "
+            "questions using these types and the entity list name."
+        ),
+    )
     HEADER_001: Detail = Detail(
         name="Headers - invalid missing header row",
         msg=(
@@ -358,6 +369,16 @@ class ErrorCode(Enum):
             "Entity lists must have a name."
         ),
     )
+    NAMES_016 = Detail(
+        name="Names - select list_name not found on choices sheet",
+        msg=(
+            "[row : {row}] On the 'survey' sheet, the 'type' value is invalid. "
+            "The select list name was not found in the 'choices' sheet. "
+            "Please add one or more rows to the 'choices' sheet for this list_name, or "
+            "check the spelling of the list name in the 'type' column and existing "
+            "choices 'list_name' rows."
+        ),
+    )
     PYREF_001: Detail = Detail(
         name="PyXForm reference - parsing failed",
         msg=(

pyxform/validators/pyxform/question_types/geo.py

@@ -21,7 +21,7 @@ def validate_parameter_incremental(value: str) -> None:
 
 def validate_parameter_reference_geometry(
     geo_references: Iterable[Iterable[str, int]],
-    secondary_instances: set[str],
+    secondary_instances: set[tuple[str, str]],
     repeat_names: set[str],
     choices: dict[str, list[dict]],
     entity_declarations: dict[str, dict[str, Any]] | None = None,
@@ -35,14 +35,15 @@ def validate_parameter_reference_geometry(
     - last-saved usages in variables
 
     :param geo_references: Pairs of (target, source row_num) for reference_geometry usage.
-    :param secondary_instances: The names of valid secondary instances in the form.
+    :param secondary_instances: The (name, ext) of valid secondary instances in the form.
     :param repeat_names: Names of repeat groups in the form.
     :param choices: The choices data as `{list_name: [choice_items[options], ...]}`.
     :param entity_declarations: The entities data `{list_name: declaration]}`.
     """
+    secondary_instance_names = {n for n, t in secondary_instances if t}
     for target, row_num in geo_references:
         if (
-            target in secondary_instances
+            target in secondary_instance_names
             or target in choices
             or (entity_declarations and target in entity_declarations)
         ):

pyxform/validators/pyxform/select_from_file.py

@@ -1,6 +1,5 @@
 from pathlib import Path
 
-from pyxform import aliases
 from pyxform import constants as co
 from pyxform.constants import EXTERNAL_INSTANCE_EXTENSIONS, ROW_FORMAT_STRING
 from pyxform.errors import ErrorCode, PyXFormError
@@ -35,7 +34,7 @@ def validate_list_name_extension(
 ) -> None:
     """For select_from_file types, the list_name should end with a supported extension."""
     list_path = Path(list_name)
-    if select_command in aliases.select_from_file and (
+    if (
         1 != len(list_path.suffixes)
         or list_path.suffix not in EXTERNAL_INSTANCE_EXTENSIONS
     ):

pyxform/xls2json.py

@@ -21,6 +21,7 @@
     get_entity_declarations,
     get_entity_references_by_question,
     get_entity_variable_references,
+    validate_update_dataset_references,
 )
 from pyxform.errors import ErrorCode, PyXFormError
 from pyxform.parsing.expression import is_xml_tag
@@ -456,7 +457,8 @@ def workbook_to_json(
     element_names = Counter()
     trigger_references: list[tuple[str, int]] = []
     geo_references: list[tuple[str, int]] = []
-    secondary_instances: set[str] = set()
+    # secondary_instances items: tuple[name, file_extension]
+    secondary_instances: set[tuple[str, str]] = set()
     repeat_names: set[str] = set()
     entity_references_by_question = {}
 
@@ -887,26 +889,27 @@ def workbook_to_json(
         question_names.add(question_name)
         if row[constants.TYPE] in constants.EXTERNAL_INSTANCE_TYPES:
             qt_external_instance.validate_scope(row_number=row_number, stack=stack)
-            secondary_instances.add(os.path.splitext(question_name)[0])
+            secondary_instances.add(
+                (question_name, f".{row[constants.TYPE].split('-')[0]}")
+            )
 
         # Try to parse question as a select:
         select_parse = RE_SELECT.search(question_type)
         if select_parse:
             parse_dict = select_parse.groupdict()
             if parse_dict.get("select_command"):
-                select_type = aliases.select[parse_dict["select_command"]]
-                if (
-                    select_type == constants.SELECT_ONE_EXTERNAL
-                    and constants.CHOICE_FILTER not in row
-                ):
-                    warnings.append(
-                        ROW_FORMAT_STRING % row_number
-                        + " select one external is only meant for filtered selects."
-                    )
+                select_command = parse_dict["select_command"]
+                select_type = aliases.select[select_command]
                 list_name = parse_dict[constants.LIST_NAME_U]
                 instance_name, file_extension = os.path.splitext(list_name)
 
+                # Validate external selects.
                 if select_type == constants.SELECT_ONE_EXTERNAL:
+                    if constants.CHOICE_FILTER not in row:
+                        warnings.append(
+                            ROW_FORMAT_STRING % row_number
+                            + " select one external is only meant for filtered selects."
+                        )
                     if not external_choices:
                         k = constants.EXTERNAL_CHOICES
                         msg = "There should be an external_choices sheet in this xlsform."
@@ -924,18 +927,11 @@ def workbook_to_json(
                             + "List name not in external choices sheet: "
                             + list_name
                         )
-                else:
-                    secondary_instances.add(instance_name)
 
-                select_from_file.validate_list_name_extension(
-                    select_command=parse_dict["select_command"],
-                    list_name=list_name,
-                    row_number=row_number,
-                )
+                # Validate internal selects.
                 if (
-                    list_name not in choices
-                    and select_type != constants.SELECT_ONE_EXTERNAL
-                    and file_extension not in EXTERNAL_INSTANCE_EXTENSIONS
+                    select_type != constants.SELECT_ONE_EXTERNAL
+                    and select_command not in aliases.select_from_file
                     and not has_pyxform_reference(list_name)
                 ):
                     if not choices:
@@ -948,28 +944,26 @@ def workbook_to_json(
                             f"{msg} Please ensure that the choices sheet has the"
                             " mandatory columns 'list_name', 'name', and 'label'."
                         )
-                    raise PyXFormError(
-                        ROW_FORMAT_STRING % row_number
-                        + " List name not in choices sheet: "
-                        + list_name
-                    )
+                    elif list_name not in choices:
+                        raise PyXFormError(
+                            code=ErrorCode.NAMES_016, context={"row": row_number}
+                        )
 
-                # Validate select_multiple choice names by making sure
-                # they have no spaces (will cause errors in exports).
-                if (
-                    select_type == constants.SELECT_ALL_THAT_APPLY
-                    and file_extension not in EXTERNAL_INSTANCE_EXTENSIONS
-                ):
-                    for choice in choices[list_name]:
-                        if " " in choice[constants.NAME]:
-                            raise PyXFormError(
-                                "Choice names with spaces cannot be added "
-                                "to multiple choice selects. See ["
-                                + choice[constants.NAME]
-                                + "] in ["
-                                + list_name
-                                + "]"
-                            )
+                    # Validate select_multiple choice names by making sure
+                    # they have no spaces (will cause errors in exports).
+                    if select_type == constants.SELECT_ALL_THAT_APPLY:
+                        for choice in choices[list_name]:
+                            if " " in choice[constants.NAME]:
+                                raise PyXFormError(
+                                    "Choice names with spaces cannot be added "
+                                    "to multiple choice selects. See ["
+                                    + choice[constants.NAME]
+                                    + "] in ["
+                                    + list_name
+                                    + "]"
+                                )
+                    # Track the secondary instance.
+                    secondary_instances.add((instance_name, ""))
 
                 specify_other_question = None
                 if parse_dict.get("specify_other") is not None:
@@ -1029,10 +1023,12 @@ def workbook_to_json(
                 new_json_dict = row.copy()
                 new_json_dict[constants.TYPE] = select_type
 
-                if parse_dict["select_command"] in {
-                    "select_one_from_file",
-                    "select_multiple_from_file",
-                }:
+                if select_command in aliases.select_from_file:
+                    select_from_file.validate_list_name_extension(
+                        select_command=select_command,
+                        list_name=list_name,
+                        row_number=row_number,
+                    )
                     qt_params = constants.ParametersSelectFromFile
                     pv.validate(
                         parameters=parameters,
@@ -1051,6 +1047,8 @@ def workbook_to_json(
                             value=parameters[qt_params.LABEL],
                             row_number=row_number,
                         )
+                    # Track the secondary instance.
+                    secondary_instances.add((instance_name, file_extension))
                 else:
                     qt_params = constants.ParametersSelect
                     pv.validate(
@@ -1372,6 +1370,10 @@ def workbook_to_json(
         )
 
     if entity_declarations:
+        validate_update_dataset_references(
+            entity_declarations=entity_declarations,
+            secondary_instances=secondary_instances,
+        )
         apply_entities_declarations(
             entity_declarations=entity_declarations,
             entity_references_by_question=entity_references_by_question,