Skip to content

Fix #30073 - Store specific text swatch attribute option label with v… #30114

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

Merged
7 changes: 4 additions & 3 deletions app/code/Magento/Swatches/Model/Plugin/EavAttribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ protected function processTextualSwatch(Attribute $attribute)
}
$defaultSwatchValue = reset($storeValues);
foreach ($storeValues as $storeId => $value) {
if (!$value) {
if ($value === null || $value === '') {
$value = $defaultSwatchValue;
}
$swatch = $this->loadSwatchIfExists($optionId, $storeId);
Expand All @@ -378,8 +378,9 @@ protected function processTextualSwatch(Attribute $attribute)
*/
protected function getAttributeOptionId($optionId)
{
if (substr($optionId, 0, 6) == self::BASE_OPTION_TITLE || substr($optionId, 0, 3) == self::API_OPTION_PREFIX) {
$optionId = isset($this->dependencyArray[$optionId]) ? $this->dependencyArray[$optionId] : null;
if (strpos((string)$optionId, self::BASE_OPTION_TITLE) === 0 ||
strpos((string)$optionId, self::API_OPTION_PREFIX) === 0) {
Comment on lines +381 to +382
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear what is checked here from the first glance. Can I ask you to move these conditions to another private method and name it so it displays what exactly it checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I'm not sure what is purpose of this check, and I'm not author of this part. I only replaced substr with strpos since it made more sense in such comparison in my opinion...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think we can live with this=)

$optionId = $this->dependencyArray[$optionId] ?? null;
}
return $optionId;
}
Expand Down
Loading