diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index d3160d7001..4e160036bc 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -32,7 +32,7 @@ from pretix.base.models.orders import ( generate_secret, ) from pretix.base.models.organizer import TeamAPIToken -from pretix.base.models.tax import TaxedPrice +from pretix.base.models.tax import TaxedPrice, TaxRule from pretix.base.payment import BasePaymentProvider, PaymentException from pretix.base.reldate import RelativeDateWrapper from pretix.base.services import tickets @@ -979,6 +979,8 @@ class OrderChangeManager: CancelOperation = namedtuple('CancelOperation', ('position',)) AddOperation = namedtuple('AddOperation', ('item', 'variation', 'price', 'addon_to', 'subevent', 'seat')) SplitOperation = namedtuple('SplitOperation', ('position',)) + FeeValueOperation = namedtuple('FeeValueOperation', ('fee', 'value')) + CancelFeeOperation = namedtuple('CancelFeeOperation', ('fee',)) RegenerateSecretOperation = namedtuple('RegenerateSecretOperation', ('position',)) def __init__(self, order: Order, user=None, auth=None, notify=True, reissue_invoice=True): @@ -1070,8 +1072,19 @@ class OrderChangeManager: self._totaldiff += price.gross - pos.price self._operations.append(self.PriceOperation(pos, price)) + def cancel_fee(self, fee: OrderFee): + self._totaldiff -= fee.value + self._operations.append(self.CancelFeeOperation(fee)) + self._invoice_dirty = True + + def change_fee(self, fee: OrderFee, value: Decimal): + value = (fee.tax_rule or TaxRule.zero()).tax(value, base_price_is='gross') + self._totaldiff += value.gross - fee.value + self._invoice_dirty = True + self._operations.append(self.FeeValueOperation(fee, value)) + def cancel(self, position: OrderPosition): - self._totaldiff += -position.price + self._totaldiff -= position.price self._quotadiff.subtract(position.quotas) self._operations.append(self.CancelOperation(position)) if position.seat: @@ -1187,19 +1200,25 @@ class OrderChangeManager: def _check_paid_to_free(self): if self.order.total == 0 and (self._totaldiff < 0 or (self.split_order and self.split_order.total > 0)) and not self.order.require_approval: - # if the order becomes free, mark it paid using the 'free' provider - # this could happen if positions have been made cheaper or removed (_totaldiff < 0) - # or positions got split off to a new order (split_order with positive total) - p = self.order.payments.create( - state=OrderPayment.PAYMENT_STATE_CREATED, - provider='free', - amount=0, - fee=None - ) - try: - p.confirm(send_mail=False, count_waitinglist=False, user=self.user, auth=self.auth) - except Quota.QuotaExceededException: - raise OrderError(self.error_messages['paid_to_free_exceeded']) + if not self.order.fees.exists() and not self.order.positions.exists(): + # The order is completely empty now, so we cancel it. + self.order.status = Order.STATUS_CANCELED + self.order.save(update_fields=['status']) + order_canceled.send(self.order.event, order=self.order) + else: + # if the order becomes free, mark it paid using the 'free' provider + # this could happen if positions have been made cheaper or removed (_totaldiff < 0) + # or positions got split off to a new order (split_order with positive total) + p = self.order.payments.create( + state=OrderPayment.PAYMENT_STATE_CREATED, + provider='free', + amount=0, + fee=None + ) + try: + p.confirm(send_mail=False, count_waitinglist=False, user=self.user, auth=self.auth) + except Quota.QuotaExceededException: + raise OrderError(self.error_messages['paid_to_free_exceeded']) if self.split_order and self.split_order.total == 0 and not self.split_order.require_approval: p = self.split_order.payments.create( @@ -1256,6 +1275,15 @@ class OrderChangeManager: }) op.position.subevent = op.subevent op.position.save() + elif isinstance(op, self.FeeValueOperation): + self.order.log_action('pretix.event.order.changed.feevalue', user=self.user, auth=self.auth, data={ + 'fee': op.fee.pk, + 'old_price': op.fee.value, + 'new_price': op.value.gross + }) + op.fee.value = op.value.gross + op.fee._calculate_tax() + op.fee.save() elif isinstance(op, self.PriceOperation): self.order.log_action('pretix.event.order.changed.price', user=self.user, auth=self.auth, data={ 'position': op.position.pk, @@ -1267,6 +1295,14 @@ class OrderChangeManager: op.position.price = op.price.gross op.position._calculate_tax() op.position.save() + elif isinstance(op, self.CancelFeeOperation): + self.order.log_action('pretix.event.order.changed.cancelfee', user=self.user, auth=self.auth, data={ + 'fee': op.fee.pk, + 'fee_type': op.fee.fee_type, + 'old_price': op.fee.value, + }) + op.fee.canceled = True + op.fee.save(update_fields=['canceled']) elif isinstance(op, self.CancelOperation): for gc in op.position.issued_gift_cards.all(): gc = GiftCard.objects.select_for_update().get(pk=gc.pk) @@ -1443,10 +1479,19 @@ class OrderChangeManager: fee = None if self.open_payment.fee: fee = self.open_payment.fee - current_fee = self.open_payment.fee.value + if any(isinstance(op, (self.FeeValueOperation, self.CancelFeeOperation)) for op in self._operations): + fee.refresh_from_db() + if not self.open_payment.fee.canceled: + current_fee = self.open_payment.fee.value total -= current_fee - if self.order.pending_sum - current_fee != 0: + if fee and any([isinstance(op, self.FeeValueOperation) and op.fee == fee for op in self._operations]): + # Do not automatically modify a fee that is being manually modified right now + payment_fee = fee.value + elif fee and any([isinstance(op, self.CancelFeeOperation) and op.fee == fee for op in self._operations]): + # Do not automatically modify a fee that is being manually removed right now + payment_fee = Decimal('0.00') + elif self.order.pending_sum - current_fee != 0: prov = self.open_payment.payment_provider if prov: payment_fee = prov.calculate_fee(total - self.completed_payment_sum) @@ -1459,7 +1504,7 @@ class OrderChangeManager: if not self.open_payment.fee: self.open_payment.fee = fee self.open_payment.save(update_fields=['fee']) - elif fee: + elif fee and not fee.canceled: fee.delete() self.order.total = total + payment_fee @@ -1489,9 +1534,10 @@ class OrderChangeManager: generate_invoice(self.order) def _check_complete_cancel(self): + current = self.order.positions.count() cancels = len([o for o in self._operations if isinstance(o, (self.CancelOperation, self.SplitOperation))]) adds = len([o for o in self._operations if isinstance(o, self.AddOperation)]) - if self.order.positions.count() - cancels + adds < 1: + if current > 0 and current - cancels + adds < 1: raise OrderError(self.error_messages['complete_cancel']) @property diff --git a/src/pretix/control/forms/orders.py b/src/pretix/control/forms/orders.py index 32cfdd28c6..50a4ad17eb 100644 --- a/src/pretix/control/forms/orders.py +++ b/src/pretix/control/forms/orders.py @@ -400,9 +400,27 @@ class OrderPositionChangeForm(forms.Form): self.fields['itemvar'].choices = choices change_decimal_field(self.fields['price'], instance.order.event.currency) - def clean(self): - if self.cleaned_data.get('operation') == 'price' and not self.cleaned_data.get('price', '') != '': - raise ValidationError(_('You need to enter a price if you want to change the product price.')) + +class OrderFeeChangeForm(forms.Form): + value = forms.DecimalField( + required=False, + max_digits=10, decimal_places=2, + localize=True, + label=_('New price (gross)') + ) + operation_cancel = forms.BooleanField( + required=False, + label=_('Remove this fee') + ) + + def __init__(self, *args, **kwargs): + instance = kwargs.pop('instance') + initial = kwargs.get('initial', {}) + + initial['value'] = instance.value + kwargs['initial'] = initial + super().__init__(*args, **kwargs) + change_decimal_field(self.fields['value'], instance.order.event.currency) class OrderContactForm(forms.ModelForm): diff --git a/src/pretix/control/logdisplay.py b/src/pretix/control/logdisplay.py index fab771dbee..78425511f2 100644 --- a/src/pretix/control/logdisplay.py +++ b/src/pretix/control/logdisplay.py @@ -65,6 +65,15 @@ def _display_order_changed(event: Event, logentry: LogEntry): old_price=money_filter(Decimal(data['old_price']), event.currency), new_price=money_filter(Decimal(data['new_price']), event.currency), ) + elif logentry.action_type == 'pretix.event.order.changed.feevalue': + return text + ' ' + _('A fee was changed from {old_price} to {new_price}.').format( + old_price=money_filter(Decimal(data['old_price']), event.currency), + new_price=money_filter(Decimal(data['new_price']), event.currency), + ) + elif logentry.action_type == 'pretix.event.order.changed.cancelfee': + return text + ' ' + _('A fee of {old_price} was removed.').format( + old_price=money_filter(Decimal(data['old_price']), event.currency), + ) elif logentry.action_type == 'pretix.event.order.changed.cancel': old_item = str(event.items.get(pk=data['old_item'])) if data['old_variation']: diff --git a/src/pretix/control/templates/pretixcontrol/order/change.html b/src/pretix/control/templates/pretixcontrol/order/change.html index fadbdb4952..bcbb68cf22 100644 --- a/src/pretix/control/templates/pretixcontrol/order/change.html +++ b/src/pretix/control/templates/pretixcontrol/order/change.html @@ -250,11 +250,67 @@ {% endescapescript %}

-

+ {% for fee in fees %} +
+
+

+ {{ fee.get_fee_type_display }} + {% if fee.description %} + – {{ fee.description }} + {% endif %} +

+
+
+
+ {% bootstrap_form_errors fee.form %} + {% if fee.custom_error %} +
+ {{ fee.custom_error }} +
+ {% endif %} +
+
+ {% trans "Current value" %} +
+
+ {% trans "Change to" %} +
+
+
+
+ {% trans "Price" %} +
+
+ {{ fee.value|money:request.event.currency }} + {% if fee.tax_rate %} +
+ + ({{ fee.net_value|money:request.event.currency }} + + {{ fee.tax_rate }}%) + + {% endif %} +
+
+ {% bootstrap_field fee.form.value addon_after=request.event.currency layout='inline' %} + {% trans "including all taxes" %} +
+
+ + {% bootstrap_field fee.form.operation_cancel layout='inline' %} + {% if fee.fee_type == "payment" %} + + {% trans "Manually modifying payment fees is discouraged since they might automatically be on subsequent order changes or when choosing a different payment method." %} + + {% endif %} +
+
+
+ {% endfor %}

diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py index cc30a22473..10d485feff 100644 --- a/src/pretix/control/views/orders.py +++ b/src/pretix/control/views/orders.py @@ -71,9 +71,9 @@ from pretix.control.forms.filter import ( ) from pretix.control.forms.orders import ( CancelForm, CommentForm, ConfirmPaymentForm, ExporterForm, ExtendForm, - MarkPaidForm, OrderContactForm, OrderLocaleForm, OrderMailForm, - OrderPositionAddForm, OrderPositionAddFormset, OrderPositionChangeForm, - OrderRefundForm, OtherOperationsForm, + MarkPaidForm, OrderContactForm, OrderFeeChangeForm, OrderLocaleForm, + OrderMailForm, OrderPositionAddForm, OrderPositionAddFormset, + OrderPositionChangeForm, OrderRefundForm, OtherOperationsForm, ) from pretix.control.permissions import EventPermissionRequiredMixin from pretix.control.views import PaginationMixin @@ -1209,6 +1209,19 @@ class OrderChange(OrderView): data=self.request.POST if self.request.method == "POST" else None ) + @cached_property + 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) + try: + ia = self.order.invoice_address + except InvoiceAddress.DoesNotExist: + ia = None + f.apply_tax = self.request.event.settings.tax_rate_default and self.request.event.settings.tax_rate_default.tax_applicable(invoice_address=ia) + return fees + @cached_property def positions(self): positions = list(self.order.positions.all()) @@ -1225,6 +1238,7 @@ class OrderChange(OrderView): def get_context_data(self, **kwargs): ctx = super().get_context_data(**kwargs) ctx['positions'] = self.positions + ctx['fees'] = self.fees ctx['add_formset'] = self.add_formset ctx['other_form'] = self.other_form return ctx @@ -1266,6 +1280,24 @@ class OrderChange(OrderView): return False return True + def _process_fees(self, ocm): + for f in self.fees: + if not f.form.is_valid(): + return False + + try: + if f.form.cleaned_data['operation_cancel']: + ocm.cancel_fee(f) + continue + + if f.form.cleaned_data['value'] != f.value: + ocm.change_fee(f, f.form.cleaned_data['value']) + + except OrderError as e: + f.custom_error = str(e) + return False + return True + def _process_change(self, ocm): for p in self.positions: if not p.form.is_valid(): @@ -1318,7 +1350,7 @@ class OrderChange(OrderView): notify=notify, reissue_invoice=self.other_form.cleaned_data['reissue_invoice'] if self.other_form.is_valid() else True ) - form_valid = self._process_add(ocm) and self._process_change(ocm) and self._process_other(ocm) + form_valid = self._process_add(ocm) and self._process_fees(ocm) and self._process_change(ocm) and self._process_other(ocm) if not form_valid: messages.error(self.request, _('An error occurred. Please see the details below.')) diff --git a/src/tests/base/test_orders.py b/src/tests/base/test_orders.py index 28b784c434..bef271b688 100644 --- a/src/tests/base/test_orders.py +++ b/src/tests/base/test_orders.py @@ -1973,6 +1973,115 @@ class OrderChangeManagerTests(TestCase): with self.assertRaises(OrderError): self.ocm.add_position(self.ticket, None, price=Decimal('13.00'), subevent=se2, seat=self.seat_a1) + @classscope(attr='o') + def test_fee_change_value(self): + fee = self.order.fees.create(fee_type="shipping", value=Decimal('5.00')) + self.order.total += Decimal('5.00') + self.order.save() + self.ocm.change_fee(fee, Decimal('3.50')) + self.ocm.commit() + self.order.refresh_from_db() + assert self.order.total == Decimal('49.50') + fee.refresh_from_db() + assert fee.value == Decimal('3.50') + + @classscope(attr='o') + def test_fee_change_value_tax_rate(self): + fee = self.order.fees.create(fee_type="shipping", value=Decimal('5.00'), tax_rule=self.tr19) + self.order.total += Decimal('5.00') + self.order.save() + self.ocm.change_fee(fee, Decimal('3.50')) + self.ocm.commit() + self.order.refresh_from_db() + assert self.order.total == Decimal('49.50') + fee.refresh_from_db() + assert fee.value == Decimal('3.50') + assert fee.tax_rate == Decimal('19.00') + assert fee.tax_value == Decimal('0.56') + + @classscope(attr='o') + def test_fee_cancel(self): + fee = self.order.fees.create(fee_type="shipping", value=Decimal('5.00')) + self.order.total += Decimal('5.00') + self.order.save() + self.ocm.cancel_fee(fee) + self.ocm.commit() + self.order.refresh_from_db() + assert self.order.total == Decimal('46.00') + fee.refresh_from_db() + assert fee.canceled + + @classscope(attr='o') + def test_clear_out_order(self): + self.order.status = Order.STATUS_PAID + self.order.save() + self.order.payments.create(amount=self.order.total, state=OrderPayment.PAYMENT_STATE_CONFIRMED, provider='manual') + cancel_order(self.order, cancellation_fee=Decimal('5.00')) + self.order.refresh_from_db() + assert self.order.total == Decimal('5.00') + self.ocm.cancel_fee(self.order.fees.get()) + self.ocm.commit() + self.order.refresh_from_db() + assert self.order.total == Decimal('0.00') + assert self.order.status == Order.STATUS_CANCELED + + @classscope(attr='o') + def test_auto_change_payment_fee(self): + fee2 = self.order.fees.create(fee_type=OrderFee.FEE_TYPE_SHIPPING, value=Decimal('0.50')) + fee = self.order.fees.create(fee_type=OrderFee.FEE_TYPE_PAYMENT, value=Decimal('0.46')) + self.order.status = Order.STATUS_PAID + self.order.total = Decimal('51.1') + self.order.save() + + self.order.payments.create(state=OrderPayment.PAYMENT_STATE_PENDING, amount=Decimal('48.5'), fee=fee, provider="banktransfer") + prov = self.ocm._get_payment_provider() + prov.settings.set('_fee_percent', Decimal('10.00')) + prov.settings.set('_fee_reverse_calc', False) + + self.ocm.cancel_fee(fee2) + self.ocm.commit() + assert self.order.total == Decimal('50.6') + fee.refresh_from_db() + assert fee.value == Decimal('4.6') + + @classscope(attr='o') + def test_change_payment_fee(self): + fee = self.order.fees.create(fee_type=OrderFee.FEE_TYPE_PAYMENT, value=Decimal('0.46')) + self.order.status = Order.STATUS_PAID + self.order.total = Decimal('50.60') + self.order.save() + + self.order.payments.create(state=OrderPayment.PAYMENT_STATE_PENDING, amount=Decimal('48.5'), fee=fee, provider="banktransfer") + prov = self.ocm._get_payment_provider() + prov.settings.set('_fee_percent', Decimal('10.00')) + prov.settings.set('_fee_reverse_calc', False) + + self.ocm.change_fee(fee, Decimal('0.2')) + self.ocm.commit() + fee.refresh_from_db() + assert fee.value == Decimal('0.20') + self.order.refresh_from_db() + assert self.order.total == Decimal('46.20') + + @classscope(attr='o') + def test_cancel_payment_fee(self): + fee = self.order.fees.create(fee_type=OrderFee.FEE_TYPE_PAYMENT, value=Decimal('0.46')) + self.order.status = Order.STATUS_PAID + self.order.total = Decimal('50.60') + self.order.save() + + self.order.payments.create(state=OrderPayment.PAYMENT_STATE_PENDING, amount=Decimal('48.5'), fee=fee, provider="banktransfer") + prov = self.ocm._get_payment_provider() + prov.settings.set('_fee_percent', Decimal('10.00')) + prov.settings.set('_fee_reverse_calc', False) + + self.ocm.cancel_fee(fee) + self.ocm.commit() + fee.refresh_from_db() + assert fee.canceled + self.order.refresh_from_db() + assert self.order.total == Decimal('46.00') + @pytest.mark.django_db def test_autocheckin(clist_autocheckin, event): diff --git a/src/tests/control/test_orders.py b/src/tests/control/test_orders.py index cc606440d3..f9d3ddf41e 100644 --- a/src/tests/control/test_orders.py +++ b/src/tests/control/test_orders.py @@ -1291,6 +1291,60 @@ class OrderChangeTests(SoupTest): assert op.tax_value == Decimal('0.00') assert op.tax_rate == Decimal('0.00') + def test_change_fee_value_success(self): + with scopes_disabled(): + fee = self.order.fees.create(fee_type="shipping", value=Decimal('5.00'), tax_rule=self.tr19) + self.order.total += Decimal('5.00') + self.order.save() + self.client.post('/control/event/{}/{}/orders/{}/change'.format( + self.event.organizer.slug, self.event.slug, self.order.code + ), { + 'add-TOTAL_FORMS': '0', + 'add-INITIAL_FORMS': '0', + 'add-MIN_NUM_FORMS': '0', + 'add-MAX_NUM_FORMS': '100', + 'op-{}-price'.format(self.op1.pk): '24.00', + 'op-{}-operation'.format(self.op2.pk): '', + 'op-{}-itemvar'.format(self.op2.pk): str(self.ticket.pk), + 'of-{}-value'.format(fee.pk): '3.50', + }) + self.op1.refresh_from_db() + self.order.refresh_from_db() + assert self.op1.item == self.ticket + assert self.op1.price == Decimal('24.00') + fee.refresh_from_db() + self.op1.refresh_from_db() + self.op2.refresh_from_db() + assert self.order.total == self.op1.price + self.op2.price + Decimal('3.50') + assert fee.value == Decimal('3.50') + + def test_cancel_fee_success(self): + with scopes_disabled(): + fee = self.order.fees.create(fee_type="shipping", value=Decimal('5.00'), tax_rule=self.tr19) + self.order.total += Decimal('5.00') + self.order.save() + self.client.post('/control/event/{}/{}/orders/{}/change'.format( + self.event.organizer.slug, self.event.slug, self.order.code + ), { + 'add-TOTAL_FORMS': '0', + 'add-INITIAL_FORMS': '0', + 'add-MIN_NUM_FORMS': '0', + 'add-MAX_NUM_FORMS': '100', + 'op-{}-operation'.format(self.op1.pk): 'price', + 'op-{}-itemvar'.format(self.op1.pk): str(self.ticket.pk), + 'op-{}-price'.format(self.op1.pk): '24.00', + 'op-{}-operation'.format(self.op2.pk): '', + 'op-{}-itemvar'.format(self.op2.pk): str(self.ticket.pk), + 'of-{}-value'.format(fee.pk): '5.00', + 'of-{}-operation_cancel'.format(fee.pk): 'on', + }) + self.order.refresh_from_db() + fee.refresh_from_db() + assert fee.canceled + self.op1.refresh_from_db() + self.op2.refresh_from_db() + assert self.order.total == self.op1.price + self.op2.price + @pytest.mark.django_db def test_check_vatid(client, env):