Skip to content

Commit ed6723f

Browse files
magento-engcom-teammagento-engcom-team
authored
🔃 [EngCom] Public Pull Requests - 2.3-develop
Accepted Public Pull Requests: - #13640: [Forwardport] #13551 Fix json encoded attribute backend type to not encode attribute value multiple times (by @nmalevanec) - #13798: [Port 2.3-develop] Save CMS Block using repository (by @JeroenVanLeusden) - #13917: Pass Expected Data Type in backgroundColor Call (2.3) (by @northernco) - #13815: [Port 2.3-develop] Load CMS Page using repository in save action (by @JeroenVanLeusden)
2 parents ff6c326 + 6f24086 commit ed6723f

File tree

10 files changed

+174
-82
lines changed

10 files changed

+174
-82
lines changed

‎app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,6 @@ public function execute()
202202
}
203203
}
204204

205-
$data = $this->presentation->convertPresentationDataToInputType($data);
206-
207205
if ($attributeId) {
208206
if (!$model->getId()) {
209207
$this->messageManager->addErrorMessage(__('This attribute no longer exists.'));
@@ -218,6 +216,7 @@ public function execute()
218216

219217
$data['attribute_code'] = $model->getAttributeCode();
220218
$data['is_user_defined'] = $model->getIsUserDefined();
219+
$data['frontend_input'] = $model->getFrontendInput();
221220
} else {
222221
/**
223222
* @todo add to helper and specify all relations for properties
@@ -230,6 +229,8 @@ public function execute()
230229
);
231230
}
232231

232+
$data = $this->presentation->convertPresentationDataToInputType($data);
233+
233234
$data += ['is_filterable' => 0, 'is_filterable_in_search' => 0];
234235

235236
if ($model->getIsUserDefined() === null || $model->getIsUserDefined() != 0) {

‎app/code/Magento/Cms/Controller/Adminhtml/Block/Save.php‎

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
namespace Magento\Cms\Controller\Adminhtml\Block;
88

99
use Magento\Backend\App\Action\Context;
10+
use Magento\Cms\Api\BlockRepositoryInterface;
1011
use Magento\Cms\Model\Block;
12+
use Magento\Cms\Model\BlockFactory;
1113
use Magento\Framework\App\Request\DataPersistorInterface;
1214
use Magento\Framework\Exception\LocalizedException;
13-
use Magento\TestFramework\Inspection\Exception;
15+
use Magento\Framework\Registry;
1416

1517
class Save extends \Magento\Cms\Controller\Adminhtml\Block
1618
{
@@ -19,17 +21,35 @@ class Save extends \Magento\Cms\Controller\Adminhtml\Block
1921
*/
2022
protected $dataPersistor;
2123

24+
/**
25+
* @var BlockFactory
26+
*/
27+
private $blockFactory;
28+
29+
/**
30+
* @var BlockRepositoryInterface
31+
*/
32+
private $blockRepository;
33+
2234
/**
2335
* @param Context $context
24-
* @param \Magento\Framework\Registry $coreRegistry
36+
* @param Registry $coreRegistry
2537
* @param DataPersistorInterface $dataPersistor
38+
* @param BlockFactory|null $blockFactory
39+
* @param BlockRepositoryInterface|null $blockRepository
2640
*/
2741
public function __construct(
2842
Context $context,
29-
\Magento\Framework\Registry $coreRegistry,
30-
DataPersistorInterface $dataPersistor
43+
Registry $coreRegistry,
44+
DataPersistorInterface $dataPersistor,
45+
BlockFactory $blockFactory = null,
46+
BlockRepositoryInterface $blockRepository = null
3147
) {
3248
$this->dataPersistor = $dataPersistor;
49+
$this->blockFactory = $blockFactory
50+
?: \Magento\Framework\App\ObjectManager::getInstance()->get(BlockFactory::class);
51+
$this->blockRepository = $blockRepository
52+
?: \Magento\Framework\App\ObjectManager::getInstance()->get(BlockRepositoryInterface::class);
3353
parent::__construct($context, $coreRegistry);
3454
}
3555

@@ -45,8 +65,6 @@ public function execute()
4565
$resultRedirect = $this->resultRedirectFactory->create();
4666
$data = $this->getRequest()->getPostValue();
4767
if ($data) {
48-
$id = $this->getRequest()->getParam('block_id');
49-
5068
if (isset($data['is_active']) && $data['is_active'] === 'true') {
5169
$data['is_active'] = Block::STATUS_ENABLED;
5270
}
@@ -55,27 +73,32 @@ public function execute()
5573
}
5674

5775
/** @var \Magento\Cms\Model\Block $model */
58-
$model = $this->_objectManager->create(\Magento\Cms\Model\Block::class)->load($id);
59-
if (!$model->getId() && $id) {
60-
$this->messageManager->addError(__('This block no longer exists.'));
61-
return $resultRedirect->setPath('*/*/');
76+
$model = $this->blockFactory->create();
77+
78+
$id = $this->getRequest()->getParam('block_id');
79+
if ($id) {
80+
try {
81+
$model = $this->blockRepository->getById($id);
82+
} catch (LocalizedException $e) {
83+
$this->messageManager->addErrorMessage(__('This block no longer exists.'));
84+
return $resultRedirect->setPath('*/*/');
85+
}
6286
}
6387

6488
$model->setData($data);
6589

6690
try {
67-
$model->save();
68-
$this->messageManager->addSuccess(__('You saved the block.'));
91+
$this->blockRepository->save($model);
92+
$this->messageManager->addSuccessMessage(__('You saved the block.'));
6993
$this->dataPersistor->clear('cms_block');
70-
7194
if ($this->getRequest()->getParam('back')) {
7295
return $resultRedirect->setPath('*/*/edit', ['block_id' => $model->getId()]);
7396
}
7497
return $resultRedirect->setPath('*/*/');
7598
} catch (LocalizedException $e) {
76-
$this->messageManager->addError($e->getMessage());
99+
$this->messageManager->addErrorMessage($e->getMessage());
77100
} catch (\Exception $e) {
78-
$this->messageManager->addException($e, __('Something went wrong while saving the block.'));
101+
$this->messageManager->addExceptionMessage($e, __('Something went wrong while saving the block.'));
79102
}
80103

81104
$this->dataPersistor->set('cms_block', $data);

‎app/code/Magento/Cms/Controller/Adminhtml/Page/Save.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ class Save extends \Magento\Backend\App\Action
4646
* @param DataPersistorInterface $dataPersistor
4747
* @param \Magento\Cms\Model\PageFactory $pageFactory
4848
* @param \Magento\Cms\Api\PageRepositoryInterface $pageRepository
49-
*
5049
*/
5150
public function __construct(
5251
Action\Context $context,
@@ -90,11 +89,10 @@ public function execute()
9089

9190
$id = $this->getRequest()->getParam('page_id');
9291
if ($id) {
93-
$model->load($id);
94-
if (!$model->getId()) {
92+
try {
93+
$model = $this->pageRepository->getById($id);
94+
} catch (LocalizedException $e) {
9595
$this->messageManager->addErrorMessage(__('This page no longer exists.'));
96-
/** \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
97-
$resultRedirect = $this->resultRedirectFactory->create();
9896
return $resultRedirect->setPath('*/*/');
9997
}
10098
}

‎app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Block/SaveTest.php‎

Lines changed: 54 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,16 @@ class SaveTest extends \PHPUnit\Framework\TestCase
6565
*/
6666
protected $saveController;
6767

68+
/**
69+
* @var \Magento\Cms\Model\BlockFactory|\PHPUnit_Framework_MockObject_MockObject
70+
*/
71+
private $blockFactory;
72+
73+
/**
74+
* @var \Magento\Cms\Api\BlockRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
75+
*/
76+
private $blockRepository;
77+
6878
/**
6979
* @var int
7080
*/
@@ -129,11 +139,22 @@ protected function setUp()
129139
->method('getResultRedirectFactory')
130140
->willReturn($this->resultRedirectFactory);
131141

142+
$this->blockFactory = $this->getMockBuilder(\Magento\Cms\Model\BlockFactory::class)
143+
->disableOriginalConstructor()
144+
->setMethods(['create'])
145+
->getMock();
146+
147+
$this->blockRepository = $this->getMockBuilder(\Magento\Cms\Api\BlockRepositoryInterface::class)
148+
->disableOriginalConstructor()
149+
->getMockForAbstractClass();
150+
132151
$this->saveController = $this->objectManager->getObject(
133152
\Magento\Cms\Controller\Adminhtml\Block\Save::class,
134153
[
135154
'context' => $this->contextMock,
136155
'dataPersistor' => $this->dataPersistorMock,
156+
'blockFactory' => $this->blockFactory,
157+
'blockRepository' => $this->blockRepository,
137158
]
138159
);
139160
}
@@ -158,26 +179,24 @@ public function testSaveAction()
158179
]
159180
);
160181

161-
$this->objectManagerMock->expects($this->atLeastOnce())
182+
$this->blockFactory->expects($this->atLeastOnce())
162183
->method('create')
163-
->with($this->equalTo(\Magento\Cms\Model\Block::class))
164184
->willReturn($this->blockMock);
165185

166-
$this->blockMock->expects($this->any())
167-
->method('load')
168-
->willReturnSelf();
169-
$this->blockMock->expects($this->any())
170-
->method('getId')
171-
->willReturn(true);
186+
$this->blockRepository->expects($this->once())
187+
->method('getById')
188+
->with($this->blockId)
189+
->willReturn($this->blockMock);
190+
172191
$this->blockMock->expects($this->once())->method('setData');
173-
$this->blockMock->expects($this->once())->method('save');
192+
$this->blockRepository->expects($this->once())->method('save')->with($this->blockMock);
174193

175194
$this->dataPersistorMock->expects($this->any())
176195
->method('clear')
177196
->with('cms_block');
178197

179198
$this->messageManagerMock->expects($this->once())
180-
->method('addSuccess')
199+
->method('addSuccessMessage')
181200
->with(__('You saved the block.'));
182201

183202
$this->resultRedirect->expects($this->atLeastOnce())->method('setPath')->with('*/*/') ->willReturnSelf();
@@ -204,20 +223,17 @@ public function testSaveActionNoId()
204223
]
205224
);
206225

207-
$this->objectManagerMock->expects($this->atLeastOnce())
226+
$this->blockFactory->expects($this->atLeastOnce())
208227
->method('create')
209-
->with($this->equalTo(\Magento\Cms\Model\Block::class))
210228
->willReturn($this->blockMock);
211229

212-
$this->blockMock->expects($this->any())
213-
->method('load')
214-
->willReturnSelf();
215-
$this->blockMock->expects($this->any())
216-
->method('getId')
217-
->willReturn(false);
230+
$this->blockRepository->expects($this->once())
231+
->method('getById')
232+
->with($this->blockId)
233+
->willThrowException(new \Magento\Framework\Exception\NoSuchEntityException(__('Error message')));
218234

219235
$this->messageManagerMock->expects($this->once())
220-
->method('addError')
236+
->method('addErrorMessage')
221237
->with(__('This block no longer exists.'));
222238

223239
$this->resultRedirect->expects($this->atLeastOnce())->method('setPath')->with('*/*/') ->willReturnSelf();
@@ -237,22 +253,20 @@ public function testSaveAndContinue()
237253
]
238254
);
239255

240-
$this->objectManagerMock->expects($this->atLeastOnce())
256+
$this->blockFactory->expects($this->atLeastOnce())
241257
->method('create')
242-
->with($this->equalTo(\Magento\Cms\Model\Block::class))
243258
->willReturn($this->blockMock);
244259

245-
$this->blockMock->expects($this->any())
246-
->method('load')
247-
->willReturnSelf();
248-
$this->blockMock->expects($this->any())
249-
->method('getId')
250-
->willReturn(true);
260+
$this->blockRepository->expects($this->once())
261+
->method('getById')
262+
->with($this->blockId)
263+
->willReturn($this->blockMock);
264+
251265
$this->blockMock->expects($this->once())->method('setData');
252-
$this->blockMock->expects($this->once())->method('save');
266+
$this->blockRepository->expects($this->once())->method('save')->with($this->blockMock);
253267

254268
$this->messageManagerMock->expects($this->once())
255-
->method('addSuccess')
269+
->method('addSuccessMessage')
256270
->with(__('You saved the block.'));
257271

258272
$this->dataPersistorMock->expects($this->any())
@@ -279,24 +293,24 @@ public function testSaveActionThrowsException()
279293
]
280294
);
281295

282-
$this->objectManagerMock->expects($this->atLeastOnce())
296+
$this->blockFactory->expects($this->atLeastOnce())
283297
->method('create')
284-
->with($this->equalTo(\Magento\Cms\Model\Block::class))
285298
->willReturn($this->blockMock);
286299

287-
$this->blockMock->expects($this->any())
288-
->method('load')
289-
->willReturnSelf();
290-
$this->blockMock->expects($this->any())
291-
->method('getId')
292-
->willReturn(true);
300+
$this->blockRepository->expects($this->once())
301+
->method('getById')
302+
->with($this->blockId)
303+
->willReturn($this->blockMock);
304+
293305
$this->blockMock->expects($this->once())->method('setData');
294-
$this->blockMock->expects($this->once())->method('save')->willThrowException(new \Exception('Error message.'));
306+
$this->blockRepository->expects($this->once())->method('save')
307+
->with($this->blockMock)
308+
->willThrowException(new \Exception('Error message.'));
295309

296310
$this->messageManagerMock->expects($this->never())
297-
->method('addSuccess');
311+
->method('addSuccessMessage');
298312
$this->messageManagerMock->expects($this->once())
299-
->method('addException');
313+
->method('addExceptionMessage');
300314

301315
$this->dataPersistorMock->expects($this->any())
302316
->method('set')

0 commit comments

Comments
 (0)