GUI to Edit EAV Attributes & Sets

[justinbeaty]

Cherry-picked commits from OpenMage/magento-lts#2317, OpenMage/magento-lts#2352, and OpenMage/magento-lts#2353.

A couple of notes:

• The files js/varien/js.js, skin/frontend/base/default/js/opcheckout.js, and skin/frontend/rwd/default/scss/module/_eav.scss had changes in GUI to Edit EAV Attributes & Sets - Customer OpenMage/magento-lts#2352, but I'm not sure where those changes go in Maho.

• The last two WIP commits were part of an experimental feature to automatically add fields into various customer forms in the frontend and backend. I had an experimental branch that did more work here, but I think I may have lost it.

• I didn't pull any of the short array syntax, phpcs, or merge commits. Instead I rebased on top of Maho/main.

[fballiano]

@justinbeaty first of all thank you for this PR, for sure questions will arise, first ones being...

1. Do you think that customer attributes should have this kind of scope?

2. If I "open customer attributes", filter something on the grid, then go to "customer address attributes" the grid is already filtered (I think the grids share the same ID or it's because they're actually the same class), is it just a matter of ID or should we implement a custom class for each one of them (instead of the observer setting the properties)?

[justinbeaty]

@justinbeaty first of all thank you for this PR, for sure questions will arise, first one being...

Do you think that customer attributes should have this kind of scope?

My initial thought is that it should support scopes. An example might be a wholesaler website that asks for tax info. Similarly an EU store view might ask for a VAT id, but a USA one would not.

We also have the "Share Customer Accounts" option in the backend that might influence the decision, but I am positive that would cause any issues.

I have to admit I have very limited experience in running more than one Website / Store / Store View.

[justinbeaty]

2. If I "open customer attributes", filter something on the grid, then go to "customer address attributes" the grid is already filtered (I think the grids share the same ID or it's because they're actually the same class), is it just a matter of ID or should we implement a custom class for each one of them (instead of the observer setting the properties)?

I am pretty sure this is because of the same ID. What if you try to change this line:

maho/app/code/core/Mage/Eav/Block/Adminhtml/Attribute/Grid/Abstract.php

Line 24 in 01a5dbb

$this->setId('attributeGrid');

to:

$this->setId('attributeGrid' . (Mage::registry('entity_type') ?? ''));

[fballiano]

2. If I "open customer attributes", filter something on the grid, then go to "customer address attributes" the grid is

$this->setId('attributeGrid' . (Mage::registry('entity_type') ?? ''));

done! I changed a bit just because Mage::registry('entity_type') is an object but thanks! works as expected.

[justinbeaty]

@justinbeaty first of all thank you for this PR, for sure questions will arise, first one being...
Do you think that customer attributes should have this kind of scope?

I just remembered that in addition to (or instead of) supporting scopes, we have the ability to set a customer attribute set per customer group. See this comment: OpenMage/magento-lts#2352 (comment)

I think the SQL in that comment isn't in this PR either. Will need to run manually or add upgrade script to test the feature. (Flush cache after running SQL) Now in install script.

alter table customer_group add customer_attribute_set_id smallint unsigned not null default 0;
alter table customer_group add customer_address_attribute_set_id smallint unsigned not null default 0;

[justinbeaty]

Looking into the scope issue again, and it turns out is_global is actually a column on the table catalog_eav_attribute and not on the base table eav_attribute, nor customer_eav_attribute. So I'll actually remove the scope dropdown from the base EAV code.

However, Magento actually has something more complicated for customer attributes with the table customer_eav_attribute_website which is a website specific override for values in customer_eav_attribute.

This is actually already functional by setting various values in System Config -> Customer Configuration -> Name and Address Options. For example, select "Default Config" and set Show Prefix to Required and it's saved in customer_eav_attribute, but change to "Main Website" and it is saved in customer_eav_attribute_website. I knew the former, but never noticed the latter.

I'm still investigating the best way to wrangle this all...

[fballiano]

Hi @justinbeaty, when I go to checkout I get this

this $groups array is empty

[justinbeaty]

Hi @justinbeaty, when I go to checkout I get this

I believe I hit that error too, but just haven't gone back to fix it yet. It is for sure just something small.

[fballiano]

@justinbeaty I've tried to solve the conflicts, hope I didn't mess things up

[justinbeaty]

@justinbeaty I've tried to solve the conflicts, hope I didn't mess things up

I double check the merge in a bit, still working locally with some commits.

[fballiano]

solved minor conflicts

[justinbeaty]

Okay, some big updates:

De-duplicate Code

The generic EAV code was adapted from the product attribute code, but previously the old code was left as-is. I made all of the product attribute blocks extend the generic EAV ones. We went from net +3,624 LOC to net +1,231 LOC, which is absolutely essential for maintainability:

I took pretty good care to make sure this change doesn't break things. While the other EAV types (category, customer, address) use observers to change things if needed, it is now also possible to extend the generic block classes for more control. Thus, all of the old Mage_Adminhtml_Block_Catalog_Product_Attribute_* classes still exist, they just now extend the Mage_Eav_Block_Adminhtml_Attribute_* classes and are minimal or even empty.

One of the main ways this was accomplished was to convert all of the hardcoded $this->getLayout()->createBlock() calls into layout XML. This has the nice benefit that for other EAV types you can override just a single block if needed. This was the reason for OpenMage/magento-lts#4337 btw.

Consolidated logic into EAV helper

Magento had tons of logic spread around probably a dozen files dealing with EAV attribute's backend types, models, display options, etc. It would have been pretty much impossible to define new input types or add logic for other entity types without overriding tons of classes.

Instead, I moved all of this info into the config.xml files, and wrote a bunch of helper methods to return this info. Everything is now super flexible.

Rewrote JS files

Instead of the inline JS in a few files, I created new files at public/js/mage/adminhtml/eav/ in vanilla JS, with one exception of using prototype's Template class, which will be easy for us to replace once prototype is finally dropped. Todo: write JSDoc annotations.

Misc

There's probably some other stuff too, but those are the major changes. It's very possible something broke here as I haven't fully tested, but all can be fixed and this is still a hundred times better for the long-term. One thing to note, you might see few attributes in the manage customer attribute grid. I have to investigate how Magento devs used the is_visible column because right now I'm hiding attributes based on that, which is how the product attributes grid works.

I also ended up merging main into this branch locally, which is the reason for the force-push. Hopefully you hadn't spent too much time resolving conflicts since I overwrote those commits.

[justinbeaty]

One more thing, I re-introduced the setTemplateIfExists method. The reason being that I deleted most of the templates at app/design/adminhtml/default/default/template/catalog/product/attribute/, but it's highly likely someone has made a customization in one of those files. Since these calls are only in the admin panel, I don't think the extra filesystem check is a huge deal. If the re-introduction of this method is okay with you, I can make a separate PR for it because I think I have another idea for an optimization here.

[justinbeaty]

Don't worry about conflicts, I'll resolve later...