fix: streamlined discount display in all breakdowns

This commit is contained in:
Björn Fromme
2026-08-18 13:04:33 +02:00
parent 8dee65d0dc
commit ef24c9a616
10 changed files with 185 additions and 118 deletions
@@ -17,17 +17,60 @@ class AccommodationBookingBreakdownCalculator
/** /**
* Returns the authoritative stored snapshot, falling back to current prices only for * 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<string, mixed>|null null when no accommodation or dates are set * @return array<string, mixed>|null null when no accommodation or dates are set
*/ */
public function compute(AccommodationBooking $booking): ?array public function compute(AccommodationBooking $booking): ?array
{ {
if (null !== $booking->getPriceBreakdown()) { $breakdown = $booking->getPriceBreakdown() ?? $this->computeCurrent($booking);
return $booking->getPriceBreakdown();
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<string, mixed> $breakdown
*
* @return array<string, mixed> 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;
} }
/** /**
+3 -15
View File
@@ -356,28 +356,16 @@ class AccommodationBookingService
return; return;
} }
$accommodationBase = (int) ($breakdown['basePrice'] ?? 0) + (int) ($breakdown['additionalPersonsPrice'] ?? 0); // The raw breakdown is what gets frozen; the discounted total comes from the same
$boardBase = (int) ($breakdown['boardPrice'] ?? 0); // calculator the templates read, so the stored number always matches what is shown.
$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());
$booking->setPriceSnapshot( $booking->setPriceSnapshot(
$breakdown, $breakdown,
$total, $this->breakdownCalculator->withDiscounts($booking, $breakdown)['discountedTotal'],
(string) ($breakdown['currency'] ?? $booking->getAccommodation()?->getCurrency() ?? 'EUR'), (string) ($breakdown['currency'] ?? $booking->getAccommodation()?->getCurrency() ?? 'EUR'),
self::PRICING_VERSION, 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 public function sendNotificationEmail(AccommodationBooking $booking): void
{ {
$this->sendBookingEmail( $this->sendBookingEmail(
@@ -146,31 +146,16 @@
</td> </td>
<td class="pt-2 text-right whitespace-nowrap">{{ (priceBreakdown.total / 100) | format_currency(currency) }}</td> <td class="pt-2 text-right whitespace-nowrap">{{ (priceBreakdown.total / 100) | format_currency(currency) }}</td>
</tr> </tr>
{% if booking.accommodationDiscount is not null or booking.boardServiceDiscount is not null or booking.additionalServicesDiscount is not null %} {% for discount in priceBreakdown.discounts %}
{% if booking.accommodationDiscount is not null %} <tr class="text-green-700">
{% set accommodationDiscountAmount = ((priceBreakdown.basePrice + priceBreakdown.additionalPersonsPrice) * booking.accommodationDiscount / 100) | round %} <td class="pt-1">Rabatt {{ discount.label }} {{ discount.percent }}&nbsp;%</td>
<tr class="text-green-700"> <td class="pt-1 text-right whitespace-nowrap"> {{ (discount.amount / 100) | format_currency(currency) }}</td>
<td class="pt-1">Rabatt Unterkunft {{ booking.accommodationDiscount }}&nbsp;%</td> </tr>
<td class="pt-1 text-right whitespace-nowrap"> {{ (accommodationDiscountAmount / 100) | format_currency(currency) }}</td> {% endfor %}
</tr> {% if priceBreakdown.discounts is not empty %}
{% endif %}
{% if booking.boardServiceDiscount is not null %}
{% set boardDiscountAmount = (priceBreakdown.boardPrice * booking.boardServiceDiscount / 100) | round %}
<tr class="text-green-700">
<td class="pt-1">Rabatt Verpflegung {{ booking.boardServiceDiscount }}&nbsp;%</td>
<td class="pt-1 text-right whitespace-nowrap"> {{ (boardDiscountAmount / 100) | format_currency(currency) }}</td>
</tr>
{% endif %}
{% if booking.additionalServicesDiscount is not null %}
{% set servicesDiscountAmount = (priceBreakdown.servicesPrice * booking.additionalServicesDiscount / 100) | round %}
<tr class="text-green-700">
<td class="pt-1">Rabatt Zusatzleistungen {{ booking.additionalServicesDiscount }}&nbsp;%</td>
<td class="pt-1 text-right whitespace-nowrap"> {{ (servicesDiscountAmount / 100) | format_currency(currency) }}</td>
</tr>
{% endif %}
<tr class="border-t border-gray-200 font-bold text-green-700"> <tr class="border-t border-gray-200 font-bold text-green-700">
<td class="pt-2">Gesamtpreis nach Rabatt</td> <td class="pt-2">Gesamtpreis nach Rabatt</td>
<td class="pt-2 text-right whitespace-nowrap">{{ ((booking.totalPrice ?? priceBreakdown.total) / 100) | format_currency(currency) }}</td> <td class="pt-2 text-right whitespace-nowrap">{{ (priceBreakdown.discountedTotal / 100) | format_currency(currency) }}</td>
</tr> </tr>
{% endif %} {% endif %}
</table> </table>
+6 -31
View File
@@ -94,42 +94,17 @@
{{ (priceBreakdown.total / 100)|format_currency(currency) }} {{ (priceBreakdown.total / 100)|format_currency(currency) }}
</td> </td>
</tr> </tr>
{% set accommodationDiscountAmount = booking.accommodationDiscount is not null {% for discount in priceBreakdown.discounts %}
? ((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 %}
<tr> <tr>
<td style="{{ cell }}">Rabatt Unterkunft {{ booking.accommodationDiscount }}&nbsp;%</td> <td style="{{ cell }}">Rabatt {{ discount.label }} {{ discount.percent }}&nbsp;%</td>
<td style="{{ amount }}"> {{ (accommodationDiscountAmount / 100)|format_currency(currency) }}</td> <td style="{{ amount }}"> {{ (discount.amount / 100)|format_currency(currency) }}</td>
</tr> </tr>
{% endif %} {% endfor %}
{% if booking.boardServiceDiscount is not null %} {% if priceBreakdown.discounts is not empty %}
<tr>
<td style="{{ cell }}">Rabatt Verpflegung {{ booking.boardServiceDiscount }}&nbsp;%</td>
<td style="{{ amount }}"> {{ (boardDiscountAmount / 100)|format_currency(currency) }}</td>
</tr>
{% endif %}
{% if booking.additionalServicesDiscount is not null %}
<tr>
<td style="{{ cell }}">Rabatt Zusatzleistungen {{ booking.additionalServicesDiscount }}&nbsp;%</td>
<td style="{{ amount }}"> {{ (servicesDiscountAmount / 100)|format_currency(currency) }}</td>
</tr>
{% 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. -#}
<tr> <tr>
<td style="{{ cell }} border-top: 1px solid #d1d5db; font-weight: bold;">Gesamtpreis nach Rabatt</td> <td style="{{ cell }} border-top: 1px solid #d1d5db; font-weight: bold;">Gesamtpreis nach Rabatt</td>
<td style="{{ amount }} border-top: 1px solid #d1d5db; font-weight: bold;"> <td style="{{ amount }} border-top: 1px solid #d1d5db; font-weight: bold;">
{{ ((booking.totalPrice ?? (priceBreakdown.total - discountSum)) / 100)|format_currency(currency) }} {{ (priceBreakdown.discountedTotal / 100)|format_currency(currency) }}
</td> </td>
</tr> </tr>
{% endif %} {% endif %}
@@ -6,7 +6,7 @@
{% if ctx.priceBreakdown is not null %} {% if ctx.priceBreakdown is not null %}
<div class="flex items-center justify-between pb-2"> <div class="flex items-center justify-between pb-2">
<span class="block uppercase font-semibold">Gesamtpreis</span> <span class="block uppercase font-semibold">Gesamtpreis</span>
<span class="font-semibold">{{ (displayTotal / 100)|format_currency(currency) }}</span> <span class="font-semibold">{{ (ctx.priceBreakdown.discountedTotal / 100)|format_currency(currency) }}</span>
</div> </div>
{% endif %} {% endif %}
<div class="flex items-center justify-between"> <div class="flex items-center justify-between">
@@ -150,31 +150,16 @@
{{ (ctx.priceBreakdown.total / 100)|format_currency(currency) }} {{ (ctx.priceBreakdown.total / 100)|format_currency(currency) }}
</td> </td>
</tr> </tr>
{% if booking.accommodationDiscount is not null or booking.boardServiceDiscount is not null or booking.additionalServicesDiscount is not null %} {% for discount in ctx.priceBreakdown.discounts %}
{% if booking.accommodationDiscount is not null %} <tr class="text-green-700">
{% set accommodationDiscountAmount = ((ctx.priceBreakdown.basePrice + ctx.priceBreakdown.additionalPersonsPrice) * booking.accommodationDiscount / 100) | round %} <td class="p-2">Rabatt {{ discount.label }} {{ discount.percent }}&nbsp;%</td>
<tr class="text-green-700"> <td class="p-2 text-right whitespace-nowrap"> {{ (discount.amount / 100)|format_currency(currency) }}</td>
<td class="p-2">Rabatt Unterkunft {{ booking.accommodationDiscount }}&nbsp;%</td> </tr>
<td class="p-2 text-right whitespace-nowrap"> {{ (accommodationDiscountAmount / 100)|format_currency(currency) }}</td> {% endfor %}
</tr> {% if ctx.priceBreakdown.discounts is not empty %}
{% endif %}
{% if booking.boardServiceDiscount is not null %}
{% set boardDiscountAmount = (ctx.priceBreakdown.boardPrice * booking.boardServiceDiscount / 100) | round %}
<tr class="text-green-700">
<td class="p-2">Rabatt Verpflegung {{ booking.boardServiceDiscount }}&nbsp;%</td>
<td class="p-2 text-right whitespace-nowrap"> {{ (boardDiscountAmount / 100)|format_currency(currency) }}</td>
</tr>
{% endif %}
{% if booking.additionalServicesDiscount is not null %}
{% set servicesDiscountAmount = (ctx.priceBreakdown.servicesPrice * booking.additionalServicesDiscount / 100) | round %}
<tr class="text-green-700">
<td class="p-2">Rabatt Zusatzleistungen {{ booking.additionalServicesDiscount }}&nbsp;%</td>
<td class="p-2 text-right whitespace-nowrap"> {{ (servicesDiscountAmount / 100)|format_currency(currency) }}</td>
</tr>
{% endif %}
<tr class="border-t border-primary-bg font-bold text-base text-green-700"> <tr class="border-t border-primary-bg font-bold text-base text-green-700">
<td class="p-2">Gesamtpreis nach Rabatt</td> <td class="p-2">Gesamtpreis nach Rabatt</td>
<td class="p-2 text-right whitespace-nowrap">{{ ((booking.totalPrice ?? ctx.priceBreakdown.total) / 100)|format_currency(currency) }}</td> <td class="p-2 text-right whitespace-nowrap">{{ (ctx.priceBreakdown.discountedTotal / 100)|format_currency(currency) }}</td>
</tr> </tr>
{% endif %} {% endif %}
</table> </table>
-2
View File
@@ -30,7 +30,6 @@
{% block content %} {% block content %}
{% set currency = booking.pricingCurrency ?? priceBreakdown.currency %} {% set currency = booking.pricingCurrency ?? priceBreakdown.currency %}
{% set displayTotal = booking.totalPrice ?? (priceBreakdown.total ?? null) %}
<div class="flex-1 flex flex-col lg:grid lg:grid-cols-5 min-h-0 relative"> <div class="flex-1 flex flex-col lg:grid lg:grid-cols-5 min-h-0 relative">
@@ -42,7 +41,6 @@
booking: booking, booking: booking,
ctx: ctx, ctx: ctx,
currency: currency, currency: currency,
displayTotal: displayTotal,
} %} } %}
</div> </div>
@@ -25,6 +25,7 @@ use App\Service\BookingEditPreFlightChecker;
use App\Service\BookingEditSubmitter; use App\Service\BookingEditSubmitter;
use App\Service\BookingSessionManager; use App\Service\BookingSessionManager;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger;
use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormInterface;
use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
@@ -816,6 +817,7 @@ final class TestableIndexController extends IndexController
$editContextFactory, $editContextFactory,
$preFlightChecker, $preFlightChecker,
$formSubmitter, $formSubmitter,
new NullLogger(),
); );
} }
@@ -26,6 +26,7 @@ use App\Security\Crypt;
use Carbon\CarbonImmutable; use Carbon\CarbonImmutable;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView; use Symfony\Component\Form\FormView;
use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -329,6 +330,7 @@ final class TestableParticipantController extends ParticipantController
$bookingSessionService, $bookingSessionService,
$prepopulationService, $prepopulationService,
$participantFormSupportService, $participantFormSupportService,
new NullLogger(),
); );
} }
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Tests\Service; namespace App\Tests\Service;
use App\Entity\Groups\Accommodation;
use App\Entity\Groups\AccommodationBooking; use App\Entity\Groups\AccommodationBooking;
use App\Repository\Groups\AccommodationPriceRepository; use App\Repository\Groups\AccommodationPriceRepository;
use App\Service\AccommodationBookingBreakdownCalculator; use App\Service\AccommodationBookingBreakdownCalculator;
@@ -22,7 +23,99 @@ class AccommodationBookingBreakdownCalculatorTest extends TestCase
$priceRepository = $this->createMock(AccommodationPriceRepository::class); $priceRepository = $this->createMock(AccommodationPriceRepository::class);
$priceRepository->expects(self::never())->method('findByHotelCodeAndDateRange'); $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<string, mixed>
*/
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(), [ new GroupsPriceCalculator(new PriceTimelineBuilder(), [
'runningCostsEur' => 0, 'runningCostsEur' => 0,
'runningCostsChf' => 0, 'runningCostsChf' => 0,
@@ -31,9 +124,7 @@ class AccommodationBookingBreakdownCalculatorTest extends TestCase
'undersubscription40Eur' => 0, 'undersubscription40Eur' => 0,
'undersubscription40Chf' => 0, 'undersubscription40Chf' => 0,
]), ]),
$priceRepository, $priceRepository ?? $this->createMock(AccommodationPriceRepository::class),
); );
self::assertSame($snapshot, $calculator->compute($booking));
} }
} }
@@ -522,21 +522,16 @@ class AccommodationBookingServiceTest extends TestCase
$service->sendBookingConfirmedCustomerEmail($booking); $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 = new AccommodationBooking();
$booking->setAccommodationDiscount(10);
$booking->setBoardServiceDiscount(20);
$booking->setAdditionalServicesDiscount(50);
$breakdown = [ $breakdown = ['total' => 12345, 'currency' => 'CHF'];
'total' => 12345,
'currency' => 'CHF',
'basePrice' => 8000,
'additionalPersonsPrice' => 2000,
'boardPrice' => 1000,
'servicesPrice' => 500,
];
$breakdownCalculator = $this->createMock(AccommodationBookingBreakdownCalculator::class); $breakdownCalculator = $this->createMock(AccommodationBookingBreakdownCalculator::class);
$breakdownCalculator $breakdownCalculator
@@ -544,13 +539,16 @@ class AccommodationBookingServiceTest extends TestCase
->method('computeCurrent') ->method('computeCurrent')
->with($booking) ->with($booking)
->willReturn($breakdown); ->willReturn($breakdown);
$breakdownCalculator
->method('withDiscounts')
->with($booking, $breakdown)
->willReturn($breakdown + ['discounts' => [], 'discountedTotal' => 10895]);
$service = $this->createServiceWithAccommodation(breakdownCalculator: $breakdownCalculator); $service = $this->createServiceWithAccommodation(breakdownCalculator: $breakdownCalculator);
$service->refreshPriceSnapshot($booking); $service->refreshPriceSnapshot($booking);
// accommodation: (8000+2000)*10% = 1000, board: 1000*20% = 200, services: 500*50% = 250 self::assertSame($breakdown, $booking->getPriceBreakdown(), 'the derived keys must not be persisted');
self::assertSame($breakdown, $booking->getPriceBreakdown()); self::assertSame(10895, $booking->getTotalPrice());
self::assertSame(12345 - 1000 - 200 - 250, $booking->getTotalPrice());
self::assertSame('CHF', $booking->getPricingCurrency()); self::assertSame('CHF', $booking->getPricingCurrency());
self::assertSame(1, $booking->getPricingVersion()); self::assertSame(1, $booking->getPricingVersion());
} }