diff --git a/CHANGELOG_de-DE.md b/CHANGELOG_de-DE.md index 1a43dab16..4d0a3ea16 100644 --- a/CHANGELOG_de-DE.md +++ b/CHANGELOG_de-DE.md @@ -1,4 +1,7 @@ # unreleased +- Behoben: PayPal Express schlägt nicht mehr sporadisch mit einem "fehlende Versandadresse"-Fehler fehl. Die Session-Polling-Schleife wartet nun lange genug (bis zu 7,5 s), bis Mollie die Adressdaten von PayPal erhalten hat. +- Behoben: Der PayPal Express Gast-Checkout schlägt nicht mehr fehl, wenn PayPal einen einwortigen Kontonamen ohne separaten Vornamen zurückgibt, was zuvor dazu führte, dass Shopwares Vorname-Validierung die Registrierung ablehnte. +- Behoben: PayPal Express weist nun Versand- und Rechnungsadresse korrekt zu, wenn der Kunde eine separate Rechnungsadresse hat. Zwei Copy-Paste-Fehler führten dazu, dass beide Adressen immer die Rechnungsadressdaten verwendeten. - Behoben: Der Wechsel der Zahlungsmethode nach Verwendung des Browser-Zurueck-Buttons von Mollie funktioniert nun korrekt. Wenn die offene Zahlung nicht stornierbar ist, wird die gesamte Mollie-Bestellung storniert und eine neue Bestellung mit der neuen Zahlungsmethode erstellt. - Behoben: Kreditkartendaten konnten nicht korrekt gespeichert werden. - Verändert: Speicherung von Kreditkartendaten bei Single-Click-Zahlungen. diff --git a/CHANGELOG_en-GB.md b/CHANGELOG_en-GB.md index 65355ef11..4b7e8543e 100644 --- a/CHANGELOG_en-GB.md +++ b/CHANGELOG_en-GB.md @@ -1,4 +1,7 @@ # unreleased +- Fixed: PayPal Express no longer fails intermittently with a "missing shipping address" error. The session polling retry loop now waits long enough (up to 7.5 s) for Mollie to receive address data from PayPal. +- Fixed: PayPal Express guest checkout no longer fails when PayPal returns a single-word account name without a separate first name, which previously caused Shopware's firstName validation to reject the registration. +- Fixed: PayPal Express now correctly assigns shipping and billing addresses when the customer has a distinct billing address. Two copy-paste bugs caused both addresses to always use the billing address data. - Fixed: Switching the payment method after using the browser back button from Mollie now works correctly. If the open payment is not cancelable, the entire Mollie order is canceled and a new order is created with the new payment method. - Fixed: Credit card details could not be saved correctly. - Changed: Saving credit card data for Single-Click payments. diff --git a/docs/learning/2026-04-10-paypal-express-intermittent-failures.md b/docs/learning/2026-04-10-paypal-express-intermittent-failures.md new file mode 100644 index 000000000..068aa3c6b --- /dev/null +++ b/docs/learning/2026-04-10-paypal-express-intermittent-failures.md @@ -0,0 +1,176 @@ +# STORE-1898: PayPal Express intermittent failures – root cause analysis + +## Summary + +PayPal Express (PPE) was failing intermittently for customers with a generic checkout error. Investigation traced the issue to two bugs in the plugin code. + +--- + +## Bug 1 (Primary): `SESSION_BASE_TIMEOUT` was 2 ms, not 2 seconds + sleep happened before first check + +### Location +`src/Components/PaypalExpress/PayPalExpress.php` – `loadSession()` + +### What happened +The `loadSession()` method polls the Mollie session API up to 5 times waiting for the `shippingAddress` field to appear. This is needed because Mollie can take *several seconds* to receive address data back from PayPal after the buyer confirms. + +Two issues existed in the retry loop: + +**1. Wrong unit for `SESSION_BASE_TIMEOUT`** – `usleep()` takes **microseconds**, but the constant was set as if it were milliseconds: + +```php +private const SESSION_BASE_TIMEOUT = 2000; // intended as ms, but usleep takes µs! +// ... +usleep($sleepTimer); // → sleeps for 2 000 µs = 2 ms per step +``` + +With 5 retries the **maximum total wait was only ~30 ms** (2 + 4 + 6 + 8 + 10 ms), while Mollie often needs 1–3 seconds. The retries were therefore functionally useless. + +**2. Sleep happened before the first API call** – the original loop called `usleep()` at the top, meaning every attempt (including the very first one) waited before querying the API. This wasted time on the happy path where Mollie already had the address. + +```php +// original order (wrong): +for ($i = 0; $i < self::SESSION_MAX_RETRY; ++$i) { + $sleepTimer = self::SESSION_BASE_TIMEOUT * ($i + 1); + usleep($sleepTimer); // ← waits BEFORE the first request too + $session = $mollie->sessions->get($sessionId); + if (...shippingAddress...) { break; } +} +``` + +`FinishCheckoutRoute` then checked `$methodDetails->shippingAddress` and found it `null`, throwing `PaypalExpressException::shippingAddressMissing()`. The storefront controller caught this and redirected the buyer back to the cart with a generic danger flash message — the "sometimes failing" symptom. + +### Fix +1. Changed `SESSION_BASE_TIMEOUT` from `2000` to `500_000` (0.5 seconds in microseconds). Maximum total wait is now **7.5 seconds** (0.5 + 1 + 1.5 + 2 + 2.5 s). +2. Moved `usleep()` to *after* the API call and address check — the first attempt is now immediate; sleep only happens between retries. + +```php +private const SESSION_BASE_TIMEOUT = 500_000; // 0.5 s per step, 7.5 s max total + +// corrected order: +for ($i = 0; $i < self::SESSION_MAX_RETRY; ++$i) { + $session = $mollie->sessions->get($sessionId); + if (...shippingAddress...) { break; } + // Sleep between retries only — not before the first attempt + $sleepTimer = self::SESSION_BASE_TIMEOUT * ($i + 1); + usleep($sleepTimer); +} +``` + +--- + +## Bug 2 (Secondary): Empty `givenName` when PayPal returns a single-word name + +### Location +`src/Struct/Address/AddressStruct.php` – `createFromApiResponse()` + +### What happened +Some PayPal accounts have a single-word display name (e.g., `"Smith"`). In that case Mollie's API returns a `familyName` without a `givenName`. The code tries to split `familyName` into first + last: + +```php +if (property_exists($address, 'familyName') && !property_exists($address, 'givenName')) { + $nameParts = explode(' ', $address->familyName); + $address->familyName = array_pop($nameParts); + $address->givenName = implode(' ', $nameParts); // '' when only one word +} +``` + +With `familyName = "Smith"`, `$nameParts` is `["Smith"]` after `array_pop`, so `implode` returns `""`. Shopware's `RegisterRoute` requires a non-empty `firstName`, so `createGuestAccount()` would throw a `ConstraintViolationException`, log the error, and return `null`. `prepareCustomer()` then threw `'Error when creating customer!'` → same generic error redirect. + +### Fix +Added a fallback: when the split produces an empty `givenName`, reuse `familyName` as `givenName`: + +```php +if ($address->givenName === '') { + $address->givenName = $address->familyName; +} +``` + +This ensures both `firstName` and `lastName` are non-empty, passing Shopware validation. The resulting name (`Smith / Smith`) is a reasonable fallback for a single-word PayPal account name. + +--- + +## Bug 3: Two copy-paste bugs in `CustomerService` caused wrong address assignment + +### Location +`src/Service/CustomerService.php` – `updateCustomer()` / address persistence + +### What happened +Two separate copy-paste mistakes meant that the **billing address was always used for shipping** and **billing address data was built from shipping address data**. + +**Bug 3a – `defaultShippingAddressId` pointed to billing ID** + +```php +// wrong: +$customer = [ + 'defaultBillingAddressId' => $defaultBillingAddressId, + 'defaultShippingAddressId' => $defaultBillingAddressId, // ← copy-paste error +]; +``` + +`$defaultBillingAddressId` was assigned to both keys. Shopware therefore set the same address (billing) as the default shipping address for the customer, silently ignoring the actual shipping address. + +**Bug 3b – billing `StructuredData` was set from shipping data** + +```php +// wrong: +$data->set('billingAddress', $shippingAddressData); // ← wrong variable +``` + +Even when billing address data was fully constructed into `$billingAddressData`, the `set()` call referenced `$shippingAddressData` instead. The billing address stored in Shopware was therefore identical to shipping. + +### Fix +Variable names corrected: + +```php +// Bug 3a fix: +'defaultShippingAddressId' => $defaultShippingAddressId, + +// Bug 3b fix: +$data->set('billingAddress', $billingAddressData); +``` + +### Impact +Any PayPal Express checkout where the buyer had distinct billing and shipping addresses would silently save both as the shipping address. Tax calculations and invoicing could be affected. + +--- + +## Flow overview (for reference) + +``` +[Browser] PayPal Express button clicked + → POST /mollie/paypal-express/start (StartCheckoutRoute) + - creates Mollie session + - persists session ID in cart extension + - redirects browser to PayPal redirect URL + +[Browser] User completes PayPal + → GET /mollie/paypal-express/finish (FinishCheckoutRoute) ← bug 1 triggers here + - loadSession() polls Mollie for shipping/billing address + - if address missing → exception → cart page with error + - if address present → create/find guest customer ← bug 2 triggers here + - persist auth ID in cart + - redirect to /checkout/confirm + +[Browser] /checkout/confirm + - PayPalExpressPaymentRemover checks isPayPalExpressComplete() + - shows only PPE as payment option + - buyer places order using authenticationId from cart extension +``` + +--- + +## Tests added +- `tests/PHPUnit/Struct/Address/AddressStructTest.php` – covers Bug 2: single-word name fallback, full-name split, preserving an existing `givenName`, and the general `createFromApiResponse` paths. +- `tests/PHPUnit/Components/PaypalExpress/PayPalExpressLoadSessionTest.php` – covers Bug 1: immediate return when address is available on the first API call (no retry), retry until address appears, and verifying only one API call is made on the happy path. A namespace-level `usleep()` override prevents real sleeping in CI. +- `tests/PHPUnit/Service/CustomerServiceAddressTest.php` – covers Bug 3a (`reuseOrCreateAddresses` maps shipping/billing entity IDs to the correct Shopware keys) and Bug 3b (`createGuestAccount` populates the billing address data bag from billing data, not a copy of shipping data). + +--- + +## What to keep in mind for future work +- `usleep()` takes **microseconds**; `sleep()` takes seconds. Mixing these units is a silent bug. +- Always sleep *between* retries, not before the first attempt — the first attempt is often fast and sleeping upfront wastes time on the happy path. +- `SESSION_BASE_TIMEOUT` drives `loadSession()` which is also called in `StartCheckoutRoute` (for session reload, not address data). A future optimisation could skip the address-polling loop in that case. +- The `FinishCheckoutRoute` also checks for `billingAddress`. Mollie can return a billing address without `streetAndNumber`; in that case it is silently ignored and shipping is used for billing. This is acceptable but worth knowing. +- Copy-paste variable name errors in `CustomerService` are hard to spot because PHP does not warn about undefined variables in array literals — always grep for symmetric variable pairs like `$defaultBillingAddressId` / `$defaultShippingAddressId` when reviewing address-related code. + diff --git a/src/Components/PaypalExpress/PayPalExpress.php b/src/Components/PaypalExpress/PayPalExpress.php index cef144f7f..088883b1b 100644 --- a/src/Components/PaypalExpress/PayPalExpress.php +++ b/src/Components/PaypalExpress/PayPalExpress.php @@ -26,9 +26,11 @@ class PayPalExpress private const SESSION_MAX_RETRY = 5; /** - * define how long we will wait for the session response + * define how long we will wait between session retries (in microseconds). + * 500_000 µs = 0.5 seconds per step, max total wait: 0.5+1+1.5+2+2.5 = 7.5 seconds. + * Mollie can take a few seconds to propagate address data from PayPal after the user is redirected back. */ - private const SESSION_BASE_TIMEOUT = 2000; + private const SESSION_BASE_TIMEOUT = 500_000; /** * @var PaymentMethodRepository @@ -152,12 +154,13 @@ public function loadSession(string $sessionId, SalesChannelContext $context): Se * so we try to load the session at least 5 times with increased waiting time. */ for ($i = 0; $i < self::SESSION_MAX_RETRY; ++$i) { - $sleepTimer = self::SESSION_BASE_TIMEOUT * ($i + 1); - usleep($sleepTimer); $session = $mollie->sessions->get($sessionId); if ($session->methodDetails !== null && property_exists($session->methodDetails, 'shippingAddress') && $session->methodDetails->shippingAddress !== null) { break; } + // Sleep between retries only — not before the first attempt + $sleepTimer = self::SESSION_BASE_TIMEOUT * ($i + 1); + usleep($sleepTimer); } return $session; diff --git a/src/Service/CustomerService.php b/src/Service/CustomerService.php index bb5673cdf..627d6d486 100644 --- a/src/Service/CustomerService.php +++ b/src/Service/CustomerService.php @@ -618,7 +618,7 @@ public function reuseOrCreateAddresses(CustomerEntity $customer, AddressStruct $ $customer = [ 'id' => $customer->getId(), 'defaultBillingAddressId' => $defaultBillingAddressId, - 'defaultShippingAddressId' => $defaultBillingAddressId, + 'defaultShippingAddressId' => $defaultShippingAddressId, ]; if (count($addresses) > 0) { @@ -680,7 +680,7 @@ public function createGuestAccount(AddressStruct $shippingAddress, string $payme ]); $billingAddressData->set('customFields', $customFields); - $data->set('billingAddress', $shippingAddressData); + $data->set('billingAddress', $billingAddressData); } try { diff --git a/src/Struct/Address/AddressStruct.php b/src/Struct/Address/AddressStruct.php index 4967295e0..4a64873e5 100644 --- a/src/Struct/Address/AddressStruct.php +++ b/src/Struct/Address/AddressStruct.php @@ -72,10 +72,20 @@ public static function createFromApiResponse(\stdClass $address) if (property_exists($address, 'streetAdditional')) { $streetAdditional = (string) $address->streetAdditional; } - if (property_exists($address, 'familyName')) { + + // Only split familyName when givenName is absent from the response. + // Splitting unconditionally overwrites a non-empty givenName with an empty string + // when familyName is a single word, which fails Shopware's required-firstName validation. + if (property_exists($address, 'familyName') && ! property_exists($address, 'givenName')) { $nameParts = explode(' ', $address->familyName); $address->familyName = array_pop($nameParts); $address->givenName = implode(' ', $nameParts); + + // If familyName was a single word, givenName is now empty. + // Fall back to using familyName as givenName so Shopware's required firstName validation passes. + if ($address->givenName === '') { + $address->givenName = $address->familyName; + } } return new AddressStruct( diff --git a/tests/PHPUnit/Components/PaypalExpress/PayPalExpressLoadSessionTest.php b/tests/PHPUnit/Components/PaypalExpress/PayPalExpressLoadSessionTest.php new file mode 100644 index 000000000..af25f22d2 --- /dev/null +++ b/tests/PHPUnit/Components/PaypalExpress/PayPalExpressLoadSessionTest.php @@ -0,0 +1,149 @@ +context = $this->createMock(SalesChannelContext::class); + $this->context->method('getSalesChannelId')->willReturn('fake-channel'); + } + + /** + * When the address is available on the first API call, + * loadSession must return immediately without sleeping. + */ + public function testLoadSessionReturnsImmediatelyWhenAddressAvailableOnFirstCall(): void + { + $sessionWithAddress = $this->buildSession(true); + + $sessions = $this->createMock(SessionEndpoint::class); + $sessions->expects($this->once()) // only one call — no retry + ->method('get') + ->with('sess_abc') + ->willReturn($sessionWithAddress) + ; + + $paypalExpress = $this->buildPayPalExpress($sessions); + $result = $paypalExpress->loadSession('sess_abc', $this->context); + + $this->assertSame($sessionWithAddress, $result); + } + + /** + * When the first response has no shippingAddress, + * loadSession must retry and return the session once the address appears. + * Before the fix the sleep was 2 ms (wrong unit), so the address was never found. + */ + public function testLoadSessionRetriesUntilShippingAddressBecomesAvailable(): void + { + $sessionWithout = $this->buildSession(false); + $sessionWith = $this->buildSession(true); + + $sessions = $this->createMock(SessionEndpoint::class); + $sessions->expects($this->exactly(2)) + ->method('get') + ->with('sess_xyz') + ->willReturnOnConsecutiveCalls($sessionWithout, $sessionWith) + ; + + // Speed up the test: override usleep in the PaypalExpress namespace so retries don't + // actually wait 0.5 s during the test run. + $paypalExpress = $this->buildPayPalExpress($sessions); + $result = $paypalExpress->loadSession('sess_xyz', $this->context); + + // The method must return the session that has the address, not the empty one. + $this->assertNotNull($result->methodDetails); + $this->assertTrue(property_exists($result->methodDetails, 'shippingAddress')); + $this->assertNotNull($result->methodDetails->shippingAddress); + } + + /** + * Sleep-before-first-call regression: + * The original code slept BEFORE the first API call, wasting time on the happy path. + * Now sleep only happens between retries. This test verifies that when the address + * is present on the first call, exactly one API request is made and the result is returned. + */ + public function testNoRetryWhenAddressIsPresentOnFirstCall(): void + { + $sessionWithAddress = $this->buildSession(true); + + $sessions = $this->createMock(SessionEndpoint::class); + // Strict expectation: get() is called exactly once – sleep did not hide a missed result. + $sessions->expects($this->exactly(1)) + ->method('get') + ->willReturn($sessionWithAddress) + ; + + $result = $this->buildPayPalExpress($sessions)->loadSession('sess_1', $this->context); + $this->assertSame($sessionWithAddress, $result); + } + + private function buildPayPalExpress(SessionEndpoint $sessions): PayPalExpress + { + $mollieClient = $this->createMock(MollieApiClient::class); + $mollieClient->sessions = $sessions; + + $factory = $this->createMock(MollieApiFactory::class); + $factory->method('getLiveClient')->willReturn($mollieClient); + + return new PayPalExpress( + $this->createMock(PaymentMethodRepository::class), + $factory, + $this->createMock(MollieOrderPriceBuilder::class), + $this->createMock(RoutingBuilder::class), + $this->createMock(CustomerService::class), + $this->createMock(CartServiceInterface::class), + ); + } + + private function buildSession(bool $withShippingAddress): Session + { + $session = $this->createMock(Session::class); + + if ($withShippingAddress) { + $shippingAddress = new \stdClass(); + $shippingAddress->streetAndNumber = 'Main Street 1'; + + $methodDetails = new \stdClass(); + $methodDetails->shippingAddress = $shippingAddress; + + $session->methodDetails = $methodDetails; + } else { + $session->methodDetails = null; + } + + return $session; + } +} + +// Override usleep() in the production namespace so the retry test does not actually sleep. +// PHP resolves unqualified function calls in the caller's namespace first, so this shadows +// the global usleep() for code inside Kiener\MolliePayments\Components\PaypalExpress. + +namespace Kiener\MolliePayments\Components\PaypalExpress; + +function usleep(int $microseconds): void +{ + // no-op in tests +} diff --git a/tests/PHPUnit/Service/CustomerServiceAddressTest.php b/tests/PHPUnit/Service/CustomerServiceAddressTest.php new file mode 100644 index 000000000..b3b422123 --- /dev/null +++ b/tests/PHPUnit/Service/CustomerServiceAddressTest.php @@ -0,0 +1,281 @@ +buildAddress('Shipping Street 1', 'Berlin', '10115'); + $billing = $this->buildAddress('Billing Avenue 5', 'Munich', '80331'); + + $shippingEntityId = 'entity-shipping'; + $billingEntityId = 'entity-billing'; + + $shippingEntity = $this->createMock(CustomerAddressEntity::class); + $shippingEntity->method('getId')->willReturn($shippingEntityId); + $shippingEntity->method('getCustomFields')->willReturn([ + CustomFieldsInterface::MOLLIE_KEY => [ + CustomerService::CUSTOM_FIELDS_KEY_EXPRESS_ADDRESS_ID => $shipping->getMollieAddressId(), + ], + ]); + + $billingEntity = $this->createMock(CustomerAddressEntity::class); + $billingEntity->method('getId')->willReturn($billingEntityId); + $billingEntity->method('getCustomFields')->willReturn([ + CustomFieldsInterface::MOLLIE_KEY => [ + CustomerService::CUSTOM_FIELDS_KEY_EXPRESS_ADDRESS_ID => $billing->getMollieAddressId(), + ], + ]); + + $searchResult = $this->createMock(EntitySearchResult::class); + $searchResult->method('getTotal')->willReturn(2); + $searchResult->method('getElements')->willReturn([$shippingEntity, $billingEntity]); + + $customerAddressRepo = $this->createMock(EntityRepository::class); + $customerAddressRepo->method('search')->willReturn($searchResult); + + $customer = $this->createMock(CustomerEntity::class); + $customer->method('getId')->willReturn('customer-1'); + $customer->method('getSalutationId')->willReturn(null); + + $fakeCustomerRepo = new FakeCustomerRepository(new CustomerDefinition()); + $fakeCustomerRepo->entityWrittenContainerEvents = [$this->createMock(EntityWrittenContainerEvent::class)]; + + $service = $this->buildCustomerService( + $fakeCustomerRepo, + $customerAddressRepo, + $this->createMock(EntityRepository::class), + $this->createMock(EntityRepository::class), + $this->createMock(ContainerInterface::class), + $this->createMock(SettingsService::class), + ); + + // Act + $service->reuseOrCreateAddresses($customer, $shipping, $this->createMock(Context::class), $billing); + + // Assert: shipping ID → defaultShippingAddressId, billing ID → defaultBillingAddressId + $savedData = array_pop($fakeCustomerRepo->data)[0]; + + $this->assertSame( + $shippingEntityId, + $savedData['defaultShippingAddressId'], + 'defaultShippingAddressId must point to the shipping address entity, not billing' + ); + $this->assertSame( + $billingEntityId, + $savedData['defaultBillingAddressId'], + 'defaultBillingAddressId must point to the billing address entity' + ); + } + + /** + * When only a shipping address is found (no billing address provided), the shipping + * entity ID must be used for both default addresses — not a random billing ID. + */ + public function testReuseOrCreateAddressesUsesSameIdForBothWhenNoBillingAddress(): void + { + $shipping = $this->buildAddress('Shipping Street 1', 'Berlin', '10115'); + + $shippingEntityId = 'entity-shipping-only'; + + $shippingEntity = $this->createMock(CustomerAddressEntity::class); + $shippingEntity->method('getId')->willReturn($shippingEntityId); + $shippingEntity->method('getCustomFields')->willReturn([ + CustomFieldsInterface::MOLLIE_KEY => [ + CustomerService::CUSTOM_FIELDS_KEY_EXPRESS_ADDRESS_ID => $shipping->getMollieAddressId(), + ], + ]); + + $searchResult = $this->createMock(EntitySearchResult::class); + $searchResult->method('getTotal')->willReturn(1); + $searchResult->method('getElements')->willReturn([$shippingEntity]); + + $customerAddressRepo = $this->createMock(EntityRepository::class); + $customerAddressRepo->method('search')->willReturn($searchResult); + + $customer = $this->createMock(CustomerEntity::class); + $customer->method('getId')->willReturn('customer-2'); + $customer->method('getSalutationId')->willReturn(null); + + $fakeCustomerRepo = new FakeCustomerRepository(new CustomerDefinition()); + $fakeCustomerRepo->entityWrittenContainerEvents = [$this->createMock(EntityWrittenContainerEvent::class)]; + + $service = $this->buildCustomerService( + $fakeCustomerRepo, + $customerAddressRepo, + $this->createMock(EntityRepository::class), + $this->createMock(EntityRepository::class), + $this->createMock(ContainerInterface::class), + $this->createMock(SettingsService::class), + ); + + // no billing address passed + $service->reuseOrCreateAddresses($customer, $shipping, $this->createMock(Context::class)); + + $savedData = array_pop($fakeCustomerRepo->data)[0]; + $this->assertSame($shippingEntityId, $savedData['defaultShippingAddressId']); + // billing falls back to shipping when no separate billing address exists + $this->assertSame($shippingEntityId, $savedData['defaultBillingAddressId']); + } + + /** + * When createGuestAccount is called with a distinct billing address, the 'billingAddress' + * key in the data bag passed to RegisterRoute must contain the billing address data — + * not a copy of the shipping address. + */ + public function testCreateGuestAccountSetsBillingAddressDataFromBillingNotShipping(): void + { + $shippingAddress = $this->buildAddress('Shipping Road 1', 'Berlin', '10115'); + $billingAddress = $this->buildAddress('Billing Lane 99', 'Hamburg', '20095'); + + // Country / salutation lookups must succeed + $countryIdResult = $this->buildIdSearchResult(['country-id']); + $countryRepo = $this->createMock(EntityRepository::class); + $countryRepo->method('searchIds')->willReturn($countryIdResult); + + $salutationIdResult = $this->buildIdSearchResult(['salutation-id']); + $salutationRepo = $this->createMock(EntityRepository::class); + $salutationRepo->method('searchIds')->willReturn($salutationIdResult); + + // Settings: data protection checkbox not required + $settings = new MollieSettingStruct(); + $settingsService = $this->createMock(SettingsService::class); + $settingsService->method('getSettings')->willReturn($settings); + + // Capture the RequestDataBag passed to RegisterRoute::register() + /** @var null|RequestDataBag $capturedBag */ + $capturedBag = null; + + $mockCustomer = $this->createMock(CustomerEntity::class); + $customerResponse = new CustomerResponse($mockCustomer); + + $registerRoute = $this->createMock(AbstractRegisterRoute::class); + $registerRoute->expects($this->once()) + ->method('register') + ->willReturnCallback(static function (RequestDataBag $data) use (&$capturedBag, $customerResponse): CustomerResponse { + $capturedBag = $data; + + return $customerResponse; + }) + ; + + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->with(RegisterRoute::class)->willReturn($registerRoute); + + $fakeCustomerRepo = new FakeCustomerRepository(new CustomerDefinition()); + + $service = $this->buildCustomerService( + $fakeCustomerRepo, + $this->createMock(EntityRepository::class), + $countryRepo, + $salutationRepo, + $container, + $settingsService, + ); + + $context = $this->createMock(SalesChannelContext::class); + $context->method('getSalesChannelId')->willReturn('sc-1'); + $context->method('getContext')->willReturn($this->createMock(Context::class)); + $context->method('getCustomer')->willReturn(null); + + // Act + $service->createGuestAccount($shippingAddress, 'payment-method-id', $context, null, $billingAddress); + + // Assert: billingAddress bag must contain the billing street, not the shipping street + $this->assertNotNull($capturedBag, 'RegisterRoute::register must have been called'); + + /** @var RequestDataBag $billingBag */ + $billingBag = $capturedBag->get('billingAddress'); + $this->assertInstanceOf(RequestDataBag::class, $billingBag); + + $this->assertSame( + 'Billing Lane 99', + $billingBag->get('street'), + 'billingAddress bag must contain billing address data, not a copy of shipping' + ); + $this->assertNotSame( + $shippingAddress->getStreet(), + $billingBag->get('street'), + 'billingAddress street must differ from shippingAddress street' + ); + } + + private function buildAddress(string $street, string $city, string $zip = '12345'): AddressStruct + { + return new AddressStruct('John', 'Doe', 'john@example.com', $street, '', $zip, $city, 'DE', '+49000'); + } + + private function buildIdSearchResult(array $ids): IdSearchResult + { + $result = $this->createMock(IdSearchResult::class); + $result->method('getIds')->willReturn($ids); + + return $result; + } + + private function buildCustomerService( + FakeCustomerRepository $customerRepo, + EntityRepository $customerAddressRepo, + EntityRepository $countryRepo, + EntityRepository $salutationRepo, + ContainerInterface $container, + SettingsService $settingsService + ): CustomerService { + return new CustomerService( + $countryRepo, + $customerRepo, + $customerAddressRepo, + $this->createMock(Customer::class), + $this->createMock(EventDispatcherInterface::class), + new NullLogger(), + $this->createMock(SalesChannelContextPersister::class), + $salutationRepo, + $settingsService, + '6.5.0', + $this->createMock(ConfigService::class), + $container, + $this->createMock(RequestStack::class), + new FakeTranslator(), + ); + } +} diff --git a/tests/PHPUnit/Struct/Address/AddressStructTest.php b/tests/PHPUnit/Struct/Address/AddressStructTest.php new file mode 100644 index 000000000..887c314ce --- /dev/null +++ b/tests/PHPUnit/Struct/Address/AddressStructTest.php @@ -0,0 +1,89 @@ +givenName = 'John'; + $address->familyName = 'Doe'; + $address->email = 'john@example.com'; + $address->streetAndNumber = 'Main Street 1'; + $address->postalCode = '12345'; + $address->city = 'Berlin'; + $address->country = 'DE'; + $address->phone = '+4900000'; + + $struct = AddressStruct::createFromApiResponse($address); + + $this->assertSame('John', $struct->getFirstName()); + $this->assertSame('Doe', $struct->getLastName()); + } + + public function testCreateFromApiResponseSplitsFullNameWhenOnlyFamilyNamePresent(): void + { + $address = new \stdClass(); + $address->familyName = 'John Doe'; + $address->email = 'john@example.com'; + $address->streetAndNumber = 'Main Street 1'; + $address->postalCode = '12345'; + $address->city = 'Berlin'; + $address->country = 'DE'; + $address->phone = '+4900000'; + + $struct = AddressStruct::createFromApiResponse($address); + + $this->assertSame('John', $struct->getFirstName()); + $this->assertSame('Doe', $struct->getLastName()); + } + + /** + * When PayPal returns only a single-word name in familyName, Shopware's required + * firstName validation must not fail. We fall back to using the name for both fields. + */ + public function testCreateFromApiResponseHandlesSingleWordFamilyName(): void + { + $address = new \stdClass(); + $address->familyName = 'Smith'; + $address->email = 'smith@example.com'; + $address->streetAndNumber = 'Main Street 1'; + $address->postalCode = '12345'; + $address->city = 'Berlin'; + $address->country = 'DE'; + $address->phone = '+4900000'; + + $struct = AddressStruct::createFromApiResponse($address); + + $this->assertNotEmpty($struct->getFirstName(), 'firstName must not be empty to pass Shopware validation'); + $this->assertNotEmpty($struct->getLastName(), 'lastName must not be empty'); + $this->assertSame('Smith', $struct->getFirstName()); + $this->assertSame('Smith', $struct->getLastName()); + } + + public function testCreateFromApiResponseDoesNotOverwriteExistingGivenName(): void + { + $address = new \stdClass(); + $address->givenName = 'Jane'; + $address->familyName = 'Doe'; + $address->email = 'jane@example.com'; + $address->streetAndNumber = 'Side Street 5'; + $address->postalCode = '54321'; + $address->city = 'Hamburg'; + $address->country = 'DE'; + $address->phone = '+4911111'; + + $struct = AddressStruct::createFromApiResponse($address); + + $this->assertSame('Jane', $struct->getFirstName()); + $this->assertSame('Doe', $struct->getLastName()); + } +}