Skip to content

Add cart rule module validation - #34875

Merged
nicosomb merged 1 commit into
PrestaShop:developfrom
Hlavtox:cart-rule-validation
Apr 12, 2024
Merged

Add cart rule module validation#34875
nicosomb merged 1 commit into
PrestaShop:developfrom
Hlavtox:cart-rule-validation

Conversation

@Hlavtox

@Hlavtox Hlavtox commented Dec 19, 2023

Copy link
Copy Markdown
Member
Questions Answers
Branch? develop
Description? Adds a hook to create endless possibilities of cart rules.
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
How to test? Indicate how to verify that this change works as expected.
UI Tests https://github.com/florine2623/testing_pr/actions/runs/8340742024
Fixed issue or discussion?
Related PRs PrestaShop/autoupgrade#672
Sponsor company TRENDO s.r.o.

New hook

  • I added a new hook called actionValidateCartRule which allows you to create absolutely endless possibilities of rules, when the cart rule should be valid. Also, you can use this to speed up prestashop in specific cases by skipping some of the validations. All of those "Buy X and X to get X", "Buy 2x this and get discount" - whatever, easily.
    • If null is provided, nothing happens and built-in validation is ran. Useful if you want your own conditions, but also want to retain functionality of the core.
    • If true is provided, the validation ends here and the rule is VALID, ignoring the rest of core validation.
    • If false is provided, the validation ends here and the rule is not VALID, ignoring the rest of core validation. In this case, it's recommended to properly alter the isValidatedByModulesError error message so the user knows why.
  • You can test it with following module - tox_cartrulevalidation.zip, it will validate a cart rule only if there is a product with "umming" in it's name in the cart.

Known issues NOT RELATED TO THIS PR

  • If the cart rule is in the cart and you change validity, active or total quantity, it won't be removed. I tried to fix it but it breaks tests, will be done in another PR, in another day. :-)

@Hlavtox
Hlavtox requested a review from a team as a code owner December 19, 2023 12:23
@Hlavtox
Hlavtox marked this pull request as draft December 19, 2023 14:32
@Hlavtox
Hlavtox force-pushed the cart-rule-validation branch 2 times, most recently from 9571d63 to ca4dc4c Compare December 20, 2023 11:13
@Hlavtox
Hlavtox marked this pull request as ready for review December 20, 2023 11:13
@Hlavtox Hlavtox added this to the 9.0.0 milestone Dec 20, 2023
@Hlavtox

Hlavtox commented Dec 20, 2023

Copy link
Copy Markdown
Member Author

Ping @kpodemski, it's ready

ShaiMagal
ShaiMagal previously approved these changes Dec 21, 2023

@ShaiMagal ShaiMagal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

boherm
boherm previously approved these changes Dec 21, 2023
@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Dec 21, 2023
@AureRita

Copy link
Copy Markdown
Contributor

Hi @Hlavtox

Thank you for your PR, I tested it and it doesn't seems to works as expected as you can see :

recording.32.webm

Thank you

@AureRita AureRita added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Dec 29, 2023
@Hlavtox

Hlavtox commented Dec 29, 2023

Copy link
Copy Markdown
Member Author

@AureRita Please read the PR description, I specially written this to avoid QA reporting this - If the cart rule is in the cart and you change validity, active or total quantity, it won't be removed.

Try to change minimum order value for example. Now the rule will be removed on page reload, before, only if you changed something in cart.

@prestashop-issue-bot prestashop-issue-bot Bot removed the Waiting for author Status: action required, waiting for author feedback label Dec 29, 2023
@Hlavtox Hlavtox added the Waiting for QA Status: action required, waiting for test feedback label Dec 29, 2023
@Hlavtox Hlavtox closed this Dec 29, 2023
@Hlavtox Hlavtox reopened this Dec 29, 2023
@Hlavtox

Hlavtox commented Jan 12, 2024

Copy link
Copy Markdown
Member Author

Bump @AureRita

@kpodemski

Copy link
Copy Markdown
Contributor

ping @AureRita @florine2623 :)

@kpodemski kpodemski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm blocking this PR.

Unfortunately, we have to fix the most common issues first.

@ps-jarvis ps-jarvis added the Waiting for author Status: action required, waiting for author feedback label Jan 29, 2024
@kpodemski kpodemski removed the Waiting for QA Status: action required, waiting for test feedback label Jan 29, 2024
@Hlavtox
Hlavtox marked this pull request as draft January 29, 2024 11:10
@Hlavtox Hlavtox added the Blocked Status: The issue is blocked by another task label Jan 31, 2024
@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix labels Jan 31, 2024
@Hlavtox Hlavtox added the Waiting for QA Status: action required, waiting for test feedback label Mar 15, 2024
@florine2623 florine2623 self-assigned this Mar 19, 2024

@florine2623 florine2623 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello @Hlavtox ,

I'm not sure I understand well this PR. This is not a bug fix but rather an improvement of an existing feature ?
I don't see a "before and after" as you usually add in the how to test section of a PR x)

The new hook actionValidateCartRule is well available on my shop.

I tested this following part : All of those "Buy X and X to get X", "Buy 2x this and get discount" - whatever, easily.
Works as expected ✅

I tested your module tox_cartrulevalidation.zip and it works as you explained.

Is there something else that should be tested ?

Thanks!

@florine2623 florine2623 added the Waiting for author Status: action required, waiting for author feedback label Mar 19, 2024
@florine2623 florine2623 removed their assignment Mar 19, 2024
@Hlavtox

Hlavtox commented Apr 2, 2024

Copy link
Copy Markdown
Member Author

Ping @kpodemski could you QA it maybe?

@prestashop-issue-bot prestashop-issue-bot Bot removed the Waiting for author Status: action required, waiting for author feedback label Apr 2, 2024
@paulnoelcholot

Copy link
Copy Markdown
Contributor

Hello @Hlavtox

Some checks haven’t completed yet

@paulnoelcholot paulnoelcholot added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Apr 4, 2024
@Hlavtox Hlavtox closed this Apr 4, 2024
@Hlavtox Hlavtox reopened this Apr 4, 2024
@Hlavtox

Hlavtox commented Apr 4, 2024

Copy link
Copy Markdown
Member Author

@paulnoelcholot Tests green now

@prestashop-issue-bot prestashop-issue-bot Bot removed the Waiting for author Status: action required, waiting for author feedback label Apr 4, 2024
@Hlavtox Hlavtox added the Waiting for QA Status: action required, waiting for test feedback label Apr 4, 2024
@paulnoelcholot

Copy link
Copy Markdown
Contributor

Hello @Hlavtox,

I tested your PR and it's GOOD for me! 🎉

Thanks!

@paulnoelcholot paulnoelcholot added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Apr 12, 2024
@paulnoelcholot paulnoelcholot removed their assignment Apr 12, 2024
@nicosomb
nicosomb merged commit 4d3c11c into PrestaShop:develop Apr 12, 2024
@kpodemski kpodemski added the Needs documentation Needs an update of the developer documentation label May 8, 2024
@gericfo gericfo added the PR Autoupgrade opened ✔️ The associated PR has been opened in the Autoupgrade module repository. label Oct 3, 2024
@Hlavtox Hlavtox removed Needs autoupgrade PR PR Autoupgrade opened ✔️ The associated PR has been opened in the Autoupgrade module repository. labels Nov 14, 2024
@jf-viguier

Copy link
Copy Markdown
Contributor

love it

@kpodemski kpodemski added the Key feature Notable feature to be highlighted label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix Type: Bug fix develop Branch Key feature Notable feature to be highlighted Needs documentation Needs an update of the developer documentation QA ✔️ Status: check done, code approved

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.