Fix copy-and-paste errors

A non-empty string is truthy, making the the for-loop useless, as the first item in inspect.stack() is always the for-loop itself, which then lead to the function returning immediately.
This commit
* fixes this typo
* changes the loop to ignore the first element of instpect.stack() (which is the loop itself)
* ignores django-internal code

This should create something similar to what I suspect the code was intended to do originally.

src/pretix/base/models/_transactions.py

@@ -32,6 +32,9 @@ import threading
 
 from django.conf import settings
 from django.db import transaction
+import django
+from pathlib import Path
+
 
 dirty_transactions = threading.local()
 
@@ -74,10 +77,14 @@ def _transactions_mark_order_dirty(order_id, using=None):
     if "PYTEST_CURRENT_TEST" in os.environ:
         # We don't care about Order.objects.create() calls in test code so let's try to figure out if this is test code
         # or not.
-        for frame in inspect.stack():
-            if 'pretix/base/models/orders' in frame.filename:
+        for frame in inspect.stack()[1:]:
+            if (
+                'pretix/base/models/orders' in frame.filename
+                or Path(frame.filename).is_relative_to(Path(django.__file__).parent)
+            ):
+                # Ignore model- and django-internal code
                 continue
-            elif 'test_' in frame.filename or 'conftest.py in frame.filename':
+            elif 'test_' in frame.filename or 'conftest.py' in frame.filename:
                 return
             elif 'pretix/' in frame.filename or 'pretix_' in frame.filename:
                 # This went through non-test code, let's consider it non-test

src/pretix/base/models/orders.py

@@ -590,7 +590,7 @@ class Order(LockModel, LoggedModel):
             not kwargs.get('force_save_with_deferred_fields', None) and
             (not update_fields or ('require_approval' not in update_fields and 'status' not in update_fields))
         ):
-            _fail("It is unsafe to call save() on an OrderFee with deferred fields since we can't check if you missed "
+            _fail("It is unsafe to call save() on an Order with deferred fields since we can't check if you missed "
                   "creating a transaction. Call save(force_save_with_deferred_fields=True) if you really want to do "
                   "this.")
 
@@ -2841,7 +2841,7 @@ class OrderPosition(AbstractPosition):
             if Transaction.key(self) != self.__initial_transaction_key or self.canceled != self.__initial_canceled or not self.pk:
                 _transactions_mark_order_dirty(self.order_id, using=kwargs.get('using', None))
         elif not kwargs.get('force_save_with_deferred_fields', None):
-            _fail("It is unsafe to call save() on an OrderFee with deferred fields since we can't check if you missed "
+            _fail("It is unsafe to call save() on an OrderPosition with deferred fields since we can't check if you missed "
                   "creating a transaction. Call save(force_save_with_deferred_fields=True) if you really want to do "
                   "this.")
 

src/tests/base/test_notifications.py

@@ -75,47 +75,42 @@ def user(team):
     return user
 
 
-@pytest.fixture
-def monkeypatch_on_commit(monkeypatch):
-    monkeypatch.setattr("django.db.transaction.on_commit", lambda t: t())
-
-
 @pytest.mark.django_db
-def test_notification_trigger_event_specific(event, order, user, monkeypatch_on_commit):
+def test_notification_trigger_event_specific(event, order, user, django_capture_on_commit_callbacks):
     djmail.outbox = []
     user.notification_settings.create(
         method='mail', event=event, action_type='pretix.event.order.paid', enabled=True
     )
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {})
     assert len(djmail.outbox) == 1
     assert djmail.outbox[0].subject.endswith("DUMMY: Order FOO has been marked as paid.")
 
 
 @pytest.mark.django_db
-def test_notification_trigger_global(event, order, user, monkeypatch_on_commit):
+def test_notification_trigger_global(event, order, user, django_capture_on_commit_callbacks):
     djmail.outbox = []
     user.notification_settings.create(
         method='mail', event=None, action_type='pretix.event.order.paid', enabled=True
     )
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {})
     assert len(djmail.outbox) == 1
 
 
 @pytest.mark.django_db
-def test_notification_trigger_global_wildcard(event, order, user, monkeypatch_on_commit):
+def test_notification_trigger_global_wildcard(event, order, user, django_capture_on_commit_callbacks):
     djmail.outbox = []
     user.notification_settings.create(
         method='mail', event=None, action_type='pretix.event.order.changed.*', enabled=True
     )
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.changed.item', {})
     assert len(djmail.outbox) == 1
 
 
 @pytest.mark.django_db
-def test_notification_enabled_global_ignored_specific(event, order, user, monkeypatch_on_commit):
+def test_notification_enabled_global_ignored_specific(event, order, user, django_capture_on_commit_callbacks):
     djmail.outbox = []
     user.notification_settings.create(
         method='mail', event=None, action_type='pretix.event.order.paid', enabled=True
@@ -123,24 +118,24 @@ def test_notification_enabled_global_ignored_specific(event, order, user, monkey
     user.notification_settings.create(
         method='mail', event=event, action_type='pretix.event.order.paid', enabled=False
     )
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {})
     assert len(djmail.outbox) == 0
 
 
 @pytest.mark.django_db
-def test_notification_ignore_same_user(event, order, user, monkeypatch_on_commit):
+def test_notification_ignore_same_user(event, order, user, django_capture_on_commit_callbacks):
     djmail.outbox = []
     user.notification_settings.create(
         method='mail', event=event, action_type='pretix.event.order.paid', enabled=True
     )
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {}, user=user)
     assert len(djmail.outbox) == 0
 
 
 @pytest.mark.django_db
-def test_notification_ignore_insufficient_permissions(event, order, user, team, monkeypatch_on_commit):
+def test_notification_ignore_insufficient_permissions(event, order, user, team, django_capture_on_commit_callbacks):
     djmail.outbox = []
     team.all_event_permissions = False
     team.limit_event_permissions = {"event.vouchers:read": True}
@@ -148,7 +143,7 @@ def test_notification_ignore_insufficient_permissions(event, order, user, team,
     user.notification_settings.create(
         method='mail', event=event, action_type='pretix.event.order.paid', enabled=True
     )
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {})
     assert len(djmail.outbox) == 0
 

src/tests/base/test_orders.py

@@ -29,7 +29,8 @@ import pytest
 from django.conf import settings
 from django.core import mail as djmail
 from django.db.models import F, Sum
-from django.test import TestCase, override_settings
+from django.test import TestCase, TransactionTestCase, override_settings
+from django.db import transaction
 from django.utils.timezone import make_aware, now
 from django_countries.fields import Country
 from django_scopes import scope
@@ -1224,13 +1225,6 @@ class DownloadReminderTests(TestCase):
         send_download_reminders(sender=self.event)
         assert len(djmail.outbox) == 0
 
-
-@pytest.fixture
-def class_monkeypatch(request, monkeypatch):
-    request.cls.monkeypatch = monkeypatch
-
-
-@pytest.mark.usefixtures("class_monkeypatch")
 class OrderCancelTests(TestCase):
     def setUp(self):
         super().setUp()
@@ -1258,7 +1252,6 @@ class OrderCancelTests(TestCase):
             self.order.create_transactions()
             generate_invoice(self.order)
             djmail.outbox = []
-        self.monkeypatch.setattr("django.db.transaction.on_commit", lambda t: t())
 
     @classscope(attr='o')
     def test_cancel_canceled(self):
@@ -1351,14 +1344,14 @@ class OrderCancelTests(TestCase):
         self.order.status = Order.STATUS_PAID
         self.order.save()
         djmail.outbox = []
-        cancel_order(self.order.pk, send_mail=True)
-        print([s.subject for s in djmail.outbox])
-        print([s.to for s in djmail.outbox])
+        with self.captureOnCommitCallbacks(execute=True):
+            cancel_order(self.order.pk, send_mail=True)
+
         assert len(djmail.outbox) == 2
-        assert ["invoice@example.org"] == djmail.outbox[0].to
-        assert any(["Invoice_" in a[0] for a in djmail.outbox[0].attachments])
-        assert ["dummy@dummy.test"] == djmail.outbox[1].to
-        assert not any(["Invoice_" in a[0] for a in djmail.outbox[1].attachments])
+        assert ["dummy@dummy.test"] == djmail.outbox[0].to
+        assert not any(["Invoice_" in a[0] for a in djmail.outbox[0].attachments])
+        assert ["invoice@example.org"] == djmail.outbox[1].to
+        assert any(["Invoice_" in a[0] for a in djmail.outbox[1].attachments])
 
     @classscope(attr='o')
     def test_cancel_paid_with_too_high_fee(self):
@@ -1487,9 +1480,7 @@ class OrderCancelTests(TestCase):
         assert not self.order.refunds.exists()
         assert self.order.all_logentries().filter(action_type='pretix.event.order.refund.requested').exists()
 
-
-@pytest.mark.usefixtures("class_monkeypatch")
-class OrderChangeManagerTests(TestCase):
+class BaseOrderChangeManagerTestCase:
     def setUp(self):
         super().setUp()
         self.o = Organizer.objects.create(name='Dummy', slug='dummy', plugins='pretix.plugins.banktransfer')
@@ -1552,7 +1543,6 @@ class OrderChangeManagerTests(TestCase):
             self.seat_a1 = self.event.seats.create(seat_number="A1", product=self.stalls, seat_guid="A1")
             self.seat_a2 = self.event.seats.create(seat_number="A2", product=self.stalls, seat_guid="A2")
             self.seat_a3 = self.event.seats.create(seat_number="A3", product=self.stalls, seat_guid="A3")
-        self.monkeypatch.setattr("django.db.transaction.on_commit", lambda t: t())
 
     def _enable_reverse_charge(self):
         self.tr7.eu_reverse_charge = True
@@ -1566,6 +1556,7 @@ class OrderChangeManagerTests(TestCase):
             country=Country('AT')
         )
 
+class OrderChangeManagerTests(BaseOrderChangeManagerTestCase, TestCase):
     @classscope(attr='o')
     def test_multiple_commits_forbidden(self):
         self.ocm.change_price(self.op1, Decimal('10.00'))
@@ -3895,15 +3886,16 @@ class OrderChangeManagerTests(TestCase):
 
     @classscope(attr='o')
     def test_set_valid_until(self):
-        self.event.settings.ticket_secret_generator = "pretix_sig1"
-        assign_ticket_secret(self.event, self.op1, force_invalidate=True, save=True)
-        old_secret = self.op1.secret
+        with transaction.atomic():
+            self.event.settings.ticket_secret_generator = "pretix_sig1"
+            assign_ticket_secret(self.event, self.op1, force_invalidate=True, save=True)
+            old_secret = self.op1.secret
 
-        dt = make_aware(datetime(2022, 9, 20, 15, 0, 0, 0))
-        self.ocm.change_valid_until(self.op1, dt)
-        self.ocm.commit()
-        self.op1.refresh_from_db()
-        assert self.op1.secret != old_secret
+            dt = make_aware(datetime(2022, 9, 20, 15, 0, 0, 0))
+            self.ocm.change_valid_until(self.op1, dt)
+            self.ocm.commit()
+            self.op1.refresh_from_db()
+            assert self.op1.secret != old_secret
 
     @classscope(attr='o')
     def test_unset_valid_from_until(self):
@@ -3928,6 +3920,7 @@ class OrderChangeManagerTests(TestCase):
         assert len(djmail.outbox) == 1
         assert len(["Invoice_" in a[0] for a in djmail.outbox[0].attachments]) == 2
 
+class OrderChangeManagerTransactionalTests(BaseOrderChangeManagerTestCase, TransactionTestCase):
     @classscope(attr='o')
     def test_new_invoice_send_somewhere_else(self):
         generate_invoice(self.order)

src/tests/base/test_webhooks.py

@@ -82,14 +82,9 @@ def force_str(v):
     return v.decode() if isinstance(v, bytes) else str(v)
 
 
-@pytest.fixture
-def monkeypatch_on_commit(monkeypatch):
-    monkeypatch.setattr("django.db.transaction.on_commit", lambda t: t())
-
-
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_trigger_event_specific(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_trigger_event_specific(event, order, webhook, django_capture_on_commit_callbacks):
     responses.add_callback(
         responses.POST, 'https://google.com',
         callback=lambda r: (200, {}, 'ok'),
@@ -97,7 +92,7 @@ def test_webhook_trigger_event_specific(event, order, webhook, monkeypatch_on_co
         match_querystring=None,  # https://github.com/getsentry/responses/issues/464
     )
 
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         le = order.log_action('pretix.event.order.paid', {})
     assert len(responses.calls) == 1
     assert json.loads(force_str(responses.calls[0].request.body)) == {
@@ -119,12 +114,12 @@ def test_webhook_trigger_event_specific(event, order, webhook, monkeypatch_on_co
 
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_trigger_global(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_trigger_global(event, order, webhook, django_capture_on_commit_callbacks):
     webhook.limit_events.clear()
     webhook.all_events = True
     webhook.save()
     responses.add(responses.POST, 'https://google.com', status=200)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         le = order.log_action('pretix.event.order.paid', {})
     assert len(responses.calls) == 1
     assert json.loads(force_str(responses.calls[0].request.body)) == {
@@ -138,13 +133,13 @@ def test_webhook_trigger_global(event, order, webhook, monkeypatch_on_commit):
 
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_trigger_global_wildcard(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_trigger_global_wildcard(event, order, webhook, django_capture_on_commit_callbacks):
     webhook.listeners.create(action_type="pretix.event.order.changed.*")
     webhook.limit_events.clear()
     webhook.all_events = True
     webhook.save()
     responses.add(responses.POST, 'https://google.com', status=200)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         le = order.log_action('pretix.event.order.changed.item', {})
     assert len(responses.calls) == 1
     assert json.loads(force_str(responses.calls[0].request.body)) == {
@@ -158,30 +153,30 @@ def test_webhook_trigger_global_wildcard(event, order, webhook, monkeypatch_on_c
 
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_ignore_wrong_action_type(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_ignore_wrong_action_type(event, order, webhook, django_capture_on_commit_callbacks):
     responses.add(responses.POST, 'https://google.com', status=200)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.changed.item', {})
     assert len(responses.calls) == 0
 
 
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_ignore_disabled(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_ignore_disabled(event, order, webhook, django_capture_on_commit_callbacks):
     webhook.enabled = False
     webhook.save()
     responses.add(responses.POST, 'https://google.com', status=200)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.changed.item', {})
     assert len(responses.calls) == 0
 
 
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_ignore_wrong_event(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_ignore_wrong_event(event, order, webhook, django_capture_on_commit_callbacks):
     webhook.limit_events.clear()
     responses.add(responses.POST, 'https://google.com', status=200)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.changed.item', {})
     assert len(responses.calls) == 0
 
@@ -189,10 +184,10 @@ def test_webhook_ignore_wrong_event(event, order, webhook, monkeypatch_on_commit
 @pytest.mark.django_db
 @pytest.mark.xfail(reason="retries can't be tested with celery_always_eager")
 @responses.activate
-def test_webhook_retry(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_retry(event, order, webhook, django_capture_on_commit_callbacks):
     responses.add(responses.POST, 'https://google.com', status=500)
     responses.add(responses.POST, 'https://google.com', status=200)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {})
     assert len(responses.calls) == 2
     with scopes_disabled():
@@ -216,9 +211,9 @@ def test_webhook_retry(event, order, webhook, monkeypatch_on_commit):
 
 @pytest.mark.django_db
 @responses.activate
-def test_webhook_disable_gone(event, order, webhook, monkeypatch_on_commit):
+def test_webhook_disable_gone(event, order, webhook, django_capture_on_commit_callbacks):
     responses.add(responses.POST, 'https://google.com', status=410)
-    with transaction.atomic():
+    with django_capture_on_commit_callbacks(execute=True):
         order.log_action('pretix.event.order.paid', {})
     assert len(responses.calls) == 1
     webhook.refresh_from_db()

src/tests/conftest.py

@@ -131,3 +131,8 @@ def set_lock_namespaces(request):
             yield
     else:
         yield
+
+
+@pytest.fixture
+def class_monkeypatch(request, monkeypatch):
+    request.cls.monkeypatch = monkeypatch

src/tests/control/test_auth.py

@@ -385,11 +385,6 @@ class RegistrationFormTest(TestCase):
         self.assertEqual(response.status_code, 403)
 
 
-@pytest.fixture
-def class_monkeypatch(request, monkeypatch):
-    request.cls.monkeypatch = monkeypatch
-
-
 @pytest.mark.usefixtures("class_monkeypatch")
 class Login2FAFormTest(TestCase):
 

src/tests/control/test_events.py

@@ -49,11 +49,6 @@ from tests.base import SoupTest, extract_form_fields
 from pretix.base.models import Event, LogEntry, Order, Organizer, Team, User
 
 
-@pytest.fixture
-def class_monkeypatch(request, monkeypatch):
-    request.cls.monkeypatch = monkeypatch
-
-
 @pytest.mark.usefixtures("class_monkeypatch")
 class EventsTest(SoupTest):
     @scopes_disabled()

src/tests/control/test_organizer.py

@@ -33,11 +33,6 @@ from tests.base import SoupTest, extract_form_fields
 from pretix.base.models import Event, Organizer, OutgoingMail, Team, User
 
 
-@pytest.fixture
-def class_monkeypatch(request, monkeypatch):
-    request.cls.monkeypatch = monkeypatch
-
-
 @pytest.mark.usefixtures("class_monkeypatch")
 class OrganizerTest(SoupTest):
     @scopes_disabled()

src/tests/control/test_user.py

@@ -286,11 +286,6 @@ class UserPasswordChangeTest(SoupTest):
         assert self.user.needs_password_change is False
 
 
-@pytest.fixture
-def class_monkeypatch(request, monkeypatch):
-    request.cls.monkeypatch = monkeypatch
-
-
 @pytest.mark.usefixtures("class_monkeypatch")
 class UserSettings2FATest(SoupTest):
     def setUp(self):

src/tests/presale/test_checkout.py

@@ -33,7 +33,7 @@ from django.conf import settings
 from django.core import mail as djmail
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.core.signing import dumps
-from django.test import TestCase
+from django.test import TestCase, TransactionTestCase
 from django.utils.crypto import get_random_string
 from django.utils.timezone import now
 from django_countries.fields import Country
@@ -60,12 +60,6 @@ from pretix.testutils.sessions import get_cart_session_key
 from .test_timemachine import TimemachineTestMixin
 
 
-@pytest.fixture
-def class_monkeypatch(request, monkeypatch):
-    request.cls.monkeypatch = monkeypatch
-
-
-@pytest.mark.usefixtures("class_monkeypatch")
 class BaseCheckoutTestCase:
     @scopes_disabled()
     def setUp(self):
@@ -104,7 +98,6 @@ class BaseCheckoutTestCase:
         self.workshopquota.items.add(self.workshop2)
         self.workshopquota.variations.add(self.workshop2a)
         self.workshopquota.variations.add(self.workshop2b)
-        self.monkeypatch.setattr("django.db.transaction.on_commit", lambda t: t())
 
     def _set_session(self, key, value):
         session = self.client.session
@@ -4420,6 +4413,7 @@ class CheckoutTestCase(BaseCheckoutTestCase, TimemachineTestMixin, TestCase):
             assert len(djmail.outbox) == 1
             assert any(["Invoice_" in a[0] for a in djmail.outbox[0].attachments])
 
+class CheckoutTransactionTestCase(BaseCheckoutTestCase, TransactionTestCase):
     def test_order_confirmation_mail_invoice_sent_somewhere_else(self):
         self.event.settings.invoice_address_asked = True
         self.event.settings.invoice_address_required = True