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.

Sign up for GitHub

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

Jump to bottom

[API][CatalogPromotion] Reapplying catalog promotions on rule update #13129

Merged
merged 6 commits into from Sep 29, 2021
Merged

[API][CatalogPromotion] Reapplying catalog promotions on rule update #13129

merged 6 commits into from Sep 29, 2021

Conversation

SirDomin
Copy link
Contributor

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

@SirDomin SirDomin requested a review from a team as a code owner September 21, 2021 21:56
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Sep 21, 2021
@SirDomin SirDomin force-pushed the reapply-cp-after-rule-change branch 2 times, most recently from 80cdd3a to b2f86a2 Compare September 21, 2021 22:57
GSadee
GSadee reviewed Sep 22, 2021
View reviewed changes
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingCatalogPromotionsContext.php Show resolved Hide resolved
@SirDomin SirDomin force-pushed the reapply-cp-after-rule-change branch 5 times, most recently from 45ea582 to 1befd29 Compare September 23, 2021 07:23
GSadee
GSadee reviewed Sep 24, 2021
View reviewed changes
Copy link
Member

@GSadee GSadee left a comment

Choose a reason for hiding this comment

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

As we have the possibility to define more than one, I would be for adding another scenario for checking reapplying after adding a second rule to catalog promotion that already has one defined

...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
...omotion/applying_catalog_promotions/reapplying_catalog_promotion_after_changing_rule.feature Outdated Show resolved Hide resolved
lchrusciel
lchrusciel reviewed Sep 27, 2021
View reviewed changes
src/Sylius/Bundle/CoreBundle/Doctrine/ORM/ChannelPricingRepository.php
@@ -21,8 +21,7 @@ class ChannelPricingRepository extends EntityRepository implements ChannelPricin
public function findWithDiscountedPrice(): array
{
return $this->createQueryBuilder('o')
->andWhere('o.originalPrice IS NOT NULL')
->andWhere('o.originalPrice > o.price')
->andWhere('o.appliedPromotions != \'[]\'')
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure, this notation will be the same for postgres?

Copy link
Member

Choose a reason for hiding this comment

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

As we don't have a build for postgres, I am not 100% sure

@GSadee GSadee changed the title [API][CatalogPromotions] reapply catalog promotions on rule update [API][CatalogPromotions] Reapplying catalog promotions on rule update Sep 28, 2021
@GSadee GSadee changed the title [API][CatalogPromotions] Reapplying catalog promotions on rule update [API][CatalogPromotion] Reapplying catalog promotions on rule update Sep 28, 2021
AdamKasp
AdamKasp approved these changes Sep 29, 2021
View reviewed changes
.../applying_catalog_promotions/reapplying_catalog_promotions_after_editing_their_rules.feature
And the visitor should see that the "PHP Mug" variant is discounted from "$5.00" to "$2.50" with "Mug sale" promotion

@api
Scenario: Reapplying catalog promotion after adding another action
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Scenario: Reapplying catalog promotion after adding another action
Scenario: Reapplying catalog promotion after adding another rule

@AdamKasp AdamKasp merged commit 18135d7 into Sylius:master Sep 29, 2021
@AdamKasp
Copy link
Contributor

AdamKasp commented Sep 29, 2021 •
edited

Thank you, Dominik and Grzegorz 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GSadee GSadee GSadee left review comments

@Zales0123 Zales0123 Zales0123 left review comments

@lchrusciel lchrusciel lchrusciel left review comments

@AdamKasp AdamKasp AdamKasp approved these changes

Assignees
No one assigned
Labels
API APIs related issues and PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@SirDomin @AdamKasp @GSadee @Zales0123 @lchrusciel