Repository: XLSForm/pyxform (python)

Issue:
Add support for offline Entity create and update
Spec discussion at https://forum.getodk.org/t/form-spec-proposal-support-entity-updates-from-multiple-offline-clients/47890

We are going to stage this. In the next version of `pyxform`, users will need to explicitly opt in to produce forms that use the v2024.1.0 spec to use offline Entities to allow for some piloting of the functionality across all components. The opt-in flag will be removed in the following version and all Entity-related forms will be produced following the v2024.1.0 spec.

Entity-related forms can create or update Entities or do both. There is existing logic in `pyxform` to determine which of these a form definition specifies.

## Forms that can create Entities
For forms that can create Entities, the only change needed in `pyxform` is to set the `entities-version` attribute on the `model` node to `2024.1.0`.

## Forms that can update Entities
For forms that can update Entities, the `entities-version` attribute on the `model` node will need to be set to `2024.1.0`.

Additionally:
* Add `trunkVersion` and `branchId` attributes to the `entity` element
* Add the following `binds`:
```
<bind nodeset="/data/meta/entity/@trunkVersion" type="string" calculate="instance('trees')/root/item[name=/data/tree]/__trunkVersion" />
<bind nodeset="/data/meta/entity/@branchId" type="string" calculate="instance('trees')/root/item[name=/data/tree]/__branchId" />
```

## Opt in
Initially, the changes described above should be opt-in so we also need to introduce a setting column named `offline` on the `entities` sheet. It should be optional and its default value should be `no`/`false`. If its value is set to `yes`, the changes above should be applied. Otherwise, Entity forms should be generated as they are today.

Pull request:
713: support offline entities spec v2024.1.0 via opt-in setting
Closes #713

#### Why is this the best possible solution? Were any other approaches considered?

Implements #713: opt-in via a new entities setting `offline` with a value `yes`. If not opted-in, retain all current behaviour. If opted-in, for the create case, use the new spec version. If opted-in, for the update case, use the new spec version and emit trunkVersion/branchId properties and bindings.

Deviations to the requirements:

1. Bindings readonly property. I used the existing function in `entity_declaration.py`, `EntityDeclaration._get_bind_node()`, which sets the attribute `readonly="true()"`, which presumably is correct since this function is used for the existing `baseVersion` property with a similar structure.
2. Opt-in behaviour. Like other yes/no settings, the exact value `yes` is accepted, or it's aliases e.g. `Yes`, `true`, etc.
3. Opt-out behaviour. A user can opt out either by omitting the `offline` column, or by providing a value other than an alias for `yes` (such as `no`, `bananas`, or anything else). This seemed more consistent with the requirements than raising an error if the column is present but the value is not an alias for `yes`, although an error may make sense too.

#### What are the regression risks?

Low risk, should be backwards compatible as per the tests suite.

In future when the new spec version is the default, presumably the `offline` setting will be removed, meaning that from then on forms will have to remove that setting or else get an error about an unexpected column. This would be a minor point of friction but beneficial in that the setting would do nothing (all forms would use spec `2024.1.0`) which may be confusing it were kept/allowed.

#### 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.

Updates to the entities spec and ODK docs are already in progress. The new `offline` setting is intended to be kept for only a few months, so I'm not sure if that's worth an update to the XLSForm template, especially if the intended audience during that period is insiders/advanced users. The xlsform.org page doesn't mention entities at all.

#### 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 74adefb..bd569af 100644
--- a/pyxform/constants.py
+++ b/pyxform/constants.py
@@ -114,7 +114,8 @@ CURRENT_XFORMS_VERSION = "1.0.0"
 
 # The ODK entities spec version that generated forms comply to
 ENTITIES_CREATE_VERSION = "2022.1.0"
-CURRENT_ENTITIES_VERSION = "2023.1.0"
+ENTITIES_UPDATE_VERSION = "2023.1.0"
+ENTITIES_OFFLINE_VERSION = "2024.1.0"
 ENTITY = "entity"
 ENTITY_FEATURES = "entity_features"
 ENTITIES_RESERVED_PREFIX = "__"
@@ -126,6 +127,7 @@ 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 51f21bc..017b675 100644
--- a/pyxform/entities/entities_parsing.py
+++ b/pyxform/entities/entities_parsing.py
@@ -1,6 +1,7 @@
 from typing import Any
 
 from pyxform import constants as const
+from pyxform.aliases import yes_no
 from pyxform.errors import PyXFormError
 from pyxform.xlsparseutils import find_sheet_misspellings, is_valid_xml_tag
 
@@ -28,6 +29,7 @@ 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(
@@ -53,6 +55,7 @@ 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 cd85991..b34950a 100644
--- a/pyxform/entities/entity_declaration.py
+++ b/pyxform/entities/entity_declaration.py
@@ -24,23 +24,28 @@ class EntityDeclaration(SurveyElement):
     """
 
     def xml_instance(self, **kwargs):
+        parameters = self.get(const.PARAMETERS, {})
+
         attributes = {
-            EC.DATASET.value: self.get(const.PARAMETERS, {}).get(EC.DATASET, ""),
+            EC.DATASET.value: parameters.get(EC.DATASET, ""),
             "id": "",
         }
 
-        entity_id_expression = self.get(const.PARAMETERS, {}).get(EC.ENTITY_ID, None)
-        create_condition = self.get(const.PARAMETERS, {}).get(EC.CREATE_IF, None)
-        update_condition = self.get(const.PARAMETERS, {}).get(EC.UPDATE_IF, None)
+        entity_id_expression = parameters.get(EC.ENTITY_ID, None)
+        create_condition = parameters.get(EC.CREATE_IF, None)
+        update_condition = parameters.get(EC.UPDATE_IF, None)
 
         if entity_id_expression:
             attributes["update"] = "1"
             attributes["baseVersion"] = ""
+            if parameters.get(EC.OFFLINE, None):
+                attributes["trunkVersion"] = ""
+                attributes["branchId"] = ""
 
         if create_condition or (not update_condition and not entity_id_expression):
             attributes["create"] = "1"
 
-        if self.get(const.PARAMETERS, {}).get(EC.LABEL, None):
+        if parameters.get(EC.LABEL, None):
             return node(const.ENTITY, node(const.LABEL), **attributes)
         else:
             return node(const.ENTITY, **attributes)
@@ -50,10 +55,11 @@ class EntityDeclaration(SurveyElement):
         See the class comment for an explanation of the logic for generating bindings.
         """
         survey = self.get_root()
-        entity_id_expression = self.get(const.PARAMETERS, {}).get(EC.ENTITY_ID, None)
-        create_condition = self.get(const.PARAMETERS, {}).get(EC.CREATE_IF, None)
-        update_condition = self.get(const.PARAMETERS, {}).get(EC.UPDATE_IF, None)
-        label_expression = self.get(const.PARAMETERS, {}).get(EC.LABEL, None)
+        parameters = self.get(const.PARAMETERS, {})
+        entity_id_expression = parameters.get(EC.ENTITY_ID, None)
+        create_condition = parameters.get(EC.CREATE_IF, None)
+        update_condition = parameters.get(EC.UPDATE_IF, None)
+        label_expression = parameters.get(EC.LABEL, None)
 
         bind_nodes = []
 
@@ -69,11 +75,20 @@ class EntityDeclaration(SurveyElement):
             bind_nodes.append(self._get_bind_node(survey, update_condition, "/@update"))
 
         if entity_id_expression:
-            dataset_name = self.get(const.PARAMETERS, {}).get(EC.DATASET, "")
-            base_version_expression = f"instance('{dataset_name}')/root/item[name={entity_id_expression}]/__version"
+            dataset_name = parameters.get(EC.DATASET, "")
+            entity = f"instance('{dataset_name}')/root/item[name={entity_id_expression}]"
             bind_nodes.append(
-                self._get_bind_node(survey, base_version_expression, "/@baseVersion")
+                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")
+                )
 
         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 d5337b6..8f1a69c 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -586,9 +586,13 @@ class Survey(Section):
 
         entity_features = getattr(self, constants.ENTITY_FEATURES, [])
         if len(entity_features) > 0:
-            if "update" in entity_features:
+            if "offline" in entity_features:
                 model_kwargs["entities:entities-version"] = (
-                    constants.CURRENT_ENTITIES_VERSION
+                    constants.ENTITIES_OFFLINE_VERSION
+                )
+            elif "update" in entity_features:
+                model_kwargs["entities:entities-version"] = (
+                    constants.ENTITIES_UPDATE_VERSION
                 )
             else:
                 model_kwargs["entities:entities-version"] = (
diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py
index 6bc7dab..2d19fda 100644
--- a/pyxform/xls2json.py
+++ b/pyxform/xls2json.py
@@ -1538,10 +1538,14 @@ def workbook_to_json(
 
     if len(entity_declaration) > 0:
         json_dict[constants.ENTITY_FEATURES] = ["create"]
+        entity_parameters = entity_declaration.get(constants.PARAMETERS, {})
 
-        if entity_declaration.get("parameters", {}).get("entity_id", None):
+        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")
+
         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 2a39b65..c935af2 100644
--- a/tests/test_entities_create.py
+++ b/tests/test_entities_create.py
@@ -1,3 +1,5 @@
+from pyxform import constants as co
+
 from tests.pyxform_test_case import PyxformTestCase
 
 
@@ -438,3 +440,43 @@ 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 9ec7616..75a0231 100644
--- a/tests/test_entities_update.py
+++ b/tests/test_entities_update.py
@@ -1,3 +1,5 @@
+from pyxform import constants as co
+
 from tests.pyxform_test_case import PyxformTestCase
 
 
@@ -174,3 +176,95 @@ 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'])
+                          ]
+                        """,
+                    ],
+                )