Skip to content

Commit 4af80a2

Browse files
dhorytskyidhorytskyi
committed
MAGETWO-86125: Sorting on price of configurable products in catalog not working properly
1 parent 59f6edb commit 4af80a2

File tree

2 files changed

+182
-93
lines changed

2 files changed

+182
-93
lines changed

‎app/code/Magento/Config/Model/Config.php

Lines changed: 146 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
*/
66
namespace Magento\Config\Model;
77

8+
use Magento\Config\Model\Config\Structure\Element\Group;
9+
use Magento\Config\Model\Config\Structure\Element\Field;
10+
811
/**
912
* Backend config model
1013
* Used to save configuration
@@ -126,9 +129,9 @@ public function save()
126129

127130
$oldConfig = $this->_getConfig(true);
128131

129-
/* @var $deleteTransaction \Magento\Framework\DB\Transaction */
132+
/** @var \Magento\Framework\DB\Transaction $deleteTransaction */
130133
$deleteTransaction = $this->_transactionFactory->create();
131-
/* @var $saveTransaction \Magento\Framework\DB\Transaction */
134+
/** @var \Magento\Framework\DB\Transaction $saveTransaction */
132135
$saveTransaction = $this->_transactionFactory->create();
133136

134137
$changedPaths = [];
@@ -147,17 +150,8 @@ public function save()
147150
$deleteTransaction
148151
);
149152

150-
if (isset($groupData['fields'])) {
151-
$groupPath = $sectionId . '/' . $groupId;
152-
foreach ($groupData['fields'] as $fieldId => $fieldData) {
153-
/** @var $field \Magento\Config\Model\Config\Structure\Element\Field */
154-
$field = $this->_configStructure->getElement($groupPath . '/' . $fieldId);
155-
$path = $field->getGroupPath() . '/' . $fieldId;
156-
if ($this->isValueChanged($oldConfig, $path, $fieldData)) {
157-
$changedPaths[] = $path;
158-
}
159-
}
160-
}
153+
$groupChangedPaths = $this->getChangedPaths($sectionId, $groupId, $groupData, $oldConfig, $extraOldGroups);
154+
$changedPaths = \array_merge($changedPaths, $groupChangedPaths);
161155
}
162156

163157
try {
@@ -185,6 +179,79 @@ public function save()
185179
return $this;
186180
}
187181

182+
/**
183+
* Map field name if they were cloned
184+
*
185+
* @param Group $group
186+
* @param string $fieldId
187+
* @return string
188+
*/
189+
private function getOriginalFieldId(Group $group, string $fieldId): string
190+
{
191+
if ($group->shouldCloneFields()) {
192+
$cloneModel = $group->getCloneModel();
193+
194+
/** @var \Magento\Config\Model\Config\Structure\Element\Field $field */
195+
foreach ($group->getChildren() as $field) {
196+
foreach ($cloneModel->getPrefixes() as $prefix) {
197+
if ($prefix['field'] . $field->getId() === $fieldId) {
198+
$fieldId = $field->getId();
199+
break(2);
200+
}
201+
}
202+
}
203+
}
204+
205+
return $fieldId;
206+
}
207+
208+
/**
209+
* Get field object
210+
*
211+
* @param string $sectionId
212+
* @param string $groupId
213+
* @param string $fieldId
214+
* @return Field
215+
*/
216+
private function getField(string $sectionId, string $groupId, string $fieldId): Field
217+
{
218+
/** @var \Magento\Config\Model\Config\Structure\Element\Group $group */
219+
$group = $this->_configStructure->getElement($sectionId . '/' . $groupId);
220+
$fieldPath = $group->getPath() . '/' . $this->getOriginalFieldId($group, $fieldId);
221+
$field = $this->_configStructure->getElement($fieldPath);
222+
223+
return $field;
224+
}
225+
226+
/**
227+
* Get field path
228+
*
229+
* @param Field $field
230+
* @param array &$oldConfig Need for compatibility with _processGroup()
231+
* @param array &$extraOldGroups Need for compatibility with _processGroup()
232+
* @return string
233+
*/
234+
private function getFieldPath(Field $field, array &$oldConfig, array &$extraOldGroups): string
235+
{
236+
$path = $field->getGroupPath() . '/' . $field->getId();
237+
238+
/**
239+
* Look for custom defined field path
240+
*/
241+
$configPath = $field->getConfigPath();
242+
if ($configPath && strrpos($configPath, '/') > 0) {
243+
// Extend old data with specified section group
244+
$configGroupPath = substr($configPath, 0, strrpos($configPath, '/'));
245+
if (!isset($extraOldGroups[$configGroupPath])) {
246+
$oldConfig = $this->extendConfig($configGroupPath, true, $oldConfig);
247+
$extraOldGroups[$configGroupPath] = true;
248+
}
249+
$path = $configPath;
250+
}
251+
252+
return $path;
253+
}
254+
188255
/**
189256
* Check is config value changed
190257
*
@@ -193,7 +260,7 @@ public function save()
193260
* @param array $fieldData
194261
* @return bool
195262
*/
196-
private function isValueChanged(array $oldConfig, string $path, array $fieldData)
263+
private function isValueChanged(array $oldConfig, string $path, array $fieldData): bool
197264
{
198265
if (isset($oldConfig[$path]['value'])) {
199266
$result = !isset($fieldData['value']) || $oldConfig[$path]['value'] !== $fieldData['value'];
@@ -204,6 +271,52 @@ private function isValueChanged(array $oldConfig, string $path, array $fieldData
204271
return $result;
205272
}
206273

274+
/**
275+
* Get changed paths
276+
*
277+
* @param string $sectionId
278+
* @param string $groupId
279+
* @param array $groupData
280+
* @param array &$oldConfig
281+
* @param array &$extraOldGroups
282+
* @return array
283+
*/
284+
private function getChangedPaths(
285+
string $sectionId,
286+
string $groupId,
287+
array $groupData,
288+
array &$oldConfig,
289+
array &$extraOldGroups
290+
): array {
291+
$changedPaths = [];
292+
293+
if (isset($groupData['fields'])) {
294+
foreach ($groupData['fields'] as $fieldId => $fieldData) {
295+
$field = $this->getField($sectionId, $groupId, $fieldId);
296+
$path = $this->getFieldPath($field, $oldConfig, $extraOldGroups);
297+
if ($this->isValueChanged($oldConfig, $path, $fieldData)) {
298+
$changedPaths[] = $path;
299+
}
300+
}
301+
}
302+
303+
if (isset($groupData['groups'])) {
304+
$subSectionId = $sectionId . '/' . $groupId;
305+
foreach ($groupData['groups'] as $subGroupId => $subGroupData) {
306+
$subGroupChangedPaths = $this->getChangedPaths(
307+
$subSectionId,
308+
$subGroupId,
309+
$subGroupData,
310+
$oldConfig,
311+
$extraOldGroups
312+
);
313+
$changedPaths = \array_merge($changedPaths, $subGroupChangedPaths);
314+
}
315+
}
316+
317+
return $changedPaths;
318+
}
319+
207320
/**
208321
* Process group data
209322
*
@@ -216,9 +329,6 @@ private function isValueChanged(array $oldConfig, string $path, array $fieldData
216329
* @param \Magento\Framework\DB\Transaction $saveTransaction
217330
* @param \Magento\Framework\DB\Transaction $deleteTransaction
218331
* @return void
219-
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
220-
* @SuppressWarnings(PHPMD.NPathComplexity)
221-
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
222332
*/
223333
protected function _processGroup(
224334
$groupId,
@@ -231,92 +341,42 @@ protected function _processGroup(
231341
\Magento\Framework\DB\Transaction $deleteTransaction
232342
) {
233343
$groupPath = $sectionPath . '/' . $groupId;
234-
$scope = $this->getScope();
235-
$scopeId = $this->getScopeId();
236-
$scopeCode = $this->getScopeCode();
237-
/**
238-
*
239-
* Map field names if they were cloned
240-
*/
241-
/** @var $group \Magento\Config\Model\Config\Structure\Element\Group */
242-
$group = $this->_configStructure->getElement($groupPath);
243344

244-
// set value for group field entry by fieldname
245-
// use extra memory
246-
$fieldsetData = [];
247345
if (isset($groupData['fields'])) {
248-
if ($group->shouldCloneFields()) {
249-
$cloneModel = $group->getCloneModel();
250-
$mappedFields = [];
251-
252-
/** @var $field \Magento\Config\Model\Config\Structure\Element\Field */
253-
foreach ($group->getChildren() as $field) {
254-
foreach ($cloneModel->getPrefixes() as $prefix) {
255-
$mappedFields[$prefix['field'] . $field->getId()] = $field->getId();
256-
}
257-
}
258-
}
259-
foreach ($groupData['fields'] as $fieldId => $fieldData) {
260-
$fieldsetData[$fieldId] = is_array(
261-
$fieldData
262-
) && isset(
263-
$fieldData['value']
264-
) ? $fieldData['value'] : null;
265-
}
346+
/** @var \Magento\Config\Model\Config\Structure\Element\Group $group */
347+
$group = $this->_configStructure->getElement($groupPath);
266348

349+
// set value for group field entry by fieldname
350+
// use extra memory
351+
$fieldsetData = [];
267352
foreach ($groupData['fields'] as $fieldId => $fieldData) {
268-
$originalFieldId = $fieldId;
269-
if ($group->shouldCloneFields() && isset($mappedFields[$fieldId])) {
270-
$originalFieldId = $mappedFields[$fieldId];
271-
}
272-
/** @var $field \Magento\Config\Model\Config\Structure\Element\Field */
273-
$field = $this->_configStructure->getElement($groupPath . '/' . $originalFieldId);
274-
353+
$field = $this->getField($sectionPath, $groupId, $fieldId);
275354
/** @var \Magento\Framework\App\Config\ValueInterface $backendModel */
276-
$backendModel = $field->hasBackendModel() ? $field
277-
->getBackendModel() : $this
278-
->_configValueFactory
279-
->create();
355+
$backendModel = $field->hasBackendModel()
356+
? $field->getBackendModel()
357+
: $this->_configValueFactory->create();
280358

359+
if (!isset($fieldData['value'])) {
360+
$fieldData['value'] = null;
361+
}
362+
$fieldsetData[$fieldId] = $fieldData['value'];
281363
$data = [
282364
'field' => $fieldId,
283365
'groups' => $groups,
284366
'group_id' => $group->getId(),
285-
'scope' => $scope,
286-
'scope_id' => $scopeId,
287-
'scope_code' => $scopeCode,
367+
'scope' => $this->getScope(),
368+
'scope_id' => $this->getScopeId(),
369+
'scope_code' => $this->getScopeCode(),
288370
'field_config' => $field->getData(),
289-
'fieldset_data' => $fieldsetData
371+
'fieldset_data' => $fieldsetData,
290372
];
291373
$backendModel->addData($data);
292-
293374
$this->_checkSingleStoreMode($field, $backendModel);
294375

295-
if (false == isset($fieldData['value'])) {
296-
$fieldData['value'] = null;
297-
}
298-
299-
$path = $field->getGroupPath() . '/' . $fieldId;
300-
/**
301-
* Look for custom defined field path
302-
*/
303-
if ($field && $field->getConfigPath()) {
304-
$configPath = $field->getConfigPath();
305-
if (!empty($configPath) && strrpos($configPath, '/') > 0) {
306-
// Extend old data with specified section group
307-
$configGroupPath = substr($configPath, 0, strrpos($configPath, '/'));
308-
if (!isset($extraOldGroups[$configGroupPath])) {
309-
$oldConfig = $this->extendConfig($configGroupPath, true, $oldConfig);
310-
$extraOldGroups[$configGroupPath] = true;
311-
}
312-
$path = $configPath;
313-
}
314-
}
315-
316-
$inherit = !empty($fieldData['inherit']);
317-
376+
$path = $this->getFieldPath($field, $extraOldGroups, $oldConfig);
318377
$backendModel->setPath($path)->setValue($fieldData['value']);
319378

379+
$inherit = !empty($fieldData['inherit']);
320380
if (isset($oldConfig[$path])) {
321381
$backendModel->setConfigId($oldConfig[$path]['config_id']);
322382

‎app/code/Magento/Config/Test/Unit/Model/ConfigTest.php

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public function testSaveToCheckScopeDataSet()
162162
)->method(
163163
'dispatch'
164164
)->with(
165-
$this->equalTo('admin_system_config_changed_section_'),
165+
$this->equalTo('admin_system_config_changed_section_section'),
166166
$this->arrayHasKey('website')
167167
);
168168

@@ -171,30 +171,59 @@ public function testSaveToCheckScopeDataSet()
171171
)->method(
172172
'dispatch'
173173
)->with(
174-
$this->equalTo('admin_system_config_changed_section_'),
174+
$this->equalTo('admin_system_config_changed_section_section'),
175175
$this->arrayHasKey('store')
176176
);
177177

178178
$group = $this->createMock(\Magento\Config\Model\Config\Structure\Element\Group::class);
179+
$group->method('getPath')->willReturn('section/1');
179180

180181
$field = $this->createMock(\Magento\Config\Model\Config\Structure\Element\Field::class);
182+
$field->method('getGroupPath')->willReturn('section/1');
183+
$field->method('getId')->willReturn('key');
181184

182185
$this->_configStructure->expects(
183186
$this->at(0)
184187
)->method(
185188
'getElement'
186189
)->with(
187-
'/1'
190+
'section/1'
188191
)->will(
189192
$this->returnValue($group)
190193
);
191-
192194
$this->_configStructure->expects(
193195
$this->at(1)
194196
)->method(
195197
'getElement'
196198
)->with(
197-
'/1/key'
199+
'section/1'
200+
)->will(
201+
$this->returnValue($group)
202+
);
203+
$this->_configStructure->expects(
204+
$this->at(2)
205+
)->method(
206+
'getElement'
207+
)->with(
208+
'section/1/key'
209+
)->will(
210+
$this->returnValue($field)
211+
);
212+
$this->_configStructure->expects(
213+
$this->at(3)
214+
)->method(
215+
'getElement'
216+
)->with(
217+
'section/1'
218+
)->will(
219+
$this->returnValue($group)
220+
);
221+
$this->_configStructure->expects(
222+
$this->at(4)
223+
)->method(
224+
'getElement'
225+
)->with(
226+
'section/1/key'
198227
)->will(
199228
$this->returnValue($field)
200229
);
@@ -206,7 +235,7 @@ public function testSaveToCheckScopeDataSet()
206235
$this->_storeManager->expects($this->any())->method('isSingleStoreMode')->will($this->returnValue(true));
207236

208237
$this->_model->setWebsite('website');
209-
238+
$this->_model->setSection('section');
210239
$this->_model->setGroups(['1' => ['fields' => ['key' => ['data']]]]);
211240

212241
$backendModel = $this->createPartialMock(
@@ -234,7 +263,7 @@ public function testSaveToCheckScopeDataSet()
234263
)->method(
235264
'setPath'
236265
)->with(
237-
'/key'
266+
'section/1/key'
238267
)->will(
239268
$this->returnValue($backendModel)
240269
);

0 commit comments

Comments
 (0)