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

magento/magento2#12294: Bug: Adding Custom Attribute - The value of A… #12755

Merged
merged 3 commits into from
Jan 3, 2018

Conversation

virtual97
Copy link

Description

Fixed Issues (if relevant)

  1. Bug: Adding Custom Attribute - The value of Admin scope can't be empty #12294: Bug: Adding Custom Attribute - The value of Admin scope can't
    be empty.- Made hidden fields disabled.

Manual testing scenarios

  1. Add customer Product Attribute
  2. Choose Dropdown
  3. Under Manage Attributes, click "Add Option" and then "Delete" empty boxes
  4. Attempt to save.

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)

…e can't be empty- Made hidden fields disabled.
@magento-engcom-team magento-engcom-team added mageconf 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 Dec 15, 2017
@okorshenko okorshenko self-assigned this Dec 26, 2017
@okorshenko okorshenko added this to the December 2017 milestone Dec 26, 2017
@omiroshnichenko
Copy link
Contributor

Hi, @virtual97. Unfortunately, your fix is incorrect. When you makes input element disabled it will not present in serialized object. For case with new option it's ok. But when attribute will have saved option, this option will not be present in serialized object and in result will not be removed from attribute on backend. To fix this problem you can make next:

  1. Mark new option with some class
  2. On remove, check if option has this class.

Example:
In app/code/Magento/Catalog/view/adminhtml/web/js/options.js

...
    rendered: 0,
    template: mageTemplate('#row-template'),
    newOptionClass: 'new-option',// Add this property
    isReadOnly: config.isReadOnly,
    add: function (data, render) {
        var isNewOption = false,
            element;

        if (typeof data.id == 'undefined') {
            data = {
                'id': 'option_' + this.itemCount,
                'sort_order': this.itemCount + 1,
                'rowClasses': this.newOptionClass// Add class if option is new
            };
            isNewOption = true;
        },
        remove: function (event) {
            var element = $(Event.findElement(event, 'tr'));

            element.ancestors().each(function (parentItem) {...});

            if (element) {...}

            if (element.hasClassName(this.newOptionClass)) {// Check if element has class newOption
                element.remove();// Remove this element
            }
        },
...

In app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/attribute/options.phtml

...
<script id="row-template" type="text/x-magento-template">
         <tr <% if (data.rowClasses) { %>class="<%- data.rowClasses %>"<% } %>>//Add class to row template
            <td class="col-draggable">
...

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 Progress: accept Release Line: 2.2 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

Successfully merging this pull request may close these issues.

5 participants