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

Catalog promotion priority #13465

Merged
merged 3 commits into from
Jan 12, 2022
Merged

Catalog promotion priority #13465

merged 3 commits into from
Jan 12, 2022

Conversation

Rafikooo
Copy link
Contributor

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

@Rafikooo Rafikooo requested a review from a team as a code owner January 11, 2022 07:49
@probot-autolabeler probot-autolabeler bot added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Jan 11, 2022
@Rafikooo Rafikooo added the Behat Issues and PRs aimed at improving Behat usage. label Jan 11, 2022
@@ -7,7 +7,7 @@
"code": "mugs_discount",
"startDate": null,
"endDate": null,
"priority": 1000,
"priority": 0,
Copy link
Contributor Author

@Rafikooo Rafikooo Jan 11, 2022

Choose a reason for hiding this comment

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

Having one row in the database when updating, gedmo sets the priority to 0, even if the value given is different, similar behavior like described in the other comment

@@ -12,7 +12,6 @@ Feature: Seeing catalog promotion's details
And it applies also on "T-Shirt" product
And it reduces also price by fixed "$10.00" in the "Web-US" channel
And the catalog promotion "Winter sale" operates between "2021-11-10" and "2022-01-08"
And its priority is 1200
Copy link
Contributor Author

@Rafikooo Rafikooo Jan 11, 2022

Choose a reason for hiding this comment

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

The default priority value when creating is 0, which is in this scenario. This step updates the database row, but gedmo cannot allow to set different priority values when only one row existing thus I decided to remove that act.
Alternatively, I can set this value to 1200 during creating, and then gedmo allows it

@@ -24,4 +23,3 @@ Feature: Seeing catalog promotion's details
And it should apply on "PHP T-Shirt" variant
And it should apply on "T-Shirt" product
And it should start at "2021-11-10" and end at "2022-01-08"
And its priority should be 1200
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With current behavior, I can assert that the priority value is 0

@Rafikooo Rafikooo requested a review from GSadee January 12, 2022 06:50

@api @ui
Scenario: Adding a catalog promotion with a priority lower than all existing ones
increases the priority value of other catalog promotions by 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if we need these details, in the scenario we are describing what we do, not an effect, IMO assertions are enough to understand priorities logic :)

@GSadee GSadee merged commit 25648cd into Sylius:1.11 Jan 12, 2022
@GSadee
Copy link
Member

GSadee commented Jan 12, 2022

Thank you, Rafał! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Behat Issues and PRs aimed at improving Behat usage. Documentation Documentation related issues and PRs - requests, fixes, proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants