From 00734f897236e908a513f3807d88a79179ed5ef8 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Tue, 12 Aug 2025 11:45:56 +0200 Subject: [PATCH] Update fee algorithm --- src/pretix/base/payment.py | 12 ++- src/pretix/base/services/cart.py | 63 ++++++++++----- src/pretix/plugins/paypal2/views.py | 11 +-- src/pretix/presale/checkoutflow.py | 52 ++++++++----- src/pretix/presale/views/__init__.py | 111 +++++++++++++++++---------- src/tests/base/test_taxrules.py | 2 +- 6 files changed, 164 insertions(+), 87 deletions(-) diff --git a/src/pretix/base/payment.py b/src/pretix/base/payment.py index 3e41ec9b1d..697f85018a 100644 --- a/src/pretix/base/payment.py +++ b/src/pretix/base/payment.py @@ -71,7 +71,7 @@ from pretix.helpers.countries import CachedCountries from pretix.helpers.format import format_map from pretix.helpers.money import DecimalTextInput from pretix.multidomain.urlreverse import build_absolute_uri -from pretix.presale.views import get_cart, get_cart_position_sum +from pretix.presale.views import get_cart from pretix.presale.views.cart import cart_session, get_or_create_cart_id logger = logging.getLogger(__name__) @@ -1147,12 +1147,16 @@ class FreeOrderProvider(BasePaymentProvider): from .services.cart import get_fees cart = get_cart(request) - total = get_cart_position_sum(request) + try: - total += sum([f.value for f in get_fees(self.event, request, total, None, None, cart)]) + fees = get_fees(event=request.event, request=request, + invoice_address=None, + payments=None, positions=cart) except TaxRule.SaleNotAllowed: # ignore for now, will fail on order creation - pass + fees = [] + total = sum([c.price for c in cart]) + sum([f.value for f in fees]) + return total == 0 def order_change_allowed(self, order: Order) -> bool: diff --git a/src/pretix/base/services/cart.py b/src/pretix/base/services/cart.py index 3b256bd62c..eb8d964432 100644 --- a/src/pretix/base/services/cart.py +++ b/src/pretix/base/services/cart.py @@ -66,8 +66,8 @@ from pretix.base.reldate import RelativeDateWrapper from pretix.base.services.checkin import _save_answers from pretix.base.services.locking import LockTimeoutException, lock_objects from pretix.base.services.pricing import ( - apply_discounts, get_line_price, get_listed_price, get_price, - is_included_for_free, + apply_discounts, apply_rounding, get_line_price, get_listed_price, + get_price, is_included_for_free, ) from pretix.base.services.quotas import QuotaAvailability from pretix.base.services.tasks import ProfiledEventTask @@ -1493,30 +1493,53 @@ def add_payment_to_cart(request, provider, min_value: Decimal=None, max_value: D add_payment_to_cart_session(cs, provider, min_value, max_value, info_data) -def get_fees(event, request, total, invoice_address, payments, positions): +def get_fees(event, request, _total_ignored_=None, invoice_address=None, payments=None, positions=None): + """ + Return all fees that would be created for the current cart. Also implicitly applies rounding on the order + positions. A recommended usage pattern to compute the total looks like this:: + + cart = get_cart(request) + fees = get_fees( + event=request.event, + request=request, + invoice_address=cached_invoice_address(request), + payments=None, + positions=cart, + ) + total = sum([c.price for c in cart]) + sum([f.value for f in fees]) + """ if payments and not isinstance(payments, list): raise TypeError("payments must now be a list") + if positions is None: + raise TypeError("Must pass positions, parameter is only optional for backwards-compat reasons") fees = [] + total = sum([c.price - c.price_includes_rounding_correction for c in positions]) for recv, resp in fee_calculation_for_cart.send(sender=event, request=request, invoice_address=invoice_address, - total=total, positions=positions, payment_requests=payments): + positions=positions, total=total, payment_requests=payments): if resp: fees += resp - total = total + sum(f.value for f in fees) + for fee in fees: + fee._calculate_tax(invoice_address=invoice_address, event=event) + if fee.tax_rule and not fee.tax_rule.pk: + fee.tax_rule = None # TODO: deprecate + + apply_rounding(event.settings.tax_rounding, event.currency, [*positions, *fees]) + total = sum([c.price for c in positions]) + sum([f.value for f in fees]) if total != 0 and payments: - total_remaining = total + payments_assigned = Decimal("0.00") for p in payments: # This algorithm of treating min/max values and fees needs to stay in sync between the following # places in the code base: # - pretix.base.services.cart.get_fees # - pretix.base.services.orders._get_fees # - pretix.presale.views.CartMixin.current_selected_payments - if p.get('min_value') and total_remaining < Decimal(p['min_value']): + if p.get('min_value') and total - payments_assigned < Decimal(p['min_value']): continue - to_pay = total_remaining + to_pay = max(total - payments_assigned, Decimal("0.00")) if p.get('max_value') and to_pay > Decimal(p['max_value']): to_pay = min(to_pay, Decimal(p['max_value'])) @@ -1525,28 +1548,32 @@ def get_fees(event, request, total, invoice_address, payments, positions): continue payment_fee = pprov.calculate_fee(to_pay) - total_remaining += payment_fee - to_pay += payment_fee - - if p.get('max_value') and to_pay > Decimal(p['max_value']): - to_pay = min(to_pay, Decimal(p['max_value'])) - - total_remaining -= to_pay - if payment_fee: if event.settings.tax_rule_payment == "default": payment_fee_tax_rule = event.cached_default_tax_rule or TaxRule.zero() else: payment_fee_tax_rule = TaxRule.zero() payment_fee_tax = payment_fee_tax_rule.tax(payment_fee, base_price_is='gross', invoice_address=invoice_address) - fees.append(OrderFee( + pf = OrderFee( fee_type=OrderFee.FEE_TYPE_PAYMENT, value=payment_fee, tax_rate=payment_fee_tax.rate, tax_value=payment_fee_tax.tax, tax_code=payment_fee_tax.code, tax_rule=payment_fee_tax_rule - )) + ) + fees.append(pf) + + # Re-apply rounding as grand total has changed + apply_rounding(event.settings.tax_rounding, event.currency, [*positions, *fees]) + total = sum([c.price for c in positions]) + sum([f.value for f in fees]) + + # Re-calculate to_pay as grand total has changed + to_pay = max(total - payments_assigned, Decimal("0.00")) + if p.get('max_value') and to_pay > Decimal(p['max_value']): + to_pay = min(to_pay, Decimal(p['max_value'])) + + payments_assigned += to_pay return fees diff --git a/src/pretix/plugins/paypal2/views.py b/src/pretix/plugins/paypal2/views.py index 41364da51b..a42d95ffec 100644 --- a/src/pretix/plugins/paypal2/views.py +++ b/src/pretix/plugins/paypal2/views.py @@ -73,7 +73,7 @@ from pretix.plugins.paypal2.payment import ( PaypalMethod, PaypalMethod as Paypal, PaypalWallet, ) from pretix.plugins.paypal.models import ReferencedPayPalObject -from pretix.presale.views import get_cart, get_cart_position_sum +from pretix.presale.views import get_cart from pretix.presale.views.cart import cart_session logger = logging.getLogger('pretix.plugins.paypal2') @@ -147,7 +147,7 @@ class XHRView(View): cart_total = order.pending_sum + fee else: - cart_total = get_cart_position_sum(request) + cart = get_cart(request) cart_payments = cart_session(request).get('payments', []) multi_use_cart_payments = [p for p in cart_payments if p.get('multi_use_supported')] simulated_payments = multi_use_cart_payments + [{ @@ -159,12 +159,13 @@ class XHRView(View): }] try: - for fee in get_fees(request.event, request, cart_total, None, simulated_payments, get_cart(request)): - cart_total += fee.value + fees = get_fees(event=request.event, request=request, invoice_address=None, + payments=simulated_payments, positions=cart) except TaxRule.SaleNotAllowed: # ignore for now, will fail on order creation - pass + fees = [] + cart_total = sum([c.price for c in cart]) + sum([f.value for f in fees]) total_remaining = cart_total for p in multi_use_cart_payments: if p.get('min_value') and total_remaining < Decimal(p['min_value']): diff --git a/src/pretix/presale/checkoutflow.py b/src/pretix/presale/checkoutflow.py index 7de3e3ee80..f51354af95 100644 --- a/src/pretix/presale/checkoutflow.py +++ b/src/pretix/presale/checkoutflow.py @@ -91,9 +91,7 @@ from pretix.presale.signals import ( question_form_fields_overrides, ) from pretix.presale.utils import customer_login -from pretix.presale.views import ( - CartMixin, get_cart, get_cart_is_free, get_cart_position_sum, -) +from pretix.presale.views import CartMixin, get_cart, get_cart_is_free from pretix.presale.views.cart import ( _items_from_post_data, cart_session, create_empty_cart_id, get_or_create_cart_id, @@ -1252,18 +1250,16 @@ class PaymentStep(CartMixin, TemplateFlowStep): @cached_property def _total_order_value(self): cart = get_cart(self.request) - total = get_cart_position_sum(self.request) try: - total += sum([ - f.value for f in get_fees( - self.request.event, self.request, total, self.invoice_address, - [p for p in self.cart_session.get('payments', []) if p.get('multi_use_supported')], - cart, - ) - ]) + fees = get_fees( + event=self.request.event, request=self.request, invoice_address=self.invoice_address, + payments=[p for p in self.cart_session.get('payments', []) if p.get('multi_use_supported')], + positions=cart, + ) except TaxRule.SaleNotAllowed: # ignore for now, will fail on order creation - pass + fees = [] + total = sum([c.price for c in cart]) + sum([f.value for f in fees]) return Decimal(total) @cached_property @@ -1389,7 +1385,13 @@ class PaymentStep(CartMixin, TemplateFlowStep): def get_context_data(self, **kwargs): ctx = super().get_context_data(**kwargs) - ctx['current_payments'] = [p for p in self.current_selected_payments(self._total_order_value) if p.get('multi_use_supported')] + ctx['cart'] = self.get_cart() + ctx['current_payments'] = [ + p for p in self.current_selected_payments( + ctx['cart']['positions'], ctx['cart']['fees'], ctx['cart']['invoice_address'], + ) + if p.get('multi_use_supported') + ] ctx['remaining'] = self._total_order_value - sum(p['payment_amount'] for p in ctx['current_payments']) + sum(p['fee'] for p in ctx['current_payments']) ctx['providers'] = self.provider_forms ctx['show_fees'] = any(p['fee'] for p in self.provider_forms) @@ -1402,7 +1404,6 @@ class PaymentStep(CartMixin, TemplateFlowStep): ctx['selected'] = self.single_use_payment['provider'] else: ctx['selected'] = '' - ctx['cart'] = self.get_cart() return ctx def _is_allowed(self, prov, request): @@ -1415,14 +1416,21 @@ class PaymentStep(CartMixin, TemplateFlowStep): return False cart = get_cart(self.request) - total = get_cart_position_sum(self.request) try: - total += sum([f.value for f in get_fees(self.request.event, self.request, total, self.invoice_address, - self.cart_session.get('payments', []), cart)]) + fees = get_fees( + event=self.request.event, + request=self.request, + invoice_address=self.invoice_address, + payments=self.cart_session.get('payments', []), + positions=cart + ) except TaxRule.SaleNotAllowed: # ignore for now, will fail on order creation - pass - selected = self.current_selected_payments(total, warn=warn, total_includes_payment_fees=True) + fees = [] + total = sum([c.price for c in cart]) + sum([f.value for f in fees]) + + selected = self.current_selected_payments(cart, fees, self.invoice_address, warn=warn) + print(sum(p['payment_amount'] for p in selected), total) if sum(p['payment_amount'] for p in selected) != total: if warn: messages.error(request, _('Please select a payment method to proceed.')) @@ -1506,7 +1514,11 @@ class ConfirmStep(CartMixin, AsyncAction, TemplateFlowStep): ctx = super().get_context_data(**kwargs) ctx['cart'] = self.get_cart(answers=True) - selected_payments = self.current_selected_payments(ctx['cart']['total'], total_includes_payment_fees=True) + selected_payments = self.current_selected_payments( + ctx['cart']['positions'], + ctx['cart']['fees'], + ctx['cart']['invoice_address'], + ) ctx['payments'] = [] for p in selected_payments: if p['provider'] == 'free': diff --git a/src/pretix/presale/views/__init__.py b/src/pretix/presale/views/__init__.py index ea9140508c..7c1d4432c0 100644 --- a/src/pretix/presale/views/__init__.py +++ b/src/pretix/presale/views/__init__.py @@ -51,7 +51,7 @@ from django_scopes import scopes_disabled from pretix.base.i18n import get_language_without_region from pretix.base.middleware import get_supported_language from pretix.base.models import ( - CartPosition, Customer, InvoiceAddress, ItemAddOn, Question, + CartPosition, Customer, InvoiceAddress, ItemAddOn, OrderFee, Question, QuestionAnswer, QuestionOption, TaxRule, ) from pretix.base.services.cart import get_fees @@ -149,16 +149,16 @@ class CartMixin: 'question': value.label }) - total = sum(p.price for p in lcp) - if order: fees = order.fees.all() elif lcp: try: fees = get_fees( - self.request.event, self.request, total, self.invoice_address, - payments if payments is not None else self.cart_session.get('payments', []), - cartpos + event=self.request.event, + request=self.request, + invoice_address=self.invoice_address, + payments=payments if payments is not None else self.cart_session.get('payments', []), + positions=cartpos, ) except TaxRule.SaleNotAllowed: # ignore for now, will fail on order creation @@ -168,13 +168,10 @@ class CartMixin: if not order: apply_rounding(self.request.event.settings.tax_rounding, self.request.event.currency, [*lcp, *fees]) - total = sum(p.price for p in lcp) - net_total = sum(p.price - p.tax_value for p in lcp) - tax_total = sum(p.tax_value for p in lcp) - total += sum([f.value for f in fees]) - net_total += sum([f.net_value for f in fees]) - tax_total += sum([f.tax_value for f in fees]) + total = sum([c.price for c in lcp]) + sum([f.value for f in fees]) + net_total = sum(p.price - p.tax_value for p in lcp) + sum([f.net_value for f in fees]) + tax_total = sum(p.tax_value for p in lcp) + sum([f.tax_value for f in fees]) # Group items of the same variation # We do this by list manipulations instead of a GROUP BY query, as @@ -261,20 +258,28 @@ class CartMixin: 'max_expiry_extend': max_expiry_extend, 'is_ordered': bool(order), 'itemcount': sum(c.count for c in positions if not c.addon_to), - 'current_selected_payments': [p for p in self.current_selected_payments(total) if p.get('multi_use_supported')] + 'current_selected_payments': [ + p for p in self.current_selected_payments(positions, fees, self.invoice_address) + if p.get('multi_use_supported') + ] } - def current_selected_payments(self, total, warn=False, total_includes_payment_fees=False): + def current_selected_payments(self, positions, fees, invoice_address, *, warn=False): raw_payments = copy.deepcopy(self.cart_session.get('payments', [])) + fees = [f for f in fees if f.fee_type != OrderFee.FEE_TYPE_PAYMENT] # we re-compute these here + + apply_rounding(self.request.event.settings.tax_rounding, self.request.event.currency, [*positions, *fees]) + total = sum([c.price for c in positions]) + sum([f.value for f in fees]) + payments = [] - total_remaining = total + payments_assigned = Decimal("0.00") for p in raw_payments: # This algorithm of treating min/max values and fees needs to stay in sync between the following # places in the code base: # - pretix.base.services.cart.get_fees # - pretix.base.services.orders._get_fees # - pretix.presale.views.CartMixin.current_selected_payments - if p.get('min_value') and total_remaining < Decimal(p['min_value']): + if p.get('min_value') and total - payments_assigned < Decimal(p['min_value']): if warn: messages.warning( self.request, @@ -285,7 +290,7 @@ class CartMixin: self._remove_payment(p['id']) continue - to_pay = total_remaining + to_pay = max(total - payments_assigned, Decimal("0.00")) if p.get('max_value') and to_pay > Decimal(p['max_value']): to_pay = min(to_pay, Decimal(p['max_value'])) @@ -294,12 +299,36 @@ class CartMixin: self._remove_payment(p['id']) continue - if not total_includes_payment_fees: - fee = pprov.calculate_fee(to_pay) - total_remaining += fee - to_pay += fee - else: - fee = Decimal('0.00') + payment_fee = pprov.calculate_fee(to_pay) + if payment_fee: + if self.request.event.settings.tax_rule_payment == "default": + payment_fee_tax_rule = self.request.event.cached_default_tax_rule or TaxRule.zero() + else: + payment_fee_tax_rule = TaxRule.zero() + try: + payment_fee_tax = payment_fee_tax_rule.tax(payment_fee, base_price_is='gross', invoice_address=invoice_address) + except TaxRule.SaleNotAllowed: + # Replicate behavior from elsewhere, will fail later at the order stage + payment_fee = Decimal("0.00") + payment_fee_tax = TaxRule.zero().tax(payment_fee) + pf = OrderFee( + fee_type=OrderFee.FEE_TYPE_PAYMENT, + value=payment_fee, + tax_rate=payment_fee_tax.rate, + tax_value=payment_fee_tax.tax, + tax_code=payment_fee_tax.code, + tax_rule=payment_fee_tax_rule + ) + fees.append(pf) + + # Re-apply rounding as grand total has changed + apply_rounding(self.request.event.settings.tax_rounding, self.request.event.currency, [*positions, *fees]) + total = sum([c.price for c in positions]) + sum([f.value for f in fees]) + + # Re-calculate to_pay as grand total has changed + to_pay = max(total - payments_assigned, Decimal("0.00")) + if p.get('max_value') and to_pay > Decimal(p['max_value']): + to_pay = min(to_pay, Decimal(p['max_value'])) if p.get('max_value') and to_pay > Decimal(p['max_value']): to_pay = min(to_pay, Decimal(p['max_value'])) @@ -307,8 +336,8 @@ class CartMixin: p['payment_amount'] = to_pay p['provider_name'] = pprov.public_name p['pprov'] = pprov - p['fee'] = fee - total_remaining -= to_pay + p['fee'] = payment_fee + payments_assigned += to_pay payments.append(p) return payments @@ -378,13 +407,22 @@ def get_cart(request): return request._cart_cache -def get_cart_position_sum(request): +def get_cart_total(request): """ - Return an estimate of the current cart total. This estimate does not account for fees and - may not include proper rounding of taxes. This means it is useful e.g. as an input for - determining fees or determining e.g. which payment provider to offer, but is no reliable - estimate for the final order value. + Use the following pattern instead:: + + cart = get_cart(request) + fees = get_fees( + event=request.event, + request=request, + invoice_address=cached_invoice_address(request), + payments=None, + positions=cart, + ) + total = sum([c.price for c in cart]) + sum([f.value for f in fees]) """ + warnings.warn('get_cart_total is deprecated and will be removed in a future release', + DeprecationWarning) from pretix.presale.views.cart import get_or_create_cart_id if not hasattr(request, '_cart_total_cache'): @@ -397,12 +435,6 @@ def get_cart_position_sum(request): return request._cart_total_cache -def get_cart_total(request): - warnings.warn('Use get_cart_position_sum() instead of get_cart_total().', - DeprecationWarning) - return get_cart_position_sum(request) - - def get_cart_invoice_address(request): from pretix.presale.views.cart import cart_session @@ -427,13 +459,14 @@ def get_cart_is_free(request): cs = cart_session(request) pos = get_cart(request) ia = get_cart_invoice_address(request) - total = get_cart_position_sum(request) try: - fees = get_fees(request.event, request, total, ia, cs.get('payments', []), pos) + fees = get_fees(event=request.event, request=request, invoice_address=ia, + payments=cs.get('payments', []), positions=pos) except TaxRule.SaleNotAllowed: # ignore for now, will fail on order creation fees = [] - request._cart_free_cache = total + sum(f.value for f in fees) == Decimal('0.00') + + request._cart_free_cache = sum(p.price for p in pos) + sum(f.value for f in fees) == Decimal('0.00') return request._cart_free_cache diff --git a/src/tests/base/test_taxrules.py b/src/tests/base/test_taxrules.py index 6498ab9678..e4819e2115 100644 --- a/src/tests/base/test_taxrules.py +++ b/src/tests/base/test_taxrules.py @@ -978,7 +978,7 @@ def test_split_fees(event): op2 = OrderPosition(price=Decimal("10.70"), item=item) op2._calculate_tax(tax_rule=tr7, invoice_address=InvoiceAddress()) of1 = OrderFee(value=Decimal("5.00"), fee_type=OrderFee.FEE_TYPE_SHIPPING) - of1._calculate_tax(tax_rule=tr7, invoice_address=InvoiceAddress()) + of1._calculate_tax(tax_rule=tr7, invoice_address=InvoiceAddress(), event=event) # Example of a 10% service fee assert split_fee_for_taxes([op1, op2], Decimal("2.26"), event) == [