From 163cf85c86f10aabb006f41a3a1cb5381ce39eec Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Tue, 28 Jul 2026 09:08:14 +0200 Subject: [PATCH] Orders: Fix N+1 query issues on order detail and change page (#6390) * rders: Fix N+1 query issues on order detail and change page There is one I couldn't fix: Loading the list of tax rules for every select box on the OrderChange page. Unfortunately, Django has a cache-breaking .all() in ModelChoiceField and that would need nasty patching that didn't feel worth it * Fix isort * Fix N+1 query in gate call * Fix leftovers * Add local cache for objects referenced in log entries * Update src/pretix/control/views/orders.py --- src/pretix/control/forms/orders.py | 4 +- src/pretix/control/logdisplay.py | 34 +++++++++------ .../templates/pretixcontrol/order/index.html | 2 +- src/pretix/control/views/orders.py | 41 ++++++++++++++----- 4 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/pretix/control/forms/orders.py b/src/pretix/control/forms/orders.py index 6c5e8b40a1..fe7398333b 100644 --- a/src/pretix/control/forms/orders.py +++ b/src/pretix/control/forms/orders.py @@ -562,9 +562,7 @@ class OrderPositionChangeForm(forms.Form): if instance.addon_to_id: del self.fields['operation_split'] - if not instance.seat and not ( - instance.item.seat_category_mappings.filter(subevent=instance.subevent).exists() - ): + if not instance.seat and not instance._seat_allowed: del self.fields['seat'] choices = [ diff --git a/src/pretix/control/logdisplay.py b/src/pretix/control/logdisplay.py index 1204fcc834..60d44f50a2 100644 --- a/src/pretix/control/logdisplay.py +++ b/src/pretix/control/logdisplay.py @@ -33,6 +33,7 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. +import functools from collections import defaultdict from datetime import datetime from decimal import Decimal @@ -69,6 +70,15 @@ OVERVIEW_BANLIST = [ ] +@functools.lru_cache(maxsize=32) +def object_id_to_string(model_class, **kwargs): + # The cache is thread-local and persists requests, but it's small enough that we can accept that + try: + return str(model_class.objects.get(**kwargs)) + except model_class.DoesNotExist: + return "?" + + class OrderChangeLogEntryType(OrderLogEntryType): prefix = _('The order has been changed:') @@ -93,10 +103,10 @@ class OrderItemChanged(OrderChangeLogEntryType): def display_prefixed(self, event: Event, logentry: LogEntry, data): old_item = str(event.items.get(pk=data['old_item'])) if data['old_variation']: - old_item += ' - ' + str(ItemVariation.objects.get(item__event=event, pk=data['old_variation'])) + old_item += ' - ' + str(object_id_to_string(ItemVariation, item__event_id=event.pk, pk=data['old_variation'])) new_item = str(event.items.get(pk=data['new_item'])) if data['new_variation']: - new_item += ' - ' + str(ItemVariation.objects.get(item__event=event, pk=data['new_variation'])) + new_item += ' - ' + str(object_id_to_string(ItemVariation, item__event_id=event.pk, pk=data['new_variation'])) return _('Position #{posid}: {old_item} ({old_price}) changed to {new_item} ({new_price}).').format( posid=data.get('positionid', '?'), old_item=old_item, new_item=new_item, @@ -153,14 +163,14 @@ class OrderTaxRuleChanged(OrderChangeLogEntryType): if 'positionid' in data: return _('Tax rule of position #{posid} changed from {old_rule} to {new_rule}.').format( posid=data.get('positionid', '?'), - old_rule=TaxRule.objects.get(pk=data['old_taxrule']) if data['old_taxrule'] else '–', - new_rule=TaxRule.objects.get(pk=data['new_taxrule']), + old_rule=object_id_to_string(TaxRule, pk=data['old_taxrule']) if data['old_taxrule'] else '–', + new_rule=object_id_to_string(TaxRule, pk=data['new_taxrule']), ) elif 'fee' in data: return _('Tax rule of fee #{fee} changed from {old_rule} to {new_rule}.').format( fee=data.get('fee', '?'), - old_rule=TaxRule.objects.get(pk=data['old_taxrule']) if data['old_taxrule'] else '–', - new_rule=TaxRule.objects.get(pk=data['new_taxrule']), + old_rule=object_id_to_string(TaxRule, pk=data['old_taxrule']) if data['old_taxrule'] else '–', + new_rule=object_id_to_string(TaxRule, pk=data['new_taxrule']), ) @@ -204,7 +214,7 @@ class OrderCanceled(OrderChangeLogEntryType): def display_prefixed(self, event: Event, logentry: LogEntry, data): old_item = str(event.items.get(pk=data['old_item'])) if data['old_variation']: - old_item += ' - ' + str(ItemVariation.objects.get(pk=data['old_variation'])) + old_item += ' - ' + object_id_to_string(ItemVariation, pk=data['old_variation']) return _('Position #{posid} ({old_item}, {old_price}) canceled.').format( posid=data.get('positionid', '?'), old_item=old_item, @@ -219,7 +229,7 @@ class OrderPositionAdded(OrderChangeLogEntryType): def display_prefixed(self, event: Event, logentry: LogEntry, data): item = str(event.items.get(pk=data['item'])) if data['variation']: - item += ' - ' + str(ItemVariation.objects.get(item__event=event, pk=data['variation'])) + item += ' - ' + object_id_to_string(ItemVariation, item__event_id=event.pk, pk=data['variation']) if data['addon_to']: addon_to = OrderPosition.objects.get(order__event=event, pk=data['addon_to']) return _('Position #{posid} created: {item} ({price}) as an add-on to position #{addon_to}.').format( @@ -283,7 +293,7 @@ class OrderChangedSplit(OrderChangeLogEntryType): def display_prefixed(self, event: Event, logentry: LogEntry, data): old_item = str(event.items.get(pk=data['old_item'])) if data['old_variation']: - old_item += ' - ' + str(ItemVariation.objects.get(pk=data['old_variation'])) + old_item += ' - ' + object_id_to_string(ItemVariation, pk=data['old_variation']) url = reverse('control:event.order', kwargs={ 'event': event.slug, 'organizer': event.organizer.slug, @@ -339,6 +349,7 @@ class OrderChangedSplitFrom(OrderLogEntryType): 'pretix.event.checkin.reverted': _('The check-in of position #{posid} on list "{list}" has been reverted.'), }) class CheckinErrorLogEntryType(OrderLogEntryType): + def display(self, logentry: LogEntry, data): return self.display_plain(self.plain, logentry, data) @@ -353,10 +364,7 @@ class CheckinErrorLogEntryType(OrderLogEntryType): event = logentry.event if 'list' in data and event: - try: - data['list'] = event.checkin_lists.get(pk=data.get('list')).name - except CheckinList.DoesNotExist: - data['list'] = _("(unknown)") + data['list'] = object_id_to_string(CheckinList, event_id=event.id, pk=data['list']) else: data['list'] = _("(unknown)") diff --git a/src/pretix/control/templates/pretixcontrol/order/index.html b/src/pretix/control/templates/pretixcontrol/order/index.html index cdba94c04b..88276398e3 100644 --- a/src/pretix/control/templates/pretixcontrol/order/index.html +++ b/src/pretix/control/templates/pretixcontrol/order/index.html @@ -833,7 +833,7 @@ {% trans "Pending total" %}
- {{ order.pending_sum|money:event.currency }} + {{ pending_sum|money:event.currency }}
diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py index 8245d78ed8..e1c19c549c 100644 --- a/src/pretix/control/views/orders.py +++ b/src/pretix/control/views/orders.py @@ -81,7 +81,7 @@ from pretix.base.i18n import language from pretix.base.models import ( CachedFile, CachedTicket, Checkin, GiftCard, Invoice, InvoiceAddress, Item, ItemVariation, LogEntry, Order, QuestionAnswer, Quota, - ScheduledEventExport, generate_secret, + ScheduledEventExport, SeatCategoryMapping, generate_secret, ) from pretix.base.models.orders import ( CancellationRequest, OrderFee, OrderPayment, OrderPosition, OrderRefund, @@ -564,10 +564,11 @@ class OrderDetail(OrderView): }) ctx['display_locale'] = dict(settings.LANGUAGES)[self.object.locale or self.request.event.settings.locale] - ctx['overpaid'] = self.order.pending_sum * -1 + pending_sum = self.order.pending_sum + ctx['overpaid'] = pending_sum * -1 ctx['download_buttons'] = self.download_buttons ctx['payment_refund_sum'] = self.order.payment_refund_sum - ctx['pending_sum'] = self.order.pending_sum + ctx['pending_sum'] = pending_sum ctx['uncancelled_invoice'] = self.order.invoices.exclude( Exists(self.order.invoices.filter(refers=OuterRef('pk'), is_cancellation=True)) ).exclude(is_cancellation=True).first() @@ -601,8 +602,9 @@ class OrderDetail(OrderView): ).prefetch_related( 'item__questions', 'issued_gift_cards', 'owned_gift_cards', 'linked_media', Prefetch('answers', queryset=QuestionAnswer.objects.prefetch_related('options').select_related('question')), - Prefetch('all_checkins', queryset=Checkin.all.select_related('list').order_by('datetime')), + Prefetch('all_checkins', queryset=Checkin.all.select_related('list', 'gate').order_by('datetime')), Prefetch('print_logs', queryset=PrintLog.objects.select_related('device').order_by('datetime')), + Prefetch('subevent', queryset=self.request.event.subevents.all()), ).order_by('positionid') positions = [] @@ -1985,20 +1987,37 @@ class OrderChange(OrderView): def fees(self): fees = list(self.order.fees.all()) for f in fees: - f.form = OrderFeeChangeForm(prefix='of-{}'.format(f.pk), instance=f, - data=self.request.POST if self.request.method == "POST" else None) + f.form = OrderFeeChangeForm( + prefix='of-{}'.format(f.pk), + instance=f, + data=self.request.POST if self.request.method == "POST" else None + ) return fees @cached_property def positions(self): positions = list(self.order.positions.select_related( 'item', 'item__tax_rule', 'used_membership', 'used_membership__membership_type', 'tax_rule', - 'seat', 'subevent', - ).prefetch_related('granted_memberships', 'issued_gift_cards')) + 'seat', + ).prefetch_related( + Prefetch( + 'subevent', + queryset=self.request.event.subevents.all(), + ), + 'granted_memberships', + 'issued_gift_cards', + 'addons', + ).annotate( + _seat_allowed=Exists(SeatCategoryMapping.objects.filter(subevent=OuterRef("subevent"), product=OuterRef("item"))) + )) for p in positions: - p.form = OrderPositionChangeForm(prefix='op-{}'.format(p.pk), instance=p, items=self.items, - initial={'seat': p.seat.seat_guid if p.seat else None}, - data=self.request.POST if self.request.method == "POST" else None) + p.form = OrderPositionChangeForm( + prefix='op-{}'.format(p.pk), + instance=p, + items=self.items, + initial={'seat': p.seat.seat_guid if p.seat else None}, + data=self.request.POST if self.request.method == "POST" else None + ) return positions def get_context_data(self, **kwargs):