Skip to content

Commit a859802

Browse files
buskamuzabuskamuza
committed
MAGETWO-62253: Refactor code to remove unserialize
- fixed tests - fixed code style - returned validation of menu item data
1 parent 9b16e36 commit a859802

File tree

8 files changed

+279
-254
lines changed

8 files changed

+279
-254
lines changed

‎app/code/Magento/Backend/Model/Menu.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,7 @@ public function populateFromArray(array $data)
305305
{
306306
$items = [];
307307
foreach ($data as $itemData) {
308-
$item = $this->menuItemFactory->create();
309-
$item->populateFromArray($itemData);
308+
$item = $this->menuItemFactory->create($itemData);
310309
$items[] = $item;
311310
}
312311
$this->exchangeArray($items);

‎app/code/Magento/Backend/Model/Menu/Item.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,7 @@ public function __construct(
168168
array $data = []
169169
) {
170170
$this->_validator = $validator;
171-
if (!empty($data)) {
172-
$this->_validator->validate($data);
173-
}
171+
$this->_validator->validate($data);
174172
$this->_moduleManager = $moduleManager;
175173
$this->_acl = $authorization;
176174
$this->_scopeConfig = $scopeConfig;

‎app/code/Magento/Backend/Test/Unit/Model/Menu/ConfigTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,27 @@
1010
class ConfigTest extends \PHPUnit_Framework_TestCase
1111
{
1212
/**
13-
* @var \PHPUnit_Framework_MockObject_MockObject
13+
* @var \Magento\Framework\App\Cache\Type\Config|\PHPUnit_Framework_MockObject_MockObject
1414
*/
1515
private $cacheInstanceMock;
1616

1717
/**
18-
* @var \PHPUnit_Framework_MockObject_MockObject
18+
* @var \Magento\Backend\Model\Menu\Config\Reader|\PHPUnit_Framework_MockObject_MockObject
1919
*/
2020
private $configReaderMock;
2121

2222
/**
23-
* @var \PHPUnit_Framework_MockObject_MockObject
23+
* @var \Magento\Backend\Model\Menu|\PHPUnit_Framework_MockObject_MockObject
2424
*/
2525
private $menuMock;
2626

2727
/**
28-
* @var \PHPUnit_Framework_MockObject_MockObject
28+
* @var \Magento\Backend\Model\Menu\Builder|\PHPUnit_Framework_MockObject_MockObject
2929
*/
3030
private $menuBuilderMock;
3131

3232
/**
33-
* @var \PHPUnit_Framework_MockObject_MockObject
33+
* @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
3434
*/
3535
private $logger;
3636

‎app/code/Magento/Backend/Test/Unit/Model/Menu/ItemTest.php

Lines changed: 10 additions & 240 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ public function testToArray(array $data, array $expected)
231231
$menuMock = $this->getMock(\Magento\Backend\Model\Menu::class, [], [], '', false);
232232
$this->_menuFactoryMock->method('create')->will($this->returnValue($menuMock));
233233
$menuMock->method('toArray')
234-
->willReturn(isset($data['sub_menu']) ? $data['sub_menu'] : null);
234+
->willReturn($data['sub_menu']);
235235

236236
$model = $this->objectManager->getObject(
237237
\Magento\Backend\Model\Menu\Item::class,
@@ -248,119 +248,12 @@ public function testToArray(array $data, array $expected)
248248
$this->assertEquals($expected, $model->toArray());
249249
}
250250

251+
/**
252+
* @return array
253+
*/
251254
public function toArrayDataProvider()
252255
{
253-
return [
254-
'No submenu' => [
255-
[
256-
'id' => 'item',
257-
'title' => 'Item Title',
258-
'action' => '/system/config',
259-
'resource' => 'Magento_Config::config',
260-
'depends_on_module' => 'Magento_Backend',
261-
'depends_on_config' => 'system/config/isEnabled',
262-
'tooltip' => 'Item tooltip',
263-
],
264-
[
265-
'parent_id' => null,
266-
'module_name' => 'Magento_Backend',
267-
'sort_index' => null,
268-
'depends_on_config' => 'system/config/isEnabled',
269-
'id' => 'item',
270-
'resource' => 'Magento_Config::config',
271-
'path' => '',
272-
'action' => '/system/config',
273-
'depends_on_module' => 'Magento_Backend',
274-
'tooltip' => 'Item tooltip',
275-
'title' => 'Item Title',
276-
'sub_menu' => null
277-
]
278-
],
279-
'with submenu' => [
280-
[
281-
'parent_id' => '1',
282-
'module_name' => 'Magento_Module1',
283-
'sort_index' => '50',
284-
'depends_on_config' => null,
285-
'id' => '5',
286-
'resource' => null,
287-
'path' => null,
288-
'action' => null,
289-
'depends_on_module' => null,
290-
'tooltip' => null,
291-
'title' => null,
292-
'sub_menu' => [
293-
'id' => 'item',
294-
'title' => 'Item Title',
295-
'action' => '/system/config',
296-
'resource' => 'Magento_Config::config',
297-
'depends_on_module' => 'Magento_Backend',
298-
'depends_on_config' => 'system/config/isEnabled',
299-
'tooltip' => 'Item tooltip',
300-
],
301-
],
302-
[
303-
'parent_id' => '1',
304-
'module_name' => 'Magento_Module1',
305-
'sort_index' => '50',
306-
'depends_on_config' => null,
307-
'id' => '5',
308-
'resource' => null,
309-
'path' => null,
310-
'action' => null,
311-
'depends_on_module' => null,
312-
'tooltip' => '',
313-
'title' => null,
314-
'sub_menu' => [
315-
'id' => 'item',
316-
'title' => 'Item Title',
317-
'action' => '/system/config',
318-
'resource' => 'Magento_Config::config',
319-
'depends_on_module' => 'Magento_Backend',
320-
'depends_on_config' => 'system/config/isEnabled',
321-
'tooltip' => 'Item tooltip',
322-
]
323-
]
324-
],
325-
'small set of data' => [
326-
[
327-
'parent_id' => '1',
328-
'module_name' => 'Magento_Module1',
329-
'sort_index' => '50',
330-
'sub_menu' => [
331-
'id' => 'item',
332-
'title' => 'Item Title',
333-
'action' => '/system/config',
334-
'resource' => 'Magento_Config::config',
335-
'depends_on_module' => 'Magento_Backend',
336-
'depends_on_config' => 'system/config/isEnabled',
337-
'tooltip' => 'Item tooltip',
338-
],
339-
],
340-
[
341-
'parent_id' => '1',
342-
'module_name' => 'Magento_Module1',
343-
'sort_index' => '50',
344-
'depends_on_config' => null,
345-
'id' => null,
346-
'resource' => null,
347-
'path' => '',
348-
'action' => null,
349-
'depends_on_module' => null,
350-
'tooltip' => '',
351-
'title' => null,
352-
'sub_menu' => [
353-
'id' => 'item',
354-
'title' => 'Item Title',
355-
'action' => '/system/config',
356-
'resource' => 'Magento_Config::config',
357-
'depends_on_module' => 'Magento_Backend',
358-
'depends_on_config' => 'system/config/isEnabled',
359-
'tooltip' => 'Item tooltip',
360-
]
361-
]
362-
]
363-
];
256+
return include __DIR__ . '/../_files/menu_item_data.php';
364257
}
365258

366259
/**
@@ -375,7 +268,7 @@ public function testPopulateFromArray(
375268
array $expected
376269
) {
377270
$menuMock = $this->getMock(\Magento\Backend\Model\Menu::class, [], [], '', false);
378-
$this->_menuFactoryMock->method('create')->will($this->returnValue($menuMock));
271+
$this->_menuFactoryMock->method('create')->willReturn($menuMock);
379272
$menuMock->method('toArray')
380273
->willReturn(['submenuArray']);
381274

@@ -395,134 +288,11 @@ public function testPopulateFromArray(
395288
$this->assertEquals($expected, $model->toArray());
396289
}
397290

291+
/**
292+
* @return array
293+
*/
398294
public function populateFromArrayDataProvider()
399295
{
400-
return [
401-
'default data to constructor' => [
402-
[],
403-
[
404-
'id' => 'item',
405-
'title' => 'Item Title',
406-
'action' => '/system/config',
407-
'resource' => 'Magento_Config::config',
408-
'depends_on_module' => 'Magento_Backend',
409-
'depends_on_config' => 'system/config/isEnabled',
410-
'tooltip' => 'Item tooltip',
411-
],
412-
[
413-
'parent_id' => null,
414-
'module_name' => 'Magento_Backend',
415-
'sort_index' => null,
416-
'depends_on_config' => 'system/config/isEnabled',
417-
'id' => 'item',
418-
'resource' => 'Magento_Config::config',
419-
'path' => '',
420-
'action' => '/system/config',
421-
'depends_on_module' => 'Magento_Backend',
422-
'tooltip' => 'Item tooltip',
423-
'title' => 'Item Title',
424-
'sub_menu' => null
425-
],
426-
],
427-
'data without submenu to constructor' => [
428-
[
429-
'id' => 'item',
430-
'title' => 'Item Title',
431-
'action' => '/system/config',
432-
'resource' => 'Magento_Config::config',
433-
'depends_on_module' => 'Magento_Backend',
434-
'depends_on_config' => 'system/config/isEnabled',
435-
'tooltip' => 'Item tooltip',
436-
],
437-
[
438-
'parent_id' => '1',
439-
'module_name' => 'Magento_Module1',
440-
'sort_index' => '50',
441-
'depends_on_config' => null,
442-
'id' => '5',
443-
'resource' => null,
444-
'path' => null,
445-
'action' => null,
446-
'depends_on_module' => null,
447-
'tooltip' => null,
448-
'title' => null,
449-
'sub_menu' => [
450-
'id' => 'item',
451-
'title' => 'Item Title',
452-
'action' => '/system/config',
453-
'resource' => 'Magento_Config::config',
454-
'depends_on_module' => 'Magento_Backend',
455-
'depends_on_config' => 'system/config/isEnabled',
456-
'tooltip' => 'Item tooltip',
457-
],
458-
],
459-
[
460-
'parent_id' => '1',
461-
'module_name' => 'Magento_Module1',
462-
'sort_index' => '50',
463-
'depends_on_config' => null,
464-
'id' => '5',
465-
'resource' => null,
466-
'path' => '',
467-
'action' => null,
468-
'depends_on_module' => null,
469-
'tooltip' => '',
470-
'title' => null,
471-
'sub_menu' => ['submenuArray']
472-
],
473-
],
474-
'data with submenu to constructor' => [
475-
[
476-
'parent_id' => '1',
477-
'module_name' => 'Magento_Module1',
478-
'sort_index' => '50',
479-
'depends_on_config' => null,
480-
'id' => '5',
481-
'resource' => null,
482-
'path' => null,
483-
'action' => null,
484-
'depends_on_module' => null,
485-
'tooltip' => null,
486-
'title' => null,
487-
'sub_menu' => [
488-
'id' => 'item',
489-
'title' => 'Item Title',
490-
'action' => '/system/config',
491-
'resource' => 'Magento_Config::config',
492-
'depends_on_module' => 'Magento_Backend',
493-
'depends_on_config' => 'system/config/isEnabled',
494-
'tooltip' => 'Item tooltip',
495-
],
496-
],
497-
[
498-
'parent_id' => '1',
499-
'module_name' => 'Magento_Module1',
500-
'sort_index' => '50',
501-
'sub_menu' => [
502-
'id' => 'item',
503-
'title' => 'Item Title',
504-
'action' => '/system/config',
505-
'resource' => 'Magento_Config::config',
506-
'depends_on_module' => 'Magento_Backend',
507-
'depends_on_config' => 'system/config/isEnabled',
508-
'tooltip' => 'Item tooltip',
509-
],
510-
],
511-
[
512-
'parent_id' => '1',
513-
'module_name' => 'Magento_Module1',
514-
'sort_index' => '50',
515-
'depends_on_config' => null,
516-
'id' => null,
517-
'resource' => null,
518-
'path' => '',
519-
'action' => null,
520-
'depends_on_module' => null,
521-
'tooltip' => '',
522-
'title' => null,
523-
'sub_menu' => ['submenuArray']
524-
],
525-
]
526-
];
296+
return include __DIR__ . '/../_files/menu_item_constructor_data.php';
527297
}
528298
}

0 commit comments

Comments
 (0)