fix: revert pickup pricing to match current BusPro behavior
Reverts split pricing calculation to use outbound-only pricing, matching BusPro's current behavior where pickup is only charged when outbound transportation is BUS. - Use pickup->price directly instead of calculateEffectivePrice() - PKW+BUS scenario now correctly charges €0 (matches BusPro loophole) - priceOutbound/priceInbound remain populated for future activation - Updated documentation with current behavior and future plan
This commit is contained in:
Björn Fromme
@@ -0,0 +1,130 @@
|
||||
# Pickup Pricing - Implementation Documentation
|
||||
|
||||
## Current BusPro Behavior (January 2026)
|
||||
|
||||
BusPro currently only charges pickup price when the **outbound** transportation is BUS, regardless of the inbound selection.
|
||||
|
||||
### Pricing Matrix (Current)
|
||||
|
||||
| Outbound | Inbound | BusPro Charges | Notes |
|
||||
|----------|---------|----------------|-------|
|
||||
| BUS | BUS | Pickup price once | Full price from outbound |
|
||||
| BUS | PKW | Pickup price once | Full price from outbound |
|
||||
| PKW | BUS | **€0** | Known loophole in BusPro |
|
||||
| PKW | PKW | €0 | No pickup available |
|
||||
|
||||
The "loophole" (PKW outbound + BUS inbound = no charge) exists on BusPro's side. Our portal must match this behavior to avoid price validation errors when submitting bookings.
|
||||
|
||||
## XML Data Structure
|
||||
|
||||
Both travel sections contain pickup data with prices:
|
||||
|
||||
```xml
|
||||
<!-- Outbound pickups -->
|
||||
<zustiege>
|
||||
<zustieg id="3" idbuspro="4" preis="5,90" .../>
|
||||
</zustiege>
|
||||
|
||||
<!-- Inbound pickups -->
|
||||
<zustiege_rueck>
|
||||
<zustieg_rueck id="3" idbuspro="4" preis="5,90" .../>
|
||||
</zustiege_rueck>
|
||||
```
|
||||
|
||||
Currently, prices are duplicated in both sections. In the future, when BusPro supports split pricing, travels may be configured with different prices per direction (e.g., 2.95 each way instead of 5.90 outbound only).
|
||||
|
||||
## Portal Implementation
|
||||
|
||||
### Forward-Compatible Data Layer
|
||||
|
||||
The portal parses and stores both direction prices for future use:
|
||||
|
||||
1. **Pickup Model** (`src/BusProNet/Model/Pickup.php`):
|
||||
- `price`: The outbound price (used for pricing calculations)
|
||||
- `priceOutbound`: Explicit outbound price (populated but not used yet)
|
||||
- `priceInbound`: Explicit inbound price (populated but not used yet)
|
||||
- `calculateEffectivePrice()`: Ready for future split pricing activation
|
||||
|
||||
2. **TravelParser** (`src/BusProNet/XmlParser/TravelParser.php`):
|
||||
- Parses `zustiege` section and sets `priceOutbound`
|
||||
- Parses `zustiege_rueck` section and sets `priceInbound`
|
||||
- Merges inbound prices into outbound pickup objects by ID
|
||||
|
||||
3. **BookingDataProcessor** (`src/BusProNet/DataProcessor/BookingDataProcessor.php`):
|
||||
- Enriches participant pickups with all price properties from travel data
|
||||
|
||||
### Current Pricing Logic
|
||||
|
||||
The pricing calculators use simple outbound-only logic to match BusPro:
|
||||
|
||||
```php
|
||||
// Only charge if outbound is BUS - matches current BusPro behavior
|
||||
$hasOutboundBus = null !== $participant->transportationOutbound
|
||||
&& 'BUS' === $participant->transportationOutbound->subType;
|
||||
|
||||
if ($hasOutboundBus && null !== $participant->pickup && null !== $participant->pickup->price) {
|
||||
$serviceTotal += $participant->pickup->price;
|
||||
}
|
||||
```
|
||||
|
||||
This ensures portal prices always match BusPro responses, avoiding validation errors.
|
||||
|
||||
## API Constraints
|
||||
|
||||
- Pickups can only be submitted via the `zustiege` (outbound) XML section
|
||||
- There is no `zustiege_rueck` (inbound) equivalent for submission
|
||||
- Booking responses return pickups in the outbound section only
|
||||
- A future API update will support separate pickup/drop-off locations
|
||||
|
||||
## Future Activation (When BusPro Supports Split Pricing)
|
||||
|
||||
When BusPro is updated to charge based on actual transportation selections, update the pricing calculators to use `calculateEffectivePrice()`:
|
||||
|
||||
```php
|
||||
$hasOutboundBus = null !== $participant->transportationOutbound
|
||||
&& 'BUS' === $participant->transportationOutbound->subType;
|
||||
$hasInboundBus = null !== $participant->transportationInbound
|
||||
&& 'BUS' === $participant->transportationInbound->subType;
|
||||
|
||||
if (null !== $participant->pickup) {
|
||||
$pickupPrice = $participant->pickup->calculateEffectivePrice($hasOutboundBus, $hasInboundBus);
|
||||
if (null !== $pickupPrice) {
|
||||
$serviceTotal += $pickupPrice;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Important Caveat for Split Pricing Configuration
|
||||
|
||||
If travels are configured with split pricing (e.g., outbound=2.95, inbound=2.95) before BusPro supports it:
|
||||
|
||||
| Scenario | Portal Charges | BusPro Charges | Intended |
|
||||
|----------|----------------|----------------|----------|
|
||||
| BUS+BUS | 2.95 | 2.95 | 5.90 |
|
||||
| BUS+PKW | 2.95 | 2.95 | 2.95 |
|
||||
| PKW+BUS | 0.00 | 0.00 | 2.95 |
|
||||
|
||||
**Recommendation**: Keep full price on outbound (`preis="5,90"`) until BusPro supports split pricing, then reconfigure to true split values.
|
||||
|
||||
## Related Code Locations
|
||||
|
||||
- `src/BusProNet/Model/Pickup.php` - Pickup model with split pricing properties and calculation methods
|
||||
- `src/BusProNet/XmlParser/TravelParser.php` - Parses and merges pickup prices from XML
|
||||
- `src/BusProNet/DataProcessor/BookingDataProcessor.php` - Enriches pickups with travel data
|
||||
- `src/BusProNet/DataProcessor/BookingPayloadBuilder.php` - Pickup submission
|
||||
- `src/BusProNet/Model/Booking.php` - `getPickupForParticipant()` method
|
||||
- `src/Form/Service/EditFieldStateProvider.php` - Pickup field visibility
|
||||
- `src/Form/Service/CreateFieldStateProvider.php` - Pickup field visibility
|
||||
- `src/Service/ParticipantPricingCalculator.php` - Pickup pricing logic (individual)
|
||||
- `src/Service/ServicePricingCalculator.php` - Pickup pricing aggregation (summary)
|
||||
|
||||
## Test Scenarios
|
||||
|
||||
1. **BUS + BUS**: Charges `pickup->price` (outbound)
|
||||
2. **BUS + PKW**: Charges `pickup->price` (outbound)
|
||||
3. **PKW + BUS**: Charges €0 (matches BusPro loophole)
|
||||
4. **PKW + PKW**: No pickup available
|
||||
|
||||
---
|
||||
|
||||
*Last updated: 2026-01-21*
|
||||
@@ -233,10 +233,20 @@ class BookingDataProcessor
|
||||
$participant->transportationInbound = $travel->transportationServices[$participant->transportationInbound->id];
|
||||
}
|
||||
|
||||
// Enrich pickup (only outbound - API limitation: pickups are only supported when
|
||||
// outbound transportation is bus; inbound-only bus bookings cannot have pickups)
|
||||
// Enrich pickup with data from travel (includes both outbound and inbound prices)
|
||||
// The outbound pickups already have inbound prices merged in from TravelParser
|
||||
if (null !== $participant->pickup && isset($travel->pickupsOutbound[$participant->pickup->id])) {
|
||||
$participant->pickup = $travel->pickupsOutbound[$participant->pickup->id];
|
||||
$enrichedPickup = $travel->pickupsOutbound[$participant->pickup->id];
|
||||
|
||||
// Copy all properties from enriched pickup to participant's pickup
|
||||
$participant->pickup->price = $enrichedPickup->price;
|
||||
$participant->pickup->priceOutbound = $enrichedPickup->priceOutbound;
|
||||
$participant->pickup->priceInbound = $enrichedPickup->priceInbound;
|
||||
$participant->pickup->time = $enrichedPickup->time;
|
||||
$participant->pickup->city = $enrichedPickup->city;
|
||||
$participant->pickup->street = $enrichedPickup->street;
|
||||
$participant->pickup->postalCode = $enrichedPickup->postalCode;
|
||||
$participant->pickup->code = $enrichedPickup->code;
|
||||
}
|
||||
|
||||
// Enrich insurance
|
||||
|
||||
@@ -39,6 +39,12 @@ class Pickup
|
||||
#[Groups(['api:single'])]
|
||||
public ?float $price = null;
|
||||
|
||||
#[Groups(['api:single'])]
|
||||
public ?float $priceOutbound = null;
|
||||
|
||||
#[Groups(['api:single'])]
|
||||
public ?float $priceInbound = null;
|
||||
|
||||
#[Groups(['api:booking'])]
|
||||
public array $mapping = [];
|
||||
|
||||
@@ -90,4 +96,75 @@ class Pickup
|
||||
|
||||
return sprintf('%s (€%s)', $label, number_format($this->price, 2, ',', '.'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the formatted label with context-aware pricing based on transportation directions.
|
||||
*
|
||||
* Calculates the effective price based on which directions use bus transportation:
|
||||
* - Both directions BUS: shows combined price (outbound + inbound)
|
||||
* - Only outbound BUS: shows outbound price only
|
||||
* - Only inbound BUS: shows inbound price only
|
||||
* - Falls back to legacy price property when split pricing data is unavailable
|
||||
*
|
||||
* @param bool $hasOutboundBus Whether outbound transportation is bus
|
||||
* @param bool $hasInboundBus Whether inbound transportation is bus
|
||||
*
|
||||
* @return string The formatted pickup label with contextual pricing
|
||||
*/
|
||||
public function getLabelWithContextualPrice(bool $hasOutboundBus, bool $hasInboundBus): string
|
||||
{
|
||||
$label = $this->getLabel();
|
||||
$effectivePrice = $this->calculateEffectivePrice($hasOutboundBus, $hasInboundBus);
|
||||
|
||||
if (null === $effectivePrice) {
|
||||
return $label;
|
||||
}
|
||||
|
||||
if (0.0 === $effectivePrice) {
|
||||
return sprintf('%s (inkl.)', $label);
|
||||
}
|
||||
|
||||
if ($effectivePrice < 0) {
|
||||
return sprintf('%s (-%s€ Rabatt)', $label, number_format(abs($effectivePrice), 2, ',', '.'));
|
||||
}
|
||||
|
||||
return sprintf('%s (€%s)', $label, number_format($effectivePrice, 2, ',', '.'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculates the effective pickup price based on transportation directions.
|
||||
*
|
||||
* Uses split pricing (priceOutbound/priceInbound) when available, falling back
|
||||
* to the legacy price property for backward compatibility with older data.
|
||||
*
|
||||
* @param bool $hasOutboundBus Whether outbound transportation is bus
|
||||
* @param bool $hasInboundBus Whether inbound transportation is bus
|
||||
*
|
||||
* @return float|null The calculated effective price or null if no applicable price
|
||||
*/
|
||||
public function calculateEffectivePrice(bool $hasOutboundBus, bool $hasInboundBus): ?float
|
||||
{
|
||||
// Neither direction is bus - no pickup price applies
|
||||
if (false === $hasOutboundBus && false === $hasInboundBus) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$effectivePrice = 0.0;
|
||||
$hasSplitPricing = null !== $this->priceOutbound || null !== $this->priceInbound;
|
||||
|
||||
if ($hasOutboundBus) {
|
||||
if ($hasSplitPricing) {
|
||||
$effectivePrice += $this->priceOutbound ?? 0.0;
|
||||
} else {
|
||||
// Fallback to legacy price for backward compatibility
|
||||
$effectivePrice += $this->price ?? 0.0;
|
||||
}
|
||||
}
|
||||
|
||||
if ($hasInboundBus && null !== $this->priceInbound) {
|
||||
$effectivePrice += $this->priceInbound;
|
||||
}
|
||||
|
||||
return $effectivePrice;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,8 +76,12 @@ class TravelParser extends AbstractParser
|
||||
$travel->transportationServices = $this
|
||||
->getTransportationServices($node->filterXPath('//lei_befoerderung/leistung'));
|
||||
$travel->rooms = $this->getRooms($hotelNode);
|
||||
$travel->pickupsOutbound = $this->getPickups($node->filterXPath('//zustiege/zustieg'), $dateFrom);
|
||||
$travel->pickupsInbound = $this->getPickups($node->filterXPath('//zustiege_rueck/zustieg_rueck'), $dateTo);
|
||||
$travel->pickupsOutbound = $this->getPickups($node->filterXPath('//zustiege/zustieg'), $dateFrom, true);
|
||||
$travel->pickupsInbound = $this->getPickups($node->filterXPath('//zustiege_rueck/zustieg_rueck'), $dateTo, false);
|
||||
|
||||
// Merge inbound prices into outbound pickups for split pricing support
|
||||
$this->mergeInboundPricesIntoOutboundPickups($travel->pickupsOutbound, $travel->pickupsInbound);
|
||||
|
||||
$travel->guide = $this->getGuide($node);
|
||||
|
||||
return $travel;
|
||||
@@ -256,14 +260,15 @@ class TravelParser extends AbstractParser
|
||||
*
|
||||
* @param Crawler $node The XML node containing pickup data
|
||||
* @param \DateTimeImmutable|null $defaultDate Default date for time parsing (outbound only)
|
||||
* @param bool $isOutbound Whether these are outbound pickups (for split pricing)
|
||||
*
|
||||
* @return array<int, Pickup> Array of pickup locations indexed by ID
|
||||
*/
|
||||
public function getPickups(Crawler $node, ?\DateTimeImmutable $defaultDate = null): array
|
||||
public function getPickups(Crawler $node, ?\DateTimeImmutable $defaultDate = null, bool $isOutbound = true): array
|
||||
{
|
||||
$pickups = [];
|
||||
|
||||
$node->each(function (Crawler $pickupNode) use (&$pickups, $defaultDate) {
|
||||
$node->each(function (Crawler $pickupNode) use (&$pickups, $defaultDate, $isOutbound) {
|
||||
$pickupId = (int) $pickupNode->attr('idbuspro');
|
||||
|
||||
$pickup = new Pickup();
|
||||
@@ -271,6 +276,13 @@ class TravelParser extends AbstractParser
|
||||
$pickup->price = $pickupNode->attr('preis') ?
|
||||
$this->stringToFloat($pickupNode->attr('preis')) : null;
|
||||
|
||||
// Set direction-specific price for split pricing support
|
||||
if ($isOutbound) {
|
||||
$pickup->priceOutbound = $pickup->price;
|
||||
} else {
|
||||
$pickup->priceInbound = $pickup->price;
|
||||
}
|
||||
|
||||
// parse date and time only for direction 'to' indicated by provided default date
|
||||
if (null !== $defaultDate) {
|
||||
$pickup->time = $this->stringToDateTimeFuzzy($pickupNode->attr('zeit'), $defaultDate);
|
||||
@@ -408,4 +420,24 @@ class TravelParser extends AbstractParser
|
||||
$service->rawAgeConstraintData = $constraintData;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges inbound pickup prices into outbound pickups for split pricing support.
|
||||
*
|
||||
* For each outbound pickup, finds the matching inbound pickup by ID and copies
|
||||
* the inbound price to the outbound pickup's priceInbound property. This enables
|
||||
* split pricing calculations where pickup costs are distributed between outbound
|
||||
* and inbound transportation.
|
||||
*
|
||||
* @param array<int, Pickup> $pickupsOutbound Outbound pickups indexed by ID (modified in place)
|
||||
* @param array<int, Pickup> $pickupsInbound Inbound pickups indexed by ID
|
||||
*/
|
||||
private function mergeInboundPricesIntoOutboundPickups(array &$pickupsOutbound, array $pickupsInbound): void
|
||||
{
|
||||
foreach ($pickupsOutbound as $pickupId => $outboundPickup) {
|
||||
if (isset($pickupsInbound[$pickupId])) {
|
||||
$outboundPickup->priceInbound = $pickupsInbound[$pickupId]->priceInbound;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -230,8 +230,9 @@ class ParticipantPricingCalculator
|
||||
}
|
||||
}
|
||||
|
||||
// Pickup pricing - only charged when outbound transportation is bus
|
||||
// BusProNet API ignores pickup surcharges for self-organized (PKW) outbound
|
||||
// Pickup pricing - only charged when outbound transportation is BUS
|
||||
// This matches current BusPro behavior where pickup price is only applied for outbound bus
|
||||
// Note: priceOutbound/priceInbound are populated for future split pricing support when BusPro is updated
|
||||
$hasOutboundBus = null !== $participant->transportationOutbound
|
||||
&& 'BUS' === $participant->transportationOutbound->subType;
|
||||
|
||||
|
||||
@@ -129,8 +129,9 @@ class ServicePricingCalculator
|
||||
}
|
||||
}
|
||||
|
||||
// Pickup pricing - only charged when outbound transportation is bus
|
||||
// BusProNet API ignores pickup surcharges for self-organized (PKW) outbound
|
||||
// Pickup pricing - only charged when outbound transportation is BUS
|
||||
// This matches current BusPro behavior where pickup price is only applied for outbound bus
|
||||
// Note: priceOutbound/priceInbound are populated for future split pricing support when BusPro is updated
|
||||
$hasOutboundBus = null !== $participant->transportationOutbound
|
||||
&& 'BUS' === $participant->transportationOutbound->subType;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user