diff --git a/pyxform/constants.py b/pyxform/constants.py index 7ee3befc..73c0ed65 100644 --- a/pyxform/constants.py +++ b/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"] diff --git a/pyxform/entities/entities_parsing.py b/pyxform/entities/entities_parsing.py index 7a629d17..f4c4a620 100644 --- a/pyxform/entities/entities_parsing.py +++ b/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( diff --git a/pyxform/entities/entity_declaration.py b/pyxform/entities/entity_declaration.py index bc55ea51..e86576d5 100644 --- a/pyxform/entities/entity_declaration.py +++ b/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() diff --git a/pyxform/question.py b/pyxform/question.py index 25a51932..1b53a0e2 100644 --- a/pyxform/question.py +++ b/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 ) diff --git a/pyxform/survey.py b/pyxform/survey.py index 19b69d06..242fbceb 100644 --- a/pyxform/survey.py +++ b/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: diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index 8ae77166..2831a82d 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -341,7 +341,7 @@ 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} @@ -349,7 +349,7 @@ def get_translations(self, default_language): # 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 diff --git a/pyxform/utils.py b/pyxform/utils.py index fc7fa6db..7786cb1c 100644 --- a/pyxform/utils.py +++ b/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 diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index a3a848a0..e1fe6cae 100644 --- a/pyxform/xls2json.py +++ b/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: diff --git a/tests/test_entities.py b/tests/test_entities_create.py similarity index 98% rename from tests/test_entities.py rename to tests/test_entities_create.py index 6127f616..83c40cb1 100644 --- a/tests/test_entities.py +++ b/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): diff --git a/tests/test_entities_update.py b/tests/test_entities_update.py new file mode 100644 index 00000000..4486f163 --- /dev/null +++ b/tests/test_entities_update.py @@ -0,0 +1,172 @@ +# -*- coding: utf-8 -*- +from tests.pyxform_test_case import PyxformTestCase + + +class EntitiesUpdateTest(PyxformTestCase): + def test_basic_entity_update_building_blocks(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | id | Tree id | + | | text | a | A | + | entities | | | | + | | dataset | entity_id | | + | | trees | ${id} | | + """, + xml__xpath_match=[ + "/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity", + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@dataset = "trees"]', + # defaults to always updating if an entity_id is specified + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@update = "1"]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@id = ""]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@id" and @type = "string" and @readonly = "true()" and @calculate = " /data/id "]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@baseVersion = ""]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@baseVersion" and @type = "string" and @readonly = "true()" and @calculate = "instance(\'trees\')/root/item[name= /data/id ]/__version"]', + '/h:html/h:head/x:model[@entities:entities-version = "2023.1.0"]', + ], + xml__xpath_count=[ + ("/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity/x:label", 0), + ("/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity/@create", 0), + ("/h:html/h:head/x:model/x:setvalue", 0), + ], + xml__contains=['xmlns:entities="http://www.opendatakit.org/xforms/entities"'], + ) + + def test_entity_id_with_creation_condition_only__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | id | Tree id | + | | text | a | A | + | entities | | | | + | | dataset | entity_id | create_if | + | | trees | ${id} | true() | + """, + errored=True, + error__contains=[ + "The entities sheet can't specify an entity creation condition and an entity_id without also including an update condition." + ], + ) + + def test_update_condition_without_entity_id__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | id | Tree id | + | | text | a | A | + | entities | | | | + | | dataset | update_if | | + | | trees | true() | | + """, + errored=True, + error__contains=[ + "The entities sheet is missing the entity_id column which is required when updating entities." + ], + ) + + def test_update_and_create_conditions_without_entity_id__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | id | Tree id | + | | integer | a | A | + | entities | | | | + | | dataset | update_if | create_if | + | | trees | ${id} != ''| ${id} = '' | + """, + errored=True, + error__contains=[ + "The entities sheet is missing the entity_id column which is required when updating entities." + ], + ) + + def test_create_if_with_entity_id_in_entities_sheet__puts_expression_on_bind(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | id | Tree id | + | | text | a | A | + | entities | | | | + | | dataset | update_if | entity_id | + | | trees | string-length(a) > 3 | ${id} | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@update" and @calculate = "string-length(a) > 3"]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@update = "1"]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@id" and @type = "string" and @readonly = "true()" and @calculate = " /data/id "]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@baseVersion = ""]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@baseVersion" and @type = "string" and @readonly = "true()" and @calculate = "instance(\'trees\')/root/item[name= /data/id ]/__version"]', + ], + xml__xpath_count=[("/h:html/h:head/x:model/x:setvalue", 0)], + ) + + def test_update_and_create_conditions_with_entity_id__puts_both_in_bind_calculations( + self, + ): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | | + | | text | id | Tree id | | + | | integer | a | A | | + | entities | | | | | + | | dataset | update_if | create_if | entity_id | + | | trees | id != '' | id = '' | ${id} | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@update" and @calculate = "id != \'\'"]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@update = "1"]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@create" and @calculate = "id = \'\'"]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]', + '/h:html/h:head/x:model/x:setvalue[@event = "odk-instance-first-load" and @type = "string" and @ref = "/data/meta/entity/@id" and @value = "uuid()"]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@id" and @type = "string" and @readonly = "true()" and @calculate = " /data/id "]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@baseVersion = ""]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@baseVersion" and @type = "string" and @readonly = "true()" and @calculate = "instance(\'trees\')/root/item[name= /data/id ]/__version"]', + ], + ) + + def test_entity_id_and_label__updates_label(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | id | Tree id | + | | text | a | A | + | entities | | | | + | | dataset | entity_id | label | + | | trees | ${id} | a | + """, + xml__xpath_match=[ + "/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity/x:label", + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/label" and @type = "string" and @readonly = "true()" and @calculate = "a"]', + ], + ) + + def test_save_to_with_entity_id__puts_save_tos_on_bind(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | id | Tree id | | + | | text | a | A | foo | + | entities | | | | | + | | dataset | entity_id | | | + | | trees | ${id} | | | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/a" and @entities:saveto = "foo"]' + ], + )