Skip to content

Commit 1a4f7c7

Browse files
committed
Solve bug #116 - Setting number of address line lower than existing entry breaks all checkouts for existing address data
1 parent e07282c commit 1a4f7c7

File tree

3 files changed

+107
-1
lines changed

3 files changed

+107
-1
lines changed

app/code/Magento/Customer/Model/Address/AbstractAddress.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Magento\Framework\App\ObjectManager;
1818
use Magento\Framework\Model\AbstractExtensibleModel;
1919
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
20+
use Magento\Customer\Helper\Address as AddressHelper;
2021

2122
/**
2223
* Address abstract model
@@ -146,6 +147,11 @@ class AbstractAddress extends AbstractExtensibleModel implements AddressModelInt
146147
*/
147148
private array $regionIdCountry = [];
148149

150+
/**
151+
* @var AddressHelper|null
152+
*/
153+
protected ?AddressHelper $addressHelper;
154+
149155
/**
150156
* @param \Magento\Framework\Model\Context $context
151157
* @param \Magento\Framework\Registry $registry
@@ -166,6 +172,7 @@ class AbstractAddress extends AbstractExtensibleModel implements AddressModelInt
166172
* @param CompositeValidator $compositeValidator
167173
* @param CountryModelsCache|null $countryModelsCache
168174
* @param RegionModelsCache|null $regionModelsCache
175+
* @param Magento\Customer\Helper\Address\AddressHelper|null $addressHelper
169176
*
170177
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
171178
*/
@@ -189,6 +196,7 @@ public function __construct(
189196
?CompositeValidator $compositeValidator = null,
190197
?CountryModelsCache $countryModelsCache = null,
191198
?RegionModelsCache $regionModelsCache = null,
199+
?AddressHelper $addressHelper = null
192200
) {
193201
$this->_directoryData = $directoryData;
194202
$data = $this->_implodeArrayField($data);
@@ -206,6 +214,8 @@ public function __construct(
206214
->get(CountryModelsCache::class);
207215
$this->regionModelsCache = $regionModelsCache ?: ObjectManager::getInstance()
208216
->get(RegionModelsCache::class);
217+
$this->addressHelper = $addressHelper ?: ObjectManager::getInstance()
218+
->get(AddressHelper::class);
209219
parent::__construct(
210220
$context,
211221
$registry,
@@ -242,6 +252,8 @@ public function getName()
242252

243253
/**
244254
* Retrieve street field of an address
255+
* Honour current configured street lines, and convert
256+
* legacy data to match
245257
*
246258
* @return string[]
247259
*/
@@ -250,7 +262,11 @@ public function getStreet()
250262
if (is_array($this->getStreetFull())) {
251263
return $this->getStreetFull();
252264
}
253-
return explode("\n", $this->getStreetFull());
265+
$maxAllowedLineCount = $this->addressHelper->getStreetLines() ?? 2;
266+
$lines = explode("\n", $this->getStreetFull());
267+
$lines = $this->addressHelper->convertStreetLines($lines, $maxAllowedLineCount);
268+
$this->setStreetFull(implode("\n", $lines));
269+
return $lines;
254270
}
255271

256272
/**

app/code/Magento/Customer/Test/Unit/Model/Address/AbstractAddressTest.php

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ class AbstractAddressTest extends TestCase
7070
/** @var CompositeValidator|MockObject */
7171
private $compositeValidatorMock;
7272

73+
private $addressHelperMock;
74+
7375
protected function setUp(): void
7476
{
7577
$this->contextMock = $this->createMock(Context::class);
@@ -491,6 +493,89 @@ function ($data) {
491493
);
492494
}
493495

496+
public function testGetStreetWithTwoLines()
497+
{
498+
// Create a partial mock for AddressHelper
499+
$this->addressHelperMock = $this->getMockBuilder(\Magento\Customer\Helper\Address::class)
500+
->disableOriginalConstructor()
501+
->onlyMethods(['getStreetLines']) // Mock only getStreetLines, keep the real convertStreetLines
502+
->getMock();
503+
504+
// Mock getStreetLines to return 2 by default
505+
$this->addressHelperMock->method('getStreetLines')->willReturn(2);
506+
507+
// Use reflection to inject the partial mock into the model
508+
$reflection = new \ReflectionClass($this->model);
509+
$property = $reflection->getProperty('addressHelper');
510+
$property->setAccessible(true);
511+
$property->setValue($this->model, $this->addressHelperMock);
512+
513+
$this->addressHelperMock->method('getStreetLines')->willReturn(2);
514+
$streetData = ["Street Line 1", "Street Line 2", "Street Line 3", "Street Line 4"];
515+
$this->model->setData('street', $streetData);
516+
517+
// Call getStreet() which should internally call convertStreetLines()
518+
$result = $this->model->getStreet();
519+
520+
// Assert that empty and whitespace-only lines are removed by convertStreetLines
521+
$this->assertEquals(["Street Line 1 Street Line 2", "Street Line 3 Street Line 4"], $result);
522+
}
523+
524+
public function testGetStreetWithThreeLines()
525+
{
526+
// Create a partial mock for AddressHelper
527+
$this->addressHelperMock = $this->getMockBuilder(\Magento\Customer\Helper\Address::class)
528+
->disableOriginalConstructor()
529+
->onlyMethods(['getStreetLines']) // Mock only getStreetLines, keep the real convertStreetLines
530+
->getMock();
531+
532+
// Mock getStreetLines to return 2 by default
533+
$this->addressHelperMock->method('getStreetLines')->willReturn(3);
534+
535+
// Use reflection to inject the partial mock into the model
536+
$reflection = new \ReflectionClass($this->model);
537+
$property = $reflection->getProperty('addressHelper');
538+
$property->setAccessible(true);
539+
$property->setValue($this->model, $this->addressHelperMock);
540+
541+
$this->addressHelperMock->method('getStreetLines')->willReturn(3);
542+
$streetData = ["Street Line 1", "Street Line 2", "Street Line 3", "Street Line 4"];
543+
$this->model->setData('street', $streetData);
544+
545+
// Call getStreet() which should internally call convertStreetLines()
546+
$result = $this->model->getStreet();
547+
548+
// Assert that empty and whitespace-only lines are removed by convertStreetLines
549+
$this->assertEquals(["Street Line 1 Street Line 2","Street Line 3","Street Line 4"], $result);
550+
}
551+
552+
public function testGetStreetWithOneLine()
553+
{
554+
// Create a partial mock for AddressHelper
555+
$this->addressHelperMock = $this->getMockBuilder(\Magento\Customer\Helper\Address::class)
556+
->disableOriginalConstructor()
557+
->onlyMethods(['getStreetLines']) // Mock only getStreetLines, keep the real convertStreetLines
558+
->getMock();
559+
560+
// Mock getStreetLines to return 2 by default
561+
$this->addressHelperMock->method('getStreetLines')->willReturn(1);
562+
563+
// Use reflection to inject the partial mock into the model
564+
$reflection = new \ReflectionClass($this->model);
565+
$property = $reflection->getProperty('addressHelper');
566+
$property->setAccessible(true);
567+
$property->setValue($this->model, $this->addressHelperMock);
568+
569+
$streetData = ["Street Line 1", "Street Line 2", "Street Line 3", "Street Line 4"];
570+
$this->model->setData('street', $streetData);
571+
572+
// Call getStreet() which should internally call convertStreetLines()
573+
$result = $this->model->getStreet();
574+
575+
// Assert that empty and whitespace-only lines are removed by convertStreetLines
576+
$this->assertEquals(["Street Line 1 Street Line 2 Street Line 3 Street Line 4"], $result);
577+
}
578+
494579
protected function tearDown(): void
495580
{
496581
$this->objectManager->setBackwardCompatibleProperty(

app/code/Magento/Customer/etc/di.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,4 +593,9 @@
593593
type="Magento\Customer\Plugin\AsyncRequestCustomerGroupAuthorization"
594594
/>
595595
</type>
596+
<type name="Magento\Customer\Model\Address\AbstractAddress">
597+
<arguments>
598+
<argument name="addressHelper" xsi:type="object">Magento\Customer\Helper\Address</argument>
599+
</arguments>
600+
</type>
596601
</config>

0 commit comments

Comments
 (0)