diff --git a/kpi/fixtures/test_data.json b/kpi/fixtures/test_data.json index 4a772bf320..f76967a5bc 100644 --- a/kpi/fixtures/test_data.json +++ b/kpi/fixtures/test_data.json @@ -112,8 +112,8 @@ "kuid": "def" } ]}, - "date_created": "2015-02-12T19:52:14.406Z", - "date_modified": "2015-02-12T19:52:14.406Z", + "date_created": "2022-04-05T21:00:22.402Z", + "date_modified": "2022-04-05T21:00:33.946Z", "owner": 2, "uid": "aPEANgNgTH3xzhJGcctURu" } diff --git a/kpi/models/asset_snapshot.py b/kpi/models/asset_snapshot.py index 78fae4ac35..8d8e5f0203 100644 --- a/kpi/models/asset_snapshot.py +++ b/kpi/models/asset_snapshot.py @@ -17,6 +17,7 @@ from kpi.utils.hash import calculate_hash from kpi.utils.log import logging from kpi.utils.models import DjangoModelABCMetaclass +from kpi.utils.pyxform_compatibility import allow_choice_duplicates class AbstractFormList( @@ -197,6 +198,8 @@ def generate_xml_from_source(self, self._populate_fields_with_autofields(source_copy) self._strip_kuids(source_copy) + allow_choice_duplicates(source_copy) + warnings = [] details = {} try: diff --git a/kpi/tests/api/v1/test_api_imports.py b/kpi/tests/api/v1/test_api_imports.py index 6ce9999de2..4e02137ac0 100644 --- a/kpi/tests/api/v1/test_api_imports.py +++ b/kpi/tests/api/v1/test_api_imports.py @@ -23,7 +23,7 @@ class AssetImportTaskTest(BaseTestCase): def setUp(self): self.client.login(username='someuser', password='someuser') self.user = User.objects.get(username='someuser') - self.asset = Asset.objects.first() + self.asset = Asset.objects.get(pk=1) def _assert_assets_contents_equal(self, a1, a2, sheet='survey'): def _prep_row_for_comparison(row): diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index 06b4bc2f20..50701f246a 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -81,7 +81,7 @@ def test_asset_list_matches_detail(self): def test_assets_hash(self): another_user = User.objects.get(username="anotheruser") - user_asset = Asset.objects.first() + user_asset = Asset.objects.get(pk=1) user_asset.save() user_asset.assign_perm(another_user, "view_asset") @@ -257,7 +257,7 @@ class AssetVersionApiTests(BaseTestCase): def setUp(self): self.client.login(username='someuser', password='someuser') - self.asset = Asset.objects.first() + self.asset = Asset.objects.get(pk=1) self.asset.save() self.version = self.asset.asset_versions.first() self.version_list_url = reverse(self._get_endpoint('asset-version-list'), diff --git a/kpi/tests/api/v2/test_api_imports.py b/kpi/tests/api/v2/test_api_imports.py index bb86c2670d..9ca534e059 100644 --- a/kpi/tests/api/v2/test_api_imports.py +++ b/kpi/tests/api/v2/test_api_imports.py @@ -23,7 +23,7 @@ class AssetImportTaskTest(BaseTestCase): def setUp(self): self.client.login(username='someuser', password='someuser') self.user = User.objects.get(username='someuser') - self.asset = Asset.objects.first() + self.asset = Asset.objects.get(pk=1) def _assert_assets_contents_equal(self, a1, a2, sheet='survey'): def _prep_row_for_comparison(row): diff --git a/kpi/tests/test_asset_snapshots.py b/kpi/tests/test_asset_snapshots.py index 85969c061a..a78cd6e831 100644 --- a/kpi/tests/test_asset_snapshots.py +++ b/kpi/tests/test_asset_snapshots.py @@ -55,3 +55,23 @@ def test_xml_export_auto_title(self): asset = Asset.objects.create(asset_type='survey', content=content) _snapshot = asset.snapshot self.assertEqual(_snapshot.source.get('settings')['form_title'], 'no_title_asset') + + def test_snapshots_allow_choice_duplicates(self): + """ + Choice duplicates should be allowed here but *not* when deploying + a survey + """ + content = { + 'survey': [ + {'type': 'select_multiple', + 'select_from_list_name': 'xxx', + 'label': 'pick one'}, + ], + 'choices': [ + {'list_name': 'xxx', 'label': 'ABC', 'name': 'ABC'}, + {'list_name': 'xxx', 'label': 'Also ABC', 'name': 'ABC'}, + ], + 'settings': {}, + } + snap = AssetSnapshot.objects.create(source=content) + assert snap.xml.count('ABC') == 2 diff --git a/kpi/tests/test_utils.py b/kpi/tests/test_utils.py index 964c02ebdc..4eb4def64c 100644 --- a/kpi/tests/test_utils.py +++ b/kpi/tests/test_utils.py @@ -11,6 +11,7 @@ from kpi.exceptions import SearchQueryTooShortException from kpi.utils.autoname import autoname_fields, autoname_fields_to_field from kpi.utils.autoname import autovalue_choices_in_place +from kpi.utils.pyxform_compatibility import allow_choice_duplicates from kpi.utils.query_parser import parse from kpi.utils.sluggify import sluggify, sluggify_label from kpi.utils.xml import strip_nodes, edit_submission_xml @@ -224,6 +225,34 @@ def test_query_parser_default_search_too_short(self): parse(query_string, default_field_lookups) assert 'Your query is too short' in str(e.exception) + def test_allow_choice_duplicates(self): + surv = { + 'survey': [ + {'type': 'select_multiple', + 'select_from_list_name': 'xxx'}, + ], + 'choices': [ + {'list_name': 'xxx', 'label': 'ABC', 'name': 'ABC'}, + {'list_name': 'xxx', 'label': 'Also ABC', 'name': 'ABC'}, + ], + 'settings': {}, + } + + # default should be 'yes' + allow_choice_duplicates(surv) + assert ( + surv['settings']['allow_choice_duplicates'] + == 'yes' + ) + + # 'no' should not be overwritten + surv['settings']['allow_choice_duplicates'] = 'no' + allow_choice_duplicates(surv) + assert ( + surv['settings']['allow_choice_duplicates'] + == 'no' + ) + class XmlUtilsTestCase(TestCase): diff --git a/kpi/utils/pyxform_compatibility.py b/kpi/utils/pyxform_compatibility.py new file mode 100644 index 0000000000..07895a4860 --- /dev/null +++ b/kpi/utils/pyxform_compatibility.py @@ -0,0 +1,15 @@ +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 + so long as that setting does not already exist. + This should *not* be done for content that's being newly updated by a user, + but rather *only* for internal conversions of existing content, e.g. when + generating XML for an `AssetVersion` that was created prior to pyxform + prohibiting duplicate choice values (aka choice names). + See https://github.com/kobotoolbox/kpi/issues/3751 + """ + settings = content.setdefault('settings', {}) + if ALLOW_CHOICE_DUPLICATES not in settings: + settings[ALLOW_CHOICE_DUPLICATES] = 'yes'