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.

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

[CatalogPromotion] Applying Catalog Promotion with hardcoded rule and action #13002

Merged
merged 7 commits into from
Aug 26, 2021

Conversation

Zales0123
Copy link
Member

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets replaces #12999, based on #12990
License MIT

It's getting serious 💃

This is the first PR proposing an actual application of Catalog Promotions. Pretty dummy one, as it has hardcoded taxon rule and discount amount... But the point of the change, is to start with some simple architecture of services and handle the event thrown in #12990 🖖

I imagine we will be iterating over the proposed solution (especially ProductCatalogPromotionProcessorInterface), especially to take into account upcoming real Catalog Promotion rules and actions 🚀

@Zales0123 Zales0123 added the Feature New feature proposals. label Aug 25, 2021
@Zales0123 Zales0123 requested a review from a team as a code owner August 25, 2021 05:27
@Zales0123 Zales0123 force-pushed the catalog-promotions-application branch from 4ed2dd8 to 0d89fa9 Compare August 25, 2021 07:58

interface ProductCatalogPromotionApplicatorInterface
{
public function applyPercentageDiscount(ProductInterface $product, float $discount): void;
Copy link
Contributor

Choose a reason for hiding this comment

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

From an interface point of view, this product catalog promotion applicator will be only related with % discounts. What kind of different implementation were you thinking of?

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume that in the future we could also want to lower the price by some fixed amount, not a percentage of it. Then, we would probably have applyFixedDiscount(ProductInterface $product, int $amount): void function

$this->entityManager = $entityManager;
}

public function process(CatalogPromotionInterface $catalogPromotion): void
Copy link
Contributor

Choose a reason for hiding this comment

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

It is more a question to the whole team. When do we want to introduce the logic behind doing this update in chunks for huge catalogs?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a good and important question, but I believe we could skip it for now. With a proper separation of services, we would be able to make the processing asynchronous/delayed in the places when it needs to be like that 🖖

@GSadee GSadee merged commit 33b7e5d into Sylius:master Aug 26, 2021
@GSadee
Copy link
Member

GSadee commented Aug 26, 2021

Thank you, Mateusz! 🥇

@Zales0123 Zales0123 deleted the catalog-promotions-application branch August 26, 2021 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants