Skip to content

Fixed issue #21650 #22699

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

Closed
wants to merge 7 commits into from
Closed

Fixed issue #21650 #22699

wants to merge 7 commits into from

Conversation

geet07
Copy link

@geet07 geet07 commented May 4, 2019

Description (*)

Saving product with options with thousands separator in price fails.

Fixed Issues (if relevant)

  1. magento/magento2#<21650>: Saving product with options with thousands separator in price fails

Manual testing scenarios (*)

  1. Add Product with custom options (Price: 1000).
  2. Once Product saved, Again edit the same product with custom options now you can save all values sucessfully.

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 on Travis CI are green)

@m2-assistant
Copy link

m2-assistant bot commented May 4, 2019

Hi @geet07. 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 2.3-develop instance - deploy vanilla Magento instance

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

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented May 4, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

@geet07 we cannot simply remove validation. Corrected validator must be used instead.

@ghost ghost assigned orlangur May 5, 2019
@geet07
Copy link
Author

geet07 commented May 7, 2019

@geet07 we cannot simply remove validation. Corrected validator must be used instead.

Hi @orlangur,

Thank you for message.

As per your comment I have added validation same a catalog main price. So now you can check this and let me know if there is another changes reqiured.

@geet07 geet07 requested a review from orlangur May 7, 2019 07:02
@orlangur
Copy link
Contributor

orlangur commented May 8, 2019

Thanks @geet07! Did you check implementation why validate-number is not working?

@geet07
Copy link
Author

geet07 commented May 8, 2019

Thanks @geet07! Did you check implementation why validate-number is not working ?

Yes I have checked, When we change the locale for admin user then there will be change in price format, So due to this "thousands separator" position also change & that 'validation-number' validation doesn't work too. I have also checked with other price field that 'validation-number' validation not use in the current system.

@geet07
Copy link
Author

geet07 commented May 21, 2019

Hi @orlangur,

Is there any another changes required?

@geet07 geet07 added partners-contribution Pull Request is created by Magento Partner Partner: Ranosys Pull Request is created by Ranosys Technologies labels May 21, 2019
@geet07
Copy link
Author

geet07 commented Jun 4, 2019

Hi @orlangur,

Can You please help me to close this issue.

@geeta07 geeta07 deleted the fix-21650 branch June 14, 2019 06:03
@geet07 geet07 closed this Jun 14, 2019
@m2-assistant
Copy link

m2-assistant bot commented Jun 14, 2019

Hi @geet07, 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.

@geet07 geet07 mentioned this pull request Jun 14, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Catalog Partner: Ranosys Pull Request is created by Ranosys Technologies partners-contribution Pull Request is created by Magento Partner Progress: on hold Release Line: 2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants