Skip to content

Commit

Permalink
Fix unit tests: patch "on_commit"
Browse files Browse the repository at this point in the history
  • Loading branch information
noliveleger committed Oct 1, 2024
1 parent 0fb90bf commit 041aeb9
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
12 changes: 5 additions & 7 deletions kobo/apps/project_ownership/models/transfer.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from __future__ import annotations

import time
from datetime import timedelta
from typing import Optional, Union

Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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:
Expand Down
23 changes: 11 additions & 12 deletions kobo/apps/project_ownership/tests/api/v2/test_api.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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


Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions kobo/apps/project_ownership/tests/test_transfer_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions kpi/tests/utils/transaction.py
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 041aeb9

Please sign in to comment.