Skip to content
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.

Sign up for GitHub

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

Custom option type select - Allow modify list of single selection option types #21744

Merged
merged 3 commits into from
May 8, 2019
Merged

Custom option type select - Allow modify list of single selection option types #21744

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ea824e
Custom option type select - Allow modify list of single selection opt…
ihor-sviziev Mar 14, 2019
996cb1a
Fix static tests.
nmalevanec Apr 12, 2019
03cfa37
Custom option type select - Allow modify list of single selection opt…
ihor-sviziev Apr 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions app/code/Magento/Catalog/Model/Product/Option/Type/Select.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,35 @@ class Select extends \Magento\Catalog\Model\Product\Option\Type\DefaultType
*/
protected $string;

/**
* @var array
*/
private $singleSelectionTypes;

/**
* @param \Magento\Checkout\Model\Session $checkoutSession
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
* @param \Magento\Framework\Stdlib\StringUtils $string
* @param \Magento\Framework\Escaper $escaper
* @param array $data
* @param array $singleSelectionTypes
*/
public function __construct(
\Magento\Checkout\Model\Session $checkoutSession,
\Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
\Magento\Framework\Stdlib\StringUtils $string,
\Magento\Framework\Escaper $escaper,
array $data = []
array $data = [],
array $singleSelectionTypes = []
) {
$this->string = $string;
$this->_escaper = $escaper;
parent::__construct($checkoutSession, $scopeConfig, $data);

$this->singleSelectionTypes = $singleSelectionTypes ?: [
\Magento\Catalog\Api\Data\ProductCustomOptionInterface::OPTION_TYPE_DROP_DOWN,
\Magento\Catalog\Api\Data\ProductCustomOptionInterface::OPTION_TYPE_RADIO,
];
Copy link
Contributor

Choose a reason for hiding this comment

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

@ihor-sviziev, could you please clarify, why we need default values here, cause we already have them in di.xml?
Also, here we have numeric array, and in di.xml we have associative array for default values.

Copy link
Contributor Author

@ihor-sviziev ihor-sviziev Apr 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @nmalevanec,
It's needed for backward compatiblity, when last argument will not be sent. For instance when someone extended this class in his code when this attribute was missing.

About numeric array VS associative array - ok, will unify them

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @ihor-sviziev , consider defining default arguments in the di.xml, with this approach this parameter will become configurable instead of hardcoded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sidolov,
As we discussed in Slack - it's needed for backward compatibility, in case if someone will extend this class & will not pass last argument in parent::__construct(...) - we have to have default value, similar as we're getting object from object manager by fallback.

}

/**
Expand Down Expand Up @@ -310,10 +322,6 @@ public function getOptionSku($optionValue, $skuDelimiter)
*/
protected function _isSingleSelection()
{
$single = [
\Magento\Catalog\Api\Data\ProductCustomOptionInterface::OPTION_TYPE_DROP_DOWN,
\Magento\Catalog\Api\Data\ProductCustomOptionInterface::OPTION_TYPE_RADIO,
];
return in_array($this->getOption()->getType(), $single);
return in_array($this->getOption()->getType(), $this->singleSelectionTypes, true);
}
}
8 changes: 8 additions & 0 deletions app/code/Magento/Catalog/etc/di.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1164,4 +1164,12 @@
</argument>
</arguments>
</type>
<type name="Magento\Catalog\Model\Product\Option\Type\Select">
<arguments>
<argument name="singleSelectionTypes" xsi:type="array">
<item name="drop_down" xsi:type="const">Magento\Catalog\Api\Data\ProductCustomOptionInterface::OPTION_TYPE_DROP_DOWN</item>
<item name="radio" xsi:type="const">Magento\Catalog\Api\Data\ProductCustomOptionInterface::OPTION_TYPE_RADIO</item>
</argument>
</arguments>
</type>
</config>