GUI to Edit EAV Attributes & Sets - Customer

[justinbeaty]

Description (*)

[WIP] This commit was previously part of #2317. It is broken out so that we can discuss ways to integrate the customer EAV attributes into customer groups, and customer forms. Discussions about this are in that PR.

Related Pull Requests

#2317
#2260

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Todo:

• Check translation strings
• Modify customer attributes to add use_in_forms option
• Modify Customers > Manage Groups to allow us to select an attribute set for each customer group (both customer and address set)

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)
• Add yourself to contributors list

[justinbeaty]

In testing the use_in_forms function, it seems like adding attributes to any of the frontend forms does not work, and would involve editing template files as mentioned by fballiano here: #2317 (comment)

However it does seem to work perfectly in all of the adminhtml areas.

[fballiano]

I can confirm you that there's some work on the frontend that needs to be done for the use_in_forms to work :-)

[kiatng]

I have custom form types, is it possible to define these in app\code\core\Mage\Customer\etc\config.xml?

[justinbeaty]

Would it maybe be better to set up an event to hook into? I'm thinking about the best way a module could define their own form types.

[kiatng]

Using event is ok, but I like the config method. An example is product types:

magento-lts/app/code/core/Mage/Catalog/etc/config.xml

Lines 479 to 487 in 35acb46

	<catalog>
	<product>
	<type>
	<simple translate="label" module="catalog">
	<label>Simple Product</label>
	<model>catalog/product_type_simple</model>
	<composite>0</composite>
	<index_priority>10</index_priority>
	</simple>

and product custom option:

magento-lts/app/code/core/Mage/Catalog/etc/config.xml

Lines 531 to 537 in 35acb46

	<options>
	<custom>
	<groups>
	<text translate="label" module="adminhtml">
	<label>Text</label>
	<render>adminhtml/catalog_product_edit_tab_options_type_text</render>
	<types>

which I have used to add more options in the HTML select element.

[justinbeaty]

@kiatng thanks for the examples, I was thinking about it more and agree the config.xml method is indeed better.

[kiatng]

Ref

magento-lts/app/code/core/Mage/Adminhtml/Model/System/Config/Source/Product/Options/Type.php

Line 34 in 35acb46

class Mage_Adminhtml_Model_System_Config_Source_Product_Options_Type

I made a sample code:

class Mage_Adminhtml_Model_System_Config_Source_Customer_Form
{
    const CUSTOMER_FORM_PATH = 'global/customer/form';

    public function toOptionArray()
    {
        $helper = Mage::helper('customer');
        foreach (Mage::getConfig()->getNode(self::CUSTOMER_FORM_PATH)->children() as $form) {
            $labelPath = self::CUSTOMER_FORM_PATH . '/' . $form->getName() . '/label';
            $forms[] = [
                'label' => $helper->__((string) Mage::getConfig()->getNode($labelPath)),
                'value' => $form->getName()
            ];
        }

        return $forms;
    }
}

The config.xml looks like this:

    <global>
        <customer>
            <form>
                <adminhtml_checkout translate="label" module="customer">
                    <label>Adminhtml Checkout</label>
                </adminhtml_checkout>
                <adminhtml_customer translate="label" module="customer">
                    <label>Adminhtml Customer</label>
                </adminhtml_customer>
                <!-- more forms -->
            </form>
        </customer>
    </global>

[kiatng]

That code sample doesn't consider the module attribute. Corrected version:

class Mage_Adminhtml_Model_System_Config_Source_Customer_Form
{
    const CUSTOMER_FORM_PATH = 'global/customer/form';

    public function toOptionArray()
    {
        foreach (Mage::getConfig()->getNode(self::CUSTOMER_FORM_PATH)->children() as $form) {
            $labelPath = self::CUSTOMER_FORM_PATH . '/' . $form->getName() . '/label';
            $moduleName = $form->getAttribute('module') ?? 'customer';
            $forms[] = [
                'label' => Mage::helper($moduleName)->__((string) Mage::getConfig()->getNode($labelPath)),
                'value' => $form->getName()
            ];
        }

        return $forms;
    }
}

Note that $form->getAttribute('module') can return null.

[justinbeaty]

@kiatng thanks for the implementation!

Do you think this XML is better? Since we need to also define customer_address forms it keeps all forms under one node, <form>.

<global>
   <form>
      <customer>
         <adminhtml_checkout translate="label" module="customer">
            <label>Adminhtml Checkout</label>
         </adminhtml_checkout>
         <adminhtml_customer translate="label" module="customer">
            <label>Adminhtml Customer</label>
         </adminhtml_customer>
         <!-- more forms -->
      </customer>
      <customer_address>
         <adminhtml_customer_address translate="label" module="customer">
            <label>Adminhtml Customer Address</label>
         </adminhtml_customer_address>
         <customer_address_edit translate="label" module="customer">
            <label>Customer Address Edit</label>
         </customer_address_edit>
         <!-- more forms -->
      </customer_address>
   </form>
</global>

I could also move the source models to:

Mage_Adminhtml_Model_System_Config_Source_Form_Customer
Mage_Adminhtml_Model_System_Config_Source_Form_Customer_Address

to match.

[kiatng]

Ref

magento-lts/app/code/core/Mage/Customer/Model/Form.php

Line 35 in 18e68a2

class Mage_Customer_Model_Form extends Mage_Eav_Model_Form

and

magento-lts/app/code/core/Mage/Catalog/etc/config.xml

Lines 620 to 621 in 18e68a2

	</catalog>
	<eav_attributes>

Use the module name like <catalog>:

<global>
   <eav>
     <form>
        <customer>
           <!-- more forms -->
        </customer>
        <customer_address>
           <!-- more forms -->
        </customer_address>
     </form>
   </eav>
</global>

or continue with the format <eav_attributes>:

<global>
   <eav_forms>
      <customer>
         <!-- more forms -->
      </customer>
      <customer_address>
         <!-- more forms -->
      </customer_address>
   </eav_forms>
</global>

Which do you prefer? I'd go with the latter.

[justinbeaty]

I like the latter as well 👍

[justinbeaty]

I've started support for use_in_forms to work on the frontend without the user having to edit templates:

Todo:

• Compile the SCSS to make the date field show up correctly.
• Date field if required doesn't pass validation even if filled 🤔
• Add changes in app/design/frontend/rwd/default/template/customer/form/edit.phtml to app/design/frontend/base as well
• Support the other frontend forms

[lgtm-com]

This pull request introduces 2 alerts and fixes 8 when merging e629678 into cf82b8f - view on LGTM.com

new alerts:

• 2 for Syntax error

fixed alerts:

• 4 for Useless assignment to local variable
• 2 for Missing variable declaration
• 1 for Useless conditional
• 1 for Unused variable, import, function or class

[fballiano]

wow!

[justinbeaty]

Working on the other frontend forms, checkout_register is a bit crowded, but it's just how Magento is with checkout & register I guess. I think a store owner should decide to not add too many fields to this form.

[justinbeaty]

I still have address forms to do tomorrow.

I am also thinking it might be nice to group these fields on the frontend based on the attribute set > groups. This way there is some more organization and the user can label the fieldset. For example in Account Information they could have a group called "Social Media" and let the user enter urls to their pages (just an example.)

But also, I am going to take a stab at allowing you to define attribute set per customer group. Thus it would be important that not only do you define use_in_forms but also put that attribute in a attribute set. Otherwise you cannot control what attributes show up for each customer group.

[justinbeaty]

@kiatng (and @fballiano) I'm seeking your opinion on the following (originally posted here)

---

I use attribute sets to organize the attributes for different customer groups. The project has over 200 customer attributes, each customer group having very different set of attributes.

My concern is that changes to EAV could make it very difficult to merge OM to existing projects with custom EAV.

The PR as it now should definitely not conflict with anything since it's all new code, but I see your point if we hook up a customer group -> customer attribute set relation. Still, I think it could be worth it maybe just waiting to merge this PR until the next major version.

How did you correlate attribute sets with customer groups? My first thought is to modify the customer_group table:

mysql> describe customer_group;
+---------------------+-------------------+------+-----+---------+----------------+
| Field               | Type              | Null | Key | Default | Extra          |
+---------------------+-------------------+------+-----+---------+----------------+
| customer_group_id   | smallint unsigned | NO   | PRI | NULL    | auto_increment |
| customer_group_code | varchar(32)       | NO   |     | NULL    |                |
| tax_class_id        | int unsigned      | NO   |     | 0       |                |
+---------------------+-------------------+------+-----+---------+----------------+
3 rows in set (0.00 sec)

To add something like customer_attribute_set_id and maybe even customer_address_attribute_set_id. Probably both are useful in the case you want to control both attribute sets per group.

If we can do it in a way that is maybe compatible with what you've done, then that's at least one site that wouldn't break.

---

In any case, I'll set up an observer so that users can change this logic.

[kiatng]

@kiatng So are you able to create two attribute sets that apply to the same customer group, but differ in the form setting? In other words, you can show different fields for the same customer group on different forms?

Yes, in fact sets 19 and 23 in the screenshot I attached are of the same customer group. The use case is for different backend users to view/edit a different set of attributes. At its height, there were a couple hundreds of backend users in this project.

There are other advantages to add apply_to at the attribute set level. Because it is upstream (EAV level), there is less coding downstream (customer level and other custom entity).

Your approach to match an attribute set at the customer group is similar to the product's attribute_set_id. However, number of customer groups (I have seen max 30 groups) is much less than the number of products (can be hundreds of thousands). It's easier to configure apply_to at the attribute set level for customer groups, but impractical for products.

But what I have is customized to a very particular use case. I only developed what is required of the project, I do not have anything on addresses and categories. Developing for general use cases is a lot more challenging.

[luigifab]

Will be ready for RC4?

[fballiano]

I wanted to revive this, I think they should target "next" but it's like my preference, because we're more free to choose how to act on the frontend.

[empiricompany]

check if extrafileds exists and add validation classes to text input
I wasn't able to send a commit

[empiricompany]

wrap into

<?php if($this->extraFields()): ?>
...
<?php endif ?>

[empiricompany]

add validation classes
class="input-text <?php echo $_attribute->getFrontendClass() ?>"

[empiricompany]

i've made some test on this PR:

• for each type of attribute (custom/address), I can't set the scope value. It's always stuck on 'store view', and in the grid column, it's always null.
• in the frontend checkout, the extra fields are not being printed

[fballiano]

there were some bad conflicts, I did what i could, not sure 100% was correct