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
[CatalogPromotions] Inform about promotions applied on variants #13035
[CatalogPromotions] Inform about promotions applied on variants #13035
Conversation
src/Sylius/Bundle/CoreBundle/Applicator/CatalogPromotionApplicator.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/CoreBundle/Migrations/Version20210830193340.php
Outdated
Show resolved
Hide resolved
@@ -30,6 +30,9 @@ class ChannelPricing implements ChannelPricingInterface | |||
/** @var int|null */ | |||
protected $originalPrice; | |||
|
|||
/** @var array */ | |||
protected $appliedPromotions = []; |
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.
Nothing to change right now, but just a question for the future - do we plan to have object references here?
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.
I don't know yet - we will see what needs we will have when the feature would be growing. But I do not rule it out 🖖
/** @var string $code */ | ||
$code = $catalogPromotion->getCode(); | ||
|
||
return [$code => ['name' => $label]]; |
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.
If we are saving only one label in one translation, there will be no possibility to translate this label on variants 🤔
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.
Indeed. Probably we should save the label in each locale - it makes this data structure a little bit more complicated, but benefits are undeniable. I would provide is as an improvement in the separate PR, wdyt?
src/Sylius/Bundle/CoreBundle/Applicator/CatalogPromotionApplicator.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/CoreBundle/Applicator/CatalogPromotionApplicator.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/CoreBundle/Processor/DummyCatalogPromotionProcessor.php
Outdated
Show resolved
Hide resolved
- improve behat steps configuration - add description to migration
802246c
to
f59b034
Compare
Thank you, Mateusz! 🎉 |
… on variant` (arti0090) This PR was merged into the 1.11-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | License | MIT Scenario that will extend #13035 Commits ------- f4585fd [CP] Add scenario for variant
We need to keep the information about applied catalog promotions on each
ChannelPricing
. I propose to start simple, with an array of arrays 💃 to make it happen sooner than later. If we decide in the future we need some more sophisticated structure - we can always change it 🚀