Repository: XLSForm/pyxform (python)

Issue:
Always produce Entity forms with the offline entities version
https://github.com/XLSForm/pyxform/pull/719 introduced support for offline Entities on an opt-in basis to allow for integrated testing of all the components.

For the next pyxform release, this should be the only behavior and the opt-in setting should be removed. 

Pull request:
725: always produce entity forms with the offline entities spec version
Closes #725

#### Why is this the best possible solution? Were any other approaches considered?

- Removed the older create/update spec versions which are now unused.
- Although it looks a bit useless now, the "entity_features" code in xls2json.py and survey.py was retained since it seems likely that this will be needed in the near future as the entities spec evolves.
- Incorporated the previous opt-in test expectations for create/update entities into the respective "basic building blocks" tests.

#### What are the regression risks?

- Pyxform will only emit entities version `2024.1.0` so if these are provided to an older version of Central (or another spec-compliant server) then they will be rejected.
- The `offline` column is now undefined for the `entities` sheet, so like other undefined columns it will now raise an error. Any forms that added the `offline` opt-in column will now need to remove it during their next form version update.

#### Does this change require updates to documentation? If so, please file an issue [here](https://github.com/XLSForm/xlsform.github.io) and include the link below.

Filed an issue to add the proposed offline version description to the spec: https://github.com/getodk/xforms-spec/issues/319

#### Before submitting this PR, please make sure you have:
- [x] included test cases for core behavior and edge cases in `tests`
- [x] run `python -m unittest` and verified all tests pass
- [x] run `ruff format pyxform tests` and `ruff check pyxform tests` to lint code
- [x] verified that any code or assets from external sources are properly credited in comments

Patch:
diff --git a/pyxform/constants.py b/pyxform/constants.py
index c33323e..793d1e0 100644
--- a/pyxform/constants.py
+++ b/pyxform/constants.py
@@ -113,8 +113,6 @@ EXTERNAL_INSTANCES = {"calculate", "constraint", "readonly", "required", "releva
 CURRENT_XFORMS_VERSION = "1.0.0"
 
 # The ODK entities spec version that generated forms comply to
-ENTITIES_CREATE_VERSION = "2022.1.0"
-ENTITIES_UPDATE_VERSION = "2023.1.0"
 ENTITIES_OFFLINE_VERSION = "2024.1.0"
 ENTITY = "entity"
 ENTITY_FEATURES = "entity_features"
@@ -127,7 +125,6 @@ class EntityColumns(StrEnum):
     CREATE_IF = "create_if"
     UPDATE_IF = "update_if"
     LABEL = "label"
-    OFFLINE = "offline"
 
 
 DEPRECATED_DEVICE_ID_METADATA_FIELDS = {"subscriberid", "simserial"}
diff --git a/pyxform/entities/entities_parsing.py b/pyxform/entities/entities_parsing.py
index 34f44e7..39f06dc 100644
--- a/pyxform/entities/entities_parsing.py
+++ b/pyxform/entities/entities_parsing.py
@@ -1,7 +1,6 @@
 from typing import Any
 
 from pyxform import constants as const
-from pyxform.aliases import yes_no
 from pyxform.errors import PyXFormError
 from pyxform.parsing.expression import is_xml_tag
 from pyxform.validators.pyxform.sheet_misspellings import find_sheet_misspellings
@@ -30,7 +29,6 @@ def get_entity_declaration(
     create_condition = entity_row.get(EC.CREATE_IF, None)
     update_condition = entity_row.get(EC.UPDATE_IF, None)
     entity_label = entity_row.get(EC.LABEL, None)
-    offline = yes_no.get(entity_row.get(EC.OFFLINE, None), None)
 
     if update_condition and not entity_id:
         raise PyXFormError(
@@ -56,7 +54,6 @@ def get_entity_declaration(
             EC.CREATE_IF: create_condition,
             EC.UPDATE_IF: update_condition,
             EC.LABEL: entity_label,
-            EC.OFFLINE: offline,
         },
     }
 
diff --git a/pyxform/entities/entity_declaration.py b/pyxform/entities/entity_declaration.py
index ed04600..49222f4 100644
--- a/pyxform/entities/entity_declaration.py
+++ b/pyxform/entities/entity_declaration.py
@@ -60,9 +60,8 @@ class EntityDeclaration(SurveyElement):
         if entity_id_expression:
             attributes["update"] = "1"
             attributes["baseVersion"] = ""
-            if parameters.get(EC.OFFLINE, None):
-                attributes["trunkVersion"] = ""
-                attributes["branchId"] = ""
+            attributes["trunkVersion"] = ""
+            attributes["branchId"] = ""
 
         if create_condition or (not update_condition and not entity_id_expression):
             attributes["create"] = "1"
@@ -101,15 +100,12 @@ class EntityDeclaration(SurveyElement):
             bind_nodes.append(
                 self._get_bind_node(survey, f"{entity}/__version", "/@baseVersion")
             )
-            if parameters.get(EC.OFFLINE, None):
-                bind_nodes.append(
-                    self._get_bind_node(
-                        survey, f"{entity}/__trunkVersion", "/@trunkVersion"
-                    )
-                )
-                bind_nodes.append(
-                    self._get_bind_node(survey, f"{entity}/__branchId", "/@branchId")
-                )
+            bind_nodes.append(
+                self._get_bind_node(survey, f"{entity}/__trunkVersion", "/@trunkVersion")
+            )
+            bind_nodes.append(
+                self._get_bind_node(survey, f"{entity}/__branchId", "/@branchId")
+            )
 
         if label_expression:
             bind_nodes.append(self._get_bind_node(survey, label_expression, "/label"))
diff --git a/pyxform/survey.py b/pyxform/survey.py
index a45ca14..75ddf3c 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -724,18 +724,7 @@ class Survey(Section):
         model_kwargs = {"odk:xforms-version": constants.CURRENT_XFORMS_VERSION}
 
         if self.entity_features:
-            if "offline" in self.entity_features:
-                model_kwargs["entities:entities-version"] = (
-                    constants.ENTITIES_OFFLINE_VERSION
-                )
-            elif "update" in self.entity_features:
-                model_kwargs["entities:entities-version"] = (
-                    constants.ENTITIES_UPDATE_VERSION
-                )
-            else:
-                model_kwargs["entities:entities-version"] = (
-                    constants.ENTITIES_CREATE_VERSION
-                )
+            model_kwargs["entities:entities-version"] = constants.ENTITIES_OFFLINE_VERSION
 
         model_children = []
         if self._translations:
diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py
index af281ef..9636c64 100644
--- a/pyxform/xls2json.py
+++ b/pyxform/xls2json.py
@@ -1514,15 +1514,7 @@ def workbook_to_json(
         )
 
     if len(entity_declaration) > 0:
-        json_dict[constants.ENTITY_FEATURES] = ["create"]
-        entity_parameters = entity_declaration.get(constants.PARAMETERS, {})
-
-        if entity_parameters.get(constants.EntityColumns.ENTITY_ID, None):
-            json_dict[constants.ENTITY_FEATURES].append("update")
-
-        if entity_parameters.get(constants.EntityColumns.OFFLINE, None):
-            json_dict[constants.ENTITY_FEATURES].append("offline")
-
+        json_dict[constants.ENTITY_FEATURES] = ["create", "update", "offline"]
         meta_children.append(entity_declaration)
 
     if len(meta_children) > 0:


Test patch:
diff --git a/tests/test_entities_create.py b/tests/test_entities_create.py
index c935af2..fef9b53 100644
--- a/tests/test_entities_create.py
+++ b/tests/test_entities_create.py
@@ -25,7 +25,7 @@ class EntitiesCreationTest(PyxformTestCase):
                 '/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: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"]',
-                '/h:html/h:head/x:model[@entities:entities-version = "2022.1.0"]',
+                f"""/h:html/h:head/x:model[@entities:entities-version = '{co.ENTITIES_OFFLINE_VERSION}']""",
             ],
             xml__xpath_count=[
                 ("/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity/@update", 0),
@@ -440,43 +440,3 @@ class EntitiesCreationTest(PyxformTestCase):
                 "'why'",
             ],
         )
-
-    def test_entities_offline_opt_in__yes(self):
-        """Should find offline spec version, if opted-in."""
-        self.assertPyxformXform(
-            md="""
-            | survey   |
-            |          | type | name | label |
-            |          | text | a    | A     |
-            | entities |
-            |          | dataset | label | offline |
-            |          | trees   | a     | yes     |
-            """,
-            xml__xpath_match=[
-                f"""
-                  /h:html/h:head/x:model[
-                    @entities:entities-version = "{co.ENTITIES_OFFLINE_VERSION}"
-                  ]
-                """,
-            ],
-        )
-
-    def test_entities_offline_opt_in__no(self):
-        """Should find create spec version, if not opted-in."""
-        self.assertPyxformXform(
-            md="""
-            | survey   |
-            |          | type      | name  | label |
-            |          | text      | a     | A     |
-            | entities |
-            |          | dataset   | label | offline |
-            |          | trees     | a     | no      |
-            """,
-            xml__xpath_match=[
-                f"""
-                  /h:html/h:head/x:model[
-                    @entities:entities-version = "{co.ENTITIES_CREATE_VERSION}"
-                  ]
-                """,
-            ],
-        )
diff --git a/tests/test_entities_update.py b/tests/test_entities_update.py
index 75a0231..1103b62 100644
--- a/tests/test_entities_update.py
+++ b/tests/test_entities_update.py
@@ -26,7 +26,29 @@ class EntitiesUpdateTest(PyxformTestCase):
                 '/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"]',
+                f"""/h:html/h:head/x:model[@entities:entities-version = '{co.ENTITIES_OFFLINE_VERSION}']""",
+                """
+                  /h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity[
+                    @trunkVersion = ''
+                    and @branchId = ''
+                  ]
+                """,
+                """
+                  /h:html/h:head/x:model/x:bind[
+                    @nodeset = '/data/meta/entity/@trunkVersion'
+                    and @calculate = "instance('trees')/root/item[name= /data/id ]/__trunkVersion"
+                    and @type = 'string'
+                    and @readonly = 'true()'
+                  ]
+                """,
+                """
+                  /h:html/h:head/x:model/x:bind[
+                    @nodeset = '/data/meta/entity/@branchId'
+                    and @calculate = "instance('trees')/root/item[name= /data/id ]/__branchId"
+                    and @type = 'string'
+                    and @readonly = 'true()'
+                  ]
+                """,
             ],
             xml__xpath_count=[
                 ("/h:html/h:head/x:model/x:instance/x:data/x:meta/x:entity/x:label", 0),
@@ -176,95 +198,3 @@ class EntitiesUpdateTest(PyxformTestCase):
                 '/h:html/h:head/x:model/x:bind[@nodeset = "/data/a" and @entities:saveto = "foo"]'
             ],
         )
-
-    def test_entities_offline_opt_in__yes(self):
-        """Should find offline spec version and trunk/branch props/binds, if opted-in."""
-        self.assertPyxformXform(
-            md="""
-            | survey   |
-            |          | type | name | label   |
-            |          | text | id   | Tree id |
-            |          | text | q1   | Q1      |
-            | entities |
-            |          | dataset | entity_id | offline |
-            |          | trees   | ${id}     | yes     |
-            """,
-            xml__xpath_match=[
-                f"""
-                  /h:html/h:head/x:model[
-                    @entities:entities-version = "{co.ENTITIES_OFFLINE_VERSION}"
-                  ]
-                """,
-                """
-                  /h:html/h:head/x:model/x:instance/x:test_name/x:meta/x:entity[
-                    @trunkVersion = ''
-                    and @branchId = ''
-                  ]
-                """,
-                """
-                  /h:html/h:head/x:model/x:bind[
-                    @nodeset = '/test_name/meta/entity/@trunkVersion'
-                    and @calculate = "instance('trees')/root/item[name= /test_name/id ]/__trunkVersion"
-                    and @type = 'string'
-                    and @readonly = 'true()'
-                  ]
-                """,
-                """
-                  /h:html/h:head/x:model/x:bind[
-                    @nodeset = '/test_name/meta/entity/@branchId'
-                    and @calculate = "instance('trees')/root/item[name= /test_name/id ]/__branchId"
-                    and @type = 'string'
-                    and @readonly = 'true()'
-                  ]
-                """,
-            ],
-        )
-
-    def test_entities_offline_opt_in__no(self):
-        """Should not find update spec version and trunk/branch props/binds, if not opted-in."""
-        cases = (
-            """
-            | entities |
-            |          | dataset | entity_id |
-            |          | trees   | ${id}     |
-            """,
-            """
-            | entities |
-            |          | dataset | entity_id | offline |
-            |          | trees   | ${id}     | no      |
-            """,
-        )
-        survey = """
-        | survey   |
-        |          | type | name | label   |
-        |          | text | id   | Tree id |
-        |          | text | q1   | Q1      |
-        """
-        for i, case in enumerate(cases):
-            with self.subTest(msg=i):
-                self.assertPyxformXform(
-                    md=survey + case,
-                    xml__xpath_match=[
-                        f"""
-                          /h:html/h:head/x:model[
-                            @entities:entities-version = "{co.ENTITIES_UPDATE_VERSION}"
-                          ]
-                        """,
-                        """
-                          /h:html/h:head/x:model/x:instance/x:test_name/x:meta/x:entity[
-                            not(@trunkVersion)
-                            and not(@branchId)
-                          ]
-                        """,
-                        """
-                          /h:html/h:head/x:model[
-                            not(x:bind[@nodeset = '/test_name/meta/entity/@trunkVersion'])
-                          ]
-                        """,
-                        """
-                          /h:html/h:head/x:model[
-                            not(x:bind[@nodeset = '/test_name/meta/entity/@branchId'])
-                          ]
-                        """,
-                    ],
-                )