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] Process all catalog promotions when catalog promotion is created #13550

Merged

Conversation

SirDomin
Copy link
Contributor

Q A
Branch? 1.11
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

During Handling CatalogPromotionCreated we weren't processing all available catalog promotions, now it takes all into account.

@SirDomin SirDomin requested a review from a team as a code owner January 26, 2022 21:41
@SirDomin SirDomin added the Bug Confirmed bugs or bugfixes. label Jan 26, 2022
Comment on lines 138 to 146
When I want to create a new catalog promotion
And I specify its code as "winter_sale"
And I name it "Winter sale"
And I specify its label as "Winter sale" in "English (United States)"
And I describe it as "This promotion gives a 50% discount on all products" in "English (United States)"
And I add scope that applies on variants "PHP T-Shirt" variant and "Kotlin T-Shirt" variant
And I add action that gives "50%" percentage discount
And I make it start yesterday and ends tomorrow
And I make it available in channel "United States"
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we should merge it into single step.

@SirDomin SirDomin changed the title Process all catalog promotions when catalog promotion is created [CatalogPromotion] Process all catalog promotions when catalog promotion is created Jan 27, 2022
@SirDomin SirDomin force-pushed the create-catalog-promotion-with-priorities branch from 0023d15 to af319f3 Compare January 27, 2022 11:48
Comment on lines +140 to +149
And I specify its code as "winter_sale"
And I name it "Winter sale"
And I specify its label as "Winter sale" in "English (United States)"
And I describe it as "This promotion gives a 5$ discount on all products" in "English (United States)"
And I set its priority to 150
And I add scope that applies on variants "PHP T-Shirt" variant and "Kotlin T-Shirt" variant
And I add action that gives "$5" of fixed discount in the "United States" channel
And I make it start yesterday and ends tomorrow
And I make it available in channel "United States"
And I add it
Copy link
Member

Choose a reason for hiding this comment

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

We should definitely think about some more complex steps for promotion creation to avoid a long wall of text each time we want to create a catalog promotion 💃 Something like:

When I create a "Winter sale" catalog promotion with priority 150 that discounting "PHP T-Shirt" variant by "$5" in "United States" channel

? Of course it would need to resolve some default values, but may be worth to be considered 🖖

@Zales0123 Zales0123 merged commit 894c6f1 into Sylius:1.11 Jan 27, 2022
@Zales0123
Copy link
Member

Thanks, @SirDomin! 🥇

GSadee added a commit that referenced this pull request Feb 2, 2022
…tion (TheMilek)

This PR was merged into the 1.11 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.11  <!-- see the comment below -->
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no <!-- don't forget to update the UPGRADE-*.md file -->
| Related tickets | #13550
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.10 or 1.11 branch(the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

We should also have scenario for creating an exclusive catalog promotion to control it's behaviour with priorities

Commits
-------

aa9ca37 [Behat] scenario for creating an exclusive catalog promotion
43e8073 Merge branch '1.11' of github.com:Sylius/Sylius into behat-test-with-exclusive-promotion
8663504 [Behat] outdated class correction
583ca24 [API] catalog promotion request content correction
3bf52b5 [Behat] refactoring implementation of the steps to use a private method instead
GSadee added a commit to Sylius/SyliusApiBundle that referenced this pull request Feb 2, 2022
…tion (TheMilek)

This PR was merged into the 1.11 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.11  <!-- see the comment below -->
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no <!-- don't forget to update the UPGRADE-*.md file -->
| Related tickets | Sylius/Sylius#13550
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.10 or 1.11 branch(the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->

We should also have scenario for creating an exclusive catalog promotion to control it's behaviour with priorities

Commits
-------

aa9ca377d0229898b0ad152eec4dbed4916ca5d3 [Behat] scenario for creating an exclusive catalog promotion
43e8073dd64ed65ee94ceea6620cb6a9f382d250 Merge branch '1.11' of github.com:Sylius/Sylius into behat-test-with-exclusive-promotion
8663504e90d7418fc69d6f7dc8887023e1d40c88 [Behat] outdated class correction
583ca248d2497a64079690ffb8d91684cc650024 [API] catalog promotion request content correction
3bf52b505e5c0391312412e523d310a280edcb76 [Behat] refactoring implementation of the steps to use a private method instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bugs or bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants