Merge pull request #664 from lognaturel/entity-update

Add support for entity updates

pyxform/constants.py

@@ -110,8 +110,9 @@
 CURRENT_XFORMS_VERSION = "1.0.0"
 
 # The ODK entities spec version that generated forms comply to
-CURRENT_ENTITIES_VERSION = "2022.1.0"
-ENTITY_RELATED = "entity_related"
+ENTITIES_CREATE_VERSION = "2022.1.0"
+CURRENT_ENTITIES_VERSION = "2023.1.0"
+ENTITY_FEATURES = "entity_features"
 ENTITIES_RESERVED_PREFIX = "__"
 
 DEPRECATED_DEVICE_ID_METADATA_FIELDS = ["subscriberid", "simserial"]

pyxform/entities/entities_parsing.py

@@ -20,7 +20,43 @@ def get_entity_declaration(
             "Currently, you can only declare a single entity per form. Please make sure your entities sheet only declares one entity."
         )
 
-    entity = entities_sheet[0]
+    entity_row = entities_sheet[0]
+
+    dataset_name = get_validated_dataset_name(entity_row)
+    entity_id = entity_row.get("entity_id", None)
+    create_condition = entity_row.get("create_if", None)
+    update_condition = entity_row.get("update_if", None)
+    entity_label = entity_row.get("label", None)
+
+    if update_condition and not entity_id:
+        raise PyXFormError(
+            "The entities sheet is missing the entity_id column which is required when updating entities."
+        )
+
+    if entity_id and create_condition and not update_condition:
+        raise PyXFormError(
+            "The entities sheet can't specify an entity creation condition and an entity_id without also including an update condition."
+        )
+
+    if not entity_id and not entity_label:
+        raise PyXFormError(
+            "The entities sheet is missing the label column which is required when creating entities."
+        )
+
+    return {
+        "name": "entity",
+        "type": "entity",
+        "parameters": {
+            "dataset": dataset_name,
+            "entity_id": entity_id,
+            "create": create_condition,
+            "update": update_condition,
+            "label": entity_label,
+        },
+    }
+
+
+def get_validated_dataset_name(entity):
     dataset = entity["dataset"]
 
     if dataset.startswith(constants.ENTITIES_RESERVED_PREFIX):
@@ -41,20 +77,7 @@ def get_entity_declaration(
             f"Invalid entity list name: '{dataset}'. Names must begin with a letter, colon, or underscore. Other characters can include numbers or dashes."
         )
 
-    if not ("label" in entity):
-        raise PyXFormError("The entities sheet is missing the required label column.")
-
-    creation_condition = entity["create_if"] if "create_if" in entity else "1"
-
-    return {
-        "name": "entity",
-        "type": "entity",
-        "parameters": {
-            "dataset": dataset,
-            "create": creation_condition,
-            "label": entity["label"],
-        },
-    }
+    return dataset
 
 
 def validate_entity_saveto(

pyxform/entities/entity_declaration.py

@@ -5,46 +5,108 @@
 
 
 class EntityDeclaration(SurveyElement):
+    """
+    An entity declaration includes an entity instance node with optional label child, some attributes, and corresponding bindings.
+
+    The ODK XForms Entities specification can be found at https://getodk.github.io/xforms-spec/entities
+
+    XLSForm uses a combination of the entity_id, create_if and update_if columns to determine what entity action is intended:
+        id    create  update  result
+        1     0       0       always update
+        1     0       1       update based on condition
+        1     1       0       error, id only acceptable when updating
+        1     1       1       include conditions for create and update, user's responsibility to make sure they're exclusive
+        0     0       0       always create
+        0     0       1       error, need id to update
+        0     1       0       create based on condition
+        0     1       1       error, need id to update
+    """
+
     def xml_instance(self, **kwargs):
         attributes = {}
         attributes["dataset"] = self.get("parameters", {}).get("dataset", "")
-        attributes["create"] = "1"
         attributes["id"] = ""
 
-        label_node = node("label")
-        return node("entity", label_node, **attributes)
+        entity_id_expression = self.get("parameters", {}).get("entity_id", None)
+        create_condition = self.get("parameters", {}).get("create", None)
+        update_condition = self.get("parameters", {}).get("update", None)
+
+        if entity_id_expression:
+            attributes["update"] = "1"
+            attributes["baseVersion"] = ""
+
+        if create_condition or (not update_condition and not entity_id_expression):
+            attributes["create"] = "1"
+
+        if self.get("parameters", {}).get("label", None):
+            return node("entity", node("label"), **attributes)
+        else:
+            return node("entity", **attributes)
 
     def xml_bindings(self):
+        """
+        See the class comment for an explanation of the logic for generating bindings.
+        """
         survey = self.get_root()
+        entity_id_expression = self.get("parameters", {}).get("entity_id", None)
+        create_condition = self.get("parameters", {}).get("create", None)
+        update_condition = self.get("parameters", {}).get("update", None)
+        label_expression = self.get("parameters", {}).get("label", None)
 
-        create_expr = survey.insert_xpaths(
-            self.get("parameters", {}).get("create", "true()"), context=self
-        )
-        create_bind = {
-            "calculate": create_expr,
-            "type": "string",
-            "readonly": "true()",
-        }
-        create_node = node("bind", nodeset=self.get_xpath() + "/@create", **create_bind)
+        bind_nodes = []
+
+        if create_condition:
+            bind_nodes.append(self._get_bind_node(survey, create_condition, "/@create"))
+
+        bind_nodes.append(self._get_id_bind_node(survey, entity_id_expression))
 
+        if create_condition or not entity_id_expression:
+            bind_nodes.append(self._get_id_setvalue_node())
+
+        if update_condition:
+            bind_nodes.append(self._get_bind_node(survey, update_condition, "/@update"))
+
+        if entity_id_expression:
+            dataset_name = self.get("parameters", {}).get("dataset", "")
+            base_version_expression = f"instance('{dataset_name}')/root/item[name={entity_id_expression}]/__version"
+            bind_nodes.append(
+                self._get_bind_node(survey, base_version_expression, "/@baseVersion")
+            )
+
+        if label_expression:
+            bind_nodes.append(self._get_bind_node(survey, label_expression, "/label"))
+
+        return bind_nodes
+
+    def _get_id_bind_node(self, survey, entity_id_expression):
         id_bind = {"type": "string", "readonly": "true()"}
-        id_node = node("bind", nodeset=self.get_xpath() + "/@id", **id_bind)
 
+        if entity_id_expression:
+            id_bind["calculate"] = survey.insert_xpaths(
+                entity_id_expression, context=self
+            )
+
+        return node("bind", nodeset=self.get_xpath() + "/@id", **id_bind)
+
+    def _get_id_setvalue_node(self):
         id_setvalue_attrs = {
             "event": "odk-instance-first-load",
             "type": "string",
             "readonly": "true()",
             "value": "uuid()",
         }
-        id_setvalue = node("setvalue", ref=self.get_xpath() + "/@id", **id_setvalue_attrs)
 
-        label_expr = survey.insert_xpaths(
-            self.get("parameters", {}).get("label", ""), context=self
-        )
-        label_bind = {
-            "calculate": label_expr,
+        return node("setvalue", ref=self.get_xpath() + "/@id", **id_setvalue_attrs)
+
+    def _get_bind_node(self, survey, expression, destination):
+        expr = survey.insert_xpaths(expression, context=self)
+        bind_attrs = {
+            "calculate": expr,
             "type": "string",
             "readonly": "true()",
         }
-        label_node = node("bind", nodeset=self.get_xpath() + "/label", **label_bind)
-        return [create_node, id_node, id_setvalue, label_node]
+
+        return node("bind", nodeset=self.get_xpath() + destination, **bind_attrs)
+
+    def xml_control(self):
+        raise NotImplementedError()

pyxform/question.py

@@ -74,7 +74,7 @@ def nest_setvalues(self, xml_node):
                     .strip(),
                     "event": "xforms-value-changed",
                 }
-                if not (setvalue[1] == ""):
+                if not setvalue[1] == "":
                     setvalue_attrs["value"] = self.get_root().insert_xpaths(
                         setvalue[1], self
                     )

pyxform/survey.py

@@ -185,7 +185,7 @@ class Survey(Section):
             "style": str,
             "attribute": dict,
             "namespaces": str,
-            constants.ENTITY_RELATED: str,
+            constants.ENTITY_FEATURES: list,
         }
     )  # yapf: disable
 
@@ -218,7 +218,7 @@ def _validate_uniqueness_of_section_names(self):
     def get_nsmap(self):
         """Add additional namespaces"""
         namespaces = getattr(self, constants.NAMESPACES, "")
-        if getattr(self, constants.ENTITY_RELATED, "false") == "true":
+        if len(getattr(self, constants.ENTITY_FEATURES, [])) > 0:
             namespaces += " entities=http://www.opendatakit.org/xforms/entities"
 
         if namespaces and isinstance(namespaces, str):
@@ -250,7 +250,7 @@ def xml(self):
         self._setup_xpath_dictionary()
 
         for triggering_reference in self.setvalues_by_triggering_ref.keys():
-            if not (re.search(BRACKETED_TAG_REGEX, triggering_reference)):
+            if not re.search(BRACKETED_TAG_REGEX, triggering_reference):
                 raise PyXFormError(
                     "Only references to other fields are allowed in the 'trigger' column."
                 )
@@ -564,8 +564,17 @@ def xml_model(self):
         self._add_empty_translations()
 
         model_kwargs = {"odk:xforms-version": constants.CURRENT_XFORMS_VERSION}
-        if getattr(self, constants.ENTITY_RELATED, "false") == "true":
-            model_kwargs["entities:entities-version"] = constants.CURRENT_ENTITIES_VERSION
+
+        entity_features = getattr(self, constants.ENTITY_FEATURES, [])
+        if len(entity_features) > 0:
+            if "update" in entity_features:
+                model_kwargs[
+                    "entities:entities-version"
+                ] = constants.CURRENT_ENTITIES_VERSION
+            else:
+                model_kwargs[
+                    "entities:entities-version"
+                ] = constants.ENTITIES_CREATE_VERSION
 
         model_children = []
         if self._translations:

pyxform/survey_element.py

@@ -341,15 +341,15 @@ def get_translations(self, default_language):
             # how they're defined - https://opendatakit.github.io/xforms-spec/#languages
             if (
                 display_element == "guidance_hint"
-                and not (isinstance(label_or_hint, dict))
+                and not isinstance(label_or_hint, dict)
                 and len(label_or_hint) > 0
             ):
                 label_or_hint = {default_language: label_or_hint}
 
             # always use itext for hint if there's a guidance hint
             if (
                 display_element == "hint"
-                and not (isinstance(label_or_hint, dict))
+                and not isinstance(label_or_hint, dict)
                 and len(label_or_hint) > 0
                 and "guidance_hint" in self.keys()
                 and len(self["guidance_hint"]) > 0

pyxform/utils.py

@@ -238,7 +238,7 @@ def get_languages_with_bad_tags(languages):
         lang_code = re.search(lang_code_regex, lang)
 
         if lang != "default" and (
-            not (lang_code) or not (lang_code.group(1) in iana_subtags)
+            not lang_code or not lang_code.group(1) in iana_subtags
         ):
             languages_with_bad_tags.append(lang)
     return languages_with_bad_tags

pyxform/xls2json.py

@@ -704,7 +704,7 @@ def workbook_to_json(
         if not question_type:
             # if name and label are also missing,
             # then its a comment row, and we skip it with warning
-            if not ((constants.NAME in row) or (constants.LABEL in row)):
+            if not (constants.NAME in row or constants.LABEL in row):
                 warnings.append(
                     ROW_FORMAT_STRING % row_number
                     + " Row without name, text, or label is being skipped:\n"
@@ -1482,7 +1482,11 @@ def workbook_to_json(
         )
 
     if len(entity_declaration) > 0:
-        json_dict[constants.ENTITY_RELATED] = "true"
+        json_dict[constants.ENTITY_FEATURES] = ["create"]
+
+        if entity_declaration.get("parameters", {}).get("entity_id", None):
+            json_dict[constants.ENTITY_FEATURES].append("update")
+
         meta_children.append(entity_declaration)
 
     if len(meta_children) > 0:

tests/test_entities.py → tests/test_entities_create.py

@@ -2,7 +2,7 @@
 from tests.pyxform_test_case import PyxformTestCase
 
 
-class EntitiesTest(PyxformTestCase):
+class EntitiesCreationTest(PyxformTestCase):
     def test_basic_entity_creation_building_blocks(self):
         self.assertPyxformXform(
             name="data",
@@ -26,6 +26,9 @@ def test_basic_entity_creation_building_blocks(self):
                 '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/label" and @type = "string" and @readonly = "true()" and @calculate = "a"]',
                 '/h:html/h:head/x:model[@entities:entities-version = "2022.1.0"]',
             ],
+            xml__xpath_count=[
+                ("/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity/@update", 0),
+            ],
             xml__contains=['xmlns:entities="http://www.opendatakit.org/xforms/entities"'],
         )
 
@@ -160,7 +163,9 @@ def test_entity_label__required(self):
             |          | trees   |       |       |
             """,
             errored=True,
-            error__contains=["The entities sheet is missing the required label column."],
+            error__contains=[
+                "The entities sheet is missing the label column which is required when creating entities."
+            ],
         )
 
     def test_entities_namespace__omitted_if_no_entities_sheet(self):