Update fee algorithm

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:

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
 

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']):

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':

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
 
 

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) == [