feat(organizations): Add mongo_uuid field to XForm model and popula…

…te it TASK-957 (#5196) ## Checklist 1. [x] 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] Run `./python-format.sh` to make sure that your code lints and that you've followed [our coding style](https://github.com/kobotoolbox/kpi/blob/main/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 8. [ ] Create a testing plan for the reviewer and add it to the Testing section ## Description This PR introduces a new field, mongo_uuid, to the XForm model in KoboToolbox. This unique identifier is designed to enhance the ownership transfer process of XForms by ensuring that each form is linked to a distinct UUID in MongoDB. The addition of mongo_uuid will streamline how XForms are managed, especially during ownership transfers, by reducing unnecessary updates to the MongoDB documents related to the XForm. ## Notes - A new column (mongo_uuid) has been added to the XForm model, which will serve as a unique identifier. Ownership Transfer: The logic for transferring an XForm's ownership has been updated to ensure that the mongo_uuid is populated if it is currently null. - All existing MongoDB documents that match the _userform_id will be updated to include the new mongo_uuid value, ensuring consistency across the database. - The ParsedInstance.to_dict_for_mongo() method has been modified to prioritize the mongo_uuid over _userform_id when exporting data. If mongo_uuid is not null, _userform_id will be set to mongo_uuid; otherwise, the original logic will be maintained. - The mongo_userform_id property in the OpenRosaDeploymentBackend class has been updated to return the mongo_uuid when available, ensuring that the correct ID is used during operations. --------- Co-authored-by: Olivier Leger <[email protected]>
kobotoolbox · Nov 6, 2024 · e41ba34 · e41ba34
1 parent 1574ad2
commit e41ba34
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 50 deletions.
diff --git a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py
@@ -287,6 +287,7 @@ def test_xform_serializer_none(self):
             'num_of_submissions': 0,
             'attachment_storage_bytes': 0,
             'kpi_asset_uid': '',
+            'mongo_uuid': '',
         }
         self.assertEqual(data, XFormSerializer(None).data)
 

diff --git a/kobo/apps/openrosa/apps/logger/migrations/0038_add_mongo_uuid_field_to_xform.py b/kobo/apps/openrosa/apps/logger/migrations/0038_add_mongo_uuid_field_to_xform.py
@@ -0,0 +1,22 @@
+# Generated by Django 4.2.15 on 2024-10-24 20:16
+
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('logger', '0037_remove_xform_has_kpi_hooks_and_instance_posted_to_kpi'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='xform',
+            name='mongo_uuid',
+            field=models.CharField(
+                db_index=True, max_length=100, null=True, unique=True
+            ),
+        ),
+    ]
diff --git a/kobo/apps/openrosa/apps/logger/models/xform.py b/kobo/apps/openrosa/apps/logger/models/xform.py
@@ -77,6 +77,9 @@ class XForm(AbstractTimeStampedModel):
     last_submission_time = models.DateTimeField(blank=True, null=True)
     has_start_time = models.BooleanField(default=False)
     uuid = models.CharField(max_length=32, default='', db_index=True)
+    mongo_uuid = models.CharField(
+        max_length=100, null=True, unique=True, db_index=True
+    )
 
     uuid_regex = re.compile(r'(<instance>.*?id="[^"]+">)(.*</instance>)(.*)',
                             re.DOTALL)

diff --git a/kobo/apps/openrosa/apps/viewer/models/parsed_instance.py b/kobo/apps/openrosa/apps/viewer/models/parsed_instance.py
@@ -259,12 +259,17 @@ def _get_paginated_and_sorted_cursor(cls, cursor, start, limit, sort):
 
     def to_dict_for_mongo(self):
         d = self.to_dict()
+
+        userform_id = (
+            self.instance.xform.mongo_uuid
+            if self.instance.xform.mongo_uuid
+            else f'{self.instance.xform.user.username}_{self.instance.xform.id_string}'
+        )
+
         data = {
             UUID: self.instance.uuid,
             ID: self.instance.id,
-            self.USERFORM_ID: '%s_%s' % (
-                self.instance.xform.user.username,
-                self.instance.xform.id_string),
+            self.USERFORM_ID: userform_id,
             ATTACHMENTS: _get_attachments_from_instance(self.instance),
             self.STATUS: self.instance.status,
             GEOLOCATION: [self.lat, self.lng],

diff --git a/kobo/apps/project_ownership/tests/api/v2/test_api.py b/kobo/apps/project_ownership/tests/api/v2/test_api.py
@@ -500,17 +500,79 @@ def test_data_accessible_to_new_user(self):
         for attachment in response.data['results'][0]['_attachments']:
             assert attachment['filename'].startswith('anotheruser/')
 
+        # Get the mongo_uuid for the transferred asset (XForm)
+        self.asset.deployment.xform.refresh_from_db()
+        mongo_uuid = self.asset.deployment.xform.mongo_uuid
+
         assert (
             settings.MONGO_DB.instances.count_documents(
                 {'_userform_id': f'someuser_{self.asset.uid}'}
             ) == 0
         )
         assert (
             settings.MONGO_DB.instances.count_documents(
-                {'_userform_id': f'anotheruser_{self.asset.uid}'}
+                {'_userform_id': mongo_uuid}
+            ) == 1
+        )
+
+    @patch(
+        'kobo.apps.project_ownership.models.transfer.reset_kc_permissions',
+        MagicMock()
+    )
+    @patch(
+        'kobo.apps.project_ownership.tasks.move_attachments',
+        MagicMock()
+    )
+    @patch(
+        'kobo.apps.project_ownership.tasks.move_media_files',
+        MagicMock()
+    )
+    @override_config(PROJECT_OWNERSHIP_AUTO_ACCEPT_INVITES=True)
+    def test_mongo_uuid_after_transfer(self):
+        """
+        Test that after an ownership transfer, the XForm's MongoDB document
+        updates to use the `mongo_uuid` as the `_userform_id` instead of the
+        original owner's identifier
+        """
+        self.client.login(username='someuser', password='someuser')
+        original_userform_id = f'someuser_{self.asset.uid}'
+        assert (
+            settings.MONGO_DB.instances.count_documents(
+                {'_userform_id': original_userform_id}
+            ) == 1
+        )
+
+        # Transfer the project from someuser to anotheruser
+        payload = {
+            'recipient': self.absolute_reverse(
+                self._get_endpoint('user-kpi-detail'),
+                args=[self.anotheruser.username]
+            ),
+            'assets': [self.asset.uid]
+        }
+
+        with immediate_on_commit():
+            response = self.client.post(self.invite_url, data=payload, format='json')
+        assert response.status_code == status.HTTP_201_CREATED
+
+        # Get the mongo_uuid for the transferred asset (XForm)
+        self.asset.deployment.xform.refresh_from_db()
+        mongo_uuid = self.asset.deployment.xform.mongo_uuid
+
+        # Verify MongoDB now uses mongo_uuid as the identifier
+        assert (
+            settings.MONGO_DB.instances.count_documents(
+                {'_userform_id': mongo_uuid}
             ) == 1
         )
 
+        # Confirm the original `_userform_id` is no longer used
+        assert (
+            settings.MONGO_DB.instances.count_documents(
+                {'_userform_id': original_userform_id}
+            ) == 0
+        )
+
 
 class ProjectOwnershipInAppMessageAPITestCase(KpiTestCase):
 

diff --git a/kobo/apps/project_ownership/utils.py b/kobo/apps/project_ownership/utils.py
@@ -172,6 +172,8 @@ def rewrite_mongo_userform_id(transfer: 'project_ownership.Transfer'):
     if not transfer.asset.has_deployment:
         return
 
+    transfer.asset.deployment.set_mongo_uuid()
+
     if not transfer.asset.deployment.transfer_submissions_ownership(
         old_owner.username
     ):

diff --git a/kpi/deployment_backends/openrosa_backend.py b/kpi/deployment_backends/openrosa_backend.py
@@ -61,6 +61,7 @@
     SubmissionNotFoundException,
     XPathNotFoundException,
 )
+from kpi.fields import KpiUidField
 from kpi.interfaces.sync_backend_media import SyncBackendMediaInterface
 from kpi.models.asset_file import AssetFile
 from kpi.models.object_permission import ObjectPermission
@@ -731,7 +732,10 @@ def get_validation_status(
 
     @property
     def mongo_userform_id(self):
-        return '{}_{}'.format(self.asset.owner.username, self.xform_id_string)
+        return (
+            self.xform.mongo_uuid
+            or f'{self.asset.owner.username}_{self.xform_id_string}'
+        )
 
     @staticmethod
     def nlp_tracking_data(
@@ -959,6 +963,14 @@ def set_enketo_open_rosa_server(self, require_auth: bool, enketo_id: str = None)
             server_url,
         )
 
+    def set_mongo_uuid(self):
+        """
+        Set the `mongo_uuid` for the associated XForm if it's not already set.
+        """
+        if not self.xform.mongo_uuid:
+            self.xform.mongo_uuid = KpiUidField.generate_unique_id()
+            self.xform.save(update_fields=['mongo_uuid'])
+
     def set_validation_status(
         self,
         submission_id: int,
@@ -1218,6 +1230,7 @@ def xform(self):
                 'attachment_storage_bytes',
                 'require_auth',
                 'uuid',
+                'mongo_uuid',
             )
             .select_related('user')  # Avoid extra query to validate username below
             .first()

diff --git a/kpi/fixtures/conflicting_versions.json b/kpi/fixtures/conflicting_versions.json
@@ -130,8 +130,7 @@
     "_deployment_data": {
       "active": true,
       "version": "vqu4EaVFry5MirsJcBbhsQ",
-      "backend": "mock",
-      "identifier": "mock://axD3Wc8ZnfgLXBcURRt5fM"
+      "backend": "mock"
     },
     "asset": 713928,
     "deployed": true,
@@ -221,8 +220,7 @@
     "_deployment_data": {
       "active": true,
       "version": "vnPWjPhx4VP6V24Cvn2EhG",
-      "backend": "mock",
-      "identifier": "mock://axD3Wc8ZnfgLXBcURRt5fM"
+      "backend": "mock"
     },
     "asset": 713928,
     "deployed": true,
@@ -330,8 +328,7 @@
     "_deployment_data": {
       "active": true,
       "version": "vGcztYhnRo9CCLv7biByAt",
-      "backend": "mock",
-      "identifier": "mock://axD3Wc8ZnfgLXBcURRt5fM"
+      "backend": "mock"
     },
     "asset": 713928,
     "deployed": true,
@@ -496,26 +493,23 @@
       "active": true,
       "version": "vKYFpP4BG5593cJrsYSUgg",
       "backend": "mock",
-      "identifier": "mock://axD3Wc8ZnfgLXBcURRt5fM",
+      "backend_response": {
+        "hash": "md5:d41d8cd98f00b204e9800998ecf8427e",
+        "formid": 713928,
+        "uuid": "d9e0d163e34c4524b5fd35b394f7aa26",
+        "id_string": "axD3Wc8ZnfgLXBcURRt5fM",
+        "kpi_asset_uid": "axD3Wc8ZnfgLXBcURRt5fM"
+      },
       "submissions": [
         {
-          "_id" : 713934,
-          "_notes" : [ ],
           "meta/instanceID" : "uuid:be5817e1-dd67-42b0-a6bd-30bf5ecfe4d3",
           "end" : "2017-12-18T17:47:09.000-05:00",
           "created_in_second_version" : "submitted to second version (2)",
-          "_bamboo_dataset_id" : "",
           "_uuid" : "be5817e1-dd67-42b0-a6bd-30bf5ecfe4d3",
           "created_in_first_version" : "submitted to second version (1)",
-          "_tags" : [ ],
-          "_attachments" : [ ],
           "_submission_time" : "2017-12-18T22:47:10",
           "start" : "2017-12-18T17:46:22.000-05:00",
-          "_submitted_by" : null,
-          "_geolocation" : [
-            null,
-            null
-          ],
+          "_submitted_by" : "someuser",
           "_xform_id_string" : "axD3Wc8ZnfgLXBcURRt5fM",
           "_userform_id" : "someuser_axD3Wc8ZnfgLXBcURRt5fM",
           "_version_" : "vnPWjPhx4VP6V24Cvn2EhG",
@@ -524,13 +518,9 @@
           "formhub/uuid" : "d9e0d163e34c4524b5fd35b394f7aa26"
         },
         {
-          "_id" : 713935,
-          "_notes" : [ ],
           "_version__001" : "vGcztYhnRo9CCLv7biByAt",
           "_uuid" : "a95a0de5-06d2-43ab-b78f-fb2885ceac61",
-          "_bamboo_dataset_id" : "",
-          "_tags" : [ ],
-          "_submitted_by" : null,
+          "_submitted_by" : "someuser",
           "_xform_id_string" : "axD3Wc8ZnfgLXBcURRt5fM",
           "created_in_third_version" : "submitted to third version (3)",
           "meta/instanceID" : "uuid:a95a0de5-06d2-43ab-b78f-fb2885ceac61",
@@ -539,26 +529,18 @@
           "start" : "2017-12-18T17:48:09.000-05:00",
           "_submission_time" : "2017-12-18T22:48:21",
           "created_in_first_version" : "submitted to third version (1)",
-          "_attachments" : [ ],
           "created_in_second_version" : "submitted to third version (2)",
-          "_geolocation" : [
-            null,
-            null
-          ],
           "_userform_id" : "someuser_axD3Wc8ZnfgLXBcURRt5fM",
           "_version_" : "vnPWjPhx4VP6V24Cvn2EhG",
           "_status" : "submitted_via_web",
           "__version__" : "vqu4EaVFry5MirsJcBbhsQ"
         },
         {
-          "_id" : 713936,
-          "_notes" : [ ],
           "_version__001" : "vGcztYhnRo9CCLv7biByAt",
           "_version__002" : "vKYFpP4BG5593cJrsYSUgg",
           "_uuid" : "bcbfcdfc-891a-4513-80b3-d9b98bd87553",
           "_bamboo_dataset_id" : "",
-          "_tags" : [ ],
-          "_submitted_by" : null,
+          "_submitted_by" : "someuser",
           "_xform_id_string" : "axD3Wc8ZnfgLXBcURRt5fM",
           "created_in_third_version" : "submitted to fourth version (3)",
           "meta/instanceID" : "uuid:bcbfcdfc-891a-4513-80b3-d9b98bd87553",
@@ -567,37 +549,24 @@
           "start" : "2017-12-18T17:49:19.000-05:00",
           "_submission_time" : "2017-12-18T22:49:38",
           "created_in_first_version" : "submitted to fourth version (1)",
-          "_attachments" : [ ],
           "created_in_second_version" : "submitted to fourth version (2)",
-          "_geolocation" : [
-            null,
-            null
-          ],
           "_userform_id" : "someuser_axD3Wc8ZnfgLXBcURRt5fM",
           "_version_" : "vnPWjPhx4VP6V24Cvn2EhG",
           "_status" : "submitted_via_web",
           "__version__" : "vqu4EaVFry5MirsJcBbhsQ",
           "created_in_fourth_version" : "submitted to fourth version (4)"
         },
         {
-          "_id" : 713937,
-          "_notes" : [ ],
           "_uuid" : "a6bac3ac-c9aa-4556-a1c0-b1d44d51ca65",
           "_bamboo_dataset_id" : "",
-          "_tags" : [ ],
-          "_submitted_by" : null,
+          "_submitted_by" : "someuser",
           "_xform_id_string" : "axD3Wc8ZnfgLXBcURRt5fM",
           "meta/instanceID" : "uuid:a6bac3ac-c9aa-4556-a1c0-b1d44d51ca65",
           "formhub/uuid" : "d9e0d163e34c4524b5fd35b394f7aa26",
           "end" : "2017-12-18T17:45:26.000-05:00",
           "_submission_time" : "2017-12-18T22:45:26",
           "created_in_first_version" : "submitted to first version",
-          "_attachments" : [ ],
           "start" : "2017-12-18T17:45:15.000-05:00",
-          "_geolocation" : [
-            null,
-            null
-          ],
           "_userform_id" : "someuser_axD3Wc8ZnfgLXBcURRt5fM",
           "_status" : "submitted_via_web",
           "__version__" : "vqu4EaVFry5MirsJcBbhsQ"
@@ -611,5 +580,17 @@
   },
   "model": "kpi.asset",
   "pk": 713928
+},
+{
+  "fields": {
+    "title": "conflicting versions",
+    "user": 2,
+    "xml": "",
+    "id_string": "axD3Wc8ZnfgLXBcURRt5fM",
+    "date_created": "2017-12-18T22:42:59.233Z",
+    "kpi_asset_uid": "axD3Wc8ZnfgLXBcURRt5fM"
+  },
+  "model": "logger.xform",
+  "pk": 713928
 }
 ]
diff --git a/kpi/tests/test_mock_data_conflicting_version_exports.py b/kpi/tests/test_mock_data_conflicting_version_exports.py
@@ -27,8 +27,13 @@ def setUp(self):
         self.maxDiff = None
         self.user = User.objects.get(username='someuser')
         self.asset = Asset.objects.get(uid='axD3Wc8ZnfgLXBcURRt5fM')
+        # To avoid cluttering the fixture, redeploy asset to set related XForm properly
+        self.asset.deployment.redeploy(active=True)
         # To avoid cluttering the fixture, assign permissions here
         self.asset.assign_perm(self.user, PERM_VIEW_SUBMISSIONS)
+        self.asset.deployment.mock_submissions(
+            submissions=self.asset._deployment_data['submissions'],
+        )
         self.submissions = self.asset.deployment.get_submissions(self.asset.owner)
         self.submission_id_field = '_id'
         self.formpack, self.submission_stream = report_data.build_formpack(