From 1574ad23776ae955d66551483f0cdce4d1fd41ac Mon Sep 17 00:00:00 2001 From: RuthShryock <81720958+RuthShryock@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:30:17 -0500 Subject: [PATCH] Upgrade pyxform to v2.1.1 (#5126) ## Description The previous version was 1.9.0. Most workflows are not affected, but if you interact directly with XForm XML, you should refer to the changes listed at /~https://github.com/XLSForm/pyxform/blob/master/CHANGES.txt, particularly the major version v2.0.0. These do affect the generated XForm XML. ## Checklist 1. [ ] If you've added code that should be tested, add tests 2. [ ] If you've changed APIs, update (or create!) the documentation 3. [x] Ensure the tests pass 4. [x] Make sure that your code lints and that you've followed [our coding style](/~https://github.com/kobotoolbox/kpi/blob/master/CONTRIBUTING.md) 5. [x] Write a title and, if necessary, a description of your work suitable for publishing in our [release notes](https://community.kobotoolbox.org/tag/release-notes) 6. [ ] Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE) 7. [ ] Open an issue in the [docs](/~https://github.com/kobotoolbox/docs/issues/new) if there are UI/UX changes ## Notes Update pyxform to version [2.1.1](/~https://github.com/XLSForm/pyxform/releases/tag/v2.1.1). This also modifies the format of the xls file that we send to the pyxform method `create_survey_from_xls`. Before the xls was a Django FieldFile object but we now covert it into a binary stream using `io.BytesIO` to create a file-like object in order to be compatible with the pyxform updates. This update is deployed to [kf.du.kbtdev.org](https://kf.du.kbtdev.org/) for testing. --------- Co-authored-by: John N. Milner --- dependencies/pip/dev_requirements.txt | 4 +- dependencies/pip/requirements.in | 2 +- dependencies/pip/requirements.txt | 4 +- .../tests/fixtures/Transportation Form.xml | 326 +++++++---------- .../transportation/transportation.xml | 329 +++++++----------- .../apps/viewer/models/data_dictionary.py | 4 +- kpi/tests/test_asset_snapshots.py | 2 +- kpi/utils/pyxform_compatibility.py | 38 ++ 8 files changed, 308 insertions(+), 401 deletions(-) diff --git a/dependencies/pip/dev_requirements.txt b/dependencies/pip/dev_requirements.txt index 13c4b905f7..076b32a089 100644 --- a/dependencies/pip/dev_requirements.txt +++ b/dependencies/pip/dev_requirements.txt @@ -414,7 +414,7 @@ oauthlib==3.2.2 # -r dependencies/pip/requirements.in # django-oauth-toolkit # requests-oauthlib -openpyxl==3.0.9 +openpyxl==3.1.3 # via # -r dependencies/pip/requirements.in # pyxform @@ -537,7 +537,7 @@ pytz==2024.1 # via # flower # pandas -pyxform==1.9.0 +pyxform==2.1.1 # via # -r dependencies/pip/requirements.in # formpack diff --git a/dependencies/pip/requirements.in b/dependencies/pip/requirements.in index 5db53d0e5e..8f179255ad 100644 --- a/dependencies/pip/requirements.in +++ b/dependencies/pip/requirements.in @@ -75,7 +75,7 @@ openpyxl psycopg pymongo python-dateutil -pyxform==1.9.0 +pyxform==2.1.1 requests regex responses diff --git a/dependencies/pip/requirements.txt b/dependencies/pip/requirements.txt index 41e5731fa0..bcf2400265 100644 --- a/dependencies/pip/requirements.txt +++ b/dependencies/pip/requirements.txt @@ -336,7 +336,7 @@ oauthlib==3.2.2 # -r dependencies/pip/requirements.in # django-oauth-toolkit # requests-oauthlib -openpyxl==3.0.9 +openpyxl==3.1.3 # via # -r dependencies/pip/requirements.in # pyxform @@ -412,7 +412,7 @@ pytz==2024.1 # via # flower # pandas -pyxform==1.9.0 +pyxform==2.1.1 # via # -r dependencies/pip/requirements.in # formpack diff --git a/kobo/apps/openrosa/apps/api/tests/fixtures/Transportation Form.xml b/kobo/apps/openrosa/apps/api/tests/fixtures/Transportation Form.xml index 073131011f..98260af363 100644 --- a/kobo/apps/openrosa/apps/api/tests/fixtures/Transportation Form.xml +++ b/kobo/apps/openrosa/apps/api/tests/fixtures/Transportation Form.xml @@ -1,5 +1,5 @@ - + transportation_2011_07_25 @@ -39,6 +39,9 @@ + + + @@ -46,18 +49,83 @@ + + + + ambulance + + + + bicycle + + + + boat_canoe + + + + bus + + + + donkey_mule_cart + + + + keke_pepe + + + + lorry + + + + motorbike + + + + taxi + + + + other + + + + + + + + daily + + + + weekly + + + + other + + + + dont_know + + + + - - - - - - - - - - - + + + + + + + + + + + + @@ -66,46 +134,10 @@ @@ -115,198 +147,100 @@ - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + + + + + + + + + + diff --git a/kobo/apps/openrosa/apps/main/tests/fixtures/transportation/transportation.xml b/kobo/apps/openrosa/apps/main/tests/fixtures/transportation/transportation.xml index 4305eaaeca..795d62af52 100644 --- a/kobo/apps/openrosa/apps/main/tests/fixtures/transportation/transportation.xml +++ b/kobo/apps/openrosa/apps/main/tests/fixtures/transportation/transportation.xml @@ -1,5 +1,4 @@ - - + transportation_2011_07_25 @@ -39,6 +38,9 @@ + + + @@ -46,19 +48,84 @@ + + + + ambulance + + + + bicycle + + + + boat_canoe + + + + bus + + + + donkey_mule_cart + + + + keke_pepe + + + + lorry + + + + motorbike + + + + taxi + + + + other + + + + + + + + daily + + + + weekly + + + + other + + + + dont_know + + + + - - - - - - - - - - - - + + + + + + + + + + + + + @@ -66,46 +133,10 @@ @@ -115,198 +146,100 @@ - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + - - - daily - - - - weekly - - - - other - - - - dont_know - + + + + + + + + + + + + diff --git a/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py b/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py index 50abab70a2..6a9922315a 100644 --- a/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py +++ b/kobo/apps/openrosa/apps/viewer/models/data_dictionary.py @@ -21,6 +21,7 @@ from kobo.apps.openrosa.libs.utils.model_tools import queryset_iterator, set_uuid from kpi.constants import DEFAULT_SURVEY_NAME from kpi.utils.mongo_helper import MongoHelper +from kpi.utils.pyxform_compatibility import NamedBytesIO class ColumnRename(models.Model): @@ -157,8 +158,9 @@ def add_instances(self): def save(self, *args, **kwargs): if self.xls: + xls_io = NamedBytesIO.fromfieldfile(self.xls) survey = create_survey_from_xls( - self.xls, default_name=DEFAULT_SURVEY_NAME + xls_io, default_name=DEFAULT_SURVEY_NAME ) if survey.name == DEFAULT_SURVEY_NAME: survey.name = survey.id_string diff --git a/kpi/tests/test_asset_snapshots.py b/kpi/tests/test_asset_snapshots.py index 724db532c4..54b8020b01 100644 --- a/kpi/tests/test_asset_snapshots.py +++ b/kpi/tests/test_asset_snapshots.py @@ -78,7 +78,7 @@ def test_snapshots_allow_choice_duplicates(self): 'settings': {}, } snap = AssetSnapshot.objects.create(source=content) - assert snap.xml.count('ABC') == 2 + assert snap.xml.count('ABC') == 2 class AssetSnapshotHousekeeping(AssetSnapshotsTestCase): diff --git a/kpi/utils/pyxform_compatibility.py b/kpi/utils/pyxform_compatibility.py index 07895a4860..6d3dc61f4a 100644 --- a/kpi/utils/pyxform_compatibility.py +++ b/kpi/utils/pyxform_compatibility.py @@ -1,5 +1,7 @@ +from io import BytesIO from pyxform.constants import ALLOW_CHOICE_DUPLICATES + def allow_choice_duplicates(content: dict) -> None: """ Modify `content` to include `allow_choice_duplicates=Yes` in the settings @@ -13,3 +15,39 @@ def allow_choice_duplicates(content: dict) -> None: settings = content.setdefault('settings', {}) if ALLOW_CHOICE_DUPLICATES not in settings: settings[ALLOW_CHOICE_DUPLICATES] = 'yes' + + +class NamedBytesIO(BytesIO): + """ + Changes in XLSForm/pyxform#718 prevent + `pyxform.builder.create_survey_from_xls()` from accepting a + `django.db.models.fields.files.FieldFile`. Only instances of + `bytes | BytesIO | IOBase` are now accepted for treatment as file-like + objects, and furthermore, anything that is not already a `BytesIO` will + have its contents placed inside a newly instantiated one. + + Problem: `BytesIO`s do not have `name`s, and the constructor for + `pyxform.xls2json.SurveyReader` fails because of that. + + Workaround: a `BytesIO` with a `name` 🙃 + + For more details, see + /~https://github.com/kobotoolbox/kpi/pull/5126#discussion_r1829763316 + """ + + def __init__(self, *args, name=None, **kwargs): + if name is None: + raise NotImplementedError('Use `BytesIO` if no `name` is needed') + super().__init__(*args, **kwargs) + self.name = name + + @classmethod + def fromfieldfile(cls, django_fieldfile): + """ + Given a Django `FieldFile`, return an instance of `NamedBytesIO` + + à la `datetime.datetime.fromtimestamp()` + """ + new_instance = cls(django_fieldfile.read(), name=django_fieldfile.name) + django_fieldfile.seek(0) # Be kind: rewind + return new_instance