From 7a3afde7b1ac50c4492fbe4e4fbf803a329851ce Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 21 Apr 2019 23:02:32 +0200 Subject: [PATCH] Change semantics of changing orders This basically does two things to the "Change products" view of orders and the OrderChangeManager program API: 1) It decouples changing items or subevents from changing prices. OrderChangeManager.change_item() and .change_subevent() no longer touch the price of a position. Instead .change_price() needs to be called explicitly. However, a client-side JavaScript component now *proposes* a new price based on the changed item or subevent. 2) The user interface now exposes the possibility of doing multiple things at the same time, i.e. changing the item, subevent and price in the same operation. OrderChangeManager already allowed this before. (1) is basically a consequence of (2), while (2) is a prerequesite for e.g. the `seating` branch, where changing the subevent will always require changing the seat. --- src/pretix/api/serializers/order.py | 21 ++- src/pretix/api/views/order.py | 85 ++++++++++- src/pretix/base/services/orders.py | 48 ++----- src/pretix/control/forms/orders.py | 92 +++++------- .../control/templates/pretixcontrol/base.html | 1 + .../templates/pretixcontrol/order/change.html | 136 +++++++++--------- src/pretix/control/views/orders.py | 25 ++-- .../static/pretixcontrol/js/ui/orderchange.js | 51 +++++++ .../static/pretixcontrol/scss/_forms.scss | 7 + src/tests/base/test_orders.py | 61 ++------ src/tests/control/test_orders.py | 34 ++--- 11 files changed, 310 insertions(+), 251 deletions(-) create mode 100644 src/pretix/static/pretixcontrol/js/ui/orderchange.js diff --git a/src/pretix/api/serializers/order.py b/src/pretix/api/serializers/order.py index bbcf8e34d6..c55cf20933 100644 --- a/src/pretix/api/serializers/order.py +++ b/src/pretix/api/serializers/order.py @@ -14,8 +14,8 @@ from pretix.api.serializers.i18n import I18nAwareModelSerializer from pretix.base.channels import get_all_sales_channels from pretix.base.i18n import language from pretix.base.models import ( - Checkin, Invoice, InvoiceAddress, InvoiceLine, Order, OrderPosition, - Question, QuestionAnswer, + Checkin, Invoice, InvoiceAddress, InvoiceLine, Item, ItemVariation, Order, + OrderPosition, Question, QuestionAnswer, SubEvent, ) from pretix.base.models.orders import ( CartPosition, OrderFee, OrderPayment, OrderRefund, @@ -288,6 +288,23 @@ class OrderSerializer(I18nAwareModelSerializer): return instance +class PriceCalcSerializer(serializers.Serializer): + item = serializers.PrimaryKeyRelatedField(queryset=Item.objects.none(), required=False, allow_null=True) + variation = serializers.PrimaryKeyRelatedField(queryset=ItemVariation.objects.none(), required=False, allow_null=True) + subevent = serializers.PrimaryKeyRelatedField(queryset=SubEvent.objects.none(), required=False, allow_null=True) + locale = serializers.CharField(allow_null=True, required=False) + + def __init__(self, *args, **kwargs): + event = kwargs.pop('event') + super().__init__(*args, **kwargs) + self.fields['item'].queryset = event.items.all() + self.fields['variation'].queryset = ItemVariation.objects.filter(item__event=event) + if event.has_subevents: + self.fields['subevent'].queryset = event.subevents.all() + else: + del self.fields['subevent'] + + class AnswerCreateSerializer(I18nAwareModelSerializer): class Meta: diff --git a/src/pretix/api/views/order.py b/src/pretix/api/views/order.py index aa2bd94258..b7969e4710 100644 --- a/src/pretix/api/views/order.py +++ b/src/pretix/api/views/order.py @@ -24,11 +24,12 @@ from pretix.api.models import OAuthAccessToken from pretix.api.serializers.order import ( InvoiceSerializer, OrderCreateSerializer, OrderPaymentSerializer, OrderPositionSerializer, OrderRefundCreateSerializer, - OrderRefundSerializer, OrderSerializer, + OrderRefundSerializer, OrderSerializer, PriceCalcSerializer, ) +from pretix.base.i18n import language from pretix.base.models import ( - CachedCombinedTicket, CachedTicket, Device, Event, Invoice, Order, - OrderPayment, OrderPosition, OrderRefund, Quota, TeamAPIToken, + CachedCombinedTicket, CachedTicket, Device, Event, Invoice, InvoiceAddress, + Order, OrderPayment, OrderPosition, OrderRefund, Quota, TeamAPIToken, generate_position_secret, generate_secret, ) from pretix.base.payment import PaymentException @@ -42,10 +43,12 @@ from pretix.base.services.orders import ( OrderChangeManager, OrderError, approve_order, cancel_order, deny_order, extend_order, mark_order_expired, mark_order_refunded, ) +from pretix.base.services.pricing import get_price from pretix.base.services.tickets import generate from pretix.base.signals import ( order_modified, order_placed, register_ticket_outputs, ) +from pretix.base.templatetags.money import money_filter class OrderFilter(FilterSet): @@ -622,6 +625,82 @@ class OrderPositionViewSet(mixins.DestroyModelMixin, viewsets.ReadOnlyModelViewS return prov raise NotFound('Unknown output provider.') + @action(detail=True, methods=['POST'], url_name='price_calc') + def price_calc(self, request, *args, **kwargs): + """ + This calculates the price assuming a change of product or subevent. This endpoint + is deliberately not documented and considered a private API, only to be used by + pretix' web interface. + + Sample input: + + { + "item": 2, + "variation": null, + "subevent": 3 + } + + Sample output: + + { + "gross": "2.34", + "gross_formatted": "2,34", + "net": "2.34", + "tax": "0.00", + "rate": "0.00", + "name": "VAT" + } + """ + serializer = PriceCalcSerializer(data=request.data, event=request.event) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + pos = self.get_object() + + try: + ia = pos.order.invoice_address + except InvoiceAddress.DoesNotExist: + ia = InvoiceAddress() + + kwargs = { + 'item': pos.item, + 'variation': pos.variation, + 'voucher': pos.voucher, + 'subevent': pos.subevent, + 'addon_to': pos.addon_to, + 'invoice_address': ia, + } + + if data.get('item'): + item = data.get('item') + kwargs['item'] = item + + if item.has_variations: + variation = data.get('variation') or pos.variation + if not variation: + raise ValidationError('No variation given') + if variation.item != item: + raise ValidationError('Variation does not belong to item') + else: + variation = None + kwargs['variation'] = None + + if pos.voucher and not pos.voucher.applies_to(item, variation): + kwargs['voucher'] = None + + if data.get('subevent'): + kwargs['subevent'] = data.get('subevent') + + price = get_price(**kwargs) + with language(data.get('locale') or self.request.event.settings.locale): + return Response({ + 'gross': price.gross, + 'gross_formatted': money_filter(price.gross, self.request.event.currency, hide_currency=True), + 'net': price.net, + 'rate': price.rate, + 'name': str(price.name), + 'tax': price.tax, + }) + @action(detail=True, url_name='download', url_path='download/(?P[^/]+)') def download(self, request, output, **kwargs): provider = self._get_output_provider(output) diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 5258120ddc..65fd451972 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -846,8 +846,8 @@ class OrderChangeManager: 'addon_invalid': _('The selected base position does not allow you to add this product as an add-on.'), 'subevent_required': _('You need to choose a subevent for the new position.'), } - ItemOperation = namedtuple('ItemOperation', ('position', 'item', 'variation', 'price')) - SubeventOperation = namedtuple('SubeventOperation', ('position', 'subevent', 'price')) + ItemOperation = namedtuple('ItemOperation', ('position', 'item', 'variation')) + SubeventOperation = namedtuple('SubeventOperation', ('position', 'subevent')) PriceOperation = namedtuple('PriceOperation', ('position', 'price')) CancelOperation = namedtuple('CancelOperation', ('position',)) AddOperation = namedtuple('AddOperation', ('item', 'variation', 'price', 'addon_to', 'subevent')) @@ -867,33 +867,18 @@ class OrderChangeManager: self.notify = notify self._invoice_dirty = False - def change_item(self, position: OrderPosition, item: Item, variation: Optional[ItemVariation], keep_price=False): + def change_item(self, position: OrderPosition, item: Item, variation: Optional[ItemVariation]): if (not variation and item.has_variations) or (variation and variation.item_id != item.pk): raise OrderError(self.error_messages['product_without_variation']) - if keep_price: - price = TaxedPrice(gross=position.price, net=position.price - position.tax_value, - tax=position.tax_value, rate=position.tax_rate, - name=position.tax_rule.name if position.tax_rule else None) - else: - price = get_price(item, variation, voucher=position.voucher, subevent=position.subevent, - invoice_address=self._invoice_address) - - if price is None: # NOQA - raise OrderError(self.error_messages['product_invalid']) - new_quotas = (variation.quotas.filter(subevent=position.subevent) if variation else item.quotas.filter(subevent=position.subevent)) if not new_quotas: raise OrderError(self.error_messages['quota_missing']) - if self.order.event.settings.invoice_include_free or price.gross != Decimal('0.00') or position.price != Decimal('0.00'): - self._invoice_dirty = True - - self._totaldiff += price.gross - position.price self._quotadiff.update(new_quotas) self._quotadiff.subtract(position.quotas) - self._operations.append(self.ItemOperation(position, item, variation, price)) + self._operations.append(self.ItemOperation(position, item, variation)) def change_subevent(self, position: OrderPosition, subevent: SubEvent): price = get_price(position.item, position.variation, voucher=position.voucher, subevent=subevent, @@ -907,19 +892,15 @@ class OrderChangeManager: if not new_quotas: raise OrderError(self.error_messages['quota_missing']) - if self.order.event.settings.invoice_include_free or price.gross != Decimal('0.00') or position.price != Decimal('0.00'): - self._invoice_dirty = True - - self._totaldiff += price.gross - position.price self._quotadiff.update(new_quotas) self._quotadiff.subtract(position.quotas) - self._operations.append(self.SubeventOperation(position, subevent, price)) + self._operations.append(self.SubeventOperation(position, subevent)) def regenerate_secret(self, position: OrderPosition): self._operations.append(self.RegenerateSecretOperation(position)) def change_price(self, position: OrderPosition, price: Decimal): - price = position.item.tax(price) + price = position.item.tax(price, base_price_is='gross') self._totaldiff += price.gross - position.price @@ -1076,14 +1057,11 @@ class OrderChangeManager: 'new_variation': op.variation.pk if op.variation else None, 'old_price': op.position.price, 'addon_to': op.position.addon_to_id, - 'new_price': op.price.gross + 'new_price': op.position.price }) op.position.item = op.item op.position.variation = op.variation - op.position.price = op.price.gross - op.position.tax_rate = op.price.rate - op.position.tax_value = op.price.tax - op.position.tax_rule = op.item.tax_rule + op.position._calculate_tax() op.position.save() elif isinstance(op, self.SubeventOperation): self.order.log_action('pretix.event.order.changed.subevent', user=self.user, auth=self.auth, data={ @@ -1092,13 +1070,9 @@ class OrderChangeManager: 'old_subevent': op.position.subevent.pk, 'new_subevent': op.subevent.pk, 'old_price': op.position.price, - 'new_price': op.price.gross + 'new_price': op.position.price }) op.position.subevent = op.subevent - op.position.price = op.price.gross - op.position.tax_rate = op.price.rate - op.position.tax_value = op.price.tax - op.position.tax_rule = op.position.item.tax_rule op.position.save() elif isinstance(op, self.PriceOperation): self.order.log_action('pretix.event.order.changed.price', user=self.user, auth=self.auth, data={ @@ -1109,9 +1083,7 @@ class OrderChangeManager: 'new_price': op.price.gross }) op.position.price = op.price.gross - op.position.tax_rate = op.price.rate - op.position.tax_value = op.price.tax - op.position.tax_rule = op.position.item.tax_rule + op.position._calculate_tax() op.position.save() elif isinstance(op, self.CancelOperation): for opa in op.position.addons.all(): diff --git a/src/pretix/control/forms/orders.py b/src/pretix/control/forms/orders.py index 37eee6d3d7..4d11a05eeb 100644 --- a/src/pretix/control/forms/orders.py +++ b/src/pretix/control/forms/orders.py @@ -9,9 +9,7 @@ from django.utils.timezone import now from django.utils.translation import pgettext_lazy, ugettext_lazy as _ from pretix.base.forms import I18nModelForm, PlaceholderValidator -from pretix.base.models import ( - InvoiceAddress, Item, ItemAddOn, Order, OrderPosition, -) +from pretix.base.models import InvoiceAddress, ItemAddOn, Order, OrderPosition from pretix.base.models.event import SubEvent from pretix.base.services.pricing import get_price from pretix.control.forms.widgets import Select2 @@ -150,15 +148,6 @@ class CommentForm(I18nModelForm): } -class SubEventChoiceField(forms.ModelChoiceField): - def label_from_instance(self, obj): - p = get_price(self.instance.item, self.instance.variation, - voucher=self.instance.voucher, - subevent=obj) - return '{} – {} ({})'.format(obj.name, obj.get_date_range_display(), - p.print(self.instance.order.event.currency)) - - class OtherOperationsForm(forms.Form): recalculate_taxes = forms.BooleanField( label=_('Re-calculate taxes'), @@ -265,12 +254,13 @@ class OrderPositionAddForm(forms.Form): class OrderPositionChangeForm(forms.Form): - itemvar = forms.ChoiceField() - subevent = SubEventChoiceField( + itemvar = forms.ChoiceField( + required=False, + ) + subevent = forms.ModelChoiceField( SubEvent.objects.none(), - label=pgettext_lazy('subevent', 'New date'), - required=True, - empty_label=None + required=False, + empty_label=_('(Unchanged)') ) price = forms.DecimalField( required=False, @@ -278,53 +268,49 @@ class OrderPositionChangeForm(forms.Form): localize=True, label=_('New price (gross)') ) - operation = forms.ChoiceField( + operation_secret = forms.BooleanField( required=False, - widget=forms.RadioSelect, - choices=( - ('product', 'Change product'), - ('price', 'Change price'), - ('subevent', 'Change event date'), - ('cancel', 'Remove product'), - ('split', 'Split into new order'), - ('secret', 'Regenerate secret'), - ) + label=_('Generate a new secret') + ) + operation_cancel = forms.BooleanField( + required=False, + label=_('Cancel this position') + ) + operation_split = forms.BooleanField( + required=False, + label=_('Split into new order') ) - change_product_keep_price = forms.BooleanField(required=False) def __init__(self, *args, **kwargs): instance = kwargs.pop('instance') initial = kwargs.get('initial', {}) - try: - ia = instance.order.invoice_address - except InvoiceAddress.DoesNotExist: - ia = None - - if instance: - try: - if instance.variation: - initial['itemvar'] = '%d-%d' % (instance.item.pk, instance.variation.pk) - elif instance.item: - initial['itemvar'] = str(instance.item.pk) - except Item.DoesNotExist: - pass - - if instance.item.tax_rule and not instance.item.tax_rule.price_includes_tax: - initial['price'] = instance.price - instance.tax_value - else: - initial['price'] = instance.price - initial['subevent'] = instance.subevent + if instance.item.tax_rule and not instance.item.tax_rule.price_includes_tax: + initial['price'] = instance.price - instance.tax_value + else: + initial['price'] = instance.price kwargs['initial'] = initial super().__init__(*args, **kwargs) if instance.order.event.has_subevents: - self.fields['subevent'].instance = instance self.fields['subevent'].queryset = instance.order.event.subevents.all() + self.fields['subevent'].widget = Select2( + attrs={ + 'data-model-select2': 'event', + 'data-select2-url': reverse('control:event.subevents.select2', kwargs={ + 'event': instance.order.event.slug, + 'organizer': instance.order.event.organizer.slug, + }), + 'data-placeholder': _('(Unchanged)') + } + ) + self.fields['subevent'].widget.choices = self.fields['subevent'].choices else: del self.fields['subevent'] - choices = [] + choices = [ + ('', _('(Unchanged)')) + ] for i in instance.order.event.items.prefetch_related('variations').all(): pname = str(i) if not i.is_available(): @@ -333,14 +319,10 @@ class OrderPositionChangeForm(forms.Form): if variations: for v in variations: - p = get_price(i, v, voucher=instance.voucher, subevent=instance.subevent, - invoice_address=ia) choices.append(('%d-%d' % (i.pk, v.pk), - '%s – %s (%s)' % (pname, v.value, p.print(instance.order.event.currency)))) + '%s – %s' % (pname, v.value))) else: - p = get_price(i, None, voucher=instance.voucher, subevent=instance.subevent, - invoice_address=ia) - choices.append((str(i.pk), '%s (%s)' % (pname, p.print(instance.order.event.currency)))) + choices.append((str(i.pk), pname)) self.fields['itemvar'].choices = choices change_decimal_field(self.fields['price'], instance.order.event.currency) diff --git a/src/pretix/control/templates/pretixcontrol/base.html b/src/pretix/control/templates/pretixcontrol/base.html index 49b1484947..7475dcc07a 100644 --- a/src/pretix/control/templates/pretixcontrol/base.html +++ b/src/pretix/control/templates/pretixcontrol/base.html @@ -43,6 +43,7 @@ + diff --git a/src/pretix/control/templates/pretixcontrol/order/change.html b/src/pretix/control/templates/pretixcontrol/order/change.html index 48f51fe7e6..7b84d542c0 100644 --- a/src/pretix/control/templates/pretixcontrol/order/change.html +++ b/src/pretix/control/templates/pretixcontrol/order/change.html @@ -1,6 +1,7 @@ {% extends "pretixcontrol/event/base.html" %} {% load i18n %} {% load bootstrap3 %} +{% load money %} {% block title %} {% blocktrans trimmed with code=order.code %} Change order: {{ code }} @@ -71,92 +72,87 @@
-
+
{% bootstrap_form_errors position.form %} {% if position.custom_error %}
{{ position.custom_error }}
{% endif %} -

- {% trans "Ticket secret:" %} {{ position.secret|slice:":12" }}… -

-
- +
+
+ {% trans "Current value" %} +
+
+ {% trans "Change to" %} +
-
-