Add background-geopoint question type which exposes `xforms-value-c…

…hanged` event with `odk:setgeopoint` action (XLSForm#726)

* fix: update github actions versions to silence warnings about node

---------

Co-authored-by: lindsay stevens <[email protected]>

.github/workflows/release.yml

@@ -19,7 +19,7 @@ jobs:
           python-version: ${{ matrix.python }}
 
       # Install dependencies.
-      - uses: actions/cache@v3
+      - uses: actions/cache@v4
         name: Python cache with dependencies.
         id: python-cache
         with:

.github/workflows/verify.yml

@@ -17,7 +17,7 @@ jobs:
           python-version: ${{ matrix.python }}
 
       # Install dependencies.
-      - uses: actions/cache@v3
+      - uses: actions/cache@v4
         name: Python cache with dependencies.
         id: python-cache
         with:
@@ -52,7 +52,7 @@ jobs:
           python-version: ${{ matrix.python }}
 
       # Install dependencies.
-      - uses: actions/cache@v3
+      - uses: actions/cache@v4
         name: Python cache with dependencies.
         id: python-cache
         with:
@@ -76,7 +76,7 @@ jobs:
           flit --debug build --no-use-vcs
       - name: Upload sdist and wheel.
         if: success()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: pyxform--on-${{ matrix.os }}--py${{ matrix.python }}
           path: ${{ github.workspace }}/dist/pyxform*

pyxform/builder.py

@@ -4,7 +4,7 @@
 
 import copy
 import os
-import re
+from collections import defaultdict
 from typing import TYPE_CHECKING, Any, Union
 
 from pyxform import constants as const
@@ -84,7 +84,9 @@ def __init__(self, **kwargs):
         self.set_sections(kwargs.get("sections", {}))
 
         # dictionary of setvalue target and value tuple indexed by triggering element
-        self.setvalues_by_triggering_ref = {}
+        self.setvalues_by_triggering_ref = defaultdict(list)
+        # dictionary of setgeopoint target and value tuple indexed by triggering element
+        self.setgeopoint_by_triggering_ref = defaultdict(list)
         # For tracking survey-level choices while recursing through the survey.
         self._choices: dict[str, Any] = {}
 
@@ -117,6 +119,7 @@ def create_survey_element_from_dict(
 
             if d[const.TYPE] == const.SURVEY:
                 section.setvalues_by_triggering_ref = self.setvalues_by_triggering_ref
+                section.setgeopoint_by_triggering_ref = self.setgeopoint_by_triggering_ref
                 section.choices = self._choices
 
             return section
@@ -138,27 +141,22 @@ def create_survey_element_from_dict(
         elif d[const.TYPE] == "entity":
             return EntityDeclaration(**d)
         else:
-            self._save_trigger_as_setvalue_and_remove_calculate(d)
-
+            self._save_trigger(d=d)
             return self._create_question_from_dict(
                 d, copy_json_dict(QUESTION_TYPE_DICT), self._add_none_option
             )
 
-    def _save_trigger_as_setvalue_and_remove_calculate(self, d):
+    def _save_trigger(self, d: dict) -> None:
         if "trigger" in d:
-            triggering_ref = re.sub(r"\s+", "", d["trigger"])
+            triggering_ref = d["trigger"].strip()
             value = ""
             if const.BIND in d and "calculate" in d[const.BIND]:
                 value = d[const.BIND]["calculate"]
-
-            if triggering_ref in self.setvalues_by_triggering_ref:
-                self.setvalues_by_triggering_ref[triggering_ref].append(
-                    (d[const.NAME], value)
-                )
+            question_ref = (d[const.NAME], value)
+            if d[const.TYPE] == "background-geopoint":
+                self.setgeopoint_by_triggering_ref[triggering_ref].append(question_ref)
             else:
-                self.setvalues_by_triggering_ref[triggering_ref] = [
-                    (d[const.NAME], value)
-                ]
+                self.setvalues_by_triggering_ref[triggering_ref].append(question_ref)
 
     @staticmethod
     def _create_question_from_dict(

pyxform/question.py

@@ -14,7 +14,12 @@
 from pyxform.errors import PyXFormError
 from pyxform.question_type_dictionary import QUESTION_TYPE_DICT
 from pyxform.survey_element import SurveyElement
-from pyxform.utils import PYXFORM_REFERENCE_REGEX, default_is_dynamic, node
+from pyxform.utils import (
+    PYXFORM_REFERENCE_REGEX,
+    DetachableElement,
+    default_is_dynamic,
+    node,
+)
 
 
 class Question(SurveyElement):
@@ -47,10 +52,13 @@ def xml_instance(self, **kwargs):
         return node(self.name, **attributes)
 
     def xml_control(self):
+        survey = self.get_root()
         if self.type == "calculate" or (
             ("calculate" in self.bind or self.trigger) and not (self.label or self.hint)
         ):
-            nested_setvalues = self.get_root().get_setvalues_for_question_name(self.name)
+            nested_setvalues = survey.get_trigger_values_for_question_name(
+                self.name, "setvalue"
+            )
             if nested_setvalues:
                 for setvalue in nested_setvalues:
                     msg = (
@@ -64,31 +72,36 @@ def xml_control(self):
         xml_node = self.build_xml()
 
         if xml_node:
-            self.nest_setvalues(xml_node)
-
-        return xml_node
-
-    def nest_setvalues(self, xml_node):
-        nested_setvalues = self.get_root().get_setvalues_for_question_name(self.name)
-
-        if nested_setvalues:
-            for setvalue in nested_setvalues:
-                setvalue_attrs = {
-                    "ref": self.get_root()
-                    .insert_xpaths(f"${{{setvalue[0]}}}", self.get_root())
-                    .strip(),
-                    "event": "xforms-value-changed",
-                }
-                if not setvalue[1] == "":
-                    setvalue_attrs["value"] = self.get_root().insert_xpaths(
-                        setvalue[1], self
-                    )
+            # Get nested setvalue and setgeopoint items
+            setvalue_items = survey.get_trigger_values_for_question_name(
+                self.name, "setvalue"
+            )
+            setgeopoint_items = survey.get_trigger_values_for_question_name(
+                self.name, "setgeopoint"
+            )
 
-                setvalue_node = node("setvalue", **setvalue_attrs)
+            # Only call nest_set_nodes if the respective nested items list is not empty
+            if setvalue_items:
+                self.nest_set_nodes(survey, xml_node, "setvalue", setvalue_items)
+            if setgeopoint_items:
+                self.nest_set_nodes(
+                    survey, xml_node, "odk:setgeopoint", setgeopoint_items
+                )
 
-                xml_node.appendChild(setvalue_node)
+        return xml_node
 
-    def build_xml(self):
+    def nest_set_nodes(self, survey, xml_node, tag, nested_items):
+        for item in nested_items:
+            node_attrs = {
+                "ref": survey.insert_xpaths(f"${{{item[0]}}}", survey).strip(),
+                "event": "xforms-value-changed",
+            }
+            if item[1]:
+                node_attrs["value"] = survey.insert_xpaths(item[1], self)
+            set_node = node(tag, **node_attrs)
+            xml_node.appendChild(set_node)
+
+    def build_xml(self) -> DetachableElement | None:
         return None
 
 

pyxform/question_type_dictionary.py

@@ -382,4 +382,8 @@ def generate_new_dict():
         "bind": {"type": "binary"},
         "action": {"name": "odk:recordaudio", "event": "odk-instance-load"},
     },
+    "background-geopoint": {
+        "control": {"tag": "trigger"},
+        "bind": {"type": "geopoint"},
+    },
 }

pyxform/survey.py

@@ -192,6 +192,7 @@ class Survey(Section):
             "_xpath": dict,
             "_created": datetime.now,  # This can't be dumped to json
             "setvalues_by_triggering_ref": dict,
+            "setgeopoint_by_triggering_ref": dict,
             "title": str,
             "id_string": str,
             "sms_keyword": str,
@@ -227,7 +228,7 @@ def validate(self):
 
     def _validate_uniqueness_of_section_names(self):
         root_node_name = self.name
-        section_names = []
+        section_names = set()
         for element in self.iter_descendants():
             if isinstance(element, Section):
                 if element.name in section_names:
@@ -242,7 +243,7 @@ def _validate_uniqueness_of_section_names(self):
                         raise PyXFormError(msg)
                     msg = f"There are two sections with the name {element.name}."
                     raise PyXFormError(msg)
-                section_names.append(element.name)
+                section_names.add(element.name)
 
     def get_nsmap(self):
         """Add additional namespaces"""
@@ -297,8 +298,13 @@ def xml(self):
             **nsmap,
         )
 
-    def get_setvalues_for_question_name(self, question_name):
-        return self.setvalues_by_triggering_ref.get(f"${{{question_name}}}")
+    def get_trigger_values_for_question_name(self, question_name, trigger_type):
+        trigger_map = {
+            "setvalue": self.setvalues_by_triggering_ref,
+            "setgeopoint": self.setgeopoint_by_triggering_ref,
+        }
+
+        return trigger_map.get(trigger_type, {}).get(f"${{{question_name}}}")
 
     def _generate_static_instances(self, list_name, choice_list) -> InstanceInfo:
         """

pyxform/validators/pyxform/question_types.py

@@ -0,0 +1,45 @@
+"""
+Validations for question types.
+"""
+
+import re
+
+from pyxform.errors import PyXFormError
+from pyxform.parsing.expression import is_single_token_expression
+from pyxform.utils import PYXFORM_REFERENCE_REGEX
+
+BACKGROUND_GEOPOINT_CALCULATION = "[row : {r}] For 'background-geopoint' questions, the 'calculation' column must be empty."
+TRIGGER_INVALID = (
+    "[row : {r}] For '{t}' questions, the 'trigger' column must be a reference to another "
+    "question that exists, in the format ${{question_name_here}}."
+)
+
+
+def validate_background_geopoint_calculation(row: dict, row_num: int) -> bool:
+    """A background-geopoint must not have a calculation."""
+    try:
+        row["bind"]["calculate"]
+    except KeyError:
+        return True
+    else:
+        raise PyXFormError(BACKGROUND_GEOPOINT_CALCULATION.format(r=row_num))
+
+
+def validate_background_geopoint_trigger(row: dict, row_num: int) -> bool:
+    """A background-geopoint must have a trigger."""
+    if not row.get("trigger", False) or not is_single_token_expression(
+        expression=row["trigger"], token_types=["PYXFORM_REF"]
+    ):
+        raise PyXFormError(TRIGGER_INVALID.format(r=row_num, t=row["type"]))
+    return True
+
+
+def validate_references(referrers: list[tuple[dict, int]], questions: set[str]) -> bool:
+    """Triggers must refer to a question that exists."""
+    for row, row_num in referrers:
+        matches = re.match(PYXFORM_REFERENCE_REGEX, row["trigger"])
+        if matches is not None:
+            trigger = matches.groups()[0]
+            if trigger not in questions:
+                raise PyXFormError(TRIGGER_INVALID.format(r=row_num, t=row["type"]))
+    return True

pyxform/xls2json.py

@@ -24,6 +24,7 @@
 from pyxform.parsing.expression import is_single_token_expression
 from pyxform.utils import PYXFORM_REFERENCE_REGEX, coalesce, default_is_dynamic
 from pyxform.validators.pyxform import parameters_generic, select_from_file
+from pyxform.validators.pyxform import question_types as qt
 from pyxform.validators.pyxform.android_package_name import validate_android_package_name
 from pyxform.validators.pyxform.translations_checks import SheetTranslations
 from pyxform.xls2json_backends import csv_to_dict, xls_to_dict, xlsx_to_dict
@@ -697,9 +698,11 @@ def workbook_to_json(
 
     # Rows from the survey sheet that should be nested in meta
     survey_meta = []
+    # To check that questions with triggers refer to other questions that exist.
+    question_names = set()
+    trigger_references = []
 
-    # row by row, validate questions, throwing errors and adding warnings
-    # where needed.
+    # row by row, validate questions, throwing errors and adding warnings where needed.
     for row in survey_sheet.data:
         row_number += 1
         if stack[-1] is not None:
@@ -727,6 +730,7 @@ def workbook_to_json(
         # Get question type
         question_type = row.get(constants.TYPE)
         question_name = row.get(constants.NAME)
+        question_names.add(question_name)
 
         if not question_type:
             # if name and label are also missing,
@@ -1493,10 +1497,17 @@ def workbook_to_json(
             continue
         # TODO: Consider adding some question_type validation here.
 
+        # Ensure that
+        if question_type == "background-geopoint":
+            qt.validate_background_geopoint_trigger(row=row, row_num=row_number)
+            qt.validate_background_geopoint_calculation(row=row, row_num=row_number)
+            trigger_references.append((row, row_number))
+
         # Put the row in the json dict as is:
         parent_children_array.append(row)
 
     sheet_translations.or_other_check(warnings=warnings)
+    qt.validate_references(referrers=trigger_references, questions=question_names)
 
     if len(stack) != 1:
         raise PyXFormError(