feat: replace custom discount with absolute value in favor of percentage

This commit is contained in:
2026-09-08 17:11:00 +02:00
parent d6c2cfaf1c
commit f8ff4b82a3
15 changed files with 105 additions and 50 deletions
+5 -5
View File
@@ -276,7 +276,7 @@ Single booking by UUID. `404 {"message":"Not found"}` if unknown.
"accommodationDiscount": null,
"boardServiceDiscount": null,
"additionalServicesDiscount": null,
"totalDiscount": null,
"totalDiscountAmount": null,
"totalDiscountLabel": null,
"totalPrice": 123456,
"pricingCurrency": "EUR",
@@ -285,19 +285,19 @@ Single booking by UUID. `404 {"message":"Not found"}` if unknown.
}
```
`totalPrice` is in integer **minor units** (cents) — unlike the contingent endpoints. The `*Discount` fields are whole **percentages** (1100) or `null`, not amounts. `dateFrom`/`dateTo` are `Y-m-d`; `acceptedAt` is a full ISO-8601 datetime and is `null` until accepted. `priceBreakdown` is a computed nested structure.
`totalPrice` and `totalDiscountAmount` are integer **minor units** (cents) — unlike the contingent endpoints. The three `*Discount` fields are whole **percentages** (1100) or `null`, not amounts. `dateFrom`/`dateTo` are `Y-m-d`; `acceptedAt` is a full ISO-8601 datetime and is `null` until accepted. `priceBreakdown` is a computed nested structure.
The first three discounts each apply to one section of the breakdown: `accommodationDiscount` to `basePrice + additionalPersonsPrice`, `boardServiceDiscount` to `boardPrice`, `additionalServicesDiscount` to `servicesPrice`. Surcharges and running costs sit in `total` but in no section, so they are never discounted.
`totalDiscount` is a freely named discount on the whole price, labelled by `totalDiscountLabel`. **It compounds**: the three section discounts reduce `total` first, and `totalDiscount` then applies to that already-reduced subtotal. Recomputing it against the gross `total` yields a different figure. Each amount is rounded to the nearest cent at the step it is applied.
`totalDiscountAmount` is a freely named discount on the whole price, labelled by `totalDiscountLabel`. It is a fixed sum, not a percentage, and is subtracted **after** the three section discounts have reduced `total`. It is capped at what is left of that subtotal, so `discountedTotal` is never negative and the amount shown in the breakdown may be smaller than `totalDiscountAmount` itself. The section discounts are each rounded to the nearest cent as they are applied.
`priceBreakdown` carries the resulting rows, all in minor units:
| field | meaning |
| --- | --- |
| `discounts` | list of `{label, percent, amount}` for the section discounts; entries rounding to `0` are omitted, and `label` already includes its `Rabatt ` prefix |
| `discountSubtotal` | `total` minus the section discounts — present only when `totalDiscount` applies *and* at least one section discount did, otherwise `null` |
| `totalDiscountDetails` | `{label, percent, amount}` for the total discount, or `null` |
| `discountSubtotal` | `total` minus the section discounts — present only when `totalDiscountAmount` applies *and* at least one section discount did, otherwise `null` |
| `totalDiscountDetails` | `{label, amount}` for the total discount, or `null`; `amount` is the capped figure actually subtracted |
| `discountedTotal` | the final price, equal to the booking's `totalPrice` |
#### `POST /api/accommodation-bookings/{uuid}/accept` — scope `api`
+30
View File
@@ -0,0 +1,30 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* The percentage column is dropped rather than converted: it shipped with Version20260908145156
* but no booking was ever given a value, so there is nothing to carry over.
*/
final class Version20260908161500 extends AbstractMigration
{
public function getDescription(): string
{
return 'Store the accommodation booking total discount as an absolute amount in minor units instead of a percentage';
}
public function up(Schema $schema): void
{
$this->addSql('ALTER TABLE accommodation_booking DROP total_discount, ADD total_discount_amount INT DEFAULT NULL');
}
public function down(Schema $schema): void
{
$this->addSql('ALTER TABLE accommodation_booking DROP total_discount_amount, ADD total_discount INT DEFAULT NULL');
}
}
@@ -91,6 +91,7 @@ class EditController extends AbstractController
'current_board_service' => $currentBoardService,
'current_additional_services' => $currentAdditionalServices,
'assignable_managers' => $assignableManagers,
'currency' => $booking->getPricingCurrency() ?? $accommodation?->getCurrency() ?? 'EUR',
]);
$form->handleRequest($request);
+8 -7
View File
@@ -156,13 +156,14 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt
#[Assert\Range(min: 1, max: 100)]
private ?int $additionalServicesDiscount = null;
/** Absolute discount on the total, in minor units. Applied after the three section discounts. */
#[ORM\Column(nullable: true)]
#[Assert\Range(min: 1, max: 100)]
private ?int $totalDiscount = null;
#[Assert\Positive]
private ?int $totalDiscountAmount = null;
#[ORM\Column(length: 255, nullable: true)]
#[Assert\Expression(
'null === this.getTotalDiscount() or (null !== value and "" !== value)',
'null === this.getTotalDiscountAmount() or (null !== value and "" !== value)',
message: 'Bitte eine Bezeichnung für den Rabatt angeben.',
)]
private ?string $totalDiscountLabel = null;
@@ -649,14 +650,14 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt
return $this;
}
public function getTotalDiscount(): ?int
public function getTotalDiscountAmount(): ?int
{
return $this->totalDiscount;
return $this->totalDiscountAmount;
}
public function setTotalDiscount(?int $totalDiscount): self
public function setTotalDiscountAmount(?int $totalDiscountAmount): self
{
$this->totalDiscount = $totalDiscount;
$this->totalDiscountAmount = $totalDiscountAmount;
return $this;
}
@@ -14,11 +14,13 @@ use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
use Symfony\Component\Form\Extension\Core\Type\DateType;
use Symfony\Component\Form\Extension\Core\Type\EmailType;
use Symfony\Component\Form\Extension\Core\Type\IntegerType;
use Symfony\Component\Form\Extension\Core\Type\MoneyType;
use Symfony\Component\Form\Extension\Core\Type\TextareaType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Validator\Constraints\Positive;
use Symfony\Component\Validator\Constraints\Range;
/** @extends AbstractType<AccommodationBooking> */
@@ -158,14 +160,17 @@ class AccommodationBookingType extends AbstractType
new Range(min: 1, max: 100),
],
])
// Applies to the total after the three section discounts. The label is not marked
// required here — a percentage may legitimately be absent — but the entity refuses
// a percentage without one, so no unnamed discount can reach the customer.
->add('totalDiscount', IntegerType::class, [
'label' => 'Rabatt Gesamtpreis (%)',
// A fixed sum, not a percentage, applied to the total after the three section
// discounts. The label is not marked required here — an amount may legitimately be
// absent — but the entity refuses an amount without one, so no unnamed discount can
// reach the customer.
->add('totalDiscountAmount', MoneyType::class, [
'label' => 'Rabatt Gesamtpreis',
'currency' => $options['currency'],
'divisor' => 100,
'required' => false,
'constraints' => [
new Range(min: 1, max: 100),
new Positive(),
],
])
->add('totalDiscountLabel', TextType::class, [
@@ -201,6 +206,7 @@ class AccommodationBookingType extends AbstractType
'current_board_service' => null,
'current_additional_services' => [],
'assignable_managers' => [],
'currency' => 'EUR',
]);
$resolver->setAllowedTypes('max_adolescent_age', 'int');
$resolver->setAllowedTypes('board_services', 'array');
@@ -208,5 +214,6 @@ class AccommodationBookingType extends AbstractType
$resolver->setAllowedTypes('current_board_service', ['null', BoardService::class]);
$resolver->setAllowedTypes('current_additional_services', 'array');
$resolver->setAllowedTypes('assignable_managers', 'array');
$resolver->setAllowedTypes('currency', 'string');
}
}
@@ -73,9 +73,9 @@ final readonly class AccommodationBookingApiResponse
#[Groups(['api:single'])]
public ?int $additionalServicesDiscount;
/** Applies to the total after the three section discounts above, not to the gross total. */
/** An absolute amount in minor units, subtracted after the three section discounts above. */
#[Groups(['api:single'])]
public ?int $totalDiscount;
public ?int $totalDiscountAmount;
#[Groups(['api:single'])]
public ?string $totalDiscountLabel;
@@ -122,7 +122,7 @@ final readonly class AccommodationBookingApiResponse
$this->accommodationDiscount = $booking->getAccommodationDiscount();
$this->boardServiceDiscount = $booking->getBoardServiceDiscount();
$this->additionalServicesDiscount = $booking->getAdditionalServicesDiscount();
$this->totalDiscount = $booking->getTotalDiscount();
$this->totalDiscountAmount = $booking->getTotalDiscountAmount();
$this->totalDiscountLabel = $booking->getTotalDiscountLabel();
$this->totalPrice = $booking->getTotalPrice();
$this->pricingCurrency = $booking->getPricingCurrency();
@@ -43,7 +43,8 @@ class AccommodationBookingBreakdownCalculator
public function withDiscounts(AccommodationBooking $booking, array $breakdown): array
{
// The labels carry their own "Rabatt" prefix so that every row renders as plain
// {label} {percent} %, including the freely named discount on the total below.
// {label} {percent} %, while the freely named discount on the total below is a fixed
// amount and renders as its label alone.
$buckets = [
['Rabatt Unterkunft', $booking->getAccommodationDiscount(), (int) ($breakdown['basePrice'] ?? 0) + (int) ($breakdown['additionalPersonsPrice'] ?? 0)],
['Rabatt Verpflegung', $booking->getBoardServiceDiscount(), (int) ($breakdown['boardPrice'] ?? 0)],
@@ -70,15 +71,15 @@ class AccommodationBookingBreakdownCalculator
// computing the new total, where the stored one is still the outdated value.
$subtotal = (int) ($breakdown['total'] ?? 0) - $discountSum;
// Compounds on the already-reduced subtotal rather than on the gross total: the discount
// is granted on what the customer would otherwise pay, not on a figure no longer asked.
$totalDiscountAmount = self::discountAmount($subtotal, $booking->getTotalDiscount());
// A fixed sum rather than a percentage, subtracted after the section discounts. Clamped to
// what is left of the subtotal so that a discount larger than the price cannot produce a
// negative total, the same guard BookingSummaryAssembler applies to vouchers.
$totalDiscountAmount = min($booking->getTotalDiscountAmount() ?? 0, max(0, $subtotal));
$breakdown['discounts'] = $discounts;
$breakdown['totalDiscountDetails'] = 0 !== $totalDiscountAmount
? [
'label' => (string) $booking->getTotalDiscountLabel(),
'percent' => $booking->getTotalDiscount(),
'amount' => $totalDiscountAmount,
]
: null;
@@ -81,7 +81,7 @@
</div>
<div class="lg:col-span-2 grid lg:grid-cols-2 gap-y-4 lg:gap-x-8">
{{ form_row(form.totalDiscountLabel) }}
{{ form_row(form.totalDiscount) }}
{{ form_row(form.totalDiscountAmount) }}
</div>
<div class="lg:col-span-2">
{{ form_row(form.remarks) }}
@@ -84,8 +84,8 @@
{% if booking.additionalServicesDiscount is not null %}
{% set discounts = discounts | merge(['Zusatzleistungen ' ~ booking.additionalServicesDiscount ~ '%']) %}
{% endif %}
{% if booking.totalDiscount is not null %}
{% set discounts = discounts | merge([(booking.totalDiscountLabel ?: 'Gesamtpreis') ~ ' ' ~ booking.totalDiscount ~ '%']) %}
{% if booking.totalDiscountAmount is not null %}
{% set discounts = discounts | merge([(booking.totalDiscountLabel ?: 'Gesamtpreis') ~ ' ' ~ ((booking.totalDiscountAmount / 100) | format_currency(booking.pricingCurrency ?: 'EUR'))]) %}
{% endif %}
<a href="{{ edit_url }}">{{ discounts | length > 0 ? (discounts | join(', ')) : '' }}</a>
</td>
@@ -160,7 +160,7 @@
{% endif %}
{% if priceBreakdown.totalDiscountDetails is not null %}
<tr class="text-green-700">
<td class="pt-1">{{ priceBreakdown.totalDiscountDetails.label }} {{ priceBreakdown.totalDiscountDetails.percent }}&nbsp;%</td>
<td class="pt-1">{{ priceBreakdown.totalDiscountDetails.label }}</td>
<td class="pt-1 text-right whitespace-nowrap"> {{ (priceBreakdown.totalDiscountDetails.amount / 100) | format_currency(currency) }}</td>
</tr>
{% endif %}
+1 -1
View File
@@ -89,7 +89,7 @@
{% endif %}
{% if priceBreakdown.totalDiscountDetails is not null %}
<tr>
<td style="{{ cell }}">{{ priceBreakdown.totalDiscountDetails.label }} {{ priceBreakdown.totalDiscountDetails.percent }}&nbsp;%</td>
<td style="{{ cell }}">{{ priceBreakdown.totalDiscountDetails.label }}</td>
<td style="{{ amount }}"> {{ (priceBreakdown.totalDiscountDetails.amount / 100)|format_currency(currency) }}</td>
</tr>
{% endif %}
+1 -1
View File
@@ -164,7 +164,7 @@
{% endif %}
{% if ctx.priceBreakdown.totalDiscountDetails is not null %}
<tr class="text-green-700">
<td class="p-2">{{ ctx.priceBreakdown.totalDiscountDetails.label }} {{ ctx.priceBreakdown.totalDiscountDetails.percent }}&nbsp;%</td>
<td class="p-2">{{ ctx.priceBreakdown.totalDiscountDetails.label }}</td>
<td class="p-2 text-right whitespace-nowrap"> {{ (ctx.priceBreakdown.totalDiscountDetails.amount / 100)|format_currency(currency) }}</td>
</tr>
{% endif %}
@@ -56,7 +56,7 @@ class AccommodationBookingControllerTest extends TestCase
$booking->setAccommodationDiscount(10);
$booking->setBoardServiceDiscount(20);
$booking->setAdditionalServicesDiscount(30);
$booking->setTotalDiscount(5);
$booking->setTotalDiscountAmount(5000);
$booking->setTotalDiscountLabel('Treuerabatt');
$booking->setStatus(AccommodationBookingStatus::Confirmed);
$booking->setOrigin(AccommodationBookingOrigin::Direct);
@@ -129,7 +129,7 @@ class AccommodationBookingControllerTest extends TestCase
'accommodationDiscount' => 10,
'boardServiceDiscount' => 20,
'additionalServicesDiscount' => 30,
'totalDiscount' => 5,
'totalDiscountAmount' => 5000,
'totalDiscountLabel' => 'Treuerabatt',
'totalPrice' => 11111,
'pricingCurrency' => 'EUR',
@@ -74,13 +74,13 @@ class AccommodationBookingTypeTest extends TestCase
}
/**
* The label is printed verbatim on the customer's PDF, so a percentage without one is
* The label is printed verbatim on the customer's PDF, so an amount without one is
* refused — and in the Default group, so that even a draft cannot store the pair half-set.
*/
public function testTotalDiscountRequiresALabel(): void
{
$booking = new AccommodationBooking();
$booking->setTotalDiscount(5);
$booking->setTotalDiscountAmount(5000);
$violations = $this->validate($booking, ['Default']);
@@ -91,17 +91,17 @@ class AccommodationBookingTypeTest extends TestCase
public function testALabelledTotalDiscountPassesValidation(): void
{
$booking = new AccommodationBooking();
$booking->setTotalDiscount(5);
$booking->setTotalDiscountAmount(5000);
$booking->setTotalDiscountLabel('Treuerabatt');
self::assertCount(0, $this->validate($booking, ['Default']));
}
/**
* The reverse is harmless — without a percentage the label never reaches a breakdown row —
* The reverse is harmless — without an amount the label never reaches a breakdown row —
* so it is deliberately left valid rather than blocking a half-typed edit.
*/
public function testALabelWithoutAPercentageIsAccepted(): void
public function testALabelWithoutAnAmountIsAccepted(): void
{
$booking = new AccommodationBooking();
$booking->setTotalDiscountLabel('Treuerabatt');
@@ -101,33 +101,32 @@ class AccommodationBookingBreakdownCalculatorTest extends TestCase
/**
* On its own the total discount has nothing to sit below, so no subtotal row is offered and
* the percentage applies to the plain total.
* the amount comes straight off the plain total.
*/
public function testTotalDiscountAloneAppliesToTheTotalWithoutASubtotal(): void
{
$booking = new AccommodationBooking();
$booking->setTotalDiscount(10);
$booking->setTotalDiscountAmount(1235);
$booking->setTotalDiscountLabel('Treuerabatt');
$breakdown = $this->createCalculator()->withDiscounts($booking, $this->breakdown());
self::assertSame([], $breakdown['discounts']);
self::assertSame(['label' => 'Treuerabatt', 'percent' => 10, 'amount' => 1235], $breakdown['totalDiscountDetails']);
self::assertSame(['label' => 'Treuerabatt', 'amount' => 1235], $breakdown['totalDiscountDetails']);
self::assertNull($breakdown['discountSubtotal']);
self::assertSame(12345 - 1235, $breakdown['discountedTotal']);
}
/**
* The worked example from the specification: the total discount compounds on the subtotal
* left by the section discounts, not on the gross total — 5 % of 930,00 € is 46,50 €, where
* 5 % of the gross 1.000,00 € would have been 50,00 €.
* The worked example from the specification: a fixed 50,00 € off the 930,00 € left by the
* section discounts, which is where the Zwischensumme row comes from.
*/
public function testTotalDiscountCompoundsOnTheSubtotalAfterSectionDiscounts(): void
public function testTotalDiscountIsSubtractedFromTheSubtotalAfterSectionDiscounts(): void
{
$booking = new AccommodationBooking();
$booking->setAccommodationDiscount(10);
$booking->setBoardServiceDiscount(5);
$booking->setTotalDiscount(5);
$booking->setTotalDiscountAmount(5000);
$booking->setTotalDiscountLabel('Treuerabatt');
$breakdown = $this->createCalculator()->withDiscounts($booking, [
@@ -138,14 +137,30 @@ class AccommodationBookingBreakdownCalculatorTest extends TestCase
// accommodation: 60000*10% = 6000, board: 20000*5% = 1000 → subtotal 93000
self::assertSame(93000, $breakdown['discountSubtotal']);
self::assertSame(['label' => 'Treuerabatt', 'percent' => 5, 'amount' => 4650], $breakdown['totalDiscountDetails']);
self::assertSame(88350, $breakdown['discountedTotal']);
self::assertSame(['label' => 'Treuerabatt', 'amount' => 5000], $breakdown['totalDiscountDetails']);
self::assertSame(88000, $breakdown['discountedTotal']);
}
/**
* A sum larger than the price is clamped rather than refused: prices move after the discount
* was agreed, and a negative total would be worse than a free stay.
*/
public function testTotalDiscountLargerThanTheSubtotalIsClampedToIt(): void
{
$booking = new AccommodationBooking();
$booking->setTotalDiscountAmount(20000);
$booking->setTotalDiscountLabel('Treuerabatt');
$breakdown = $this->createCalculator()->withDiscounts($booking, $this->breakdown());
self::assertSame(['label' => 'Treuerabatt', 'amount' => 12345], $breakdown['totalDiscountDetails']);
self::assertSame(0, $breakdown['discountedTotal']);
}
public function testTotalDiscountThatAmountsToNothingIsSkipped(): void
{
$booking = new AccommodationBooking();
$booking->setTotalDiscount(20);
$booking->setTotalDiscountAmount(2000);
$booking->setTotalDiscountLabel('Treuerabatt');
$breakdown = $this->createCalculator()->withDiscounts($booking, ['total' => 0]);