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

Custom option values do not save correctly #5067

Closed
samtay opened this issue Jun 16, 2016 · 9 comments
Closed

Custom option values do not save correctly #5067

samtay opened this issue Jun 16, 2016 · 9 comments
Labels
bug report Event: distributed-cd Distributed Contribution Day Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@samtay
Copy link

samtay commented Jun 16, 2016

Certain permutations of product custom options are impossible to save. For example,

  1. First save a product with a custom option dropdown of two values with differing fields:
    2016-06-16-191853_1331x521_scrot
  2. Now try to save the second value to be identical to the first, like so:
    2016-06-16-193922_1344x530_scrot
  3. Notice that many of the values are not saved correctly. For me, this time around only the SKU was problematic, but you will have problems saving any number of these fields.

This is because

  • the abstract resource model generates the UPDATE query based on differences between the new data array being saved and the preexisting $object->storedData array. code reference
  • the OptionValue model loops through setData commands to save multiple OptionValue instances, hence storedData property refers to the previously saved Value instance, which is set during the previous setData()->..->save() iteration. code reference

So, those two reasons result in any identical OptionValue fields not able to be saved consecutively, causing the bug shown in steps 1-3.

I think it is rather odd/lazy that the option value model does not have its own resource model.. it seems like a poor idea to just loop through $this->setData()->save() calls. But, if you want to fix this quickly, just issue a $this->load($this->getOptionTypeId()) before the save. This will set the proper storedData property onto the model.

@antboiko
Copy link

Hello @samtay , could you please advise me if you have any errors upon saving the product with identical values in custom option? What Magento version are you using?

Thanks,
Anton.

@samtay
Copy link
Author

samtay commented Jun 17, 2016

No, see the above code references, this fails silently. Occurs on latest version.

@antboiko
Copy link

@samtay thanks for the details, I've created internal ticket MAGETWO-54516 to investigate this issue.

Best,
Anton.

@antboiko antboiko added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development and removed Progress: needs update labels Jun 17, 2016
@vkorotun vkorotun removed the MX label Aug 4, 2016
@ttribeiro
Copy link

ttribeiro commented Nov 18, 2016

I have a similar problem. I edit the price of the option and do not save the price correctly.

Before Save

screen shot 2016-11-18 at 2 14 14 am

After Save

screen shot 2016-11-18 at 2 15 34 am

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed labels Sep 11, 2017
@yutv
Copy link

yutv commented Oct 17, 2017

Can anyone from magento tell when this bug will be fixed? Really guys, you have had 1+ year to fix it but this bug still exists in your recent 2.2 release.

Here is hotfix - just modify Magento\Catalog\Model\Product\Option\Value::saveValues method:

  1. add $clone = clone $this; after foreach
  2. use $clone instead of $this in the foreach body.

@magento-engcom-team
Copy link
Contributor

@samtay, thank you for your report.
We've created internal ticket(s) MAGETWO-54516 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 18, 2017
@yutv
Copy link

yutv commented Jan 29, 2018

Here is a module with clone based workaround mentioned in my previous comment:
composer require orange35/magento2-custom-option-value-save-handler

@magento-engcom-team magento-engcom-team added the Event: distributed-cd Distributed Contribution Day label Mar 19, 2018
@VladimirZaets
Copy link
Contributor

Hi @samtay. Thank you for your report.
The issue has been fixed in #13569 by @JeroenVanLeusden in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.6 release.

@magento-engcom-team
Copy link
Contributor

Hi @samtay. Thank you for your report.
The issue has been fixed in #16838 by @JeroenVanLeusden in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Jul 18, 2018
magento-team pushed a commit that referenced this issue Jan 28, 2020
[TSG] Fixes for 2.3 (pr87) (2.3.4-develop)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Event: distributed-cd Distributed Contribution Day Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

8 participants