diff --git a/src/Service/AccommodationBookingBreakdownCalculator.php b/src/Service/AccommodationBookingBreakdownCalculator.php index f2eebe7..6bf51b3 100644 --- a/src/Service/AccommodationBookingBreakdownCalculator.php +++ b/src/Service/AccommodationBookingBreakdownCalculator.php @@ -17,17 +17,60 @@ class AccommodationBookingBreakdownCalculator /** * Returns the authoritative stored snapshot, falling back to current prices only for - * bookings created before snapshot persistence was introduced. + * bookings created before snapshot persistence was introduced, enriched with the + * discount rows and the discounted total for rendering. * * @return array|null null when no accommodation or dates are set */ public function compute(AccommodationBooking $booking): ?array { - if (null !== $booking->getPriceBreakdown()) { - return $booking->getPriceBreakdown(); + $breakdown = $booking->getPriceBreakdown() ?? $this->computeCurrent($booking); + + return null !== $breakdown ? $this->withDiscounts($booking, $breakdown) : null; + } + + /** + * Adds the discount rows and the price the customer actually pays. Derived on every read + * and never persisted — refreshPriceSnapshot() stores the raw breakdown from + * computeCurrent() — so the stored total and this one come out of the same code and + * cannot drift apart. + * + * @param array $breakdown + * + * @return array the breakdown plus `discounts` and `discountedTotal` + */ + public function withDiscounts(AccommodationBooking $booking, array $breakdown): array + { + $buckets = [ + ['Unterkunft', $booking->getAccommodationDiscount(), (int) ($breakdown['basePrice'] ?? 0) + (int) ($breakdown['additionalPersonsPrice'] ?? 0)], + ['Verpflegung', $booking->getBoardServiceDiscount(), (int) ($breakdown['boardPrice'] ?? 0)], + ['Zusatzleistungen', $booking->getAdditionalServicesDiscount(), (int) ($breakdown['servicesPrice'] ?? 0)], + ]; + + $discounts = []; + $discountSum = 0; + + foreach ($buckets as [$label, $percent, $base]) { + if (null === $percent) { + continue; + } + + $amount = self::discountAmount($base, $percent); + $discountSum += $amount; + $discounts[] = ['label' => $label, 'percent' => $percent, 'amount' => $amount]; } - return $this->computeCurrent($booking); + $breakdown['discounts'] = $discounts; + // Deliberately not $booking->getTotalPrice(): refreshPriceSnapshot() calls this while + // computing the new total, where the stored one is still the outdated value. + $breakdown['discountedTotal'] = (int) ($breakdown['total'] ?? 0) - $discountSum; + + return $breakdown; + } + + private static function discountAmount(int $base, ?int $percent): int + { + return null !== $percent ? (int) round($base * $percent / 100) : 0; } /** diff --git a/src/Service/AccommodationBookingService.php b/src/Service/AccommodationBookingService.php index 0ea56b7..71d6251 100644 --- a/src/Service/AccommodationBookingService.php +++ b/src/Service/AccommodationBookingService.php @@ -356,28 +356,16 @@ class AccommodationBookingService return; } - $accommodationBase = (int) ($breakdown['basePrice'] ?? 0) + (int) ($breakdown['additionalPersonsPrice'] ?? 0); - $boardBase = (int) ($breakdown['boardPrice'] ?? 0); - $servicesBase = (int) ($breakdown['servicesPrice'] ?? 0); - - $total = (int) ($breakdown['total'] ?? 0) - - $this->discountAmount($accommodationBase, $booking->getAccommodationDiscount()) - - $this->discountAmount($boardBase, $booking->getBoardServiceDiscount()) - - $this->discountAmount($servicesBase, $booking->getAdditionalServicesDiscount()); - + // The raw breakdown is what gets frozen; the discounted total comes from the same + // calculator the templates read, so the stored number always matches what is shown. $booking->setPriceSnapshot( $breakdown, - $total, + $this->breakdownCalculator->withDiscounts($booking, $breakdown)['discountedTotal'], (string) ($breakdown['currency'] ?? $booking->getAccommodation()?->getCurrency() ?? 'EUR'), self::PRICING_VERSION, ); } - private function discountAmount(int $base, ?int $percent): int - { - return null !== $percent ? (int) round($base * $percent / 100) : 0; - } - public function sendNotificationEmail(AccommodationBooking $booking): void { $this->sendBookingEmail( diff --git a/templates/admin/accommodation_booking/show.html.twig b/templates/admin/accommodation_booking/show.html.twig index d812387..7f02673 100644 --- a/templates/admin/accommodation_booking/show.html.twig +++ b/templates/admin/accommodation_booking/show.html.twig @@ -146,31 +146,16 @@ {{ (priceBreakdown.total / 100) | format_currency(currency) }} - {% if booking.accommodationDiscount is not null or booking.boardServiceDiscount is not null or booking.additionalServicesDiscount is not null %} - {% if booking.accommodationDiscount is not null %} - {% set accommodationDiscountAmount = ((priceBreakdown.basePrice + priceBreakdown.additionalPersonsPrice) * booking.accommodationDiscount / 100) | round %} - - Rabatt Unterkunft {{ booking.accommodationDiscount }} % - – {{ (accommodationDiscountAmount / 100) | format_currency(currency) }} - - {% endif %} - {% if booking.boardServiceDiscount is not null %} - {% set boardDiscountAmount = (priceBreakdown.boardPrice * booking.boardServiceDiscount / 100) | round %} - - Rabatt Verpflegung {{ booking.boardServiceDiscount }} % - – {{ (boardDiscountAmount / 100) | format_currency(currency) }} - - {% endif %} - {% if booking.additionalServicesDiscount is not null %} - {% set servicesDiscountAmount = (priceBreakdown.servicesPrice * booking.additionalServicesDiscount / 100) | round %} - - Rabatt Zusatzleistungen {{ booking.additionalServicesDiscount }} % - – {{ (servicesDiscountAmount / 100) | format_currency(currency) }} - - {% endif %} + {% for discount in priceBreakdown.discounts %} + + Rabatt {{ discount.label }} {{ discount.percent }} % + – {{ (discount.amount / 100) | format_currency(currency) }} + + {% endfor %} + {% if priceBreakdown.discounts is not empty %} Gesamtpreis nach Rabatt - {{ ((booking.totalPrice ?? priceBreakdown.total) / 100) | format_currency(currency) }} + {{ (priceBreakdown.discountedTotal / 100) | format_currency(currency) }} {% endif %} diff --git a/templates/email/_price_breakdown.html.twig b/templates/email/_price_breakdown.html.twig index 406c566..76913f0 100644 --- a/templates/email/_price_breakdown.html.twig +++ b/templates/email/_price_breakdown.html.twig @@ -94,42 +94,17 @@ {{ (priceBreakdown.total / 100)|format_currency(currency) }} - {% set accommodationDiscountAmount = booking.accommodationDiscount is not null - ? ((priceBreakdown.basePrice + priceBreakdown.additionalPersonsPrice) * booking.accommodationDiscount / 100) | round - : 0 %} - {% set boardDiscountAmount = booking.boardServiceDiscount is not null - ? (priceBreakdown.boardPrice * booking.boardServiceDiscount / 100) | round - : 0 %} - {% set servicesDiscountAmount = booking.additionalServicesDiscount is not null - ? (priceBreakdown.servicesPrice * booking.additionalServicesDiscount / 100) | round - : 0 %} - {% set discountSum = accommodationDiscountAmount + boardDiscountAmount + servicesDiscountAmount %} - - {% if booking.accommodationDiscount is not null %} + {% for discount in priceBreakdown.discounts %} - Rabatt Unterkunft {{ booking.accommodationDiscount }} % - – {{ (accommodationDiscountAmount / 100)|format_currency(currency) }} + Rabatt {{ discount.label }} {{ discount.percent }} % + – {{ (discount.amount / 100)|format_currency(currency) }} - {% endif %} - {% if booking.boardServiceDiscount is not null %} - - Rabatt Verpflegung {{ booking.boardServiceDiscount }} % - – {{ (boardDiscountAmount / 100)|format_currency(currency) }} - - {% endif %} - {% if booking.additionalServicesDiscount is not null %} - - Rabatt Zusatzleistungen {{ booking.additionalServicesDiscount }} % - – {{ (servicesDiscountAmount / 100)|format_currency(currency) }} - - {% endif %} - {% if discountSum > 0 %} - {#- The frozen snapshot wins where it exists; without one the discounted total has to - be derived from the very rows shown above, so the mail can never contradict itself. -#} + {% endfor %} + {% if priceBreakdown.discounts is not empty %} Gesamtpreis nach Rabatt - {{ ((booking.totalPrice ?? (priceBreakdown.total - discountSum)) / 100)|format_currency(currency) }} + {{ (priceBreakdown.discountedTotal / 100)|format_currency(currency) }} {% endif %} diff --git a/templates/groups/booking/_offer_summary.html.twig b/templates/groups/booking/_offer_summary.html.twig index 9970256..43988e5 100644 --- a/templates/groups/booking/_offer_summary.html.twig +++ b/templates/groups/booking/_offer_summary.html.twig @@ -6,7 +6,7 @@ {% if ctx.priceBreakdown is not null %}
Gesamtpreis - {{ (displayTotal / 100)|format_currency(currency) }} + {{ (ctx.priceBreakdown.discountedTotal / 100)|format_currency(currency) }}
{% endif %}
@@ -150,31 +150,16 @@ {{ (ctx.priceBreakdown.total / 100)|format_currency(currency) }} - {% if booking.accommodationDiscount is not null or booking.boardServiceDiscount is not null or booking.additionalServicesDiscount is not null %} - {% if booking.accommodationDiscount is not null %} - {% set accommodationDiscountAmount = ((ctx.priceBreakdown.basePrice + ctx.priceBreakdown.additionalPersonsPrice) * booking.accommodationDiscount / 100) | round %} - - Rabatt Unterkunft {{ booking.accommodationDiscount }} % - – {{ (accommodationDiscountAmount / 100)|format_currency(currency) }} - - {% endif %} - {% if booking.boardServiceDiscount is not null %} - {% set boardDiscountAmount = (ctx.priceBreakdown.boardPrice * booking.boardServiceDiscount / 100) | round %} - - Rabatt Verpflegung {{ booking.boardServiceDiscount }} % - – {{ (boardDiscountAmount / 100)|format_currency(currency) }} - - {% endif %} - {% if booking.additionalServicesDiscount is not null %} - {% set servicesDiscountAmount = (ctx.priceBreakdown.servicesPrice * booking.additionalServicesDiscount / 100) | round %} - - Rabatt Zusatzleistungen {{ booking.additionalServicesDiscount }} % - – {{ (servicesDiscountAmount / 100)|format_currency(currency) }} - - {% endif %} + {% for discount in ctx.priceBreakdown.discounts %} + + Rabatt {{ discount.label }} {{ discount.percent }} % + – {{ (discount.amount / 100)|format_currency(currency) }} + + {% endfor %} + {% if ctx.priceBreakdown.discounts is not empty %} Gesamtpreis nach Rabatt - {{ ((booking.totalPrice ?? ctx.priceBreakdown.total) / 100)|format_currency(currency) }} + {{ (ctx.priceBreakdown.discountedTotal / 100)|format_currency(currency) }} {% endif %} diff --git a/templates/groups/booking/offer.html.twig b/templates/groups/booking/offer.html.twig index b5d3c96..30481cc 100644 --- a/templates/groups/booking/offer.html.twig +++ b/templates/groups/booking/offer.html.twig @@ -30,7 +30,6 @@ {% block content %} {% set currency = booking.pricingCurrency ?? priceBreakdown.currency %} - {% set displayTotal = booking.totalPrice ?? (priceBreakdown.total ?? null) %}
@@ -42,7 +41,6 @@ booking: booking, ctx: ctx, currency: currency, - displayTotal: displayTotal, } %}
diff --git a/tests/Controller/Booking/Edit/IndexControllerTest.php b/tests/Controller/Booking/Edit/IndexControllerTest.php index a7efbd8..4a9427b 100644 --- a/tests/Controller/Booking/Edit/IndexControllerTest.php +++ b/tests/Controller/Booking/Edit/IndexControllerTest.php @@ -25,6 +25,7 @@ use App\Service\BookingEditPreFlightChecker; use App\Service\BookingEditSubmitter; use App\Service\BookingSessionManager; use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; @@ -816,6 +817,7 @@ final class TestableIndexController extends IndexController $editContextFactory, $preFlightChecker, $formSubmitter, + new NullLogger(), ); } diff --git a/tests/Controller/Booking/Edit/ParticipantControllerTest.php b/tests/Controller/Booking/Edit/ParticipantControllerTest.php index 2e330cd..eb64cd0 100644 --- a/tests/Controller/Booking/Edit/ParticipantControllerTest.php +++ b/tests/Controller/Booking/Edit/ParticipantControllerTest.php @@ -26,6 +26,7 @@ use App\Security\Crypt; use Carbon\CarbonImmutable; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormView; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -329,6 +330,7 @@ final class TestableParticipantController extends ParticipantController $bookingSessionService, $prepopulationService, $participantFormSupportService, + new NullLogger(), ); } diff --git a/tests/Service/AccommodationBookingBreakdownCalculatorTest.php b/tests/Service/AccommodationBookingBreakdownCalculatorTest.php index f736bc4..734f997 100644 --- a/tests/Service/AccommodationBookingBreakdownCalculatorTest.php +++ b/tests/Service/AccommodationBookingBreakdownCalculatorTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Tests\Service; +use App\Entity\Groups\Accommodation; use App\Entity\Groups\AccommodationBooking; use App\Repository\Groups\AccommodationPriceRepository; use App\Service\AccommodationBookingBreakdownCalculator; @@ -22,7 +23,99 @@ class AccommodationBookingBreakdownCalculatorTest extends TestCase $priceRepository = $this->createMock(AccommodationPriceRepository::class); $priceRepository->expects(self::never())->method('findByHotelCodeAndDateRange'); - $calculator = new AccommodationBookingBreakdownCalculator( + $breakdown = $this->createCalculator($priceRepository)->compute($booking); + + self::assertSame(12345, $breakdown['total']); + self::assertSame('EUR', $breakdown['currency']); + } + + public function testComputeEnrichesTheStoredSnapshotWithDiscountRowsAndTotal(): void + { + $booking = new AccommodationBooking(); + $booking->setPriceSnapshot($this->breakdown(), 12345, 'CHF', 1); + $booking->setAccommodationDiscount(10); + + $breakdown = $this->createCalculator()->compute($booking); + + self::assertSame([['label' => 'Unterkunft', 'percent' => 10, 'amount' => 1000]], $breakdown['discounts']); + self::assertSame(12345 - 1000, $breakdown['discountedTotal']); + } + + public function testWithDiscountsListsOneRowPerConfiguredDiscount(): void + { + $booking = new AccommodationBooking(); + $booking->setAccommodationDiscount(10); + $booking->setBoardServiceDiscount(20); + $booking->setAdditionalServicesDiscount(50); + + $breakdown = $this->createCalculator()->withDiscounts($booking, $this->breakdown()); + + // accommodation: (8000+2000)*10% = 1000, board: 1000*20% = 200, services: 500*50% = 250 + self::assertSame([ + ['label' => 'Unterkunft', 'percent' => 10, 'amount' => 1000], + ['label' => 'Verpflegung', 'percent' => 20, 'amount' => 200], + ['label' => 'Zusatzleistungen', 'percent' => 50, 'amount' => 250], + ], $breakdown['discounts']); + self::assertSame(12345 - 1000 - 200 - 250, $breakdown['discountedTotal']); + } + + public function testWithDiscountsRoundsToWholeCents(): void + { + $booking = new AccommodationBooking(); + $booking->setBoardServiceDiscount(15); + + // 333 * 15% = 49.95 — the amount has to land on whole cents, and the discounted total + // has to be reduced by the very number that is shown on the row. + $breakdown = $this->createCalculator()->withDiscounts($booking, ['total' => 12345, 'boardPrice' => 333]); + + self::assertSame(50, $breakdown['discounts'][0]['amount']); + self::assertSame(12345 - 50, $breakdown['discountedTotal']); + } + + public function testWithoutDiscountsTheDiscountedTotalIsTheTotal(): void + { + $breakdown = $this->createCalculator()->withDiscounts(new AccommodationBooking(), $this->breakdown()); + + self::assertSame([], $breakdown['discounts']); + self::assertSame(12345, $breakdown['discountedTotal']); + } + + /** + * The derived keys must never reach the database: refreshPriceSnapshot() freezes exactly + * what computeCurrent() returns. + */ + public function testComputeCurrentReturnsTheRawBreakdown(): void + { + $booking = new AccommodationBooking(); + $booking->setAccommodationDiscount(10); + $booking->setAccommodation((new Accommodation())->setCalendarCode('HOTEL')->setCurrency('CHF')); + $booking->setDateFrom(new \DateTimeImmutable('2026-08-01')); + $booking->setDateTo(new \DateTimeImmutable('2026-08-03')); + + $breakdown = $this->createCalculator()->computeCurrent($booking); + + self::assertArrayNotHasKey('discounts', $breakdown); + self::assertArrayNotHasKey('discountedTotal', $breakdown); + } + + /** + * @return array + */ + private function breakdown(): array + { + return [ + 'total' => 12345, + 'currency' => 'CHF', + 'basePrice' => 8000, + 'additionalPersonsPrice' => 2000, + 'boardPrice' => 1000, + 'servicesPrice' => 500, + ]; + } + + private function createCalculator(?AccommodationPriceRepository $priceRepository = null): AccommodationBookingBreakdownCalculator + { + return new AccommodationBookingBreakdownCalculator( new GroupsPriceCalculator(new PriceTimelineBuilder(), [ 'runningCostsEur' => 0, 'runningCostsChf' => 0, @@ -31,9 +124,7 @@ class AccommodationBookingBreakdownCalculatorTest extends TestCase 'undersubscription40Eur' => 0, 'undersubscription40Chf' => 0, ]), - $priceRepository, + $priceRepository ?? $this->createMock(AccommodationPriceRepository::class), ); - - self::assertSame($snapshot, $calculator->compute($booking)); } } diff --git a/tests/Service/AccommodationBookingServiceTest.php b/tests/Service/AccommodationBookingServiceTest.php index ee23504..8ae9919 100644 --- a/tests/Service/AccommodationBookingServiceTest.php +++ b/tests/Service/AccommodationBookingServiceTest.php @@ -522,21 +522,16 @@ class AccommodationBookingServiceTest extends TestCase $service->sendBookingConfirmedCustomerEmail($booking); } - public function testRefreshPriceSnapshotStoresDiscountedFinalTotal(): void + /** + * The discount arithmetic itself lives in — and is tested with — + * AccommodationBookingBreakdownCalculator; what matters here is that the raw breakdown is + * frozen and the stored total is the calculator's discounted one. + */ + public function testRefreshPriceSnapshotStoresTheRawBreakdownAndTheDiscountedTotal(): void { $booking = new AccommodationBooking(); - $booking->setAccommodationDiscount(10); - $booking->setBoardServiceDiscount(20); - $booking->setAdditionalServicesDiscount(50); - $breakdown = [ - 'total' => 12345, - 'currency' => 'CHF', - 'basePrice' => 8000, - 'additionalPersonsPrice' => 2000, - 'boardPrice' => 1000, - 'servicesPrice' => 500, - ]; + $breakdown = ['total' => 12345, 'currency' => 'CHF']; $breakdownCalculator = $this->createMock(AccommodationBookingBreakdownCalculator::class); $breakdownCalculator @@ -544,13 +539,16 @@ class AccommodationBookingServiceTest extends TestCase ->method('computeCurrent') ->with($booking) ->willReturn($breakdown); + $breakdownCalculator + ->method('withDiscounts') + ->with($booking, $breakdown) + ->willReturn($breakdown + ['discounts' => [], 'discountedTotal' => 10895]); $service = $this->createServiceWithAccommodation(breakdownCalculator: $breakdownCalculator); $service->refreshPriceSnapshot($booking); - // accommodation: (8000+2000)*10% = 1000, board: 1000*20% = 200, services: 500*50% = 250 - self::assertSame($breakdown, $booking->getPriceBreakdown()); - self::assertSame(12345 - 1000 - 200 - 250, $booking->getTotalPrice()); + self::assertSame($breakdown, $booking->getPriceBreakdown(), 'the derived keys must not be persisted'); + self::assertSame(10895, $booking->getTotalPrice()); self::assertSame('CHF', $booking->getPricingCurrency()); self::assertSame(1, $booking->getPricingVersion()); }