Skip to content

Conversation

@BernardRobbins
Copy link

…stead, but cart would reflect correct price of 0.

Fixed Issues (if relevant)

None found.

Manual testing scenarios (*)

  1. Create product
  2. Set Product price > 0
  3. Advanced Pricing -> Price -> Create Customer Group Price with All Websites, All Groups, 1 quantity, Fixed Price, 0 cost.

Without fix, product price will display, with fix correct price of 0. Cart will display correct price of 0 in either case.

…stead, but cart would reflect correct price of 0.
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 17, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @BernardRobbins. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@rodrigowebjump
Copy link
Member

Hi @BernardRobbins
Thanks for your PR, but there is an already approved change in PR #18631 that also fix this issue.
So I am closing yours.

@BernardRobbins
Copy link
Author

Hi @rodrigowebjump,

That's great that someone else has gotten this. I apologize about not finding it when I searched. Not sure why their fix didn't show.

I don't believe that Mahesh's fix solves the issue properly. I feel that checking that the value is numeric instead of anything not bool false would be a better way to go in that it better conveys what we are truly testing for. Let me know if there is a formal way of requesting this change to the pull request.

@sidolov sidolov added Complex Review Complex review provided by Maintainer and removed Complex Review Complex review provided by Maintainer labels Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants