magento/magento2#40104 #40105
magento/magento2#40104 #40105
Conversation
Refactoring. Add tests to check single-option-price issue.
|
Hi @swnsma. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
@magento run all tests |
ct-prd-projects-boards-automation
bot
added
the
Priority: P2
| @@ -196,26 +196,25 @@ public function _resetState(): void | |||
| } | |||
|
|
|||
| /** | |||
| * Check whether Configurable Product have more than one children products | |||
| * Check whether Configurable options do not have price difference | |||
| * | |||
| * @param SaleableInterface $product | |||
| * @return bool | |||
| */ | |||
| public function isChildProductsOfEqualPrices(SaleableInterface $product): bool | |||
There was a problem hiding this comment.
Should an early return be added to the beginning of this method to exit if the product is not configurable?
if ($product->getTypeId() !== 'configurable') {
return false;
}There was a problem hiding this comment.
Thank you for your comment!
I've just realized that we don't need any arguments, as an object of this class will already have a product assigned.
There was a problem hiding this comment.
I should not need a new instance of a class to call a one method with a different product.
- isChildProductsOfEqualPrices is no longer backwards compatible checks $this->product instead of parameter.
- proposed alternative reduces flexibility, I should not need a new instance to just check a different product. the change basically makes the code single use.
I propose to revert back to checking the parameter.
EDIT: I just saw that the price is checked against the instance on the class. Please disregard above.
Refactoring. Simplify original logic. Fix static tests.
| @@ -63,7 +63,7 @@ public function getMaxPriceForConfigurableProduct($productId) | |||
| $result = $connection->fetchRow($select); | |||
|
|
|||
| if ($result && isset($result['max_price'])) { | |||
| return $result['max_price']; | |||
| return (float)$result['max_price']; | |||
There was a problem hiding this comment.
return (float) $result['max_price'];
| if ($maxPrice == 0) { | ||
|
|
||
| $maxPrice = $this->configurableMaxPriceCalculator | ||
| ->getMaxPriceForConfigurableProduct((int)$this->product->getId()); |
There was a problem hiding this comment.
->getMaxPriceForConfigurableProduct((int) $this->product->getId());
Fix code styles.
|
@magento run all tests |
Description (*)
Refactoring.
Add tests to check single-option-price issue.
Fixed Issues (if relevant)
Manual testing scenarios (*)
Contribution checklist (*)