-
Notifications
You must be signed in to change notification settings - Fork 102
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
Customer EAV attribute generation #583
Customer EAV attribute generation #583
Conversation
Hi @ProkopovVitaliy, thanks for your very useful contribution. But can we please not use deprecated methods? Like the model @VitaliyBoyko, I'm assuming we don't want to use deprecated methods or bad practices in this project. Since every Magento developer in the world will be using this and we're only spreading bad code by doing so since we are the platform. :) |
'used_in_forms', | ||
[${CUSTOMER_FORMS}] | ||
); | ||
$attribute->save(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a resource model instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ProkopovVitaliy thank you for the feature, could you please check the below comments? Additionally, I'd have the following points, that can be done separately:
- Can we have the dialog height more compact? Now, the fields' spaces are dependent by the number of shown fields.
- Add some dependencies between
Input
andType
? Which means, that if you selectedtext
for Input, the only available options for Type (which should be Backend Type) would betext
, 'varchar'. Magento default the following info \Magento\CustomAttributeManagement\Helper\Data::getAttributeInputTypes while creating the attributes from the backend.
Thank you.
|
||
public enum CustomerForm { | ||
|
||
ADMINHTML_CHECKOUT("adminhtml_checkout"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USER_DEFINED("user_defined"), | ||
POSITION("position"), | ||
SYSTEM("system"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that it makes sense to give the possibility to configure both user_defined
and system
. Since user_defined = true
, automatically means system = false
, and vice versa.
/* | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* |
/* | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove all such occurrences.
* |
Hello, @ProkopovVitaliy! Could you please tell me when you can finish development here? Regards, |
@bohdan-harniuk I'll cover @ProkopovVitaliy back here. |
…to/magento2-phpstorm-plugin into customer-aev-attribute-generation
I suggest making such improvements in a separate PR. All other things were fixed. |
Description (*)
Added customer eav attribute generation
For example lent's add multiselect attribute with options:
Open and fill attribute dialog:
Check generated Data Patch file:
After
setup:upgrade
execution check customer in admin panel:Let's create required attribute that will show in customers grid:
Open and fill dialog form:
Check generated Data Patch file:
After
setup:upgrade
execution check customer in admin panel:Fixed Issues (if relevant)
Questions or comments
Contribution checklist (*)