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

Update Mage_SalesRule_Model_Quote_Discount #4293

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Hanmac
Copy link
Contributor

@Hanmac Hanmac commented Oct 18, 2024

Description (*)

  • DiscountDescription should only be string or null (not array)
  • strlen needs a type check for string before
  • sales_quote_address_discount_item needs to be done after calculator->process

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes OpenMage/magento-lts#<issue_number>

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

* DiscountDescription should only be string or null (not array)
* strlen needs a type check for string before
* sales_quote_address_discount_item needs to be done after calculator->process
@github-actions github-actions bot added the Component: SalesRule Relates to Mage_SalesRule label Oct 18, 2024
@sreichel
Copy link
Contributor

Wouldnt it be better to dispatch an _after event?

@Hanmac
Copy link
Contributor Author

Hanmac commented Oct 19, 2024

Wouldnt it be better to dispatch an _after event?

The problem right now is that the existing event is either called before or after the calculator process

If you want a new event, you probably want one before process

@kiatng
Copy link
Contributor

kiatng commented Oct 22, 2024

I agree with the change.

@Hanmac Hanmac marked this pull request as ready for review December 19, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: SalesRule Relates to Mage_SalesRule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants