diff --git a/kobo/apps/project_ownership/management/commands/resume_failed_transfers_2_024_25_fix.py b/kobo/apps/project_ownership/management/commands/resume_failed_transfers_2_024_25_fix.py index 34e43a49f9..6be2719b24 100644 --- a/kobo/apps/project_ownership/management/commands/resume_failed_transfers_2_024_25_fix.py +++ b/kobo/apps/project_ownership/management/commands/resume_failed_transfers_2_024_25_fix.py @@ -41,7 +41,8 @@ def handle(self, *args, **options): if not self._validate_whether_transfer_can_be_fixed(transfer): if verbosity: self.stdout.write( - f'Project `{transfer.asset}` transfer cannot be fixed automatically' + f'Project `{transfer.asset}` transfer cannot be fixed' + f' automatically' ) continue @@ -56,9 +57,7 @@ def handle(self, *args, **options): move_attachments(transfer) move_media_files(transfer) if verbosity: - self.stdout.write( - f'\tDone!' - ) + self.stdout.write('\tDone!') usernames.add(transfer.invite.recipient.username) # Update attachment storage bytes counters diff --git a/kobo/apps/project_ownership/models/transfer.py b/kobo/apps/project_ownership/models/transfer.py index 6c97ddfd2f..23c90b0098 100644 --- a/kobo/apps/project_ownership/models/transfer.py +++ b/kobo/apps/project_ownership/models/transfer.py @@ -1,6 +1,5 @@ from __future__ import annotations -import time from datetime import timedelta from typing import Optional, Union @@ -102,6 +101,7 @@ def transfer_project(self): success = False try: if not self.asset.has_deployment: + _rewrite_mongo = False with transaction.atomic(): self._reassign_project_permissions(update_deployment=False) self._sent_in_app_messages() @@ -115,7 +115,7 @@ def transfer_project(self): status=TransferStatusChoices.SUCCESS ) else: - _rewrite_mongo = False + _rewrite_mongo = True with transaction.atomic(): with kc_transaction_atomic(): deployment = self.asset.deployment @@ -131,12 +131,10 @@ def transfer_project(self): deployment.rename_enketo_id_key(previous_owner_username) self._sent_in_app_messages() - _rewrite_mongo = True - - # Do not delegate anything to Celery before the transaction has - # been validated. Otherwise, Celery could fetch outdated data. - transaction.on_commit(lambda: self._start_async_jobs(_rewrite_mongo)) + # Do not delegate anything to Celery before the transaction has + # been validated. Otherwise, Celery could fetch outdated data. + transaction.on_commit(lambda: self._start_async_jobs(_rewrite_mongo)) success = True finally: if not success: diff --git a/kobo/apps/project_ownership/tests/api/v2/test_api.py b/kobo/apps/project_ownership/tests/api/v2/test_api.py index 58f708f1de..f8a4027772 100644 --- a/kobo/apps/project_ownership/tests/api/v2/test_api.py +++ b/kobo/apps/project_ownership/tests/api/v2/test_api.py @@ -1,15 +1,12 @@ import uuid from constance.test import override_config -from datetime import timedelta -from dateutil.parser import isoparse from django.conf import settings from django.contrib.auth import get_user_model from django.utils import timezone from mock import patch, MagicMock from rest_framework import status from rest_framework.reverse import reverse -from unittest.mock import ANY from kobo.apps.project_ownership.models import ( Invite, @@ -18,11 +15,11 @@ ) from kobo.apps.project_ownership.tests.utils import MockServiceUsageSerializer from kobo.apps.trackers.utils import update_nlp_counter - from kpi.constants import PERM_VIEW_ASSET from kpi.models import Asset from kpi.tests.base_test_case import BaseAssetTestCase from kpi.tests.kpi_test_case import KpiTestCase +from kpi.tests.utils.transaction import immediate_on_commit from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE @@ -432,7 +429,7 @@ def test_account_usage_transferred_to_new_user(self): response = self.client.get(service_usage_url) assert response.data == expected_empty_data - # Transfer project from someuser to anotheruser + # Transfer the project from someuser to anotheruser self.client.login(username='someuser', password='someuser') payload = { 'recipient': self.absolute_reverse( @@ -445,9 +442,10 @@ def test_account_usage_transferred_to_new_user(self): 'kpi.deployment_backends.backends.MockDeploymentBackend.xform', MagicMock(), ): - response = self.client.post( - self.invite_url, data=payload, format='json' - ) + with immediate_on_commit(): + response = self.client.post( + self.invite_url, data=payload, format='json' + ) assert response.status_code == status.HTTP_201_CREATED # someuser should have no usage reported anymore @@ -495,7 +493,7 @@ def test_data_accessible_to_new_user(self): ) == 0 ) - # Transfer project from someuser to anotheruser + # Transfer the project from someuser to anotheruser self.client.login(username='someuser', password='someuser') payload = { 'recipient': self.absolute_reverse( @@ -508,9 +506,10 @@ def test_data_accessible_to_new_user(self): 'kpi.deployment_backends.backends.MockDeploymentBackend.xform', MagicMock(), ): - response = self.client.post( - self.invite_url, data=payload, format='json' - ) + with immediate_on_commit(): + response = self.client.post( + self.invite_url, data=payload, format='json' + ) assert response.status_code == status.HTTP_201_CREATED # anotheruser is the owner and should see the project diff --git a/kobo/apps/project_ownership/tests/test_transfer_status.py b/kobo/apps/project_ownership/tests/test_transfer_status.py index 11985bdc2c..2853083e66 100644 --- a/kobo/apps/project_ownership/tests/test_transfer_status.py +++ b/kobo/apps/project_ownership/tests/test_transfer_status.py @@ -2,6 +2,7 @@ from django.test import TestCase from kpi.models import Asset +from kpi.tests.utils.transaction import immediate_on_commit from ..models import ( Invite, InviteStatusChoices, @@ -105,9 +106,11 @@ def test_calculated_failed_transfer_status(self): assert self.invite.status == InviteStatusChoices.FAILED def test_draft_project_transfer(self): - # when project is a draft, there are no celery tasks called to move + # When a project is a draft, there are no celery tasks called to move # submissions (and related attachments). - self.transfer.transfer_project() + with immediate_on_commit(): + self.transfer.transfer_project() + assert self.transfer.status == TransferStatusChoices.SUCCESS # However, the status of each async task should still be updated to diff --git a/kpi/tests/utils/transaction.py b/kpi/tests/utils/transaction.py new file mode 100644 index 0000000000..7d30838425 --- /dev/null +++ b/kpi/tests/utils/transaction.py @@ -0,0 +1,25 @@ +from contextlib import contextmanager +from unittest import mock + +from django.contrib.auth.management import DEFAULT_DB_ALIAS + + +@contextmanager +def immediate_on_commit(using=None): + """ + Context manager executing transaction.on_commit() hooks immediately as + if the connection was in auto-commit mode. This is required when + using a subclass of django.test.TestCase as all tests are wrapped in + a transaction that never gets committed. + + Source: https://code.djangoproject.com/ticket/30457#comment:1 + """ + immediate_using = DEFAULT_DB_ALIAS if using is None else using + + def on_commit(func, using=None): + using = DEFAULT_DB_ALIAS if using is None else using + if using == immediate_using: + func() + + with mock.patch('django.db.transaction.on_commit', side_effect=on_commit) as patch: + yield patch