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

26214: Bundle Product / Create / Price validation for dynamic price #27409

Closed
wants to merge 5 commits into from
Closed

26214: Bundle Product / Create / Price validation for dynamic price #27409

wants to merge 5 commits into from

Conversation

phuocnh
Copy link

@phuocnh phuocnh commented Mar 24, 2020

Description (*)

  • Fixed Bundle Product / Create / Price validation for dynamic price

Fixed Issues (if relevant)

  1. Bundle Product / Create / Price validation for dynamic price #26214: Bundle Product / Create / Price validation for dynamic price

Manual testing scenarios (*)

  1. Enter "New Product / Bundle" form
  2. Turn off "Dynamic price"
  3. Enter the price and remove it. You'll see a validation error.
  4. Turn on "Dynamic price". You'll see remove a validation error.

video fixed https://youtu.be/zyPJw6udqO0

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Copy link
Contributor

@eduard13 eduard13 left a comment

Choose a reason for hiding this comment

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

Hi @phuocnh, thank you for your contribution. Due to Magento Definition of Done the changes should be covered by tests. Could you please cover your fix by a MFTF test and also fix the static tests?
Thank you.

@ghost ghost moved this from Pending Review to Changes Requested in Pull Requests Dashboard Mar 26, 2020
@phuocnh
Copy link
Author

phuocnh commented Mar 26, 2020

Hi @eduard13, please check again
thanks

@eduard13
Copy link
Contributor

Hi @phuocnh, as I mentioned previously, your changes have to be covered by an automated test. I'd suggest covering by a simple MFTF test.

@magento magento deleted a comment from iibrah Apr 9, 2020
@magento magento deleted a comment from iibrah Apr 9, 2020
@eduard13
Copy link
Contributor

eduard13 commented Apr 9, 2020

Hi @phuocnh, are you willing to continue the progress on this one? There is no much left till the end, as you should only cover your fix by a test.
Please let me know if you need any assistance.
Thank you.

@eduard13 eduard13 added Award: bug fix Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests labels Apr 9, 2020
@phuocnh
Copy link
Author

phuocnh commented Apr 14, 2020

Hi @eduard13

now all checks have passed
1

But I have problem when install MFTF test with branch 2.4-develop and flow by document link https://devdocs.magento.com/mftf/docs/getting-started.html. I think it only support with Magento 2.3.x

2

thanks

@eduard13
Copy link
Contributor

Closing this one, as it looks like the repo doesn't exist anymore.

@eduard13 eduard13 closed this Apr 30, 2020
@m2-assistant
Copy link

m2-assistant bot commented Apr 30, 2020

Hi @phuocnh, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@ghost ghost removed this from Changes Requested in Pull Requests Dashboard Apr 30, 2020
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.

Bundle Product / Create / Price validation for dynamic price
3 participants