-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fallback for Product_links position attribute if not set in request #12650
Changes from 10 commits
944adbf
aa5bc4f
828f18b
005e3eb
27cc0b6
12653a1
6e93513
9e51c51
ccaf748
0b14b4e
da1833a
3d87303
30b494b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,18 +31,27 @@ class SaveHandler | |
private $linkResource; | ||
|
||
/** | ||
* @var linkTypeProvider | ||
*/ | ||
private $linkTypeProvider; | ||
|
||
/** | ||
* SaveHandler constructor. | ||
* @param MetadataPool $metadataPool | ||
* @param Link $linkResource | ||
* @param ProductLinkRepositoryInterface $productLinkRepository | ||
* @param \Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider | ||
*/ | ||
public function __construct( | ||
MetadataPool $metadataPool, | ||
Link $linkResource, | ||
ProductLinkRepositoryInterface $productLinkRepository | ||
ProductLinkRepositoryInterface $productLinkRepository, | ||
\Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider | ||
) { | ||
$this->metadataPool = $metadataPool; | ||
$this->linkResource = $linkResource; | ||
$this->productLinkRepository = $productLinkRepository; | ||
$this->linkTypeProvider = $linkTypeProvider; | ||
} | ||
|
||
/** | ||
|
@@ -55,17 +64,71 @@ public function execute($entityType, $entity) | |
{ | ||
$link = $entity->getData($this->metadataPool->getMetadata($entityType)->getLinkField()); | ||
if ($this->linkResource->hasProductLinks($link)) { | ||
/** @var \Magento\Catalog\Api\Data\ProductInterface $entity*/ | ||
/** @var \Magento\Catalog\Api\Data\ProductInterface $entity */ | ||
foreach ($this->productLinkRepository->getList($entity) as $link) { | ||
$this->productLinkRepository->delete($link); | ||
} | ||
} | ||
$productLinks = $entity->getProductLinks(); | ||
|
||
// Build links per type | ||
$linksByType = []; | ||
foreach ($productLinks as $link) { | ||
$linksByType[$link->getLinkType()][] = $link; | ||
} | ||
|
||
// Do check | ||
$hasPositionLinkType = $this->isPositionSet($linksByType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's confusing that a variable name starting with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it contains an array of boolean values per link type, I had the case that the positions were set for the products in "Related" but not set for products under "Cross sell" and so on, the same that under on Link type got the case of some products had position attribute and others don't, therefor I had to check if at least one product doesn't have a position under a product Link type all positions under this type will be ignored and user array index instead. regarding the question whether we need that amount of iterations, |
||
|
||
// Set array position as a fallback position if necessary | ||
foreach ($hasPositionLinkType as $linkType => $hasPosition) { | ||
if (!$hasPosition) { | ||
array_walk($linksByType[$linkType], function ($productLink, $position) { | ||
$productLink->setPosition(++$position); | ||
}); | ||
} | ||
} | ||
|
||
// Flatten multi-dimensional linksByType in ProductLinks | ||
$productLinks = array_reduce($linksByType, 'array_merge', []); | ||
|
||
if (count($productLinks) > 0) { | ||
foreach ($entity->getProductLinks() as $link) { | ||
$this->productLinkRepository->save($link); | ||
} | ||
} | ||
return $entity; | ||
} | ||
|
||
/** | ||
* Check if the position is set for all product links per link type. | ||
* array with boolean per type | ||
* | ||
* @param $linksByType | ||
* @return array | ||
*/ | ||
private function isPositionSet($linksByType) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking some more about it, it's confusing that a method name starting with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naming convention will be improved |
||
{ | ||
$linkTypes = $this->linkTypeProvider->getLinkTypes(); | ||
|
||
// Initialize isPositionSet for existent link types | ||
$isPositionSet = []; | ||
foreach (array_keys($linkTypes) as $typeName) { | ||
if (array_key_exists($typeName, $linksByType)) { | ||
$isPositionSet[$typeName] = count($linksByType[$typeName]) > 0; | ||
} | ||
} | ||
|
||
// Check if at least one link without position exists per Link type | ||
foreach ($linksByType as $type => $links) { | ||
foreach ($links as $link) { | ||
if (!array_key_exists('position', $link->getData())) { | ||
$isPositionSet[$type] = false; | ||
break; | ||
} | ||
} | ||
} | ||
|
||
return $isPositionSet; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After your last refactoring, we don't need the variable
$productLinks
in line 72 anymore. Call$entity->getProductLinks()
directly in line 72. By doing this, we adhere to the best practice that a variable shouldn't be changed anymore after initialisation.