From 57b86a8fbc89895b1dc10f00bf83e28810a05bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Mon, 19 Oct 2026 10:29:12 +0200 Subject: [PATCH] chore: remove boarding point price levelling --- .env | 9 --- config/services.yaml | 4 -- docs/buspro-database-reference.md | 16 ++--- src/BusProNet/XmlParser/TravelParser.php | 58 ---------------- src/Service/ParticipantPricingCalculator.php | 2 +- src/Service/ProductExportBuilder.php | 4 -- .../BusProNet/XmlParser/TravelParserTest.php | 67 ++++--------------- ...articipantFieldOptionsProviderBabyTest.php | 2 +- tests/Service/ProductExportBuilderTest.php | 3 +- tests/Service/TravelExportBuilderTest.php | 2 +- 10 files changed, 23 insertions(+), 144 deletions(-) diff --git a/.env b/.env index 7f0975c9..75f4bd79 100644 --- a/.env +++ b/.env @@ -98,15 +98,6 @@ APP_TRAVEL_PREFER_REMOTE=false APP_TRAVEL_ENABLE_FALLBACK=true APP_TRAVEL_SNAPSHOT_RETENTION_BUFFER_DAYS=14 -# Compensates a BusPro export defect: a can omit its preis although the booking -# engine still bills the return-leg Abfahrtsortzuschlag, so we undercharge by exactly one leg. -# A boarding point costs the same in both directions, so the stated price is applied to both. -# Off = report the export verbatim. Provisional until the vendor fixes the export; see -# docs/buspro-database-reference.md section A7. -# Flipping this needs `app:bpn:xml-cache-invalidate` and `app:bpn:refresh-travel-snapshot`, -# because the cached travels and TravelSnapshot rows carry the parsed price. -APP_BPN_LEVEL_BOARDING_POINT_PRICES=true - # Off turns every navigation into a full page load, which the web debug toolbar needs: its loader # script never runs on a boosted body swap. Explicit hx-get/hx-post requests are unaffected. # Only for local debugging; prod and the boosted code paths (HxRedirectResponse, error modal) need it on. diff --git a/config/services.yaml b/config/services.yaml index d00df647..de381405 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -201,10 +201,6 @@ services: arguments: $retentionBufferDays: '%travel_snapshot_retention_buffer_days%' - App\BusProNet\XmlParser\TravelParser: - arguments: - $levelBoardingPointPrices: '%env(bool:APP_BPN_LEVEL_BOARDING_POINT_PRICES)%' - App\Service\TravelDataProvider: arguments: $logger: '@monolog.logger.core' diff --git a/docs/buspro-database-reference.md b/docs/buspro-database-reference.md index 977db965..a26c07f7 100644 --- a/docs/buspro-database-reference.md +++ b/docs/buspro-database-reference.md @@ -225,16 +225,12 @@ JOIN dbo.ZustiegPreis zp ON zp.IDArtZustiegPreis_FS=lb.IDArtZustiegPreis_FS WHERE l.IDLeistung IN (@OutboundServiceId,@ReturnServiceId); ``` -**V 2026-09-28 — the XML export can omit the return-leg price.** Since one price row is -surfaced twice, an asymmetric pair is an export defect: on products 701 and 2435 a -`` carries no `preis` although the engine still bills the return -`Abfahrtsortzuschlag`, so reading the export verbatim undercharges by one leg. Measured over -all exports, a stated return price never differs from its outbound counterpart (510 pairs, -zero counter-examples). `TravelParser` therefore levels a boarding point's price across both -directions, switchable through `APP_BPN_LEVEL_BOARDING_POINT_PRICES` while the defect is open -with the vendor. Flipping the flag needs `app:bpn:xml-cache-invalidate` and -`app:bpn:refresh-travel-snapshot`. A point appearing **only** in `zustiege_rueck` without a -price cannot be priced from our side — that is a catalog job. +**V 2026-10-02 — an unpriced return leg is free.** The export can state a `preis` on the +outbound `` but none on the matching `` (seen on products 701 and +2435). The booking engine bills a return `Abfahrtsortzuschlag` only where the return entry +states a price, so the export is read verbatim, per leg. A missing return price is a catalog +omission for the office to fix, not something to compensate locally: pricing the return leg +from the outbound one makes the booking inquiry fail with a price mismatch. **N — the departure surcharge is computed.** Booking responses carry a position `art="SON" unterart="TRA"` (*Abfahrtsortzuschlag*) with no matching row type: `ArtZuschlag` diff --git a/src/BusProNet/XmlParser/TravelParser.php b/src/BusProNet/XmlParser/TravelParser.php index 15dd010f..3a620843 100644 --- a/src/BusProNet/XmlParser/TravelParser.php +++ b/src/BusProNet/XmlParser/TravelParser.php @@ -29,20 +29,8 @@ class TravelParser extends AbstractParser Constants::PSEUDO_PRICE_ROOM_CODE, ]; - /** - * `$levelBoardingPointPrices` carries no default on purpose: a hand-built parser would - * otherwise silently ignore APP_BPN_LEVEL_BOARDING_POINT_PRICES and price one travel tier - * differently from the others. The registry gave up its own default along with it, so that - * there is one way to build this class — through the container, or explicitly in a test. - * - * @param bool $levelBoardingPointPrices Whether to compensate the BusPro export defect that - * omits a boarding point's return-leg price. The - * container wires it from - * APP_BPN_LEVEL_BOARDING_POINT_PRICES. - */ public function __construct( private readonly AgeConstraintParserRegistry $ageConstraintRegistry, - private readonly bool $levelBoardingPointPrices, ) { } @@ -99,11 +87,6 @@ class TravelParser extends AbstractParser $travel->pickups = $this->getPickups($node->filterXPath('.//zustiege/zustieg'), $dateFrom, true); $travel->dropOffs = $this->getPickups($node->filterXPath('.//zustiege_rueck/zustieg_rueck'), $dateTo, false); - // A boarding point costs the same in both directions - if ($this->levelBoardingPointPrices) { - $this->applyBoardingPointPriceLevelling($travel->pickups, $travel->dropOffs); - } - $travel->guide = $this->getGuide($node); return $travel; @@ -466,47 +449,6 @@ class TravelParser extends AbstractParser } } - /** - * Levels the price of a boarding point across both travel directions. - * - * A boarding point is either free or costs the same in both directions: - * `ZustiegPreis` is keyed by (point, tariff scheme) and holds no per-leg price, so - * BusPro bills an `Abfahrtsortzuschlag` per leg at one and the same rate. An - * asymmetric pair is therefore always an export defect — on some products a - * `` carries no `preis` although the engine still charges the return - * leg — and the stated price is the one that applies to both legs. - * - * Where both directions state a price they agree in practice; the outbound one wins - * to keep the outcome deterministic. Points without a counterpart are left as they are. - * - * Compensating the defect locally is provisional, so it is switchable: with - * `$levelBoardingPointPrices` off the export is reported verbatim and the affected - * travels undercharge the return leg again. See - * `docs/buspro-database-reference.md` section A7. - * - * @param array $pickups Pickups indexed by ID (modified in place) - * @param array $dropOffs Drop-offs indexed by ID (modified in place) - */ - private function applyBoardingPointPriceLevelling(array $pickups, array $dropOffs): void - { - foreach ($dropOffs as $pickupId => $dropOff) { - if (false === isset($pickups[$pickupId])) { - continue; - } - - $pickup = $pickups[$pickupId]; - $price = $pickup->price ?? $dropOff->price; - - $pickup->price = $price; - $pickup->priceOutbound = $price; - $pickup->priceInbound = $price; - - $dropOff->price = $price; - $dropOff->priceOutbound = $price; - $dropOff->priceInbound = $price; - } - } - /** * Adjusts time-only pickup entries for overnight bus routes. * diff --git a/src/Service/ParticipantPricingCalculator.php b/src/Service/ParticipantPricingCalculator.php index 9ba63411..1ff07a8c 100644 --- a/src/Service/ParticipantPricingCalculator.php +++ b/src/Service/ParticipantPricingCalculator.php @@ -194,7 +194,7 @@ class ParticipantPricingCalculator } } - // Boarding surcharges are charged per leg, at the same rate in both directions + // Boarding surcharges are charged per leg, each at the price its own direction states $hasOutboundBus = null !== $participant->transportationOutbound && DirectionMapper::isBus($participant->transportationOutbound->subType); $hasInboundBus = null !== $participant->transportationInbound diff --git a/src/Service/ProductExportBuilder.php b/src/Service/ProductExportBuilder.php index ad5228c0..23085077 100644 --- a/src/Service/ProductExportBuilder.php +++ b/src/Service/ProductExportBuilder.php @@ -31,10 +31,6 @@ use Symfony\Contracts\Cache\ItemInterface; * * Hotels are identified by the code and id on the travel XML's own `` node, so * unlike the travel export no row depends on hotel.xml master data. - * - * Boarding-point prices are the export's, verbatim. APP_BPN_LEVEL_BOARDING_POINT_PRICES - * is a booking-path compensation and is deliberately not applied: TravelParser::getPickups() - * does not level, only TravelParser::parse() does. */ class ProductExportBuilder { diff --git a/tests/BusProNet/XmlParser/TravelParserTest.php b/tests/BusProNet/XmlParser/TravelParserTest.php index ca836ef9..4e2b3ea9 100644 --- a/tests/BusProNet/XmlParser/TravelParserTest.php +++ b/tests/BusProNet/XmlParser/TravelParserTest.php @@ -357,49 +357,23 @@ class TravelParserTest extends TestCase $this->assertNull($travel->status); // No status_hin node should result in null } - public function testDropOffsInheritThePickupPriceWhenTheReturnLegStatesNone(): void + public function testBoardingPricesAreReportedVerbatimPerLeg(): void { - // Shape of product 701: the return boarding points carry no preis although - // BusPro bills an Abfahrtsortzuschlag for the return leg as well. + // BusPro bills the return-leg Abfahrtsortzuschlag only where the return entry + // states a preis, so an unpriced return leg is free and each leg keeps its own price. $travel = $this->parseTravelWithBoardingPoints( ' - ', + ', ' - ' - ); - - $this->assertSame(17.9, $travel->dropOffs[18]->price); - $this->assertSame(17.9, $travel->dropOffs[18]->priceInbound); - $this->assertSame(17.9, $travel->pickups[18]->priceInbound); - - // A point that is free outbound stays free on the return leg - $this->assertNull($travel->dropOffs[27]->price); - $this->assertNull($travel->pickups[27]->priceInbound); - } - - public function testPickupInheritsTheDropOffPriceWhenTheOutboundLegStatesNone(): void - { - // The mirror image of the product 701 defect: an asymmetric pair is a defect - // whichever leg is missing, so the stated price applies to both. - $travel = $this->parseTravelWithBoardingPoints( - '', - '' + ' ); $this->assertSame(17.9, $travel->pickups[18]->price); - $this->assertSame(17.9, $travel->pickups[18]->priceOutbound); - $this->assertSame(17.9, $travel->dropOffs[18]->price); - } + $this->assertNull($travel->pickups[18]->priceInbound); + $this->assertNull($travel->dropOffs[18]->price); - public function testConflictingBoardingPricesResolveToTheOutboundOne(): void - { - $travel = $this->parseTravelWithBoardingPoints( - '', - '' - ); - - $this->assertSame(17.9, $travel->pickups[18]->price); - $this->assertSame(17.9, $travel->dropOffs[18]->price); + $this->assertSame(17.9, $travel->pickups[4]->price); + $this->assertSame(9.9, $travel->dropOffs[4]->price); } public function testDropOffWithoutAPickupCounterpartKeepsItsOwnPrice(): void @@ -412,27 +386,12 @@ class TravelParserTest extends TestCase $this->assertSame(5.9, $travel->dropOffs[99]->price); } - public function testBoardingPricesAreReportedVerbatimWhenLevellingIsDisabled(): void + private function createParser(): TravelParser { - // The levelling compensates a vendor export defect and is therefore switchable. - // With it off the product 701 shape undercharges the return leg again. - $travel = $this->parseTravelWithBoardingPoints( - '', - '', - $this->createParser(levelBoardingPointPrices: false) - ); - - $this->assertSame(17.9, $travel->pickups[18]->price); - $this->assertNull($travel->pickups[18]->priceInbound); - $this->assertNull($travel->dropOffs[18]->price); + return new TravelParser(new AgeConstraintParserRegistry()); } - private function createParser(bool $levelBoardingPointPrices = true): TravelParser - { - return new TravelParser(new AgeConstraintParserRegistry(), $levelBoardingPointPrices); - } - - private function parseTravelWithBoardingPoints(string $pickups, string $dropOffs, ?TravelParser $parser = null): Travel + private function parseTravelWithBoardingPoints(string $pickups, string $dropOffs): Travel { $xmlContent = ' @@ -447,7 +406,7 @@ class TravelParserTest extends TestCase $crawler = XmlCrawlerFactory::create($xmlContent); - return ($parser ?? $this->parser)->parse($crawler->filterXPath('//reise/termin')->first()); + return $this->parser->parse($crawler->filterXPath('//reise/termin')->first()); } public function testGetPickupsAdjustsOvernightTimes(): void diff --git a/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php b/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php index 7be900eb..b17c2fb5 100644 --- a/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php +++ b/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php @@ -488,7 +488,7 @@ class ParticipantFieldOptionsProviderBabyTest extends TestCase */ private function createTravelWithExportedSkiPasses(array $serviceIds): Travel { - $parser = new TravelParser(new AgeConstraintParserRegistry(), true); + $parser = new TravelParser(new AgeConstraintParserRegistry()); $crawler = XmlCrawlerFactory::create(file_get_contents('/var/www/html/var/xmlexport/Ziel_2456.xml')); $travelNode = $crawler->filterXPath('//reise/termin')->first(); $travel = $parser->parse($travelNode); diff --git a/tests/Service/ProductExportBuilderTest.php b/tests/Service/ProductExportBuilderTest.php index b5c26bda..29f2a8c5 100644 --- a/tests/Service/ProductExportBuilderTest.php +++ b/tests/Service/ProductExportBuilderTest.php @@ -288,8 +288,7 @@ class ProductExportBuilderTest extends TestCase return (new ProductExportBuilder( $pickupLoader, - // Levelling on, to prove the export does not go through it. - new TravelParser(new AgeConstraintParserRegistry([]), true), + new TravelParser(new AgeConstraintParserRegistry([])), new ProductExportParser(), $filesystem, new TagAwareAdapter(new ArrayAdapter()), diff --git a/tests/Service/TravelExportBuilderTest.php b/tests/Service/TravelExportBuilderTest.php index 759b42af..d615569e 100644 --- a/tests/Service/TravelExportBuilderTest.php +++ b/tests/Service/TravelExportBuilderTest.php @@ -264,7 +264,7 @@ class TravelExportBuilderTest extends TestCase return (new TravelExportBuilder( $hotelLoader, $pickupLoader, - new TravelParser(new AgeConstraintParserRegistry([]), false), + new TravelParser(new AgeConstraintParserRegistry([])), $filesystem, new TagAwareAdapter(new ArrayAdapter()), new NullLogger(),