From 012a45506e3b167e058d56b0ed0e8789447c42d1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 20:13:10 +0000 Subject: [PATCH 1/5] Initial plan From 10ecdf3391edeaaf0cad45181eabad672f2082ef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 20:21:01 +0000 Subject: [PATCH 2/5] Fix invalid customer-address ID error by checking customer ID first Co-authored-by: ihor-sviziev <1873745+ihor-sviziev@users.noreply.github.com> --- .../Quote/Model/QuoteAddressValidator.php | 9 +- .../Unit/Model/QuoteAddressValidatorTest.php | 256 ++++++++++++++++++ 2 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php diff --git a/app/code/Magento/Quote/Model/QuoteAddressValidator.php b/app/code/Magento/Quote/Model/QuoteAddressValidator.php index 01d4632ec77c6..07a8cef704033 100644 --- a/app/code/Magento/Quote/Model/QuoteAddressValidator.php +++ b/app/code/Magento/Quote/Model/QuoteAddressValidator.php @@ -122,7 +122,14 @@ public function validate(AddressInterface $addressData): bool */ public function validateForCart(CartInterface $cart, AddressInterface $address): void { - $this->doValidate($address, $cart->getCustomerIsGuest() ? null : (int) $cart->getCustomer()->getId()); + // If cart has a customer ID, use it regardless of the is_guest flag + // This handles cases where the flags are out of sync + $customerId = $cart->getCustomer()->getId(); + if (!$customerId) { + $customerId = null; + } + + $this->doValidate($address, $customerId ? (int) $customerId : null); } /** diff --git a/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php b/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php new file mode 100644 index 0000000000000..af323b7b7a3fb --- /dev/null +++ b/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php @@ -0,0 +1,256 @@ +addressRepositoryMock = $this->getMockBuilder(AddressRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->customerRepositoryMock = $this->getMockBuilder(CustomerRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->customerSessionMock = $this->getMockBuilder(Session::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->validator = new QuoteAddressValidator( + $this->addressRepositoryMock, + $this->customerRepositoryMock, + $this->customerSessionMock + ); + } + + /** + * Test that validation uses customer ID when available, regardless of is_guest flag + * + * This tests the fix for the issue where is_guest flag and customer_id are out of sync + */ + public function testValidateForCartWithCustomerIdIgnoresGuestFlag() + { + $customerId = 123; + $customerAddressId = 456; + + $cartMock = $this->getMockBuilder(CartInterface::class) + ->getMock(); + + $customerMock = $this->getMockBuilder(CustomerInterface::class) + ->getMock(); + + $addressMock = $this->getMockBuilder(AddressInterface::class) + ->getMock(); + + $customerAddressMock = $this->getMockBuilder(CustomerAddressInterface::class) + ->getMock(); + + // Set up cart to have a customer ID + $customerMock->expects($this->once()) + ->method('getId') + ->willReturn($customerId); + + $cartMock->expects($this->once()) + ->method('getCustomer') + ->willReturn($customerMock); + + // Set up address with customer address ID + $addressMock->expects($this->once()) + ->method('getCustomerAddressId') + ->willReturn($customerAddressId); + + // Mock customer repository to return customer with addresses + $customerMock->expects($this->once()) + ->method('getAddresses') + ->willReturn([$customerAddressMock]); + + $customerAddressMock->expects($this->once()) + ->method('getId') + ->willReturn($customerAddressId); + + $this->customerRepositoryMock->expects($this->once()) + ->method('getById') + ->with($customerId) + ->willReturn($customerMock); + + $this->addressRepositoryMock->expects($this->once()) + ->method('getById') + ->with($customerAddressId) + ->willReturn($customerAddressMock); + + // Execute validation - should not throw exception + $this->validator->validateForCart($cartMock, $addressMock); + } + + /** + * Test that validation correctly handles guest cart (no customer ID) + */ + public function testValidateForCartWithGuestCart() + { + $cartMock = $this->getMockBuilder(CartInterface::class) + ->getMock(); + + $customerMock = $this->getMockBuilder(CustomerInterface::class) + ->getMock(); + + $addressMock = $this->getMockBuilder(AddressInterface::class) + ->getMock(); + + // Set up cart without customer ID (guest) + $customerMock->expects($this->once()) + ->method('getId') + ->willReturn(null); + + $cartMock->expects($this->once()) + ->method('getCustomer') + ->willReturn($customerMock); + + // Address should not have customer address ID for guest + $addressMock->expects($this->once()) + ->method('getCustomerAddressId') + ->willReturn(null); + + // Execute validation - should not throw exception + $this->validator->validateForCart($cartMock, $addressMock); + } + + /** + * Test that validation throws exception when guest cart has customer address ID + */ + public function testValidateForCartThrowsExceptionWhenGuestHasCustomerAddress() + { + $this->expectException(NoSuchEntityException::class); + $this->expectExceptionMessage('Invalid customer address id 456'); + + $customerAddressId = 456; + + $cartMock = $this->getMockBuilder(CartInterface::class) + ->getMock(); + + $customerMock = $this->getMockBuilder(CustomerInterface::class) + ->getMock(); + + $addressMock = $this->getMockBuilder(AddressInterface::class) + ->getMock(); + + // Set up cart without customer ID (guest) + $customerMock->expects($this->once()) + ->method('getId') + ->willReturn(null); + + $cartMock->expects($this->once()) + ->method('getCustomer') + ->willReturn($customerMock); + + // Address has customer address ID even though cart is guest + $addressMock->expects($this->once()) + ->method('getCustomerAddressId') + ->willReturn($customerAddressId); + + // Execute validation - should throw exception + $this->validator->validateForCart($cartMock, $addressMock); + } + + /** + * Test that validation throws exception when customer address doesn't belong to customer + */ + public function testValidateForCartThrowsExceptionWhenAddressDoesNotBelongToCustomer() + { + $this->expectException(NoSuchEntityException::class); + $this->expectExceptionMessage('Invalid customer address id 456'); + + $customerId = 123; + $customerAddressId = 456; + $differentAddressId = 789; + + $cartMock = $this->getMockBuilder(CartInterface::class) + ->getMock(); + + $customerMock = $this->getMockBuilder(CustomerInterface::class) + ->getMock(); + + $addressMock = $this->getMockBuilder(AddressInterface::class) + ->getMock(); + + $customerAddressMock = $this->getMockBuilder(CustomerAddressInterface::class) + ->getMock(); + + // Set up cart with customer ID + $customerMock->expects($this->once()) + ->method('getId') + ->willReturn($customerId); + + $cartMock->expects($this->once()) + ->method('getCustomer') + ->willReturn($customerMock); + + // Address has customer address ID + $addressMock->expects($this->once()) + ->method('getCustomerAddressId') + ->willReturn($customerAddressId); + + // Mock customer with different address + $customerMock->expects($this->once()) + ->method('getAddresses') + ->willReturn([$customerAddressMock]); + + $customerAddressMock->expects($this->once()) + ->method('getId') + ->willReturn($differentAddressId); + + $this->customerRepositoryMock->expects($this->once()) + ->method('getById') + ->with($customerId) + ->willReturn($customerMock); + + $this->addressRepositoryMock->expects($this->once()) + ->method('getById') + ->with($customerAddressId) + ->willReturn($customerAddressMock); + + // Execute validation - should throw exception + $this->validator->validateForCart($cartMock, $addressMock); + } +} From e5201b121528c1a9e8c02f4210dac033409d166b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 20:23:18 +0000 Subject: [PATCH 3/5] Add null check for customer before calling getId() Co-authored-by: ihor-sviziev <1873745+ihor-sviziev@users.noreply.github.com> --- app/code/Magento/Quote/Model/QuoteAddressValidator.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Quote/Model/QuoteAddressValidator.php b/app/code/Magento/Quote/Model/QuoteAddressValidator.php index 07a8cef704033..22bf3de705e34 100644 --- a/app/code/Magento/Quote/Model/QuoteAddressValidator.php +++ b/app/code/Magento/Quote/Model/QuoteAddressValidator.php @@ -124,10 +124,8 @@ public function validateForCart(CartInterface $cart, AddressInterface $address): { // If cart has a customer ID, use it regardless of the is_guest flag // This handles cases where the flags are out of sync - $customerId = $cart->getCustomer()->getId(); - if (!$customerId) { - $customerId = null; - } + $customer = $cart->getCustomer(); + $customerId = $customer ? $customer->getId() : null; $this->doValidate($address, $customerId ? (int) $customerId : null); } From b097dfcd3cc8668332de33c2a6a471fb3a186e47 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 20:39:09 +0000 Subject: [PATCH 4/5] Avoid expensive customer object load by using getCustomerId() Co-authored-by: ihor-sviziev <1873745+ihor-sviziev@users.noreply.github.com> --- .../Quote/Model/QuoteAddressValidator.php | 8 ++- .../Unit/Model/QuoteAddressValidatorTest.php | 51 +++++++------------ 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/app/code/Magento/Quote/Model/QuoteAddressValidator.php b/app/code/Magento/Quote/Model/QuoteAddressValidator.php index 22bf3de705e34..4dab3bf639ceb 100644 --- a/app/code/Magento/Quote/Model/QuoteAddressValidator.php +++ b/app/code/Magento/Quote/Model/QuoteAddressValidator.php @@ -124,8 +124,12 @@ public function validateForCart(CartInterface $cart, AddressInterface $address): { // If cart has a customer ID, use it regardless of the is_guest flag // This handles cases where the flags are out of sync - $customer = $cart->getCustomer(); - $customerId = $customer ? $customer->getId() : null; + $customerId = null; + if ($cart instanceof Quote) { + $customerId = $cart->getCustomerId(); + } elseif (!$cart->getCustomerIsGuest()) { + $customerId = $cart->getCustomer()->getId(); + } $this->doValidate($address, $customerId ? (int) $customerId : null); } diff --git a/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php b/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php index af323b7b7a3fb..10a400c34f51b 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php @@ -15,6 +15,7 @@ use Magento\Framework\Exception\NoSuchEntityException; use Magento\Quote\Api\Data\AddressInterface; use Magento\Quote\Api\Data\CartInterface; +use Magento\Quote\Model\Quote; use Magento\Quote\Model\QuoteAddressValidator; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -75,7 +76,8 @@ public function testValidateForCartWithCustomerIdIgnoresGuestFlag() $customerId = 123; $customerAddressId = 456; - $cartMock = $this->getMockBuilder(CartInterface::class) + $cartMock = $this->getMockBuilder(Quote::class) + ->disableOriginalConstructor() ->getMock(); $customerMock = $this->getMockBuilder(CustomerInterface::class) @@ -88,13 +90,9 @@ public function testValidateForCartWithCustomerIdIgnoresGuestFlag() ->getMock(); // Set up cart to have a customer ID - $customerMock->expects($this->once()) - ->method('getId') - ->willReturn($customerId); - $cartMock->expects($this->once()) - ->method('getCustomer') - ->willReturn($customerMock); + ->method('getCustomerId') + ->willReturn($customerId); // Set up address with customer address ID $addressMock->expects($this->once()) @@ -129,23 +127,17 @@ public function testValidateForCartWithCustomerIdIgnoresGuestFlag() */ public function testValidateForCartWithGuestCart() { - $cartMock = $this->getMockBuilder(CartInterface::class) - ->getMock(); - - $customerMock = $this->getMockBuilder(CustomerInterface::class) + $cartMock = $this->getMockBuilder(Quote::class) + ->disableOriginalConstructor() ->getMock(); $addressMock = $this->getMockBuilder(AddressInterface::class) ->getMock(); // Set up cart without customer ID (guest) - $customerMock->expects($this->once()) - ->method('getId') - ->willReturn(null); - $cartMock->expects($this->once()) - ->method('getCustomer') - ->willReturn($customerMock); + ->method('getCustomerId') + ->willReturn(null); // Address should not have customer address ID for guest $addressMock->expects($this->once()) @@ -166,23 +158,17 @@ public function testValidateForCartThrowsExceptionWhenGuestHasCustomerAddress() $customerAddressId = 456; - $cartMock = $this->getMockBuilder(CartInterface::class) - ->getMock(); - - $customerMock = $this->getMockBuilder(CustomerInterface::class) + $cartMock = $this->getMockBuilder(Quote::class) + ->disableOriginalConstructor() ->getMock(); $addressMock = $this->getMockBuilder(AddressInterface::class) ->getMock(); // Set up cart without customer ID (guest) - $customerMock->expects($this->once()) - ->method('getId') - ->willReturn(null); - $cartMock->expects($this->once()) - ->method('getCustomer') - ->willReturn($customerMock); + ->method('getCustomerId') + ->willReturn(null); // Address has customer address ID even though cart is guest $addressMock->expects($this->once()) @@ -205,7 +191,8 @@ public function testValidateForCartThrowsExceptionWhenAddressDoesNotBelongToCust $customerAddressId = 456; $differentAddressId = 789; - $cartMock = $this->getMockBuilder(CartInterface::class) + $cartMock = $this->getMockBuilder(Quote::class) + ->disableOriginalConstructor() ->getMock(); $customerMock = $this->getMockBuilder(CustomerInterface::class) @@ -218,13 +205,9 @@ public function testValidateForCartThrowsExceptionWhenAddressDoesNotBelongToCust ->getMock(); // Set up cart with customer ID - $customerMock->expects($this->once()) - ->method('getId') - ->willReturn($customerId); - $cartMock->expects($this->once()) - ->method('getCustomer') - ->willReturn($customerMock); + ->method('getCustomerId') + ->willReturn($customerId); // Address has customer address ID $addressMock->expects($this->once()) From 4b838bfcc8eb30676f142e77bd004d14c9eb67cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 20:40:32 +0000 Subject: [PATCH 5/5] Add null check for getCustomer() in fallback path Co-authored-by: ihor-sviziev <1873745+ihor-sviziev@users.noreply.github.com> --- app/code/Magento/Quote/Model/QuoteAddressValidator.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Quote/Model/QuoteAddressValidator.php b/app/code/Magento/Quote/Model/QuoteAddressValidator.php index 4dab3bf639ceb..22f2f48850c48 100644 --- a/app/code/Magento/Quote/Model/QuoteAddressValidator.php +++ b/app/code/Magento/Quote/Model/QuoteAddressValidator.php @@ -128,7 +128,8 @@ public function validateForCart(CartInterface $cart, AddressInterface $address): if ($cart instanceof Quote) { $customerId = $cart->getCustomerId(); } elseif (!$cart->getCustomerIsGuest()) { - $customerId = $cart->getCustomer()->getId(); + $customer = $cart->getCustomer(); + $customerId = $customer ? $customer->getId() : null; } $this->doValidate($address, $customerId ? (int) $customerId : null);