magento-lts
magento-lts copied to clipboard
GUI to Edit EAV Attributes & Sets - Customer
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
- [x] Modify customer attributes to add use_in_forms option
- [x] Modify Customers > Manage Groups to allow us to select an attribute set for each customer group (both customer and address set)
Contribution checklist (*)
- [x] Pull request has a meaningful description of its purpose
- [x] All commits are accompanied by meaningful commit messages
- [ ] All automated tests passed successfully (all builds are green)
- [x] Add yourself to contributors list
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: https://github.com/OpenMage/magento-lts/pull/2317#issuecomment-1201662652
However it does seem to work perfectly in all of the adminhtml areas.
I can confirm you that there's some work on the frontend that needs to be done for the use_in_forms to work :-)
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.phtmltoapp/design/frontend/baseas well - Support the other frontend forms
This pull request introduces 2 alerts and fixes 8 when merging e62967875febbf3d670aa7ae9e55790ef04a9b4a into cf82b8f623e754e63c9a64dbf3c21adaf46ddc56 - 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
wow!
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.

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.
@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.
Unit Test Results
1 files ±0 1 suites ±0 0s :stopwatch: ±0s 0 tests ±0 0 :heavy_check_mark: ±0 0 :zzz: ±0 0 :x: ±0 7 runs ±0 7 :heavy_check_mark: ±0 0 :zzz: ±0 0 :x: ±0
Results for commit cf82b8f6. ± Comparison against base commit cf82b8f6.
I have some customer group -> attribute set integration. Here's screenshots:


So basically I made a new customer attribute set "Special Customer" and then I can add or remove certain attributes to that set. If I then set a group to use that attribute set, attributes will show or hide based on the set in adminhtml and frontend forms.
I will have some extra care to not BC break but not implemented yet.
I think this is a really nice feature because you can easily show or hide certain fields for customers on their edit account page.
Edit: err I just realized I didn't push an upgrade script to add the new columns to the database ( I just added them manually ) with this sql:
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;
Also in working on this, I realize there's a lot of eav_form_* tables which I assume were to implement similar functionality into EE. This PR doesn't use those, but it's a whole 'nother can of worms to try and implement that functionality into CE.
@kiatng (and @fballiano) I'm seeking your opinion on the following (originally posted here)
How did you correlate attribute sets with customer groups? My first thought is to modify the
customer_grouptable:...
To add something like
customer_attribute_set_idand maybe evencustomer_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.
What I did 10 years ago was to add a column apply_to to the table eav_attribute_set. apply_to sets comma-delimited customer groups. I needed to also drop the unique index on array('entity_type_id', 'attribute_set_name'). It looks like this:

Since then, I also use the same apply_to for a custom EAV, which is not for customer group.
I do not see how you can make it compatible without using the same approach above.
@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?
If so that's a nice feature that my PR doesn't have, but I am not sure it's something to introduce to the core this way. I would rather try and reverse engineer the eav_form tables.
So no this PR wouldn't be compatible with your way, but I think I can make it at least not interfere with what you've done.
@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.
Will be ready for RC4?
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.
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
there were some bad conflicts, I did what i could, not sure 100% was correct