Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TASK-957] Add mongo_uuid field to XForm model and populate it #5196

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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
),
),
]
3 changes: 3 additions & 0 deletions kobo/apps/openrosa/apps/logger/models/xform.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 8 additions & 3 deletions kobo/apps/openrosa/apps/viewer/models/parsed_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
65 changes: 64 additions & 1 deletion kobo/apps/project_ownership/tests/api/v2/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from rest_framework import status
from rest_framework.reverse import reverse

from kobo.apps.openrosa.apps.logger.models import XForm
from kobo.apps.project_ownership.models import Invite, InviteStatusChoices, Transfer
from kobo.apps.trackers.utils import update_nlp_counter
from kpi.constants import PERM_VIEW_ASSET
Expand Down Expand Up @@ -500,17 +501,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)
xform = XForm.objects.get(kpi_asset_uid=self.asset.uid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the XForm object from the deployment, i.e.: self.asset.deployment.xform

mongo_uuid = 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

# Retrieve the mongo_uuid for the transferred asset (XForm)
xform = XForm.objects.get(kpi_asset_uid=self.asset.uid)
mongo_uuid = 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):

Expand Down
2 changes: 2 additions & 0 deletions kobo/apps/project_ownership/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
):
Expand Down
25 changes: 24 additions & 1 deletion kpi/deployment_backends/openrosa_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
except ImportError:
from backports.zoneinfo import ZoneInfo

import secrets
import string
import redis.exceptions
import requests
from defusedxml import ElementTree as DET
Expand Down Expand Up @@ -731,7 +733,13 @@ def get_validation_status(

@property
def mongo_userform_id(self):
return '{}_{}'.format(self.asset.owner.username, self.xform_id_string)
try:
return (
self.xform.mongo_uuid
or f'{self.asset.owner.username}_{self.xform_id_string}'
)
except KeyError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KeyError? when can this error happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review Olivier. The KeyError can occur when the formid key is missing from the backend_response dictionary. This is typically the case in certain test scenarios, such as ConflictingVersionsMockDataExports, where mock data doesn't include the formid key.

When self.xform is accessed, it attempts to retrieve the formid from backend_response, which raises a KeyError if it's not present.

return f'{self.asset.owner.username}_{self.xform_id_string}'

@staticmethod
def nlp_tracking_data(
Expand Down Expand Up @@ -1218,6 +1226,7 @@ def xform(self):
'attachment_storage_bytes',
'require_auth',
'uuid',
'mongo_uuid',
)
.select_related('user') # Avoid extra query to validate username below
.first()
Expand All @@ -1234,6 +1243,20 @@ def xform(self):
self._xform = xform
return self._xform

def generate_unique_key(self, length=20):

characters = string.ascii_letters + string.digits
# Generate a secure key with the specified length
return ''.join(secrets.choice(characters) for _ in range(length))

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 = self.generate_unique_key()
self.xform.save(update_fields=['mongo_uuid'])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep methods sorted alphabetically and let's use KpiUidField.generate_unique_id() which already generates a random string (don't forget to clean imports after).

@property
def xform_id(self):
return self.xform.pk
Expand Down
Loading