chore: remove boarding point price levelling
This commit is contained in:
1 parent
3f941bb4bf
commit
57b86a8fbc
10 files changed
+23
-144
No files matched your search
@@ -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 <zustieg_rueck> 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.
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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
|
||||
`<zustieg_rueck>` 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 `<zustieg>` but none on the matching `<zustieg_rueck>` (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`
|
||||
|
||||
@@ -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
|
||||
* `<zustieg_rueck>` 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<int, Pickup> $pickups Pickups indexed by ID (modified in place)
|
||||
* @param array<int, Pickup> $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.
|
||||
*
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -31,10 +31,6 @@ use Symfony\Contracts\Cache\ItemInterface;
|
||||
*
|
||||
* Hotels are identified by the code and id on the travel XML's own `<hotel>` 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
|
||||
{
|
||||
|
||||
@@ -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(
|
||||
'<zustieg idbuspro="18" zeit="12.12.2026 21:45:00" preis="17,90" />
|
||||
<zustieg idbuspro="27" zeit="03:45" />',
|
||||
<zustieg idbuspro="4" zeit="12.12.2026 22:30:00" preis="17,90" />',
|
||||
'<zustieg_rueck idbuspro="18" />
|
||||
<zustieg_rueck idbuspro="27" />'
|
||||
);
|
||||
|
||||
$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(
|
||||
'<zustieg idbuspro="18" zeit="12.12.2026 21:45:00" />',
|
||||
'<zustieg_rueck idbuspro="18" preis="17,90" />'
|
||||
<zustieg_rueck idbuspro="4" preis="9,90" />'
|
||||
);
|
||||
|
||||
$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(
|
||||
'<zustieg idbuspro="18" zeit="12.12.2026 21:45:00" preis="17,90" />',
|
||||
'<zustieg_rueck idbuspro="18" preis="9,90" />'
|
||||
);
|
||||
|
||||
$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(
|
||||
'<zustieg idbuspro="18" zeit="12.12.2026 21:45:00" preis="17,90" />',
|
||||
'<zustieg_rueck idbuspro="18" />',
|
||||
$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 = '<?xml version="1.0" encoding="utf-8"?>
|
||||
<reisen>
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in new issue
Block a user