-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Catalog Promotions] Fixing issues with multiple variants #13080
Conversation
arti0090
commented
Sep 9, 2021
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
License | MIT |
public function addAppliedPromotion(array $promotion): void | ||
{ | ||
if ($this->appliedPromotions === null) { | ||
$this->appliedPromotions = $promotion; | ||
return; | ||
} | ||
|
||
$this->appliedPromotions = array_merge_recursive($this->appliedPromotions, $promotion); | ||
} |
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.
We should consider some changes for
/** @var array */ protected $appliedPromotions = [];
value.
At this moment it does not safe us from storing null value. This lead to problem with array_merge where one value sometimes is null.
Sugestion: change this to protected array $appliedPromotions = [];
make adjust in orm file and create the migration that will make this field not null and put default value of empty array.
f8fbe1c
to
376602d
Compare
376602d
to
7dccf0f
Compare
...otion/applying_catalog_promotions/seeing_applied_catalog_promotions_on_variants_list.feature
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/CoreBundle/Provider/CatalogPromotionVariantsProvider.php
Show resolved
Hide resolved
...es/promotion/applying_catalog_promotions/seeing_applied_catalog_promotion_on_variant.feature
Outdated
Show resolved
Hide resolved
...otion/applying_catalog_promotions/seeing_applied_catalog_promotions_on_variants_list.feature
Outdated
Show resolved
Hide resolved
|
||
/** @var CatalogPromotionRuleInterface $catalogPromotionRule */ | ||
$catalogPromotionRule = $this->catalogPromotionRuleFactory->createNew(); | ||
$catalogPromotionRule->setType(CatalogPromotionRuleInterface::TYPE_FOR_VARIANTS); | ||
$catalogPromotionRule->setConfiguration(['variants' => [$variant->getCode()]]); | ||
$catalogPromotionRule->setConfiguration(['variants' => array_values($variantCodes)]); |
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.
$catalogPromotionRule->setConfiguration(['variants' => array_values($variantCodes)]); | |
$catalogPromotionRule->setConfiguration(['variants' => $variantCodes]); |
should be enough 🤔
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.
then it creates something like: 'variants' => 0: 'code1' , 1:'code2' that is why I used array_values
src/Sylius/Bundle/CoreBundle/Provider/CatalogPromotionVariantsProvider.php
Outdated
Show resolved
Hide resolved
f52a947
to
ee8b0e6
Compare
ee8b0e6
to
41d7c2f
Compare
Thanks, @arti0090! 🥇 |