Skip to content

Commit 44fd77e

Browse files
sumesh-GLsumesh-GL
committed
AC-13335: Customer group code saved does not match the input when using multibyte characters
Changed substr() to mb_substr() for multibyte character handling
1 parent 43a5442 commit 44fd77e

File tree

5 files changed

+538
-8
lines changed

5 files changed

+538
-8
lines changed

‎app/code/Magento/Customer/Model/Group.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
*/
1818
class Group extends \Magento\Framework\Model\AbstractModel
1919
{
20-
const NOT_LOGGED_IN_ID = 0;
20+
public const NOT_LOGGED_IN_ID = 0;
2121

22-
const CUST_GROUP_ALL = 32000;
22+
public const CUST_GROUP_ALL = 32000;
2323

24-
const ENTITY = 'customer_group';
24+
public const ENTITY = 'customer_group';
2525

26-
const GROUP_CODE_MAX_LENGTH = 32;
26+
public const GROUP_CODE_MAX_LENGTH = 32;
2727

2828
/**
2929
* Prefix of model events names
@@ -92,6 +92,8 @@ public function __construct(
9292
}
9393

9494
/**
95+
* Initialize model
96+
*
9597
* @return void
9698
*/
9799
protected function _construct()
@@ -172,7 +174,7 @@ public function beforeSave()
172174
*/
173175
protected function _prepareData()
174176
{
175-
$this->setCode(substr($this->getCode(), 0, self::GROUP_CODE_MAX_LENGTH));
177+
$this->setCode(mb_substr($this->getCode(), 0, self::GROUP_CODE_MAX_LENGTH));
176178
return $this;
177179
}
178180
}

‎app/code/Magento/Customer/Model/ResourceModel/Group.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ protected function _createCustomersCollection()
124124
protected function _beforeSave(\Magento\Framework\Model\AbstractModel $group)
125125
{
126126
/** @var \Magento\Customer\Model\Group $group */
127-
$group->setCode(substr($group->getCode(), 0, $group::GROUP_CODE_MAX_LENGTH));
127+
$group->setCode(mb_substr($group->getCode(), 0, $group::GROUP_CODE_MAX_LENGTH));
128128
return parent::_beforeSave($group);
129129
}
130130

‎app/code/Magento/Customer/Test/Unit/Model/GroupTest.php‎

Lines changed: 333 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,333 @@
1+
<?php
2+
/**
3+
* Copyright 2025 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Customer\Test\Unit\Model;
9+
10+
use Magento\Customer\Model\Group;
11+
use Magento\Customer\Model\GroupManagement;
12+
use Magento\Customer\Model\ResourceModel\Group as GroupResource;
13+
use Magento\Framework\Model\Context;
14+
use Magento\Framework\Registry;
15+
use Magento\Framework\Reflection\DataObjectProcessor;
16+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
17+
use Magento\Store\Model\StoresConfig;
18+
use Magento\Tax\Model\ClassModel;
19+
use Magento\Tax\Model\ClassModelFactory;
20+
use PHPUnit\Framework\MockObject\MockObject;
21+
use PHPUnit\Framework\TestCase;
22+
23+
/**
24+
* Unit test for Customer Group model
25+
*/
26+
class GroupTest extends TestCase
27+
{
28+
/**
29+
* @var Group
30+
*/
31+
private $model;
32+
33+
/**
34+
* @var Context|MockObject
35+
*/
36+
private $contextMock;
37+
38+
/**
39+
* @var Registry|MockObject
40+
*/
41+
private $registryMock;
42+
43+
/**
44+
* @var StoresConfig|MockObject
45+
*/
46+
private $storesConfigMock;
47+
48+
/**
49+
* @var DataObjectProcessor|MockObject
50+
*/
51+
private $dataObjectProcessorMock;
52+
53+
/**
54+
* @var ClassModelFactory|MockObject
55+
*/
56+
private $classModelFactoryMock;
57+
58+
/**
59+
* @var GroupResource|MockObject
60+
*/
61+
private $resourceMock;
62+
63+
/**
64+
* @var ObjectManager
65+
*/
66+
private $objectManager;
67+
68+
/**
69+
* @inheritdoc
70+
*/
71+
protected function setUp(): void
72+
{
73+
$this->contextMock = $this->createMock(Context::class);
74+
$this->registryMock = $this->createMock(Registry::class);
75+
$this->storesConfigMock = $this->createMock(StoresConfig::class);
76+
$this->dataObjectProcessorMock = $this->createMock(DataObjectProcessor::class);
77+
$this->classModelFactoryMock = $this->createMock(ClassModelFactory::class);
78+
$this->resourceMock = $this->createMock(GroupResource::class);
79+
80+
// Mock event manager to prevent dispatch errors
81+
$eventManagerMock = $this->getMockBuilder(\Magento\Framework\Event\ManagerInterface::class)
82+
->disableOriginalConstructor()
83+
->getMock();
84+
$eventManagerMock->expects($this->any())
85+
->method('dispatch')
86+
->willReturn(true);
87+
88+
$this->contextMock->expects($this->any())
89+
->method('getEventDispatcher')
90+
->willReturn($eventManagerMock);
91+
92+
$this->objectManager = new ObjectManager($this);
93+
$this->model = $this->objectManager->getObject(
94+
Group::class,
95+
[
96+
'context' => $this->contextMock,
97+
'registry' => $this->registryMock,
98+
'storesConfig' => $this->storesConfigMock,
99+
'dataObjectProcessor' => $this->dataObjectProcessorMock,
100+
'classModelFactory' => $this->classModelFactoryMock,
101+
'resource' => $this->resourceMock
102+
]
103+
);
104+
}
105+
106+
/**
107+
* Test setCode and getCode methods
108+
*
109+
* @return void
110+
*/
111+
public function testSetCodeAndGetCode(): void
112+
{
113+
$code = 'test_group_code';
114+
$result = $this->model->setCode($code);
115+
116+
$this->assertSame($this->model, $result, 'setCode should return $this for method chaining');
117+
$this->assertEquals($code, $this->model->getCode(), 'getCode should return the code set by setCode');
118+
}
119+
120+
/**
121+
* Test that _prepareData truncates code to max length with ASCII characters
122+
*
123+
* @return void
124+
*/
125+
public function testPrepareDataTruncatesAsciiCode(): void
126+
{
127+
$longCode = str_repeat('a', 50); // 50 characters, exceeds max of 32
128+
$expectedCode = str_repeat('a', Group::GROUP_CODE_MAX_LENGTH);
129+
130+
$this->model->setCode($longCode);
131+
$this->model->beforeSave();
132+
133+
$this->assertEquals(
134+
$expectedCode,
135+
$this->model->getCode(),
136+
'Code should be truncated to GROUP_CODE_MAX_LENGTH characters'
137+
);
138+
$this->assertEquals(
139+
Group::GROUP_CODE_MAX_LENGTH,
140+
strlen($this->model->getCode()),
141+
'Code length should equal GROUP_CODE_MAX_LENGTH'
142+
);
143+
}
144+
145+
/**
146+
* Test that _prepareData correctly handles multibyte characters
147+
*
148+
* @dataProvider multibyteCharacterProvider
149+
* @param string $input
150+
* @param string $expected
151+
* @param int $expectedLength
152+
* @return void
153+
*/
154+
public function testPrepareDataHandlesMultibyteCharacters(
155+
string $input,
156+
string $expected,
157+
int $expectedLength
158+
): void {
159+
$this->model->setCode($input);
160+
$this->model->beforeSave();
161+
162+
$this->assertEquals(
163+
$expected,
164+
$this->model->getCode(),
165+
'Code should be correctly truncated based on character count, not byte count'
166+
);
167+
$this->assertEquals(
168+
$expectedLength,
169+
mb_strlen($this->model->getCode()),
170+
'Character count should match expected length'
171+
);
172+
}
173+
174+
/**
175+
* Data provider for multibyte character tests
176+
*
177+
* @return array
178+
*/
179+
public static function multibyteCharacterProvider(): array
180+
{
181+
return [
182+
'multibyte_within_limit' => [
183+
'input' => str_repeat('ö', 31),
184+
'expected' => str_repeat('ö', 31),
185+
'expectedLength' => 31
186+
],
187+
'multibyte_at_limit' => [
188+
'input' => str_repeat('ö', 32),
189+
'expected' => str_repeat('ö', 32),
190+
'expectedLength' => 32
191+
],
192+
'multibyte_over_limit' => [
193+
'input' => str_repeat('ö', 40),
194+
'expected' => str_repeat('ö', 32),
195+
'expectedLength' => 32
196+
],
197+
'chinese_over_limit' => [
198+
'input' => str_repeat('中', 40),
199+
'expected' => str_repeat('中', 32),
200+
'expectedLength' => 32
201+
],
202+
'mixed_multibyte' => [
203+
'input' => str_repeat('aö', 20), // 40 characters
204+
'expected' => str_repeat('aö', 16), // 32 characters
205+
'expectedLength' => 32
206+
],
207+
'emoji_over_limit' => [
208+
'input' => str_repeat('😀', 40),
209+
'expected' => str_repeat('😀', 32),
210+
'expectedLength' => 32
211+
]
212+
];
213+
}
214+
215+
/**
216+
* Test getTaxClassName when tax_class_name is already set
217+
*
218+
* @return void
219+
*/
220+
public function testGetTaxClassNameWhenAlreadySet(): void
221+
{
222+
$taxClassName = 'Retail Customer';
223+
$this->model->setData('tax_class_name', $taxClassName);
224+
225+
// Should not call classModelFactory since name is already set
226+
$this->classModelFactoryMock->expects($this->never())
227+
->method('create');
228+
229+
$result = $this->model->getTaxClassName();
230+
231+
$this->assertEquals($taxClassName, $result);
232+
}
233+
234+
/**
235+
* Test getTaxClassName when it needs to be loaded from tax class
236+
*
237+
* @return void
238+
*/
239+
public function testGetTaxClassNameLoadsFromTaxClass(): void
240+
{
241+
$taxClassId = 3;
242+
$taxClassName = 'Retail Customer';
243+
244+
$this->model->setTaxClassId($taxClassId);
245+
246+
$classModelMock = $this->createMock(ClassModel::class);
247+
$classModelMock->expects($this->once())
248+
->method('load')
249+
->with($taxClassId)
250+
->willReturnSelf();
251+
$classModelMock->expects($this->once())
252+
->method('getClassName')
253+
->willReturn($taxClassName);
254+
255+
$this->classModelFactoryMock->expects($this->once())
256+
->method('create')
257+
->willReturn($classModelMock);
258+
259+
$result = $this->model->getTaxClassName();
260+
261+
$this->assertEquals($taxClassName, $result);
262+
$this->assertEquals($taxClassName, $this->model->getData('tax_class_name'));
263+
}
264+
265+
/**
266+
* Test usesAsDefault returns true when group is default
267+
*
268+
* @return void
269+
*/
270+
public function testUsesAsDefaultReturnsTrue(): void
271+
{
272+
$groupId = 1;
273+
$this->model->setId($groupId);
274+
275+
$this->storesConfigMock->expects($this->once())
276+
->method('getStoresConfigByPath')
277+
->with(GroupManagement::XML_PATH_DEFAULT_ID)
278+
->willReturn([1, 2, 3]);
279+
280+
$result = $this->model->usesAsDefault();
281+
282+
$this->assertTrue($result);
283+
}
284+
285+
/**
286+
* Test usesAsDefault returns false when group is not default
287+
*
288+
* @return void
289+
*/
290+
public function testUsesAsDefaultReturnsFalse(): void
291+
{
292+
$groupId = 5;
293+
$this->model->setId($groupId);
294+
295+
$this->storesConfigMock->expects($this->once())
296+
->method('getStoresConfigByPath')
297+
->with(GroupManagement::XML_PATH_DEFAULT_ID)
298+
->willReturn([1, 2, 3]);
299+
300+
$result = $this->model->usesAsDefault();
301+
302+
$this->assertFalse($result);
303+
}
304+
305+
/**
306+
* Test beforeSave calls _prepareData
307+
*
308+
* @return void
309+
*/
310+
public function testBeforeSaveCallsPrepareData(): void
311+
{
312+
$code = str_repeat('a', 50);
313+
$this->model->setCode($code);
314+
315+
$this->model->beforeSave();
316+
317+
// Verify that code was truncated by _prepareData
318+
$this->assertEquals(
319+
Group::GROUP_CODE_MAX_LENGTH,
320+
strlen($this->model->getCode())
321+
);
322+
}
323+
324+
/**
325+
* Test GROUP_CODE_MAX_LENGTH constant value
326+
*
327+
* @return void
328+
*/
329+
public function testGroupCodeMaxLengthConstant(): void
330+
{
331+
$this->assertEquals(32, Group::GROUP_CODE_MAX_LENGTH);
332+
}
333+
}

0 commit comments

Comments
 (0)