Skip to content

Updating attribute option data through API will set unwanted source_model on the attribute #13156

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
koenner01 opened this issue Jan 12, 2018 · 6 comments
Labels
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: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development 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

@koenner01
Copy link
Contributor

When updating an eav attribute's options through the API, the source_model of the attribute will be set to 'Magento\Eav\Model\Entity\Attribute\Source\Table'. Not only is this unwanted, it also completely destroys the ability to update this attribute's options through the backend.

Preconditions

  1. PHP7.0
  2. CE 2.2.2 (also tested EE 2.2.2)

Steps to reproduce

  1. Create a new multiselect product attribute programmatically through an install script. Make sure to set the is_user_defined to 0 as we don't want te be able to delete this attribute.

  2. Notice how the source_model of the eav_attribute is nice and empty in the database and how you can manage the attribute options manually in the backend

  3. Add a new option manually through the backend for example 'I love Magento' and save the attribute

screen shot 2018-01-12 at 15 09 43

  1. Call the '../rest/all/V1/products/attributes/{newlyCreatedAttributecode}/options api with body:
{
    "option": {
        "label": "Magento rocks"
    }
}
  1. Look at the database and see how the source_model was unwillingly set to 'Magento\Eav\Model\Entity\Attribute\Source\Table' and see how this completely destroyed the ability to access the options through the backend.

screen shot 2018-01-12 at 15 09 58

Expected result

  1. Attribute option should be created (AND NOTHING ELSE SHOULD HAPPEN)

Actual result

  1. Attribute option is created and the attribute source_model has been changed

Why is it that the flow in the backend of adding a new option is not EXACTLY the same as adding a new option through the api. I can understand that there would be frontend validation etc. in the backend and the save controller for the attribute, but in the end they should both use the repository in the same way...

#magentomagic

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jan 12, 2018
@koenner01
Copy link
Contributor Author

Added bonus is that the storeview values for the options are also not rendered because the _getOptionValuesCollection function in Magento\Eav\Block\Adminhtml\Attribute\Edit\Options\Options only fetches the option values for 1 storeId (which in most cases is also not the correct one)

@magento-engcom-team magento-engcom-team added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed labels Feb 8, 2018
@magento-engcom-team
Copy link
Contributor

@koenner01, thank you for your report.
We've acknowledged the issue and added to our backlog.

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed 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 Feb 8, 2018
@hostep
Copy link
Contributor

hostep commented May 30, 2018

For the folks interested in this problem, we've received a patch from the Cloud support a while ago (MDVA-8298) which fixes this problem, apply to the magento/module-eav module:

diff --git a/Model/Entity/Attribute/AbstractAttribute.php b/Model/Entity/Attribute/AbstractAttribute.php
index 19877a51c48..68a4e1dbe7b 100644
--- a/Model/Entity/Attribute/AbstractAttribute.php
+++ b/Model/Entity/Attribute/AbstractAttribute.php
@@ -585,9 +585,9 @@ abstract class AbstractAttribute extends \Magento\Framework\Model\AbstractExtens
     {
         if (empty($this->_source)) {
             if (!$this->getSourceModel()) {
-                $this->setSourceModel($this->_getDefaultSourceModel());
+                $this->_source = $this->_getDefaultSourceModel();
             }
-            $source = $this->_universalFactory->create($this->getSourceModel());
+            $source = $this->_universalFactory->create($this->_source);
             if (!$source) {
                 throw new LocalizedException(
                     __(

UPDATE: this patch caused new problems, best not to apply until there is a better solution which works in all cases.

@hostep
Copy link
Contributor

hostep commented Sep 25, 2018

Here is a new and proper patch which I just created and tested pretty well and has no downsides (apply to magento/module-eav modules):

diff --git a/Model/Entity/Attribute/AbstractAttribute.php b/Model/Entity/Attribute/AbstractAttribute.php
index 0c97c7f5e27..401fdc7037e 100644
--- a/Model/Entity/Attribute/AbstractAttribute.php
+++ b/Model/Entity/Attribute/AbstractAttribute.php
@@ -585,9 +585,11 @@ abstract class AbstractAttribute extends \Magento\Framework\Model\AbstractExtens
     {
         if (empty($this->_source)) {
             if (!$this->getSourceModel()) {
-                $this->setSourceModel($this->_getDefaultSourceModel());
+                $this->_source = $this->_getDefaultSourceModel();
+            } else {
+                $this->_source = $this->getSourceModel();
             }
-            $source = $this->_universalFactory->create($this->getSourceModel());
+            $source = $this->_universalFactory->create($this->_source);
             if (!$source) {
                 throw new LocalizedException(
                     __(

I'll create a PR for this later.

And you might need to run the following query on your database, to remove the source_model again:

UPDATE `eav_attribute` SET `source_model` = NULL WHERE `source_model` = 'Magento\\Eav\\Model\\Entity\\Attribute\\Source\\Table' AND `frontend_input` = 'multiselect';

Be sure to first test this query first on a harmless environment, not directly on a production server!

@slavvka
Copy link
Member

slavvka commented Oct 3, 2018

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

The fix will be available with the upcoming 2.3.1 release.

@slavvka slavvka added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Oct 3, 2018
@magento-engcom-team
Copy link
Contributor

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

The fix will be available with the upcoming 2.2.8 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development 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

4 participants