From f5b95ec8ce1cff8d63d5b44023563324daa99797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Mon, 14 Nov 2022 15:23:42 -0800 Subject: [PATCH 01/17] Add initial support for entities --- pyxform/constants.py | 2 ++ pyxform/xls2json.py | 16 ++++++++++++++++ tests/test_entities.py | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 tests/test_entities.py diff --git a/pyxform/constants.py b/pyxform/constants.py index a9261fce..19c5c84a 100644 --- a/pyxform/constants.py +++ b/pyxform/constants.py @@ -68,6 +68,7 @@ SURVEY = "survey" SETTINGS = "settings" EXTERNAL_CHOICES = "external_choices" +ENTITIES = "entities" OSM = "osm" OSM_TYPE = "binary" @@ -81,6 +82,7 @@ SETTINGS, EXTERNAL_CHOICES, OSM, + ENTITIES, ] XLS_EXTENSIONS = [".xls"] XLSX_EXTENSIONS = [".xlsx", ".xlsm"] diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 282eb15a..c65de14a 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -345,6 +345,18 @@ def find_sheet_misspellings(key: str, keys: "KeysView") -> "Optional[str]": return None +def get_entity_declaration(workbook_dict: Dict) -> Dict: + entities_sheet = workbook_dict.get(constants.ENTITIES, []) + if len(entities_sheet) == 0: + return [] + elif len(entities_sheet) > 1: + raise PyXFormError( + "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." + ) + + return {"name": "entity", "type": "calculate"} + + def workbook_to_json( workbook_dict, form_name=None, @@ -1401,6 +1413,10 @@ def workbook_to_json( } ) + entity_declaration = get_entity_declaration(workbook_dict) + if len(entity_declaration) > 0: + meta_children.append(entity_declaration) + if len(meta_children) > 0: meta_element = { "name": "meta", diff --git a/tests/test_entities.py b/tests/test_entities.py new file mode 100644 index 00000000..51b5a4f8 --- /dev/null +++ b/tests/test_entities.py @@ -0,0 +1,36 @@ +# -*- coding: utf-8 -*- +from tests.pyxform_test_case import PyxformTestCase + + +class EntitiesTest(PyxformTestCase): + def test_dataset_in_entities_sheet__adds_meta_entity_block(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | | | + | | trees | | | + """, + xml__xpath_match=["/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity"], + ) + + def test_multiple_dataset_rows_in_entities_sheet__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | | | + | | trees | | | + | | shovels | | | + """, + errored=True, + error__contains=[ + "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." + ], + ) From 3df7231a08ffe627c761db46f0a5437fe8a945c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 08:41:13 -0800 Subject: [PATCH 02/17] Add dataset attribute --- pyxform/builder.py | 4 ++++ pyxform/entity_declaration.py | 20 ++++++++++++++++++++ pyxform/xls2json.py | 6 +++++- tests/test_entities.py | 16 ++++++++++++++++ 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 pyxform/entity_declaration.py diff --git a/pyxform/builder.py b/pyxform/builder.py index 38ca086d..cf2f1ea1 100644 --- a/pyxform/builder.py +++ b/pyxform/builder.py @@ -7,6 +7,7 @@ import re from pyxform import file_utils, utils +from pyxform.entity_declaration import EntityDeclaration from pyxform.errors import PyXFormError from pyxform.external_instance import ExternalInstance from pyxform.question import ( @@ -94,6 +95,7 @@ def create_survey_element_from_dict(self, d): """ if "add_none_option" in d: self._add_none_option = d["add_none_option"] + if d["type"] in self.SECTION_CLASSES: section = self._create_section_from_dict(d) @@ -116,6 +118,8 @@ def create_survey_element_from_dict(self, d): return full_survey.children elif d["type"] in ["xml-external", "csv-external"]: return ExternalInstance(**d) + elif d["type"] == "entity": + return EntityDeclaration(**d) else: self._save_trigger_as_setvalue_and_remove_calculate(d) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py new file mode 100644 index 00000000..f783e4a0 --- /dev/null +++ b/pyxform/entity_declaration.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- + +from pyxform.survey_element import SurveyElement +from pyxform.utils import node + + +class EntityDeclaration(SurveyElement): + def xml_instance(self, **kwargs): + attributes = {} + attributes["dataset"] = self.get("parameters", {}).get("dataset", "") + + return node("entity", **attributes) + + def xml_control(self): + """ + No-op since there is no associated form control to place under . + + Exists here because there's a soft abstractmethod in SurveyElement. + """ + pass diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index c65de14a..26025489 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -354,7 +354,11 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." ) - return {"name": "entity", "type": "calculate"} + return { + "name": "entity", + "type": "entity", + "parameters": {"dataset": entities_sheet[0]["dataset"]}, + } def workbook_to_json( diff --git a/tests/test_entities.py b/tests/test_entities.py index 51b5a4f8..d2d04c4b 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -34,3 +34,19 @@ def test_multiple_dataset_rows_in_entities_sheet__errors(self): "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." ], ) + + def test_dataset_in_entities_sheet__adds_dataset_attribute_to_entity(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | | | + | | trees | | | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@dataset = "trees"]' + ], + ) From e718aed19c2c983f73e6f8baf8820ae135d85b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 10:50:41 -0800 Subject: [PATCH 03/17] Default to always creating --- pyxform/entity_declaration.py | 9 +-------- pyxform/xls2json.py | 8 +++++++- tests/test_entities.py | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index f783e4a0..adfb58bf 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -8,13 +8,6 @@ class EntityDeclaration(SurveyElement): def xml_instance(self, **kwargs): attributes = {} attributes["dataset"] = self.get("parameters", {}).get("dataset", "") + attributes["create"] = self.get("parameters", {}).get("create", "1") return node("entity", **attributes) - - def xml_control(self): - """ - No-op since there is no associated form control to place under . - - Exists here because there's a soft abstractmethod in SurveyElement. - """ - pass diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 26025489..33567b26 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -353,11 +353,17 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: raise PyXFormError( "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." ) + entity = entities_sheet[0] + + creation_condition = entities_sheet[0]["create_if"] if "create_if" in entity else "1" return { "name": "entity", "type": "entity", - "parameters": {"dataset": entities_sheet[0]["dataset"]}, + "parameters": { + "dataset": entities_sheet[0]["dataset"], + "create": creation_condition, + }, } diff --git a/tests/test_entities.py b/tests/test_entities.py index d2d04c4b..1a736c7b 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -50,3 +50,19 @@ def test_dataset_in_entities_sheet__adds_dataset_attribute_to_entity(self): '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@dataset = "trees"]' ], ) + + def test_dataset_in_entities_sheet__defaults_to_always_creating(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | | | + | | trees | | | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]' + ], + ) From ab09eff6852ff3babd470f8b00aa6e0dff9fa6b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 11:19:59 -0800 Subject: [PATCH 04/17] Use entity create_if expression on bind --- pyxform/entity_declaration.py | 6 +++++- pyxform/survey.py | 2 +- pyxform/survey_element.py | 14 +++++++------- tests/j2x_question_tests.py | 14 +++++++------- tests/test_entities.py | 17 +++++++++++++++++ 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index adfb58bf..87e3b089 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -8,6 +8,10 @@ class EntityDeclaration(SurveyElement): def xml_instance(self, **kwargs): attributes = {} attributes["dataset"] = self.get("parameters", {}).get("dataset", "") - attributes["create"] = self.get("parameters", {}).get("create", "1") + attributes["create"] = "1" return node("entity", **attributes) + + def xml_bindings(self): + bind_dict = {"calculate": self.get("parameters", {}).get("create", "true()")} + return [node("bind", nodeset=self.get_xpath() + "/@create", **bind_dict)] diff --git a/pyxform/survey.py b/pyxform/survey.py index 90a91d59..bee37241 100644 --- a/pyxform/survey.py +++ b/pyxform/survey.py @@ -556,7 +556,7 @@ def xml_model(self): model_children.append(self.itext()) model_children += [node("instance", self.xml_instance())] model_children += list(self._generate_instances()) - model_children += self.xml_bindings() + model_children += self.xml_descendent_bindings() model_children += self.xml_actions() if self.submission_url or self.public_key or self.auto_send or self.auto_delete: diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index 99091422..c003c610 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -438,9 +438,9 @@ def xml_label_and_hint(self) -> "List[DetachableElement]": return result - def xml_binding(self): + def xml_bindings(self): """ - Return the binding for this survey element. + Return the binding(s) for this survey element. """ survey = self.get_root() bind_dict = self.bind.copy() @@ -472,18 +472,18 @@ def xml_binding(self): if k == "jr:noAppErrorString" and type(v) is dict: v = "jr:itext('%s')" % self._translation_path("jr:noAppErrorString") bind_dict[k] = survey.insert_xpaths(v, context=self) - return node("bind", nodeset=self.get_xpath(), **bind_dict) + return [node("bind", nodeset=self.get_xpath(), **bind_dict)] return None - def xml_bindings(self): + def xml_descendent_bindings(self): """ Return a list of bindings for this node and all its descendants. """ result = [] for e in self.iter_descendants(): - xml_binding = e.xml_binding() - if xml_binding is not None: - result.append(xml_binding) + xml_bindings = e.xml_bindings() + if xml_bindings is not None: + result.extend(xml_bindings) # dynamic defaults for repeats go in the body. All other dynamic defaults (setvalue actions) go in the model if ( diff --git a/tests/j2x_question_tests.py b/tests/j2x_question_tests.py index 30b4e9d6..693928c5 100644 --- a/tests/j2x_question_tests.py +++ b/tests/j2x_question_tests.py @@ -55,7 +55,7 @@ def test_question_type_string(self): self.assertEqual(ctw(q.xml_control()), expected_string_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_string_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_string_binding_xml) def test_select_one_question_multilingual(self): """ @@ -86,7 +86,7 @@ def test_select_one_question_multilingual(self): self.assertEqual(ctw(q.xml_control()), expected_select_one_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_select_one_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_select_one_binding_xml) def test_simple_integer_question_type_multilingual(self): """ @@ -114,7 +114,7 @@ def test_simple_integer_question_type_multilingual(self): self.assertEqual(ctw(q.xml_control()), expected_integer_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_integer_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_integer_binding_xml) def test_simple_date_question_type_multilingual(self): """ @@ -140,7 +140,7 @@ def test_simple_date_question_type_multilingual(self): self.assertEqual(ctw(q.xml_control()), expected_date_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_date_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_date_binding_xml) def test_simple_phone_number_question_type_multilingual(self): """ @@ -165,7 +165,7 @@ def test_simple_phone_number_question_type_multilingual(self): self.assertEqual(ctw(q.xml_control()), expected_phone_number_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_phone_number_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_phone_number_binding_xml) def test_simple_select_all_question_multilingual(self): """ @@ -195,7 +195,7 @@ def test_simple_select_all_question_multilingual(self): self.assertEqual(ctw(q.xml_control()), expected_select_all_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_select_all_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_select_all_binding_xml) def test_simple_decimal_question_multilingual(self): """ @@ -221,4 +221,4 @@ def test_simple_decimal_question_multilingual(self): self.assertEqual(ctw(q.xml_control()), expected_decimal_control_xml) if TESTING_BINDINGS: - self.assertEqual(ctw(q.xml_binding()), expected_decimal_binding_xml) + self.assertEqual(ctw(q.xml_bindings()), expected_decimal_binding_xml) diff --git a/tests/test_entities.py b/tests/test_entities.py index 1a736c7b..6743af16 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -66,3 +66,20 @@ def test_dataset_in_entities_sheet__defaults_to_always_creating(self): '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]' ], ) + + def test_create_if_in_entities_sheet__puts_expression_on_bind(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | create_if | | + | | trees | string-length(a) > 3 | | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@create" and @calculate = "string-length(a) > 3"]', + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]' + ], + ) \ No newline at end of file From 217d21921f57e6bee5c34f347ec33d9ed870dfa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 12:42:23 -0800 Subject: [PATCH 05/17] Add entities id --- pyxform/entity_declaration.py | 21 +++++++++++++++++++-- tests/test_entities.py | 22 ++++++++++++++++++++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index 87e3b089..13144b47 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -9,9 +9,26 @@ def xml_instance(self, **kwargs): attributes = {} attributes["dataset"] = self.get("parameters", {}).get("dataset", "") attributes["create"] = "1" + attributes["id"] = "" return node("entity", **attributes) def xml_bindings(self): - bind_dict = {"calculate": self.get("parameters", {}).get("create", "true()")} - return [node("bind", nodeset=self.get_xpath() + "/@create", **bind_dict)] + create_bind = { + "calculate": self.get("parameters", {}).get("create", "true()"), + "type": "string", + "readonly": "true()", + } + create_node = node("bind", nodeset=self.get_xpath() + "/@create", **create_bind) + + id_bind = {"type": "string", "readonly": "true()"} + id_node = node("bind", nodeset=self.get_xpath() + "/@id", **id_bind) + + 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) + return [create_node, id_node, id_setvalue] diff --git a/tests/test_entities.py b/tests/test_entities.py index 6743af16..10df6286 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -80,6 +80,24 @@ def test_create_if_in_entities_sheet__puts_expression_on_bind(self): """, xml__xpath_match=[ '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@create" and @calculate = "string-length(a) > 3"]', - '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]' + '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]', + ], + ) + + def test_dataset_in_entities_sheet__adds_id_attribute_and_model_nodes_to_entity(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | | | + | | trees | | | + """, + xml__xpath_match=[ + '/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()"]', + '/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()"]', ], - ) \ No newline at end of file + ) From 5af9c964bb939bd43643e39008fc5bc75afd4d51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 12:57:48 -0800 Subject: [PATCH 06/17] Add entities label --- pyxform/entity_declaration.py | 12 ++++++++++-- pyxform/xls2json.py | 4 +++- tests/test_entities.py | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index 13144b47..674ee40f 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -11,7 +11,8 @@ def xml_instance(self, **kwargs): attributes["create"] = "1" attributes["id"] = "" - return node("entity", **attributes) + label_node = node("label") + return node("entity", label_node, **attributes) def xml_bindings(self): create_bind = { @@ -31,4 +32,11 @@ def xml_bindings(self): "value": "uuid()", } id_setvalue = node("setvalue", ref=self.get_xpath() + "/@id", **id_setvalue_attrs) - return [create_node, id_node, id_setvalue] + + label_bind = { + "calculate": self.get("parameters", {}).get("label", ""), + "type": "string", + "readonly": "true()", + } + label_node = node("bind", nodeset=self.get_xpath() + "/label", **label_bind) + return [create_node, id_node, id_setvalue, label_node] diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 33567b26..a02c6798 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -355,7 +355,8 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: ) entity = entities_sheet[0] - creation_condition = entities_sheet[0]["create_if"] if "create_if" in entity else "1" + creation_condition = entity["create_if"] if "create_if" in entity else "1" + label = entity["label"] if "label" in entity else "" return { "name": "entity", @@ -363,6 +364,7 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: "parameters": { "dataset": entities_sheet[0]["dataset"], "create": creation_condition, + "label": label, }, } diff --git a/tests/test_entities.py b/tests/test_entities.py index 10df6286..3122d492 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -101,3 +101,20 @@ def test_dataset_in_entities_sheet__adds_id_attribute_and_model_nodes_to_entity( '/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()"]', ], ) + + def test_label_in_entities_sheet__adds_label_and_bind_to_entity(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | 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"]', + ], + ) \ No newline at end of file From ab088872540fd1647e9e6ee6f789925385ddc4af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 13:11:48 -0800 Subject: [PATCH 07/17] Support `${}` notation for label and create_if --- pyxform/entity_declaration.py | 12 ++++++++++-- tests/test_entities.py | 21 +++++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index 674ee40f..bc55ea51 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -15,8 +15,13 @@ def xml_instance(self, **kwargs): return node("entity", label_node, **attributes) def xml_bindings(self): + survey = self.get_root() + + create_expr = survey.insert_xpaths( + self.get("parameters", {}).get("create", "true()"), context=self + ) create_bind = { - "calculate": self.get("parameters", {}).get("create", "true()"), + "calculate": create_expr, "type": "string", "readonly": "true()", } @@ -33,8 +38,11 @@ def xml_bindings(self): } 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": self.get("parameters", {}).get("label", ""), + "calculate": label_expr, "type": "string", "readonly": "true()", } diff --git a/tests/test_entities.py b/tests/test_entities.py index 3122d492..50ef8f9b 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -114,7 +114,24 @@ def test_label_in_entities_sheet__adds_label_and_bind_to_entity(self): | | trees | 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: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"]', ], - ) \ No newline at end of file + ) + + def test_label_and_create_if_in_entities_sheet__expand_node_selectors_to_xpath(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | create_if | + | | trees | ${a} | string-length(${a}) > 3 | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@create" and @calculate = "string-length( /data/a ) > 3"]', + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/label" and @type = "string" and @readonly = "true()" and @calculate = " /data/a "]', + ], + ) From cd8d35794503add20c86e9efda8684fb694d6437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 13:20:59 -0800 Subject: [PATCH 08/17] Require the label column --- pyxform/xls2json.py | 6 ++-- tests/test_entities.py | 77 +++++++++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index a02c6798..cfb4eac2 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -355,8 +355,10 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: ) entity = entities_sheet[0] + 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" - label = entity["label"] if "label" in entity else "" return { "name": "entity", @@ -364,7 +366,7 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: "parameters": { "dataset": entities_sheet[0]["dataset"], "create": creation_condition, - "label": label, + "label": entity["label"], }, } diff --git a/tests/test_entities.py b/tests/test_entities.py index 50ef8f9b..97f991cd 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -7,12 +7,12 @@ def test_dataset_in_entities_sheet__adds_meta_entity_block(self): self.assertPyxformXform( name="data", md=""" - | survey | | | | - | | type | name | label | - | | text | a | A | - | entities | | | | - | | dataset | | | - | | trees | | | + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | a | | """, xml__xpath_match=["/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity"], ) @@ -39,12 +39,12 @@ def test_dataset_in_entities_sheet__adds_dataset_attribute_to_entity(self): self.assertPyxformXform( name="data", md=""" - | survey | | | | - | | type | name | label | - | | text | a | A | - | entities | | | | - | | dataset | | | - | | trees | | | + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | a | | """, xml__xpath_match=[ '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@dataset = "trees"]' @@ -55,12 +55,12 @@ def test_dataset_in_entities_sheet__defaults_to_always_creating(self): self.assertPyxformXform( name="data", md=""" - | survey | | | | - | | type | name | label | - | | text | a | A | - | entities | | | | - | | dataset | | | - | | trees | | | + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | a | | """, xml__xpath_match=[ '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@create = "1"]' @@ -75,8 +75,8 @@ def test_create_if_in_entities_sheet__puts_expression_on_bind(self): | | type | name | label | | | text | a | A | | entities | | | | - | | dataset | create_if | | - | | trees | string-length(a) > 3 | | + | | dataset | create_if | label | + | | trees | string-length(a) > 3 | a | """, xml__xpath_match=[ '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/@create" and @calculate = "string-length(a) > 3"]', @@ -88,12 +88,12 @@ def test_dataset_in_entities_sheet__adds_id_attribute_and_model_nodes_to_entity( self.assertPyxformXform( name="data", md=""" - | survey | | | | - | | type | name | label | - | | text | a | A | - | entities | | | | - | | dataset | | | - | | trees | | | + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | a | | """, xml__xpath_match=[ '/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[@id = ""]', @@ -123,11 +123,11 @@ def test_label_and_create_if_in_entities_sheet__expand_node_selectors_to_xpath(s self.assertPyxformXform( name="data", md=""" - | survey | | | | - | | type | name | label | - | | text | a | A | - | entities | | | | - | | dataset | label | create_if | + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | create_if | | | trees | ${a} | string-length(${a}) > 3 | """, xml__xpath_match=[ @@ -135,3 +135,18 @@ def test_label_and_create_if_in_entities_sheet__expand_node_selectors_to_xpath(s '/h:html/h:head/x:model/x:bind[@nodeset = "/data/meta/entity/label" and @type = "string" and @readonly = "true()" and @calculate = " /data/a "]', ], ) + + def test_entity_label__required(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | | | + | | trees | | | + """, + errored=True, + error__contains=["The entities sheet is missing the required label column."], + ) From f3e309211394006c300f9d98cbc0d739804e5a90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 14:14:50 -0800 Subject: [PATCH 09/17] Add entities versioning and namespace --- pyxform/constants.py | 4 ++++ pyxform/survey.py | 7 +++++- pyxform/xls2json.py | 1 + tests/test_entities.py | 50 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pyxform/constants.py b/pyxform/constants.py index 19c5c84a..c2652970 100644 --- a/pyxform/constants.py +++ b/pyxform/constants.py @@ -101,6 +101,10 @@ # The ODK XForms version that generated forms comply to 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" + DEPRECATED_DEVICE_ID_METADATA_FIELDS = ["subscriberid", "simserial"] AUDIO_QUALITY_VOICE_ONLY = "voice-only" diff --git a/pyxform/survey.py b/pyxform/survey.py index bee37241..bbbf26a6 100644 --- a/pyxform/survey.py +++ b/pyxform/survey.py @@ -173,6 +173,7 @@ class Survey(Section): "style": str, "attribute": dict, "namespaces": str, + constants.ENTITY_RELATED: str, } ) # yapf: disable @@ -204,7 +205,9 @@ def _validate_uniqueness_of_section_names(self): def get_nsmap(self): """Add additional namespaces""" - namespaces = getattr(self, constants.NAMESPACES, None) + namespaces = getattr(self, constants.NAMESPACES, "") + if getattr(self, constants.ENTITY_RELATED, "false") == "true": + namespaces += " entities=http://www.opendatakit.org/xforms/entities" if namespaces and isinstance(namespaces, str): nslist = [ @@ -550,6 +553,8 @@ 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 model_children = [] if self._translations: diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index cfb4eac2..5953b833 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -1429,6 +1429,7 @@ def workbook_to_json( entity_declaration = get_entity_declaration(workbook_dict) if len(entity_declaration) > 0: + json_dict[constants.ENTITY_RELATED] = "true" meta_children.append(entity_declaration) if len(meta_children) > 0: diff --git a/tests/test_entities.py b/tests/test_entities.py index 97f991cd..4d5e49a9 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -150,3 +150,53 @@ def test_entity_label__required(self): errored=True, error__contains=["The entities sheet is missing the required label column."], ) + + def test_dataset_in_entities_sheet__adds_entities_namespace(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | a | | + """, + xml__contains=["xmlns:entities=\"http://www.opendatakit.org/xforms/entities\""], + ) + + def test_entities_namespace__omitted_if_no_entities_sheet(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + """, + xml__excludes=["xmlns:entities=\"http://www.opendatakit.org/xforms/entities\""], + ) + + def test_dataset_in_entities_sheet__adds_entities_version(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | trees | a | | + """, + xml__xpath_match=['/h:html/h:head/x:model[@entities:entities-version = "2022.1.0"]'] + ) + + def test_entities_version__omitted_if_no_entities_sheet(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + """, + xml__excludes=["entities:entities-version = \"2022.1.0\""], + ) \ No newline at end of file From 20a4c3f106b03fa3ab4b18465428052c563dd7de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 15 Nov 2022 14:55:17 -0800 Subject: [PATCH 10/17] Take binding out of array Unclear to me whether these tests verify an API that's intended to be public or just represent an earlier attempt at unit testing. This may point to an issue with the Survey.xml_bindings change. --- pyxform/entity_declaration.py | 2 +- tests/j2x_question_tests.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index bc55ea51..53851807 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -47,4 +47,4 @@ def xml_bindings(self): "readonly": "true()", } label_node = node("bind", nodeset=self.get_xpath() + "/label", **label_bind) - return [create_node, id_node, id_setvalue, label_node] + return [create_node, id_node, id_setvalue, label_node] \ No newline at end of file diff --git a/tests/j2x_question_tests.py b/tests/j2x_question_tests.py index 693928c5..bc242ade 100644 --- a/tests/j2x_question_tests.py +++ b/tests/j2x_question_tests.py @@ -16,6 +16,8 @@ def ctw(control): ctw stands for control_test_wrap, but ctw is shorter and easier. using begin_str and end_str to take out the wrap that xml gives us """ + if isinstance(control, list) and len(control) == 1: + control = control[0] return control.toxml() From dbd9faef22fb51a73c8ddf28c022b973908fdab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 16 Nov 2022 13:25:12 -0800 Subject: [PATCH 11/17] Add support and validation for save_to column --- pyxform/aliases.py | 1 + pyxform/constants.py | 5 +- pyxform/entity_declaration.py | 2 +- pyxform/survey_element.py | 16 +++-- pyxform/xls2json.py | 51 +++++++++++--- tests/test_entities.py | 128 ++++++++++++++++++++++++++++++++-- tests/test_sheet_columns.py | 7 +- 7 files changed, 183 insertions(+), 27 deletions(-) diff --git a/pyxform/aliases.py b/pyxform/aliases.py index f33523f5..2d24fd53 100644 --- a/pyxform/aliases.py +++ b/pyxform/aliases.py @@ -105,6 +105,7 @@ "required message": "bind::jr:requiredMsg", "body": "control", "parameters": "parameters", + constants.ENTITIES_SAVETO: "bind::entities:saveto", } # Key is the pyxform internal name, Value is the name used in error/warning messages. TRANSLATABLE_SURVEY_COLUMNS = { diff --git a/pyxform/constants.py b/pyxform/constants.py index c2652970..546b51e8 100644 --- a/pyxform/constants.py +++ b/pyxform/constants.py @@ -12,6 +12,7 @@ TYPE = "type" TITLE = "title" NAME = "name" +ENTITIES_SAVETO = "save_to" ID_STRING = "id_string" SMS_KEYWORD = "sms_keyword" SMS_FIELD = "sms_field" @@ -103,7 +104,8 @@ # The ODK entities spec version that generated forms comply to CURRENT_ENTITIES_VERSION = "2022.1.0" -ENTITY_RELATED = "entity-related" +ENTITY_RELATED = "entityrelated" +ENTITIES_RESERVED_PREFIX = "__" DEPRECATED_DEVICE_ID_METADATA_FIELDS = ["subscriberid", "simserial"] @@ -121,3 +123,4 @@ EXTERNAL_CHOICES_ITEMSET_REF_VALUE_GEOJSON = "id" ROW_FORMAT_STRING: str = "[row : %s]" +XML_IDENTIFIER_ERROR_MESSAGE = "must begin with a letter, colon, or underscore. Other characters can include numbers, dashes, and periods." diff --git a/pyxform/entity_declaration.py b/pyxform/entity_declaration.py index 53851807..bc55ea51 100644 --- a/pyxform/entity_declaration.py +++ b/pyxform/entity_declaration.py @@ -47,4 +47,4 @@ def xml_bindings(self): "readonly": "true()", } label_node = node("bind", nodeset=self.get_xpath() + "/label", **label_bind) - return [create_node, id_node, id_setvalue, label_node] \ No newline at end of file + return [create_node, id_node, id_setvalue, label_node] diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index c003c610..72f44823 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -157,13 +157,9 @@ def add_children(self, children): def validate(self): if not is_valid_xml_tag(self.name): invalid_char = re.search(INVALID_XFORM_TAG_REGEXP, self.name) - msg = ( - "The name '{}' is an invalid XML tag, it contains an " - "invalid character '{}'. Names must begin with a letter, " - "colon, or underscore, subsequent characters can include " - "numbers, dashes, and periods".format(self.name, invalid_char.group(0)) + raise PyXFormError( + f"The name '{self.name}' contains an invalid character '{invalid_char.group(0)}'. Names {constants.XML_IDENTIFIER_ERROR_MESSAGE}" ) - raise PyXFormError(msg) # TODO: Make sure renaming this doesn't cause any problems def iter_descendants(self): @@ -452,6 +448,14 @@ def xml_bindings(self): if self.trigger and "calculate" in self.bind: del bind_dict["calculate"] + if ( + "entities:saveto" in bind_dict + and getattr(survey, constants.ENTITY_RELATED, "false") != "true" + ): + raise PyXFormError( + "To save entity properties using the save_to column, you must add an entities sheet and declare an entity." + ) + for k, v in bind_dict.items(): # I think all the binding conversions should be happening on # the xls2json side. diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 5953b833..b1e327e6 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -11,7 +11,13 @@ from typing import Any, Dict, KeysView, List, Optional from pyxform import aliases, constants -from pyxform.constants import EXTERNAL_INSTANCE_EXTENSIONS, ROW_FORMAT_STRING +from pyxform.constants import ( + ENTITIES_RESERVED_PREFIX, + EXTERNAL_INSTANCE_EXTENSIONS, + ROW_FORMAT_STRING, + TYPE, + XML_IDENTIFIER_ERROR_MESSAGE, +) from pyxform.errors import PyXFormError from pyxform.utils import default_is_dynamic, is_valid_xml_tag, levenshtein_distance from pyxform.validators.pyxform import parameters_generic, select_from_file_params @@ -371,6 +377,37 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: } +def validate_entity_saveto(row: dict, row_number: int): + save_to = row.get("bind", {}).get("entities:saveto", "") + if not save_to: + return + + if constants.GROUP in row.get(TYPE) or constants.REPEAT in row.get(TYPE): + raise PyXFormError( + f"{ROW_FORMAT_STRING % row_number} Groups and repeats can't be saved as entity properties." + ) + + error_start = f"{ROW_FORMAT_STRING % row_number} Invalid save_to name:" + + if save_to == "name" or save_to == "label": + raise PyXFormError( + f"{error_start} the entity property name '{save_to}' is reserved." + ) + + if save_to.startswith(ENTITIES_RESERVED_PREFIX): + raise PyXFormError( + f"{error_start} the entity property name '{save_to}' starts with reserved prefix {ENTITIES_RESERVED_PREFIX}." + ) + + if not is_valid_xml_tag(save_to): + if isinstance(save_to, bytes): + save_to = save_to.encode("utf-8") + + raise PyXFormError( + f"{error_start} '{save_to}'. Entity property names {XML_IDENTIFIER_ERROR_MESSAGE}" + ) + + def workbook_to_json( workbook_dict, form_name=None, @@ -927,13 +964,12 @@ def workbook_to_json( if not is_valid_xml_tag(question_name): if isinstance(question_name, bytes): question_name = question_name.encode("utf-8") - error_message = ROW_FORMAT_STRING % row_number - error_message += " Invalid question name [" + question_name + "] " - error_message += "Names must begin with a letter, colon," + " or underscore." - error_message += ( - "Subsequent characters can include numbers," + " dashes, and periods." + + raise PyXFormError( + f"{ROW_FORMAT_STRING % row_number} Invalid question name '{question_name}'. Names {XML_IDENTIFIER_ERROR_MESSAGE}" ) - raise PyXFormError(error_message) + + validate_entity_saveto(row, row_number) # Try to parse question as begin control statement # (i.e. begin loop/repeat/group): @@ -1383,7 +1419,6 @@ def workbook_to_json( parent_children_array.append(new_dict) continue - # TODO: Consider adding some question_type validation here. # Put the row in the json dict as is: diff --git a/tests/test_entities.py b/tests/test_entities.py index 4d5e49a9..63c38a7c 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -162,7 +162,7 @@ def test_dataset_in_entities_sheet__adds_entities_namespace(self): | | dataset | label | | | | trees | a | | """, - xml__contains=["xmlns:entities=\"http://www.opendatakit.org/xforms/entities\""], + xml__contains=['xmlns:entities="http://www.opendatakit.org/xforms/entities"'], ) def test_entities_namespace__omitted_if_no_entities_sheet(self): @@ -173,7 +173,7 @@ def test_entities_namespace__omitted_if_no_entities_sheet(self): | | type | name | label | | | text | a | A | """, - xml__excludes=["xmlns:entities=\"http://www.opendatakit.org/xforms/entities\""], + xml__excludes=['xmlns:entities="http://www.opendatakit.org/xforms/entities"'], ) def test_dataset_in_entities_sheet__adds_entities_version(self): @@ -187,7 +187,9 @@ def test_dataset_in_entities_sheet__adds_entities_version(self): | | dataset | label | | | | trees | a | | """, - xml__xpath_match=['/h:html/h:head/x:model[@entities:entities-version = "2022.1.0"]'] + xml__xpath_match=[ + '/h:html/h:head/x:model[@entities:entities-version = "2022.1.0"]' + ], ) def test_entities_version__omitted_if_no_entities_sheet(self): @@ -198,5 +200,121 @@ def test_entities_version__omitted_if_no_entities_sheet(self): | | type | name | label | | | text | a | A | """, - xml__excludes=["entities:entities-version = \"2022.1.0\""], - ) \ No newline at end of file + xml__excludes=['entities:entities-version = "2022.1.0"'], + ) + + def test_saveto_column__added_to_xml(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | a | A | foo | + | entities | | | | | + | | dataset | label | | | + | | trees | a | | | + """, + xml__xpath_match=[ + '/h:html/h:head/x:model/x:bind[@nodeset = "/data/a" and @entities:saveto = "foo"]' + ], + ) + + def test_saveto_without_entities_sheet__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | a | A | foo | + """, + errored=True, + error__contains=[ + "To save entity properties using the save_to column, you must add an entities sheet and declare an entity." + ], + ) + + def test_name_in_saveto_column__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | a | A | name | + | entities | | | | | + | | dataset | label | | | + | | trees | a | | | + """, + errored=True, + error__contains=[ + "[row : 2] Invalid save_to name: the entity property name 'name' is reserved." + ], + ) + + def test_label_in_saveto_column__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | a | A | label | + | entities | | | | | + | | dataset | label | | | + | | trees | a | | | + """, + errored=True, + error__contains=[ + "[row : 2] Invalid save_to name: the entity property name 'label' is reserved." + ], + ) + + def test_system_prefix_in_saveto_column__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | a | A | __a | + | entities | | | | | + | | dataset | label | | | + | | trees | a | | | + """, + errored=True, + error__contains=[ + "[row : 2] Invalid save_to name: the entity property name '__a' starts with reserved prefix __." + ], + ) + + def test_invalid_xml_identifier_in_saveto_column__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | text | a | A | $a | + | entities | | | | | + | | dataset | label | | | + | | trees | a | | | + """, + errored=True, + error__contains=[ + "[row : 2] Invalid save_to name: '$a'. Entity property names must begin with a letter, colon, or underscore. Other characters can include numbers, dashes, and periods." + ], + ) + + def test_saveto_on_group__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | save_to | + | | begin_group | a | A | a | + | | end_group | | | | + | entities | | | | | + | | dataset | label | | | + | | trees | a | | | + """, + errored=True, + error__contains=[ + "[row : 2] Groups and repeats can't be saved as entity properties." + ], + ) diff --git a/tests/test_sheet_columns.py b/tests/test_sheet_columns.py index 2fff4d48..106d5c04 100644 --- a/tests/test_sheet_columns.py +++ b/tests/test_sheet_columns.py @@ -158,12 +158,7 @@ def test_missing_list_name(self): def test_clear_filename_error_message(self): """Test clear filename error message""" - error_message = ( - "The name 'bad@filename' is an invalid XML tag, it " - "contains an invalid character '@'. Names must begin" - " with a letter, colon, or underscore, subsequent " - "characters can include numbers, dashes, and periods" - ) + error_message = "The name 'bad@filename' contains an invalid character '@'. Names must begin with a letter, colon, or underscore. Other characters can include numbers, dashes, and periods." self.assertPyxformXform( name="bad@filename", ss_structure=self._simple_choice_ss( From 9dd3ada761370f07816f8abf549e035718ea840b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 16 Nov 2022 13:31:16 -0800 Subject: [PATCH 12/17] Centralize saveto validation --- pyxform/survey_element.py | 8 -------- pyxform/xls2json.py | 15 +++++++++++---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index 72f44823..f963fb55 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -448,14 +448,6 @@ def xml_bindings(self): if self.trigger and "calculate" in self.bind: del bind_dict["calculate"] - if ( - "entities:saveto" in bind_dict - and getattr(survey, constants.ENTITY_RELATED, "false") != "true" - ): - raise PyXFormError( - "To save entity properties using the save_to column, you must add an entities sheet and declare an entity." - ) - for k, v in bind_dict.items(): # I think all the binding conversions should be happening on # the xls2json side. diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index b1e327e6..771177c4 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -354,7 +354,7 @@ def find_sheet_misspellings(key: str, keys: "KeysView") -> "Optional[str]": def get_entity_declaration(workbook_dict: Dict) -> Dict: entities_sheet = workbook_dict.get(constants.ENTITIES, []) if len(entities_sheet) == 0: - return [] + return {} elif len(entities_sheet) > 1: raise PyXFormError( "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." @@ -377,11 +377,16 @@ def get_entity_declaration(workbook_dict: Dict) -> Dict: } -def validate_entity_saveto(row: dict, row_number: int): +def validate_entity_saveto(row: Dict, row_number: int, entity_declaration: Dict): save_to = row.get("bind", {}).get("entities:saveto", "") if not save_to: return + if len(entity_declaration) == 0: + raise PyXFormError( + "To save entity properties using the save_to column, you must add an entities sheet and declare an entity." + ) + if constants.GROUP in row.get(TYPE) or constants.REPEAT in row.get(TYPE): raise PyXFormError( f"{ROW_FORMAT_STRING % row_number} Groups and repeats can't be saved as entity properties." @@ -623,6 +628,9 @@ def workbook_to_json( ) ) # noqa + # ########## Entities sheet ########### + entity_declaration = get_entity_declaration(workbook_dict) + # ########## Survey sheet ########### survey_sheet = workbook_dict[constants.SURVEY] # Process the headers: @@ -969,7 +977,7 @@ def workbook_to_json( f"{ROW_FORMAT_STRING % row_number} Invalid question name '{question_name}'. Names {XML_IDENTIFIER_ERROR_MESSAGE}" ) - validate_entity_saveto(row, row_number) + validate_entity_saveto(row, row_number, entity_declaration) # Try to parse question as begin control statement # (i.e. begin loop/repeat/group): @@ -1462,7 +1470,6 @@ def workbook_to_json( } ) - entity_declaration = get_entity_declaration(workbook_dict) if len(entity_declaration) > 0: json_dict[constants.ENTITY_RELATED] = "true" meta_children.append(entity_declaration) From e5c8e6d7fa4f460cd842ec6150857d86968ba2f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 16 Nov 2022 13:40:07 -0800 Subject: [PATCH 13/17] Warn for sheet names close to entities --- pyxform/xls2json.py | 9 +++++++-- tests/test_entities.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 771177c4..fe1dedab 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -351,14 +351,19 @@ def find_sheet_misspellings(key: str, keys: "KeysView") -> "Optional[str]": return None -def get_entity_declaration(workbook_dict: Dict) -> Dict: +def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: entities_sheet = workbook_dict.get(constants.ENTITIES, []) + if len(entities_sheet) == 0: + similar = find_sheet_misspellings(key=constants.ENTITIES, keys=workbook_dict.keys()) + if similar is not None: + warnings.append(similar + _MSG_SUPPRESS_SPELLING) return {} elif len(entities_sheet) > 1: raise PyXFormError( "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." ) + entity = entities_sheet[0] if not ("label" in entity): @@ -629,7 +634,7 @@ def workbook_to_json( ) # noqa # ########## Entities sheet ########### - entity_declaration = get_entity_declaration(workbook_dict) + entity_declaration = get_entity_declaration(workbook_dict, warnings) # ########## Survey sheet ########### survey_sheet = workbook_dict[constants.SURVEY] diff --git a/tests/test_entities.py b/tests/test_entities.py index 63c38a7c..40026f80 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -51,6 +51,20 @@ def test_dataset_in_entities_sheet__adds_dataset_attribute_to_entity(self): ], ) + def test_worksheet_name_close_to_entities__produces_warning(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entitoes | | | | + | | dataset | label | | + | | trees | a | | + """, + warnings__contains=["When looking for a sheet named 'entities', the following sheets with similar names were found: 'entitoes'."] + ) + def test_dataset_in_entities_sheet__defaults_to_always_creating(self): self.assertPyxformXform( name="data", From 935685c8568f697ef297384163bd3e92f3d1c142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 16 Nov 2022 13:53:07 -0800 Subject: [PATCH 14/17] Validate dataset name --- pyxform/xls2json.py | 20 ++++++++++++++++++-- tests/test_entities.py | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index fe1dedab..59ded054 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -355,7 +355,9 @@ def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: entities_sheet = workbook_dict.get(constants.ENTITIES, []) if len(entities_sheet) == 0: - similar = find_sheet_misspellings(key=constants.ENTITIES, keys=workbook_dict.keys()) + similar = find_sheet_misspellings( + key=constants.ENTITIES, keys=workbook_dict.keys() + ) if similar is not None: warnings.append(similar + _MSG_SUPPRESS_SPELLING) return {} @@ -365,6 +367,20 @@ def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: ) entity = entities_sheet[0] + dataset = entity["dataset"] + + if dataset.startswith(ENTITIES_RESERVED_PREFIX): + raise PyXFormError( + f"Invalid dataset name: '{dataset}' starts with reserved prefix {ENTITIES_RESERVED_PREFIX}." + ) + + if not is_valid_xml_tag(dataset): + if isinstance(dataset, bytes): + dataset = dataset.encode("utf-8") + + raise PyXFormError( + f"Invalid dataset name: '{dataset}'. Dataset names {XML_IDENTIFIER_ERROR_MESSAGE}" + ) if not ("label" in entity): raise PyXFormError("The entities sheet is missing the required label column.") @@ -375,7 +391,7 @@ def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: "name": "entity", "type": "entity", "parameters": { - "dataset": entities_sheet[0]["dataset"], + "dataset": dataset, "create": creation_condition, "label": entity["label"], }, diff --git a/tests/test_entities.py b/tests/test_entities.py index 40026f80..2b99db69 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -51,6 +51,40 @@ def test_dataset_in_entities_sheet__adds_dataset_attribute_to_entity(self): ], ) + def test_dataset_with_reserved_prefix__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | __sweet | a | | + """, + errored=True, + error__contains=[ + "Invalid dataset name: '__sweet' starts with reserved prefix __." + ], + ) + + def test_dataset_with_invalid_xml_name__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | $sweet | a | | + """, + errored=True, + error__contains=[ + "Invalid dataset name: '$sweet'. Dataset names must begin with a letter, colon, or underscore. Other characters can include numbers, dashes, and periods." + ], + ) + def test_worksheet_name_close_to_entities__produces_warning(self): self.assertPyxformXform( name="data", @@ -62,7 +96,9 @@ def test_worksheet_name_close_to_entities__produces_warning(self): | | dataset | label | | | | trees | a | | """, - warnings__contains=["When looking for a sheet named 'entities', the following sheets with similar names were found: 'entitoes'."] + warnings__contains=[ + "When looking for a sheet named 'entities', the following sheets with similar names were found: 'entitoes'." + ], ) def test_dataset_in_entities_sheet__defaults_to_always_creating(self): From f6cd4a822edbd797464b380a5314de26441aa83b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 16 Nov 2022 14:59:48 -0800 Subject: [PATCH 15/17] Further isolate entities implementation --- pyxform/builder.py | 2 +- pyxform/constants.py | 5 + pyxform/entities/entities_parsing.py | 90 +++++++++++++ pyxform/{ => entities}/entity_declaration.py | 0 pyxform/survey_element.py | 2 +- pyxform/utils.py | 12 -- pyxform/xls2json.py | 128 ++----------------- pyxform/xlsparseutils.py | 45 +++++++ 8 files changed, 150 insertions(+), 134 deletions(-) create mode 100644 pyxform/entities/entities_parsing.py rename pyxform/{ => entities}/entity_declaration.py (100%) create mode 100644 pyxform/xlsparseutils.py diff --git a/pyxform/builder.py b/pyxform/builder.py index cf2f1ea1..95e52936 100644 --- a/pyxform/builder.py +++ b/pyxform/builder.py @@ -7,7 +7,7 @@ import re from pyxform import file_utils, utils -from pyxform.entity_declaration import EntityDeclaration +from pyxform.entities.entity_declaration import EntityDeclaration from pyxform.errors import PyXFormError from pyxform.external_instance import ExternalInstance from pyxform.question import ( diff --git a/pyxform/constants.py b/pyxform/constants.py index 546b51e8..e159749f 100644 --- a/pyxform/constants.py +++ b/pyxform/constants.py @@ -124,3 +124,8 @@ ROW_FORMAT_STRING: str = "[row : %s]" XML_IDENTIFIER_ERROR_MESSAGE = "must begin with a letter, colon, or underscore. Other characters can include numbers, dashes, and periods." +_MSG_SUPPRESS_SPELLING = ( + " If you do not mean to include a sheet, to suppress this message, " + "prefix the sheet name with an underscore. For example 'setting' " + "becomes '_setting'." +) diff --git a/pyxform/entities/entities_parsing.py b/pyxform/entities/entities_parsing.py new file mode 100644 index 00000000..7f6b9460 --- /dev/null +++ b/pyxform/entities/entities_parsing.py @@ -0,0 +1,90 @@ +from typing import Dict, List + +from pyxform import constants +from pyxform.errors import PyXFormError +from pyxform.xlsparseutils import find_sheet_misspellings, is_valid_xml_tag + + +def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: + entities_sheet = workbook_dict.get(constants.ENTITIES, []) + + if len(entities_sheet) == 0: + similar = find_sheet_misspellings( + key=constants.ENTITIES, keys=workbook_dict.keys() + ) + if similar is not None: + warnings.append(similar + constants._MSG_SUPPRESS_SPELLING) + return {} + elif len(entities_sheet) > 1: + raise PyXFormError( + "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." + ) + + entity = entities_sheet[0] + dataset = entity["dataset"] + + if dataset.startswith(constants.ENTITIES_RESERVED_PREFIX): + raise PyXFormError( + f"Invalid dataset name: '{dataset}' starts with reserved prefix {constants.ENTITIES_RESERVED_PREFIX}." + ) + + if not is_valid_xml_tag(dataset): + if isinstance(dataset, bytes): + dataset = dataset.encode("utf-8") + + raise PyXFormError( + f"Invalid dataset name: '{dataset}'. Dataset names {constants.XML_IDENTIFIER_ERROR_MESSAGE}" + ) + + 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"], + }, + } + + +def validate_entity_saveto(row: Dict, row_number: int, entity_declaration: Dict): + save_to = row.get("bind", {}).get("entities:saveto", "") + if not save_to: + return + + if len(entity_declaration) == 0: + raise PyXFormError( + "To save entity properties using the save_to column, you must add an entities sheet and declare an entity." + ) + + if constants.GROUP in row.get(constants.TYPE) or constants.REPEAT in row.get( + constants.TYPE + ): + raise PyXFormError( + f"{constants.ROW_FORMAT_STRING % row_number} Groups and repeats can't be saved as entity properties." + ) + + error_start = f"{constants.ROW_FORMAT_STRING % row_number} Invalid save_to name:" + + if save_to == "name" or save_to == "label": + raise PyXFormError( + f"{error_start} the entity property name '{save_to}' is reserved." + ) + + if save_to.startswith(constants.ENTITIES_RESERVED_PREFIX): + raise PyXFormError( + f"{error_start} the entity property name '{save_to}' starts with reserved prefix {constants.ENTITIES_RESERVED_PREFIX}." + ) + + if not is_valid_xml_tag(save_to): + if isinstance(save_to, bytes): + save_to = save_to.encode("utf-8") + + raise PyXFormError( + f"{error_start} '{save_to}'. Entity property names {constants.XML_IDENTIFIER_ERROR_MESSAGE}" + ) diff --git a/pyxform/entity_declaration.py b/pyxform/entities/entity_declaration.py similarity index 100% rename from pyxform/entity_declaration.py rename to pyxform/entities/entity_declaration.py diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py index f963fb55..ade87aba 100644 --- a/pyxform/survey_element.py +++ b/pyxform/survey_element.py @@ -14,10 +14,10 @@ BRACKETED_TAG_REGEX, INVALID_XFORM_TAG_REGEXP, default_is_dynamic, - is_valid_xml_tag, node, ) from pyxform.xls2json import print_pyobj_to_json +from pyxform.xlsparseutils import is_valid_xml_tag if TYPE_CHECKING: from typing import List diff --git a/pyxform/utils.py b/pyxform/utils.py index 73a53051..de5c7fde 100644 --- a/pyxform/utils.py +++ b/pyxform/utils.py @@ -20,11 +20,6 @@ SEP = "_" -# http://www.w3.org/TR/REC-xml/ -TAG_START_CHAR = r"[a-zA-Z:_]" -TAG_CHAR = r"[a-zA-Z:_0-9\-.]" -XFORM_TAG_REGEXP = "%(start)s%(char)s*" % {"start": TAG_START_CHAR, "char": TAG_CHAR} - INVALID_XFORM_TAG_REGEXP = r"[^a-zA-Z:_][^a-zA-Z:_0-9\-.]*" LAST_SAVED_INSTANCE_NAME = "__last-saved" @@ -67,13 +62,6 @@ def writexml(self, writer, indent="", addindent="", newl=""): writer.write(data) -def is_valid_xml_tag(tag): - """ - Use a regex to see if there are any invalid characters (i.e. spaces). - """ - return re.search(r"^" + XFORM_TAG_REGEXP + r"$", tag) - - def node(*args, **kwargs): """ args[0] -- a XML tag diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 59ded054..47c58752 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -8,30 +8,29 @@ import re import sys from collections import Counter -from typing import Any, Dict, KeysView, List, Optional +from typing import Any, Dict, List from pyxform import aliases, constants from pyxform.constants import ( - ENTITIES_RESERVED_PREFIX, + _MSG_SUPPRESS_SPELLING, EXTERNAL_INSTANCE_EXTENSIONS, ROW_FORMAT_STRING, - TYPE, XML_IDENTIFIER_ERROR_MESSAGE, ) +from pyxform.entities.entities_parsing import ( + get_entity_declaration, + validate_entity_saveto, +) from pyxform.errors import PyXFormError -from pyxform.utils import default_is_dynamic, is_valid_xml_tag, levenshtein_distance +from pyxform.utils import default_is_dynamic from pyxform.validators.pyxform import parameters_generic, select_from_file_params from pyxform.validators.pyxform.missing_translations_check import ( missing_translations_check, ) from pyxform.xls2json_backends import csv_to_dict, xls_to_dict, xlsx_to_dict +from pyxform.xlsparseutils import find_sheet_misspellings, is_valid_xml_tag SMART_QUOTES = {"\u2018": "'", "\u2019": "'", "\u201c": '"', "\u201d": '"'} -_MSG_SUPPRESS_SPELLING = ( - " If you do not mean to include a sheet, to suppress this message, " - "prefix the sheet name with an underscore. For example 'setting' " - "becomes '_setting'." -) def print_pyobj_to_json(pyobj, path=None): @@ -323,117 +322,6 @@ def process_image_default(default_value): return default_value -def find_sheet_misspellings(key: str, keys: "KeysView") -> "Optional[str]": - """ - Find possible sheet name misspellings to warn the user about. - - It's possible that this will warn about sheet names for sheets that have - auxilliary metadata that is not meant for processing by pyxform. For - example the "osm" sheet name may be similar to many other initialisms. - - :param key: The sheet name to look for. - :param keys: The workbook sheet names. - """ - candidates = tuple( - _k # thanks to black - for _k in keys - if 2 >= levenshtein_distance(_k.lower(), key) - and _k not in constants.SUPPORTED_SHEET_NAMES - and not _k.startswith("_") - ) - if 0 < len(candidates): - msg = ( - "When looking for a sheet named '{k}', the following sheets with " - "similar names were found: {c}." - ).format(k=key, c=str(", ".join(("'{}'".format(c) for c in candidates)))) - return msg - else: - return None - - -def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: - entities_sheet = workbook_dict.get(constants.ENTITIES, []) - - if len(entities_sheet) == 0: - similar = find_sheet_misspellings( - key=constants.ENTITIES, keys=workbook_dict.keys() - ) - if similar is not None: - warnings.append(similar + _MSG_SUPPRESS_SPELLING) - return {} - elif len(entities_sheet) > 1: - raise PyXFormError( - "This version of pyxform only supports declaring a single entity per form. Please make sure your entities sheet only declares one entity." - ) - - entity = entities_sheet[0] - dataset = entity["dataset"] - - if dataset.startswith(ENTITIES_RESERVED_PREFIX): - raise PyXFormError( - f"Invalid dataset name: '{dataset}' starts with reserved prefix {ENTITIES_RESERVED_PREFIX}." - ) - - if not is_valid_xml_tag(dataset): - if isinstance(dataset, bytes): - dataset = dataset.encode("utf-8") - - raise PyXFormError( - f"Invalid dataset name: '{dataset}'. Dataset names {XML_IDENTIFIER_ERROR_MESSAGE}" - ) - - 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"], - }, - } - - -def validate_entity_saveto(row: Dict, row_number: int, entity_declaration: Dict): - save_to = row.get("bind", {}).get("entities:saveto", "") - if not save_to: - return - - if len(entity_declaration) == 0: - raise PyXFormError( - "To save entity properties using the save_to column, you must add an entities sheet and declare an entity." - ) - - if constants.GROUP in row.get(TYPE) or constants.REPEAT in row.get(TYPE): - raise PyXFormError( - f"{ROW_FORMAT_STRING % row_number} Groups and repeats can't be saved as entity properties." - ) - - error_start = f"{ROW_FORMAT_STRING % row_number} Invalid save_to name:" - - if save_to == "name" or save_to == "label": - raise PyXFormError( - f"{error_start} the entity property name '{save_to}' is reserved." - ) - - if save_to.startswith(ENTITIES_RESERVED_PREFIX): - raise PyXFormError( - f"{error_start} the entity property name '{save_to}' starts with reserved prefix {ENTITIES_RESERVED_PREFIX}." - ) - - if not is_valid_xml_tag(save_to): - if isinstance(save_to, bytes): - save_to = save_to.encode("utf-8") - - raise PyXFormError( - f"{error_start} '{save_to}'. Entity property names {XML_IDENTIFIER_ERROR_MESSAGE}" - ) - - def workbook_to_json( workbook_dict, form_name=None, diff --git a/pyxform/xlsparseutils.py b/pyxform/xlsparseutils.py new file mode 100644 index 00000000..9a10a770 --- /dev/null +++ b/pyxform/xlsparseutils.py @@ -0,0 +1,45 @@ +import re +from pyxform import constants +from typing import KeysView, Optional + +from pyxform.utils import levenshtein_distance + +# http://www.w3.org/TR/REC-xml/ +TAG_START_CHAR = r"[a-zA-Z:_]" +TAG_CHAR = r"[a-zA-Z:_0-9\-.]" +XFORM_TAG_REGEXP = "%(start)s%(char)s*" % {"start": TAG_START_CHAR, "char": TAG_CHAR} + + +def find_sheet_misspellings(key: str, keys: "KeysView") -> "Optional[str]": + """ + Find possible sheet name misspellings to warn the user about. + + It's possible that this will warn about sheet names for sheets that have + auxilliary metadata that is not meant for processing by pyxform. For + example the "osm" sheet name may be similar to many other initialisms. + + :param key: The sheet name to look for. + :param keys: The workbook sheet names. + """ + candidates = tuple( + _k # thanks to black + for _k in keys + if 2 >= levenshtein_distance(_k.lower(), key) + and _k not in constants.SUPPORTED_SHEET_NAMES + and not _k.startswith("_") + ) + if 0 < len(candidates): + msg = ( + "When looking for a sheet named '{k}', the following sheets with " + "similar names were found: {c}." + ).format(k=key, c=str(", ".join(("'{}'".format(c) for c in candidates)))) + return msg + else: + return None + + +def is_valid_xml_tag(tag): + """ + Use a regex to see if there are any invalid characters (i.e. spaces). + """ + return re.search(r"^" + XFORM_TAG_REGEXP + r"$", tag) From 2fb3bd8d06eedc4dae0062a5ac6f50e1e03feeac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Thu, 17 Nov 2022 11:07:40 -0800 Subject: [PATCH 16/17] Don't allow periods in dataset names --- pyxform/entities/entities_parsing.py | 7 ++++++- tests/test_entities.py | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/pyxform/entities/entities_parsing.py b/pyxform/entities/entities_parsing.py index 7f6b9460..e985ee44 100644 --- a/pyxform/entities/entities_parsing.py +++ b/pyxform/entities/entities_parsing.py @@ -28,12 +28,17 @@ def get_entity_declaration(workbook_dict: Dict, warnings: List) -> Dict: f"Invalid dataset name: '{dataset}' starts with reserved prefix {constants.ENTITIES_RESERVED_PREFIX}." ) + if "." in dataset: + raise PyXFormError( + f"Invalid dataset name: '{dataset}'. Dataset names may not include periods." + ) + if not is_valid_xml_tag(dataset): if isinstance(dataset, bytes): dataset = dataset.encode("utf-8") raise PyXFormError( - f"Invalid dataset name: '{dataset}'. Dataset names {constants.XML_IDENTIFIER_ERROR_MESSAGE}" + f"Invalid dataset name: '{dataset}'. Dataset names must begin with a letter, colon, or underscore. Other characters can include numbers or dashes." ) if not ("label" in entity): diff --git a/tests/test_entities.py b/tests/test_entities.py index 2b99db69..4b62365a 100644 --- a/tests/test_entities.py +++ b/tests/test_entities.py @@ -81,7 +81,24 @@ def test_dataset_with_invalid_xml_name__errors(self): """, errored=True, error__contains=[ - "Invalid dataset name: '$sweet'. Dataset names must begin with a letter, colon, or underscore. Other characters can include numbers, dashes, and periods." + "Invalid dataset name: '$sweet'. Dataset names must begin with a letter, colon, or underscore. Other characters can include numbers or dashes." + ], + ) + + def test_dataset_with_period_in_name__errors(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | + | | type | name | label | + | | text | a | A | + | entities | | | | + | | dataset | label | | + | | s.w.eet | a | | + """, + errored=True, + error__contains=[ + "Invalid dataset name: 's.w.eet'. Dataset names may not include periods." ], ) From 000ccce1dd565b84cc918b549752fba0e37e6083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Fri, 18 Nov 2022 16:05:19 -0800 Subject: [PATCH 17/17] Use underscore for consistency --- pyxform/constants.py | 2 +- pyxform/xls2json.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyxform/constants.py b/pyxform/constants.py index e159749f..7576d569 100644 --- a/pyxform/constants.py +++ b/pyxform/constants.py @@ -104,7 +104,7 @@ # The ODK entities spec version that generated forms comply to CURRENT_ENTITIES_VERSION = "2022.1.0" -ENTITY_RELATED = "entityrelated" +ENTITY_RELATED = "entity_related" ENTITIES_RESERVED_PREFIX = "__" DEPRECATED_DEVICE_ID_METADATA_FIELDS = ["subscriberid", "simserial"] diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 47c58752..3e524dff 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -1393,7 +1393,7 @@ def workbook_to_json( survey_children_array = stack[0]["parent_children"] survey_children_array.append(meta_element) - # print_pyobj_to_json(json_dict) + print_pyobj_to_json(json_dict) return json_dict