fix: don't restore selections from draft for immutable fields
This commit is contained in:
@@ -331,6 +331,10 @@ class BookingEditDraftService
|
||||
*/
|
||||
private function applyServiceSelections(ParticipantDto $participant, array $data, Travel $travel): void
|
||||
{
|
||||
// Additional services category — only apply draft data when services are mutable.
|
||||
// When immutable, the booking's current services must be preserved as-is to avoid
|
||||
// API rejection (stale draft data could differ from the locked booking state).
|
||||
if (true === $travel->additionalServicesMutable) {
|
||||
// Ski pass (single service) - merge strategy: only apply if resolves to valid service
|
||||
if (true === array_key_exists('skiPass', $data) && null !== $data['skiPass']) {
|
||||
$draftSkiPassId = $data['skiPass'];
|
||||
@@ -365,7 +369,6 @@ class BookingEditDraftService
|
||||
}
|
||||
}
|
||||
|
||||
// Additional services (array) - overwrite strategy with mandatory service preservation
|
||||
// Additional services (array) - overwrite strategy with mandatory service preservation
|
||||
// User can deselect optional services, but mandatory services from API must be preserved
|
||||
if (true === array_key_exists('additionalServices', $data) && true === is_array($data['additionalServices'])) {
|
||||
@@ -376,7 +379,10 @@ class BookingEditDraftService
|
||||
$travel
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Transportation category — only apply draft data when transportation is mutable
|
||||
if (true === $travel->transportationServicesMutable) {
|
||||
// Transportation outbound (single) - merge strategy: only apply if resolves to valid service
|
||||
if (true === array_key_exists('transportationOutbound', $data) && null !== $data['transportationOutbound']) {
|
||||
$resolved = $this->resolveService($data['transportationOutbound'], $travel->transportationServices);
|
||||
@@ -393,6 +399,14 @@ class BookingEditDraftService
|
||||
}
|
||||
}
|
||||
|
||||
// Parking (boolean) - overwrite strategy: user can uncheck
|
||||
if (true === array_key_exists('parking', $data)) {
|
||||
$participant->parking = (bool) $data['parking'];
|
||||
}
|
||||
}
|
||||
|
||||
// Pickups category — only apply draft data when pickups are mutable
|
||||
if (true === $travel->pickupsMutable) {
|
||||
// Pickup (single) - merge strategy: only apply if resolves to valid pickup
|
||||
if (true === array_key_exists('pickup', $data) && null !== $data['pickup']) {
|
||||
$resolved = $this->resolvePickup($data['pickup'], $travel);
|
||||
@@ -409,13 +423,9 @@ class BookingEditDraftService
|
||||
$participant->differentDropOff = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Parking (boolean) - overwrite strategy: user can uncheck
|
||||
if (true === array_key_exists('parking', $data)) {
|
||||
$participant->parking = (bool) $data['parking'];
|
||||
}
|
||||
|
||||
// Insurance (single) - merge strategy: only apply if resolves to valid insurance
|
||||
// Insurance (not governed by mutability categories — always apply from draft)
|
||||
if (true === array_key_exists('insurance', $data) && null !== $data['insurance']) {
|
||||
$resolved = $this->resolveInsurance($data['insurance'], $travel);
|
||||
if (null !== $resolved) {
|
||||
|
||||
@@ -0,0 +1,448 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service;
|
||||
|
||||
use App\BusProNet\Model\Insurance;
|
||||
use App\BusProNet\Model\Pickup;
|
||||
use App\BusProNet\Model\Service;
|
||||
use App\BusProNet\Model\Travel;
|
||||
use App\Entity\BookingEditDraft;
|
||||
use App\Entity\User;
|
||||
use App\Form\Model\BookingDto;
|
||||
use App\Form\Model\ParticipantDto;
|
||||
use App\Repository\BookingEditDraftRepository;
|
||||
use App\Service\BookingEditDraftService;
|
||||
use App\Service\BookingFingerprintService;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Psr\Log\NullLogger;
|
||||
|
||||
/**
|
||||
* Tests that draft restoration respects travel mutability flags.
|
||||
*
|
||||
* When services are immutable, draft data must NOT overwrite the fresh API
|
||||
* service selections on the DTO. This prevents stale draft data from causing
|
||||
* BusPro API rejections when submitting booking updates.
|
||||
*/
|
||||
class BookingEditDraftServiceMutabilityTest extends TestCase
|
||||
{
|
||||
private BookingEditDraftService $service;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
$this->service = new BookingEditDraftService(
|
||||
$this->createMock(BookingEditDraftRepository::class),
|
||||
$this->createMock(EntityManagerInterface::class),
|
||||
$this->createMock(BookingFingerprintService::class),
|
||||
new NullLogger(),
|
||||
);
|
||||
}
|
||||
|
||||
// --- Additional Services Mutability ---
|
||||
|
||||
public function testDraftSkipsAdditionalServicesWhenImmutable(): void
|
||||
{
|
||||
$originalService = $this->createService(10, 'Original');
|
||||
$participant = new ParticipantDto();
|
||||
$participant->additionalServices = [$originalService];
|
||||
$participant->courses = [];
|
||||
$participant->board = [];
|
||||
$participant->rentals = [];
|
||||
$participant->skiPass = null;
|
||||
$participant->rentalInsurance = null;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
additionalServicesMutable: false,
|
||||
additionalServices: [10 => $originalService, 99 => $this->createService(99, 'Draft Added')],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'additionalServices' => [99],
|
||||
'courses' => [99],
|
||||
'board' => [99],
|
||||
'rentals' => [99],
|
||||
'skiPass' => 99,
|
||||
'rentalInsurance' => 99,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
// All additional services category fields must remain unchanged
|
||||
$this->assertSame([$originalService], $participant->additionalServices);
|
||||
$this->assertSame([], $participant->courses);
|
||||
$this->assertSame([], $participant->board);
|
||||
$this->assertSame([], $participant->rentals);
|
||||
$this->assertNull($participant->skiPass);
|
||||
$this->assertNull($participant->rentalInsurance);
|
||||
}
|
||||
|
||||
public function testDraftAppliesAdditionalServicesWhenMutable(): void
|
||||
{
|
||||
$originalService = $this->createService(10, 'Original');
|
||||
$draftService = $this->createService(99, 'Draft Added');
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->additionalServices = [$originalService];
|
||||
$participant->courses = [];
|
||||
$participant->skiPass = null;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
additionalServicesMutable: true,
|
||||
additionalServices: [10 => $originalService, 99 => $draftService],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'additionalServices' => [99],
|
||||
'courses' => [99],
|
||||
'skiPass' => 99,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
// Draft values should be applied
|
||||
$this->assertCount(1, $participant->additionalServices);
|
||||
$this->assertSame(99, $participant->additionalServices[0]->id);
|
||||
$this->assertCount(1, $participant->courses);
|
||||
$this->assertSame(99, $participant->courses[0]->id);
|
||||
$this->assertNotNull($participant->skiPass);
|
||||
$this->assertSame(99, $participant->skiPass->id);
|
||||
}
|
||||
|
||||
// --- Transportation Mutability ---
|
||||
|
||||
public function testDraftSkipsTransportationWhenImmutable(): void
|
||||
{
|
||||
$originalOutbound = $this->createService(20, 'Outbound', isTransportation: true);
|
||||
$originalInbound = $this->createService(21, 'Inbound', isTransportation: true);
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->transportationOutbound = $originalOutbound;
|
||||
$participant->transportationInbound = $originalInbound;
|
||||
$participant->parking = false;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
transportationServicesMutable: false,
|
||||
transportationServices: [
|
||||
20 => $originalOutbound,
|
||||
21 => $originalInbound,
|
||||
88 => $this->createService(88, 'Draft Transport', isTransportation: true),
|
||||
],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'transportationOutbound' => 88,
|
||||
'transportationInbound' => 88,
|
||||
'parking' => true,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
// Transportation fields must remain unchanged
|
||||
$this->assertSame($originalOutbound, $participant->transportationOutbound);
|
||||
$this->assertSame($originalInbound, $participant->transportationInbound);
|
||||
$this->assertFalse($participant->parking);
|
||||
}
|
||||
|
||||
public function testDraftAppliesTransportationWhenMutable(): void
|
||||
{
|
||||
$originalOutbound = $this->createService(20, 'Outbound', isTransportation: true);
|
||||
$draftTransport = $this->createService(88, 'Draft Transport', isTransportation: true);
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->transportationOutbound = $originalOutbound;
|
||||
$participant->parking = false;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
transportationServicesMutable: true,
|
||||
transportationServices: [20 => $originalOutbound, 88 => $draftTransport],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'transportationOutbound' => 88,
|
||||
'parking' => true,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
$this->assertSame($draftTransport, $participant->transportationOutbound);
|
||||
$this->assertTrue($participant->parking);
|
||||
}
|
||||
|
||||
// --- Pickups Mutability ---
|
||||
|
||||
public function testDraftSkipsPickupsWhenImmutable(): void
|
||||
{
|
||||
$originalPickup = $this->createPickup(30, 'Original Pickup');
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->pickup = $originalPickup;
|
||||
$participant->dropOff = null;
|
||||
$participant->differentDropOff = false;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
pickupsMutable: false,
|
||||
pickups: [30 => $originalPickup, 77 => $this->createPickup(77, 'Draft Pickup')],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'pickup' => 77,
|
||||
'dropOff' => 77,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
// Pickup fields must remain unchanged
|
||||
$this->assertSame($originalPickup, $participant->pickup);
|
||||
$this->assertNull($participant->dropOff);
|
||||
$this->assertFalse($participant->differentDropOff);
|
||||
}
|
||||
|
||||
public function testDraftAppliesPickupsWhenMutable(): void
|
||||
{
|
||||
$draftPickup = $this->createPickup(77, 'Draft Pickup');
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->pickup = null;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
pickupsMutable: true,
|
||||
pickups: [77 => $draftPickup],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'pickup' => 77,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
$this->assertSame($draftPickup, $participant->pickup);
|
||||
}
|
||||
|
||||
// --- Insurance (always applied, not governed by mutability) ---
|
||||
|
||||
public function testDraftAlwaysAppliesInsuranceRegardlessOfMutability(): void
|
||||
{
|
||||
$draftInsurance = $this->createInsurance('INS-50');
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->insurance = null;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
additionalServicesMutable: false,
|
||||
transportationServicesMutable: false,
|
||||
pickupsMutable: false,
|
||||
insurances: ['INS-50' => $draftInsurance],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'insurance' => 'INS-50',
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
$this->assertSame($draftInsurance, $participant->insurance);
|
||||
}
|
||||
|
||||
// --- Mixed mutability: only immutable categories are protected ---
|
||||
|
||||
public function testMixedMutabilityProtectsOnlyImmutableCategories(): void
|
||||
{
|
||||
$originalAdditional = $this->createService(10, 'Original Additional');
|
||||
$draftTransport = $this->createService(88, 'Draft Transport', isTransportation: true);
|
||||
$draftPickup = $this->createPickup(77, 'Draft Pickup');
|
||||
|
||||
$participant = new ParticipantDto();
|
||||
$participant->additionalServices = [$originalAdditional];
|
||||
$participant->transportationOutbound = null;
|
||||
$participant->pickup = null;
|
||||
|
||||
$travel = $this->createTravel(
|
||||
additionalServicesMutable: false, // immutable
|
||||
transportationServicesMutable: true, // mutable
|
||||
pickupsMutable: true, // mutable
|
||||
additionalServices: [10 => $originalAdditional, 99 => $this->createService(99, 'Draft')],
|
||||
transportationServices: [88 => $draftTransport],
|
||||
pickups: [77 => $draftPickup],
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'services' => [
|
||||
'additionalServices' => [99],
|
||||
'transportationOutbound' => 88,
|
||||
'pickup' => 77,
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
// Additional services: immutable → unchanged
|
||||
$this->assertSame([$originalAdditional], $participant->additionalServices);
|
||||
|
||||
// Transportation: mutable → draft applied
|
||||
$this->assertSame($draftTransport, $participant->transportationOutbound);
|
||||
|
||||
// Pickups: mutable → draft applied
|
||||
$this->assertSame($draftPickup, $participant->pickup);
|
||||
}
|
||||
|
||||
// --- Personal data is always applied regardless of mutability ---
|
||||
|
||||
public function testDraftAlwaysAppliesPersonalDataRegardlessOfMutability(): void
|
||||
{
|
||||
$participant = new ParticipantDto();
|
||||
$participant->firstName = 'Original';
|
||||
|
||||
$travel = $this->createTravel(
|
||||
additionalServicesMutable: false,
|
||||
transportationServicesMutable: false,
|
||||
pickupsMutable: false,
|
||||
);
|
||||
|
||||
$dto = $this->createBookingDto($travel, [$participant]);
|
||||
|
||||
$draft = $this->createDraft([
|
||||
'participants' => [
|
||||
0 => [
|
||||
'personalData' => [
|
||||
'firstName' => 'Updated',
|
||||
],
|
||||
],
|
||||
],
|
||||
]);
|
||||
|
||||
$this->service->applyDraftToDto($draft, $dto, $travel);
|
||||
|
||||
$this->assertSame('Updated', $participant->firstName);
|
||||
}
|
||||
|
||||
// --- Helpers ---
|
||||
|
||||
private function createService(int $id, string $label = 'Test', bool $isTransportation = false): Service
|
||||
{
|
||||
$service = new Service();
|
||||
$service->id = $id;
|
||||
$service->label = $label;
|
||||
|
||||
return $service;
|
||||
}
|
||||
|
||||
private function createPickup(int $id, string $label = 'Test Pickup'): Pickup
|
||||
{
|
||||
$pickup = new Pickup();
|
||||
$pickup->id = $id;
|
||||
$pickup->city = $label;
|
||||
|
||||
return $pickup;
|
||||
}
|
||||
|
||||
private function createInsurance(string $id): Insurance
|
||||
{
|
||||
$insurance = new Insurance();
|
||||
$insurance->id = $id;
|
||||
$insurance->label = 'Test Insurance';
|
||||
|
||||
return $insurance;
|
||||
}
|
||||
|
||||
private function createTravel(
|
||||
bool $additionalServicesMutable = true,
|
||||
bool $transportationServicesMutable = true,
|
||||
bool $pickupsMutable = true,
|
||||
array $additionalServices = [],
|
||||
array $transportationServices = [],
|
||||
array $pickups = [],
|
||||
array $dropOffs = [],
|
||||
array $insurances = [],
|
||||
): Travel {
|
||||
$travel = new Travel();
|
||||
$travel->additionalServicesMutable = $additionalServicesMutable;
|
||||
$travel->transportationServicesMutable = $transportationServicesMutable;
|
||||
$travel->pickupsMutable = $pickupsMutable;
|
||||
$travel->additionalServices = $additionalServices;
|
||||
$travel->transportationServices = $transportationServices;
|
||||
$travel->pickups = $pickups;
|
||||
$travel->dropOffs = $dropOffs;
|
||||
$travel->insurances = $insurances;
|
||||
|
||||
return $travel;
|
||||
}
|
||||
|
||||
private function createBookingDto(Travel $travel, array $participants): BookingDto
|
||||
{
|
||||
$dto = new BookingDto($travel, 1);
|
||||
$dto->participants = $participants;
|
||||
|
||||
return $dto;
|
||||
}
|
||||
|
||||
private function createDraft(array $formData): BookingEditDraft
|
||||
{
|
||||
$user = $this->createMock(User::class);
|
||||
|
||||
return new BookingEditDraft($user, 123, new \DateTimeImmutable(), $formData);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user