Bugfix/ogc 3406 surcharges or discounts zeros invoice

src/onegov/org/upgrade.py

@@ -22,6 +22,7 @@
 from onegov.form import FormDefinition
 from onegov.form.parser import ParsedForm
 from onegov.newsletter import Newsletter
+from onegov.org import log
 from onegov.org.models import (
     Organisation, Topic, News, ExtendedDirectory, PushNotification)
 from onegov.org.models.political_business import (
@@ -997,3 +998,87 @@ def switch_to_parsed_event_filters(context: UpgradeContext) -> None:
         return
 
     org.event_filter_parsed_definition = ParsedForm.from_formcode(definition)
+
+
+@upgrade_task(
+    'Refresh reservation invoices zeroed by the pricing backfill',
+    requires='onegov.reservation:'
+             'Store pricing settings on reservations (fixed)'
+)
+def refresh_zeroed_reservation_invoices(context: UpgradeContext) -> None:
+    """ `Store pricing settings on reservations (fixed)` repairs the
+    reservation data, but invoice lines already zeroed by an earlier refresh
+    still show 0. Recompute them now that the reservation prices are restored.
+
+    Scoped to reservation invoices touched since the backfill rollout that are
+    tied to an allocation the bug could have disturbed (`per_item`/`per_hour`/
+    `free` — the original broken OR only ever matched `free`), or to a
+    `per_item` resource (whose content fallback was the broken one). Only
+    refreshed when safe (manual, still-open payment).
+    """
+    from onegov.ticket import Ticket
+
+    if not context.has_table('tickets'):
+        return
+    if not context.has_table('invoice_items'):
+        return
+    if not context.has_table('reservations'):
+        return
+
+    # only org-based apps have reservation tickets and a rounding base
+    org = getattr(context.app, 'org', None)
+    if org is None:
+        return
+    rounding_base = org.price_rounding
+
+    ticket_ids = context.session.execute(text("""
+        SELECT DISTINCT t.id
+          FROM tickets t
+          JOIN invoice_items ii
+            ON ii.invoice_id = t.invoice_id
+           AND ii.group = 'reservation'
+           AND COALESCE(ii.modified, ii.created) >= '2026-08-18'
+          JOIN reservations r
+            ON r.token = t.handler_id::uuid
+          JOIN resources res
+            ON res.id = r.resource
+         WHERE t.handler_code = 'RSV'
+           AND (
+                res.content->>'pricing_method' = 'per_item'
+             OR EXISTS (
+                    SELECT 1 FROM allocations a
+                     WHERE a."group" = r.target
+                       AND a.data->>'pricing_method'
+                           IN ('per_item', 'per_hour', 'free')
+                )
+           )
+    """)).scalars().all()
+
+    refreshed: list[str] = []
+    skipped: list[str] = []
+    for ticket_id in ticket_ids:
+        ticket = context.session.get(Ticket, ticket_id)
+        if ticket is None:
+            continue
+
+        handler = ticket.handler
+        if not handler.refreshing_invoice_is_safe(
+            context.request, rounding_base
+        ):
+            skipped.append(ticket.number)
+            continue
+
+        handler.refresh_invoice_items(context.request, rounding_base)
+        refreshed.append(ticket.number)
+
+    if refreshed:
+        log.info(
+            'Refreshed %d reservation invoice(s): %s',
+            len(refreshed), ', '.join(refreshed)
+        )
+    if skipped:
+        log.warning(
+            'Skipped %d reservation invoice(s) that could not be safely '
+            'refreshed (non-manual or non-open payment): %s',
+            len(skipped), ', '.join(skipped)
+        )

src/onegov/reservation/upgrade.py

@@ -15,7 +15,6 @@
 from sqlalchemy import (
     bindparam, text, Column, Enum, ForeignKey, Integer, Text, UUID)
 
-
 from typing import Any, TYPE_CHECKING
 if TYPE_CHECKING:
     from onegov.core.upgrade import UpgradeContext
@@ -506,8 +505,26 @@ def add_source_id_to_reserved_slots(context: UpgradeContext) -> None:
     )
 
 
-@upgrade_task('Store pricing settings on reservations')
-def store_pricing_settings_on_reservations(context: UpgradeContext) -> None:
+@upgrade_task('Store pricing settings on reservations (fixed)')
+def store_pricing_settings_on_reservations_fixed(
+    context: UpgradeContext
+) -> None:
+    """ Snapshots each reservation's pricing onto its own ``data``, from the
+    master allocation when it defines a price, otherwise from the resource
+    content.
+
+    Re-run of `Store pricing settings on reservations` under a new name so it
+    executes again on already-upgraded databases. Three bugs in the original
+    zeroed prices:
+
+    - the allocation lookup matched on the wrong ``pricing_method`` constants
+      (``price_per_item``/``price_per_hour`` instead of ``per_item``/
+      ``per_hour``), so allocation-priced reservations never matched,
+    - the ``resource = mirror_of`` guard was mis-parenthesised (``AND`` bound
+      tighter than the following ``OR``s), and
+    - the resource-content fallback read ``content->'price_per_item'``, but the
+      resource stores that value under ``price_per_reservation``.
+    """
     if not context.has_table('resources'):
         return
 
@@ -526,9 +543,7 @@ def store_pricing_settings_on_reservations(context: UpgradeContext) -> None:
                    ) AS pricing
               FROM allocations
              WHERE resource = mirror_of
-               AND data->>'pricing_method' = 'price_per_item'
-                OR data->>'pricing_method' = 'price_per_hour'
-                OR data->>'pricing_method' = 'free'
+               AND data->>'pricing_method' IN ('per_item', 'per_hour', 'free')
         )
         UPDATE reservations
            SET data = COALESCE(data, '{}'::jsonb) ||
@@ -555,7 +570,7 @@ def store_pricing_settings_on_reservations(context: UpgradeContext) -> None:
                         ),
                         'price_per_item',
                         COALESCE(
-                            resources.content->'price_per_item',
+                            resources.content->'price_per_reservation',
                             '0.0'::jsonb
                         ),
                         'currency',
@@ -566,7 +581,6 @@ def store_pricing_settings_on_reservations(context: UpgradeContext) -> None:
                END
            FROM resources
           WHERE resources.id = resource
-
     """))
 
 

tests/onegov/org/test_pricing_schemes.py

@@ -77,9 +77,9 @@ def test_stadtschulen_zug(client: Client) -> None:
     transaction.begin()
 
     scheduler = (
-        ResourceCollection(client.app.libres_context)  # type: ignore[union-attr]
+        ResourceCollection(client.app.libres_context)
         .by_name('tageskarte')
-        .get_scheduler(client.app.libres_context)
+        .get_scheduler(client.app.libres_context)  # type: ignore[union-attr]
     )
 
     allocations = scheduler.allocate(

tests/onegov/org/test_upgrade.py

@@ -0,0 +1,199 @@
+from __future__ import annotations
+
+import transaction
+
+from datetime import datetime
+from decimal import Decimal
+from freezegun import freeze_time
+from sqlalchemy.orm.attributes import flag_modified
+from onegov.core.utils import Bunch
+from onegov.org.upgrade import refresh_zeroed_reservation_invoices
+from onegov.reservation import ResourceCollection
+from onegov.ticket import TicketCollection
+
+from typing import cast, TYPE_CHECKING
+if TYPE_CHECKING:
+    from onegov.core.upgrade import UpgradeContext
+    from onegov.org.models.ticket import ReservationHandler
+    from tests.onegov.org.conftest import Client
+
+
+@freeze_time('2026-08-20', tick=True)
+def test_refresh_zeroed_reservation_invoices(client: Client) -> None:
+    """ A reservation invoice whose line was zeroed (before the price data was
+    corrected) is recomputed by the upgrade, restoring the price and the
+    payment.
+    """
+    resources = ResourceCollection(client.app.libres_context)
+
+    transaction.begin()
+    resource = resources.add(
+        'Parktower Panorama 24', 'Europe/Zurich', type='room')
+    resource.pricing_method = 'per_item'
+    resource.price_per_item = 200.00
+    resource.payment_method = 'manual'
+    resource.currency = 'CHF'
+    scheduler = resource.get_scheduler(client.app.libres_context)
+    allocations = scheduler.allocate(
+        dates=(datetime(2026, 8, 20), datetime(2026, 8, 20)),
+        whole_day=True,
+        quota=4,
+    )
+    reserve = client.bound_reserve(allocations[0])
+    transaction.commit()
+
+    reserve(quota=1, whole_day=True)
+    page = client.get('/resource/parktower-panorama-24/form')
+    page.form['email'] = 'info@example.org'
+    ticket_page = page.form.submit().follow().form.submit().follow()
+    assert 'RSV-' in ticket_page.text
+
+    client.login_editor()
+    invoice = (
+        client.get('/tickets/ALL/open')
+        .click('Annehmen').follow()
+        .click('Rechnung anzeigen')
+    )
+    assert '200.00' in invoice
+
+    # already-zeroed state: line at 0, payment dropped, price data still 200
+    transaction.begin()
+    session = client.app.session()
+    ticket = TicketCollection(session).query().filter_by(
+        handler_code='RSV').one()
+    handler = cast('ReservationHandler', ticket.handler)
+    payment = handler.payment
+    assert ticket.invoice is not None
+    for item in ticket.invoice.items:
+        if item.group == 'reservation':
+            item.unit = Decimal('0')
+        item.payments = []
+    if payment is not None:
+        for reservation in handler.reservations:
+            reservation.payment = None
+        ticket.payment = None
+        ticket.payment_id = None
+        session.delete(payment)
+    session.flush()
+    ticket_id = ticket.id
+    transaction.commit()
+
+    # sanity: the invoice is collapsed and has no payment
+    session = client.app.session()
+    ticket = TicketCollection(session).query().filter_by(id=ticket_id).one()
+    assert ticket.invoice is not None
+    assert ticket.invoice.total_amount == Decimal('0')
+    assert ticket.handler.payment is None
+
+    # run the upgrade task
+    context = Bunch(
+        has_table=lambda table: True,
+        session=session,
+        app=Bunch(org=Bunch(price_rounding=None)),
+        request=Bunch(session=session, translate=lambda text: text),
+    )
+    refresh_zeroed_reservation_invoices(cast('UpgradeContext', context))
+    transaction.commit()
+
+    # the reservation line and the payment are restored
+    session = client.app.session()
+    ticket = TicketCollection(session).query().filter_by(id=ticket_id).one()
+    assert ticket.invoice is not None
+    reservation_items = [
+        item for item in ticket.invoice.items if item.group == 'reservation'
+    ]
+    assert reservation_items
+    assert all(item.unit == Decimal('200') for item in reservation_items)
+    assert ticket.invoice.total_amount == Decimal('200')
+    assert ticket.handler.payment is not None
+    assert ticket.handler.payment.amount == Decimal('200')
+
+
+@freeze_time('2026-08-20', tick=True)
+def test_refresh_zeroed_reservation_invoices_allocation_priced(
+    client: Client,
+) -> None:
+    """ Same recovery, but the price comes from a per_item allocation override
+    on an otherwise free resource. Exercises the allocation branch of the
+    refresh scoping (not the per_item resource branch) (OGC-3406).
+    """
+    resources = ResourceCollection(client.app.libres_context)
+
+    transaction.begin()
+    resource = resources.add(
+        'Free Room', 'Europe/Zurich', type='room')
+    resource.pricing_method = 'free'  # resource branch must not match
+    resource.payment_method = 'manual'
+    resource.currency = 'CHF'
+    scheduler = resource.get_scheduler(client.app.libres_context)
+    allocations = scheduler.allocate(
+        dates=(datetime(2026, 8, 20), datetime(2026, 8, 20)),
+        whole_day=True,
+        quota=4,
+    )
+    allocations[0].data = {
+        'pricing_method': 'per_item', 'price_per_item': 200.0,
+        'price_per_hour': 0.0, 'currency': 'CHF',
+    }
+    flag_modified(allocations[0], 'data')
+    reserve = client.bound_reserve(allocations[0])
+    transaction.commit()
+
+    reserve(quota=1, whole_day=True)
+    page = client.get('/resource/free-room/form')
+    page.form['email'] = 'info@example.org'
+    ticket_page = page.form.submit().follow().form.submit().follow()
+    assert 'RSV-' in ticket_page.text
+
+    client.login_editor()
+    invoice = (
+        client.get('/tickets/ALL/open')
+        .click('Annehmen').follow()
+        .click('Rechnung anzeigen')
+    )
+    assert '200.00' in invoice
+
+    # already-zeroed state: line at 0, payment dropped
+    transaction.begin()
+    session = client.app.session()
+    ticket = TicketCollection(session).query().filter_by(
+        handler_code='RSV').one()
+    handler = cast('ReservationHandler', ticket.handler)
+    payment = handler.payment
+    assert ticket.invoice is not None
+    for item in ticket.invoice.items:
+        if item.group == 'reservation':
+            item.unit = Decimal('0')
+        item.payments = []
+    if payment is not None:
+        for reservation in handler.reservations:
+            reservation.payment = None
+        ticket.payment = None
+        ticket.payment_id = None
+        session.delete(payment)
+    session.flush()
+    ticket_id = ticket.id
+    transaction.commit()
+
+    session = client.app.session()
+    context = Bunch(
+        has_table=lambda table: True,
+        session=session,
+        app=Bunch(org=Bunch(price_rounding=None)),
+        request=Bunch(session=session, translate=lambda text: text),
+    )
+    refresh_zeroed_reservation_invoices(cast('UpgradeContext', context))
+    transaction.commit()
+
+    # restored via the allocation branch
+    session = client.app.session()
+    ticket = TicketCollection(session).query().filter_by(id=ticket_id).one()
+    assert ticket.invoice is not None
+    reservation_items = [
+        item for item in ticket.invoice.items if item.group == 'reservation'
+    ]
+    assert reservation_items
+    assert all(item.unit == Decimal('200') for item in reservation_items)
+    assert ticket.invoice.total_amount == Decimal('200')
+    assert ticket.handler.payment is not None
+    assert ticket.handler.payment.amount == Decimal('200')