From 91de0f93e6086b1a7488d3ad8a297cea3ecda781 Mon Sep 17 00:00:00 2001 From: Mira Date: Tue, 5 Nov 2024 14:37:50 +0100 Subject: [PATCH] Allow organizers to manually add fees to an existing order (#4590) --- src/pretix/control/forms/orders.py | 43 +++++ .../templates/pretixcontrol/order/change.html | 96 +++++++++-- src/pretix/control/views/orders.py | 69 ++++++-- src/tests/control/test_orders.py | 154 +++++++++++++----- src/tests/presale/test_event.py | 20 +-- 5 files changed, 299 insertions(+), 83 deletions(-) diff --git a/src/pretix/control/forms/orders.py b/src/pretix/control/forms/orders.py index 70bab945e7..e251d64ffd 100644 --- a/src/pretix/control/forms/orders.py +++ b/src/pretix/control/forms/orders.py @@ -609,6 +609,49 @@ class OrderFeeChangeForm(forms.Form): change_decimal_field(self.fields['value'], instance.order.event.currency) +class OrderFeeAddForm(forms.Form): + fee_type = forms.ChoiceField(choices=OrderFee.FEE_TYPES) + value = forms.DecimalField( + max_digits=13, decimal_places=2, + localize=True, + label=_('Price'), + help_text=_("including all taxes"), + ) + tax_rule = forms.ModelChoiceField( + TaxRule.objects.none(), + required=False, + ) + description = forms.CharField(required=False) + + def __init__(self, *args, **kwargs): + order = kwargs.pop('order') + super().__init__(*args, **kwargs) + self.fields['tax_rule'].queryset = order.event.tax_rules.all() + change_decimal_field(self.fields['value'], order.event.currency) + + +class OrderFeeAddFormset(forms.BaseFormSet): + def __init__(self, *args, **kwargs): + self.order = kwargs.pop('order', None) + super().__init__(*args, **kwargs) + + def _construct_form(self, i, **kwargs): + kwargs['order'] = self.order + return super()._construct_form(i, **kwargs) + + @property + def empty_form(self): + form = self.form( + auto_id=self.auto_id, + prefix=self.add_prefix('__prefix__'), + empty_permitted=True, + use_required_attribute=False, + order=self.order, + ) + self.add_fields(form, None) + return form + + class OrderContactForm(forms.ModelForm): regenerate_secrets = forms.BooleanField(required=False, label=_('Invalidate secrets'), help_text=_('Regenerates the order and ticket secrets. You will ' diff --git a/src/pretix/control/templates/pretixcontrol/order/change.html b/src/pretix/control/templates/pretixcontrol/order/change.html index 92aa9bece7..03d8b3398e 100644 --- a/src/pretix/control/templates/pretixcontrol/order/change.html +++ b/src/pretix/control/templates/pretixcontrol/order/change.html @@ -296,11 +296,11 @@ {% endfor %} -
- {{ add_formset.management_form }} - {% bootstrap_formset_errors add_formset %} +
+ {{ add_position_formset.management_form }} + {% bootstrap_formset_errors add_position_formset %}
- {% for add_form in add_formset %} + {% for add_form in add_position_formset %}

@@ -351,25 +351,25 @@ {% trans "Add product" %}
- {{ add_formset.empty_form.id }} - {% bootstrap_field add_formset.empty_form.DELETE form_group_class="" layout="inline" %} + {{ add_position_formset.empty_form.id }} + {% bootstrap_field add_position_formset.empty_form.DELETE form_group_class="" layout="inline" %}

- {% bootstrap_field add_formset.empty_form.itemvar layout="control" %} - {% bootstrap_field add_formset.empty_form.price addon_after=request.event.currency layout="control" %} - {% if add_formset.empty_form.addon_to %} - {% bootstrap_field add_formset.empty_form.addon_to layout="control" %} + {% bootstrap_field add_position_formset.empty_form.itemvar layout="control" %} + {% bootstrap_field add_position_formset.empty_form.price addon_after=request.event.currency layout="control" %} + {% if add_position_formset.empty_form.addon_to %} + {% bootstrap_field add_position_formset.empty_form.addon_to layout="control" %} {% endif %} - {% if add_formset.empty_form.subevent %} - {% bootstrap_field add_formset.empty_form.subevent layout="control" %} + {% if add_position_formset.empty_form.subevent %} + {% bootstrap_field add_position_formset.empty_form.subevent layout="control" %} {% endif %} - {% if add_formset.empty_form.used_membership %} - {% bootstrap_field add_formset.empty_form.used_membership layout="control" %} + {% if add_position_formset.empty_form.used_membership %} + {% bootstrap_field add_position_formset.empty_form.used_membership layout="control" %} {% endif %} - {% bootstrap_field add_formset.empty_form.seat layout="control" %} + {% bootstrap_field add_position_formset.empty_form.seat layout="control" %}
@@ -431,13 +431,77 @@ {% 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." %} + {% trans "Manually modifying payment fees is discouraged since they might automatically be updated on subsequent order changes or when choosing a different payment method." %} {% endif %}
{% endfor %} + +
+ {{ add_fee_formset.management_form }} + {% bootstrap_formset_errors add_fee_formset %} +
+ {% for add_form in add_fee_formset %} +
+
+

+ + {% trans "Add fee" %} +
+ {{ add_form.id }} + {% bootstrap_field add_form.DELETE form_group_class="" layout="inline" %} +
+

+
+
+
+ {% bootstrap_field add_form.fee_type layout='control' %} + {% bootstrap_field add_form.value addon_after=request.event.currency layout='control' %} + {% bootstrap_field add_form.tax_rule layout='control' %} + {% bootstrap_field add_form.description layout='control' %} +
+
+
+ {% endfor %} +
+ +

+ +

+
+

diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py index 274f69fa1e..9e98e29068 100644 --- a/src/pretix/control/views/orders.py +++ b/src/pretix/control/views/orders.py @@ -122,10 +122,11 @@ from pretix.control.forms.filter import ( ) from pretix.control.forms.orders import ( CancelForm, CommentForm, DenyForm, EventCancelForm, ExporterForm, - ExtendForm, MarkPaidForm, OrderContactForm, OrderFeeChangeForm, - OrderLocaleForm, OrderMailForm, OrderPositionAddForm, - OrderPositionAddFormset, OrderPositionChangeForm, OrderPositionMailForm, - OrderRefundForm, OtherOperationsForm, ReactivateOrderForm, + ExtendForm, MarkPaidForm, OrderContactForm, OrderFeeAddForm, + OrderFeeAddFormset, OrderFeeChangeForm, OrderLocaleForm, OrderMailForm, + OrderPositionAddForm, OrderPositionAddFormset, OrderPositionChangeForm, + OrderPositionMailForm, OrderRefundForm, OtherOperationsForm, + ReactivateOrderForm, ) from pretix.control.forms.rrule import RRuleForm from pretix.control.permissions import EventPermissionRequiredMixin @@ -1874,18 +1875,30 @@ class OrderChange(OrderView): data=self.request.POST if self.request.method == "POST" else None) @cached_property - def add_formset(self): + def add_position_formset(self): ff = formset_factory( OrderPositionAddForm, formset=OrderPositionAddFormset, can_order=False, can_delete=True, extra=0 ) return ff( - prefix='add', + prefix='add_position', order=self.order, items=self.items, data=self.request.POST if self.request.method == "POST" else None ) + @cached_property + def add_fee_formset(self): + ff = formset_factory( + OrderFeeAddForm, formset=OrderFeeAddFormset, + can_order=False, can_delete=True, extra=0 + ) + return ff( + prefix='add_fee', + order=self.order, + data=self.request.POST if self.request.method == "POST" else None + ) + @cached_property def items(self): return self.request.event.items.prefetch_related('variations', 'tax_rule').all() @@ -1914,7 +1927,8 @@ class OrderChange(OrderView): ctx = super().get_context_data(**kwargs) ctx['positions'] = self.positions ctx['fees'] = self.fees - ctx['add_formset'] = self.add_formset + ctx['add_position_formset'] = self.add_position_formset + ctx['add_fee_formset'] = self.add_fee_formset ctx['other_form'] = self.other_form ctx['use_revocation_list'] = self.request.event.ticket_secret_generator.use_revocation_list return ctx @@ -1929,12 +1943,35 @@ class OrderChange(OrderView): ) return True - def _process_add(self, ocm): - if not self.add_formset.is_valid(): + def _process_add_fees(self, ocm): + if not self.add_fee_formset.is_valid(): return False else: - for f in self.add_formset.forms: - if f in self.add_formset.deleted_forms or not f.has_changed(): + for f in self.add_fee_formset.forms: + if f in self.add_fee_formset.deleted_forms or not f.has_changed(): + continue + + f = OrderFee( + fee_type=f.cleaned_data['fee_type'], + value=f.cleaned_data['value'], + order=ocm.order, + tax_rule=f.cleaned_data['tax_rule'], + description=f.cleaned_data['description'], + ) + f._calculate_tax() + try: + ocm.add_fee(f) + except OrderError as e: + f.custom_error = str(e) + return False + return True + + def _process_add_positions(self, ocm): + if not self.add_position_formset.is_valid(): + return False + else: + for f in self.add_position_formset.forms: + if f in self.add_position_formset.deleted_forms or not f.has_changed(): continue if '-' in f.cleaned_data['itemvar']: @@ -1959,7 +1996,7 @@ class OrderChange(OrderView): return False return True - def _process_fees(self, ocm): + def _process_change_fees(self, ocm): for f in self.fees: if not f.form.is_valid(): return False @@ -1980,7 +2017,7 @@ class OrderChange(OrderView): return False return True - def _process_change(self, ocm): + def _process_change_positions(self, ocm): for p in self.positions: if not p.form.is_valid(): return False @@ -2061,7 +2098,11 @@ 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_fees(ocm) and self._process_change(ocm) and self._process_other(ocm) + form_valid = (self._process_add_fees(ocm) and + self._process_add_positions(ocm) and + self._process_change_fees(ocm) and + self._process_change_positions(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/control/test_orders.py b/src/tests/control/test_orders.py index 59ca91f3d4..28a338b5cd 100644 --- a/src/tests/control/test_orders.py +++ b/src/tests/control/test_orders.py @@ -1309,10 +1309,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', 'op-{}-itemvar'.format(self.op1.pk): str(self.shirt.pk), 'op-{}-price'.format(self.op1.pk): str('12.00'), }) @@ -1341,10 +1345,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', 'op-{}-subevent'.format(self.op1.pk): str(se2.pk), }) self.op1.refresh_from_db() @@ -1373,10 +1381,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', 'op-{}-used_membership'.format(self.op1.pk): str(m_correct1.pk), 'op-{}-used_membership'.format(self.op2.pk): str(m_correct1.pk), 'op-{}-used_membership'.format(self.op3.pk): str(m_correct1.pk), @@ -1389,10 +1401,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-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', @@ -1409,10 +1425,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', 'op-{}-operation_cancel'.format(self.op1.pk): 'on', }) self.order.refresh_from_db() @@ -1424,13 +1444,17 @@ class OrderChangeTests(SoupTest): self.client.post('/control/event/{}/{}/orders/{}/change'.format( self.event.organizer.slug, self.event.slug, self.order.code ), { - 'add-TOTAL_FORMS': '1', - 'add-INITIAL_FORMS': '0', - 'add-MIN_NUM_FORMS': '0', - 'add-MAX_NUM_FORMS': '100', - 'add-0-itemvar': str(self.shirt.pk), - 'add-0-do': 'on', - 'add-0-price': '14.00', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '1', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', + 'add_position-0-itemvar': str(self.shirt.pk), + 'add_position-0-do': 'on', + 'add_position-0-price': '14.00', }) with scopes_disabled(): assert self.order.positions.count() == 3 @@ -1453,10 +1477,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', 'other-recalculate_taxes': 'net', 'op-{}-operation'.format(self.op1.pk): '', 'op-{}-operation'.format(self.op2.pk): '', @@ -1489,10 +1517,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', 'other-recalculate_taxes': 'gross', 'op-{}-operation'.format(self.op1.pk): '', 'op-{}-operation'.format(self.op2.pk): '', @@ -1517,10 +1549,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-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), @@ -1544,10 +1580,14 @@ class OrderChangeTests(SoupTest): 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', + 'add_fee-TOTAL_FORMS': '0', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-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', @@ -1563,6 +1603,34 @@ class OrderChangeTests(SoupTest): self.op2.refresh_from_db() assert self.order.total == self.op1.price + self.op2.price + def test_add_fee_success(self): + old_total = self.order.total + r = self.client.post('/control/event/{}/{}/orders/{}/change'.format( + self.event.organizer.slug, self.event.slug, self.order.code + ), { + 'add_fee-TOTAL_FORMS': '1', + 'add_fee-INITIAL_FORMS': '0', + 'add_fee-MIN_NUM_FORMS': '0', + 'add_fee-MAX_NUM_FORMS': '100', + 'add_position-TOTAL_FORMS': '0', + 'add_position-INITIAL_FORMS': '0', + 'add_position-MIN_NUM_FORMS': '0', + 'add_position-MAX_NUM_FORMS': '100', + 'add_fee-0-do': 'on', + 'add_fee-0-fee_type': 'other', + 'add_fee-0-description': 'Surprise Fee', + 'add_fee-0-value': '5.00', + }) + assert r.status_code == 302 + self.order.refresh_from_db() + with scopes_disabled(): + fee = self.order.fees.get() + assert fee.fee_type == OrderFee.FEE_TYPE_OTHER + assert fee.description == 'Surprise Fee' + assert fee.value == Decimal('5.00') + assert not fee.canceled + assert self.order.total == old_total + 5 + @pytest.mark.django_db def test_check_vatid(client, env): diff --git a/src/tests/presale/test_event.py b/src/tests/presale/test_event.py index a3981b0695..2ed36fd568 100644 --- a/src/tests/presale/test_event.py +++ b/src/tests/presale/test_event.py @@ -405,11 +405,11 @@ class ItemDisplayTest(EventTestMixin, SoupTest): SubEventItem.objects.create(subevent=se1, item=item, price=12) resp = self.client.get('/%s/%s/%d/' % (self.orga.slug, self.event.slug, se1.pk)) - self.assertIn("12.00", resp.rendered_content) - self.assertNotIn("15.00", resp.rendered_content) + self.assertIn("€12.00", resp.rendered_content) + self.assertNotIn("€15.00", resp.rendered_content) resp = self.client.get('/%s/%s/%d/' % (self.orga.slug, self.event.slug, se2.pk)) - self.assertIn("15.00", resp.rendered_content) - self.assertNotIn("12.00", resp.rendered_content) + self.assertIn("€15.00", resp.rendered_content) + self.assertNotIn("€12.00", resp.rendered_content) def test_subevent_net_prices(self): self.event.has_subevents = True @@ -429,14 +429,14 @@ class ItemDisplayTest(EventTestMixin, SoupTest): resp = self.client.get('/%s/%s/%d/' % (self.orga.slug, self.event.slug, se1.pk)) doc = BeautifulSoup(resp.rendered_content, "lxml") - self.assertIn("10.08", doc.text) - self.assertNotIn("12.00", doc.text) - self.assertNotIn("15.00", doc.text) + self.assertIn("€10.08", doc.text) + self.assertNotIn("€12.00", doc.text) + self.assertNotIn("€15.00", doc.text) resp = self.client.get('/%s/%s/%d/' % (self.orga.slug, self.event.slug, se2.pk)) doc = BeautifulSoup(resp.rendered_content, "lxml") - self.assertIn("12.61", doc.text) - self.assertNotIn("12.00", doc.text) - self.assertNotIn("15.00", doc.text) + self.assertIn("€12.61", doc.text) + self.assertNotIn("€12.00", doc.text) + self.assertNotIn("€15.00", doc.text) def test_variations_subevent_disabled(self): self.event.has_subevents = True