Merge pull request #3757 from kobotoolbox/allow-duplicate-choices-in-…

…snapshots

Allow duplicate choices in snapshots

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"
         }

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:

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):

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'),

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):

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('<value>ABC</value>') == 2

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):
 

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'