[Bugfix] #7333 Unable to set negative custom option fixed price in admin view. #15267

dverkade · 2018-05-16T22:08:02Z

Fixes the ability to set a negative custom option price in the backend.

Description

In Magento 1 it was possible to set a negative price change to a custom option. This PR fixes issue #7333 and makes it possible to set a negative price in the backend of Magento. This PR fixes the backend so that the negative price change can actually be entered in the Magento admin. There is a separate PR for the frontend changes which is PR #14975

There also seem to be two other PR's for this issue which are stale and are not moving along:
#13393
#14342

Fixed Issues

Unable to set negative custom option fixed price in admin view. #7333: Unable to set negative custom option fixed price in admin view.

Manual testing scenarios

Create a new simple product in the backend and try to create new custom options where the price change has a negative value (for instance, "-10").

…iew.

miguelbalparda · 2018-05-17T12:55:33Z

Tests are failing, any chance you can review them?

dverkade · 2018-05-30T17:43:04Z

Will work on this at the MM18NL contribution day.

…ice in admin view. - Fixed unit tests - Fixed integration tests - Changed defaultvalidator to have the locale format interface. Because a value entered in the backoffice of Magento is passed as the value like "40,000.00", which needs to be converted to a float in order to do a valid check on it.

dverkade · 2018-06-02T20:00:42Z

All tests are now green. Please proceed with processing this PR.

dmanners · 2018-06-05T09:01:55Z

app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php

+     * @param string $value
+     * @return bool
+     */
+    protected function isNumber($value)


So @dverkade looking over this pr again I think we should be making this method public. We are trying to stay away from protected and since these classes are built with inheritance in mind I would suggest that having this method public would allow for a user to create a new validator and change the isNumber functionality.

Hi @dmanners, thanks for reviewing this. I have changed the function to public.

- Changed validator method to public

dverkade · 2018-06-05T09:43:56Z

Changes have been made, this PR can be processed / reviewed again.

dmanners · 2018-06-06T10:00:00Z

Thanks for those changes @dverkade there are two failures with the api tests but I will investigate these for you and then process this pr.

ihor-sviziev · 2018-06-22T05:18:09Z

Hi @dmanners,
Could you also look into #14342? Looks like these PRs fixing one issue

dmanners · 2018-06-27T08:55:56Z

Will do @ihor-sviziev thanks for the note.

dmanners · 2018-06-27T10:01:07Z

Hi @dverkade just to keep you updated I am checking with the core team to understand the reasoning for making negative prices not allowed a bit better, just to make sure that we do not miss something important out. After I have more information I will update you via this PR. Thanks for your patience.

dmanners · 2018-07-10T08:51:55Z

Hi @dverkade sorry for the delay. Would you be able to look into the test Magento\Catalog\Api\ProductCustomOptionRepositoryTest::testAddNegative It is used to validate that there are no negative prices added for options. In this case I would suggest that we test the addition of negative prices and validate that you can add them. This way if someone happens in the future to this feature we are able to see it.

dmanners · 2018-09-04T16:36:59Z

Hi @dverkade any update here?

…m-option # Conflicts: # app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php # app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php # app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/DefaultValidatorTest.php # app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/FileTest.php # app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/SelectTest.php # app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/TextTest.php

dverkade · 2018-09-05T09:57:34Z

Hi @dmanners,

Thanks for your patience. This has been resolved by merging the 2.3 develop branch into this PR. Merge conflicts have been resolved. You can proceed with checking this PR.

dmanners · 2018-09-17T16:23:12Z

Thanks @dverkade got one more failure coming through.

Magento\Catalog\Api\ProductCustomOptionRepositoryTest::testGetList
Failed asserting that 10 matches expected 11.

Could you take a look at this for me.

dverkade · 2018-09-30T13:30:28Z

Hi @dmanners,

I have taken a look at this test and fixed the issue. The API functional test should now pass as well.

sidolov · 2018-10-02T07:44:06Z

Hi @dverkade , please, resolve merge conflict.
Thanks!

…7333-negative-value-for-custom-option # Conflicts: # app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php

dverkade · 2018-10-02T08:40:30Z

@dmanners @sidolov merge conflict is resolved.

magento-engcom-team · 2018-10-04T09:10:46Z

Hi @sidolov, thank you for the review.
ENGCOM-1862 has been created to process this Pull Request

dverkade · 2018-12-13T13:09:34Z

@dmanners @sidolov when will this PR be processed? Had been in this state since the 4th of October.

ghost · 2019-01-14T11:46:32Z

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

magento-engcom-team · 2019-01-14T11:47:06Z

Hi @dverkade. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

…d price in admin view. #15267

Fix #7333 Unable to set negative custom option fixed price in admin v…

dc4de31

…iew.

dverkade changed the title ~~Fix #7333 Unable to set negative custom option fixed price in admin view.~~ [Bugfix] #7333 Unable to set negative custom option fixed price in admin view. May 16, 2018

dverkade self-assigned this May 23, 2018

dmanners added the mm18nl label Jun 2, 2018

dverkade assigned dmanners and unassigned dverkade Jun 2, 2018

magento-engcom-team added this to the Release: 2.3.0 milestone Jun 5, 2018

magento-engcom-team added Release Line: 2.3 Component: Catalog labels Jun 5, 2018

dmanners suggested changes Jun 5, 2018

View reviewed changes

magento-engcom-team added the Progress: needs update label Jun 5, 2018

#7333: Unable to set negative custom option fixed price in admin view.

9a8e7af

- Changed validator method to public

okorshenko removed the mm18nl label Jun 5, 2018

magento-engcom-team removed the Progress: needs update label Jun 6, 2018

This was referenced Jun 27, 2018

Add ability to set custom product option with negative price #14342

Closed

Allow negative price for custom option: fixes #7333 in GitHub (internal MAGETWO-60573) #13393

Closed

magento-engcom-team added the Progress: needs update label Jul 10, 2018

Fixed API test.

04caefa

magento-engcom-team added Progress: needs update and removed Progress: needs update labels Jul 10, 2018

dverkade added 3 commits September 5, 2018 09:53

Resolved merge conflict with merging 2.3 develop branch.

d43a52f

Fixed DefaultValidatorTest

d66329a

dverkade mentioned this pull request Sep 5, 2018

Travis CI does not check API functional tests #17929

Closed

magento-engcom-team removed the Progress: needs update label Sep 13, 2018

magento-engcom-team added the Progress: needs update label Sep 17, 2018

magento-engcom-team modified the milestones: Release: 2.3.0, Release: 2.3.1 Sep 18, 2018

Removed negative price from product options test file.

08ea73c

Merge commit '8460e4ee5cdc662bd489e60175664f1da6cf5087' into 2.3-Fix-…

6baf928

…7333-negative-value-for-custom-option # Conflicts: # app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php

Fixed coding style errors.

4927e56

sidolov approved these changes Oct 4, 2018

View reviewed changes

sidolov removed the Progress: needs update label Oct 4, 2018

magento-engcom-team assigned sidolov Oct 4, 2018

sidolov added community-insider-contribution Community Insider: Cream labels Nov 20, 2018

magento-engcom-team added the Progress: accept label Dec 24, 2018

magento-engcom-team merged commit 4927e56 into magento:2.3-develop Jan 14, 2019

magento-engcom-team added the Award: bug fix label Jan 14, 2019

magento-engcom-team pushed a commit that referenced this pull request Jan 14, 2019

ENGCOM-1862: [Bugfix] #7333 Unable to set negative custom option fixe…

caefa5b

…d price in admin view. #15267

[Bugfix] #7333 Unable to set negative custom option fixed price in admin view. #15267

[Bugfix] #7333 Unable to set negative custom option fixed price in admin view. #15267

Uh oh!

Conversation

dverkade commented May 16, 2018

Description

Fixed Issues

Manual testing scenarios

Uh oh!

miguelbalparda commented May 17, 2018

Uh oh!

dverkade commented May 30, 2018

Uh oh!

dverkade commented Jun 2, 2018

Uh oh!

dmanners Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

dverkade Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

dverkade commented Jun 5, 2018

Uh oh!

dmanners commented Jun 6, 2018

Uh oh!

ihor-sviziev commented Jun 22, 2018

Uh oh!

dmanners commented Jun 27, 2018

Uh oh!

dmanners commented Jun 27, 2018

Uh oh!

dmanners commented Jul 10, 2018

Uh oh!

dmanners commented Sep 4, 2018

Uh oh!

dverkade commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmanners commented Sep 17, 2018

Uh oh!

dverkade commented Sep 30, 2018

Uh oh!

sidolov commented Oct 2, 2018

Uh oh!

dverkade commented Oct 2, 2018

Uh oh!

magento-engcom-team commented Oct 4, 2018

Uh oh!

dverkade commented Dec 13, 2018

Uh oh!

ghost commented Jan 14, 2019

Uh oh!

magento-engcom-team commented Jan 14, 2019

Uh oh!

Uh oh!

dverkade commented Sep 5, 2018 •

edited

Loading