[18.0][MIG] attribute_set: Migration to 18.0 #192
Conversation
New organization : - attribute_set (former base_custom_attribute) - product_attribute_set (former pim_custom_attribute but without menus) - pim (The "PIM application" former pim_base) - pim_attrubute_set (depends on product_attribute_set and adds menus in the PIM application)
And small FIX in _build_attribute_field()
[UPD] README.rst [UPD] README.rst
[UPD] README.rst
…d attribute_set [CHG] add required columns on attributes on attribute set form view attribute_set 13.0.1.0.1
[UPD] README.rst [UPD] README.rst [ADD] icon.png Apply dotfiles [UPD] Update attribute_set.pot attribute_set 14.0.1.0.1 [UPD] README.rst [IMP] update dotfiles [ci skip]
[UPD] Update attribute_set.pot attribute_set 14.0.1.1.0
[UPD] Update attribute_set.pot [UPD] README.rst
When you define an attribute as a select field and add a related model, you have the option to "Load attribute options". On this wizard, an "option_ids" dummy field is created via fields_view_get. This change deletes option_ids from create (once the options are created) and read
menu items are visible only for two levels
[UPD] Update attribute_set.pot [UPD] README.rst [UPD] README.rst
…ss data (tree/search...) Use get_view() to get the view_type directly. Moreover, this will allow to test it through odoo.tests.Form(). Add attribute_set_id field in test form (as it should be included in inherited views
If user is not in the base.group_erp_manager group, the value_ref field selection function will be computed without access rights on ir.model. So, hide the field for users not in that group.
Add all the attributes defined for the model into the list of fields to load to display the form view. Wihout this change a JS error occurs when displaying the form view of a x2many field for a model defining attributes. The origin of the error comes from the the get_views method which is called for the type 'form' only. In this case it's important to add all the custom attributes to the list of fields defined on the model for the view. When the same method is called without restriction it will load all the views. Since odoo include all the fields defined on the model for the search view, the error doesn't occur when using the form view from a menu action.
dnplkndll
force-pushed
the
18.0-mig-attribute_set
branch
from
75801f6 to
5eb64de
Compare
kobros-tech
force-pushed
the
18.0-mig-attribute_set
branch
2 times, most recently
from
fb0f45c to
45cf3a9
Compare
kobros-tech
force-pushed
the
18.0-mig-attribute_set
branch
3 times, most recently
from
3e78e2b to
64e9353
Compare
| string="Attribute Nature", | ||
| required=True, | ||
| default="custom", | ||
| store=True, |
There was a problem hiding this comment.
| store=True, |
no need
| ast.literal_eval(self.domain) | ||
| except ValueError: | ||
| raise ValidationError( | ||
| _( |
There was a problem hiding this comment.
| _( | |
| self.env. _( |
| # context since 17.0 will be dropped in views | ||
| # unless we suffix it's key with _view_ref | ||
| "context": {"attribute_id_view_ref": self.id}, | ||
| "name": _("Options Wizard"), |
There was a problem hiding this comment.
| "name": _("Options Wizard"), | |
| "name": self.env._("Options Wizard"), |
| """Delete outdated attribute's field values on existing records.""" | ||
| self.ensure_one() | ||
| custom_field = self.name | ||
| for obj in self.env[self.model].search([]): |
There was a problem hiding this comment.
Should use the filtered function to reduce nesting of if and for statements?
There was a problem hiding this comment.
can you write the code and I apply?
There was a problem hiding this comment.
I see filtered method is great, but also regarding migration we focus on the scope.
but reducing syntax and improving it is better to be in it's own [IMP] PR, right?
| def write(self, vals): | ||
| # Prevent from changing Attribute's type | ||
| if "attribute_type" in list(vals.keys()): | ||
| if self.search_count( | ||
| [ | ||
| ("attribute_type", "!=", vals["attribute_type"]), | ||
| ("id", "in", self.ids), | ||
| ] | ||
| ): | ||
| raise ValidationError( | ||
| _( | ||
| "Can't change the type of an attribute. " | ||
| "Please create a new one." | ||
| ) | ||
| ) | ||
| else: | ||
| vals.pop("attribute_type") | ||
| # Prevent from changing relation_model_id for multiselect Attributes | ||
| # as the values of the existing many2many Attribute fields won't be | ||
| # deleted if changing relation_model_id | ||
| if "relation_model_id" in list(vals.keys()): | ||
| if self.search_count( | ||
| [ | ||
| ("relation_model_id", "!=", vals["relation_model_id"]), | ||
| ("id", "in", self.ids), | ||
| ] | ||
| ): | ||
| raise ValidationError( | ||
| _( | ||
| """Can't change the attribute's Relational Model in order to | ||
| avoid conflicts with existing objects using this attribute. | ||
| Please create a new one.""" | ||
| ) | ||
| ) | ||
| # Prevent from changing 'Serialized' | ||
| if "serialized" in list(vals.keys()): | ||
| if self.search_count( | ||
| [("serialized", "!=", vals["serialized"]), ("id", "in", self.ids)] | ||
| ): | ||
| raise ValidationError( | ||
| _( | ||
| """It is not allowed to change the boolean 'Serialized'. | ||
| A serialized field can not be change to non-serialized \ | ||
| and vice versa.""" | ||
| ) | ||
| ) | ||
| # Set the new values to self | ||
| res = super().write(vals) |
There was a problem hiding this comment.
| def write(self, vals): | |
| # Prevent from changing Attribute's type | |
| if "attribute_type" in list(vals.keys()): | |
| if self.search_count( | |
| [ | |
| ("attribute_type", "!=", vals["attribute_type"]), | |
| ("id", "in", self.ids), | |
| ] | |
| ): | |
| raise ValidationError( | |
| _( | |
| "Can't change the type of an attribute. " | |
| "Please create a new one." | |
| ) | |
| ) | |
| else: | |
| vals.pop("attribute_type") | |
| # Prevent from changing relation_model_id for multiselect Attributes | |
| # as the values of the existing many2many Attribute fields won't be | |
| # deleted if changing relation_model_id | |
| if "relation_model_id" in list(vals.keys()): | |
| if self.search_count( | |
| [ | |
| ("relation_model_id", "!=", vals["relation_model_id"]), | |
| ("id", "in", self.ids), | |
| ] | |
| ): | |
| raise ValidationError( | |
| _( | |
| """Can't change the attribute's Relational Model in order to | |
| avoid conflicts with existing objects using this attribute. | |
| Please create a new one.""" | |
| ) | |
| ) | |
| # Prevent from changing 'Serialized' | |
| if "serialized" in list(vals.keys()): | |
| if self.search_count( | |
| [("serialized", "!=", vals["serialized"]), ("id", "in", self.ids)] | |
| ): | |
| raise ValidationError( | |
| _( | |
| """It is not allowed to change the boolean 'Serialized'. | |
| A serialized field can not be change to non-serialized \ | |
| and vice versa.""" | |
| ) | |
| ) | |
| # Set the new values to self | |
| res = super().write(vals) | |
| def write(self, vals): | |
| def _check_field_change(field, error_message): | |
| """Helper function to check if a field is being changed and raise an error if so.""" | |
| if field in vals: | |
| if self.search_count([(field, "!=", vals[field]), ("id", "in", self.ids)]): | |
| raise ValidationError(self.env._(error_message)) | |
| else: | |
| vals.pop(field) | |
| # Prevent changing Attribute's type | |
| _check_field_change( | |
| "attribute_type", | |
| "Can't change the type of an attribute. Please create a new one.", | |
| ) | |
| # Prevent changing relation_model_id for multiselect Attributes | |
| _check_field_change( | |
| "relation_model_id", | |
| """Can't change the attribute's Relational Model to avoid conflicts with | |
| existing objects using this attribute. Please create a new one.""", | |
| ) | |
| # Prevent changing 'Serialized' | |
| _check_field_change( | |
| "serialized", | |
| """It is not allowed to change the boolean 'Serialized'. | |
| A serialized field cannot be changed to non-serialized and vice versa.""", | |
| ) | |
| # Set the new values to self | |
| res = super().write(vals) |
This is just a small suggestion on how to reuse another function, you can split it out and besides the labels of the fields like Serialized, Relational Model and type of an attribute should come from the fields (and respect eventual translations).
hint: you can refer to this forum: https://www.odoo.com/forum/help-1/get-the-value-of-string-in-fields-150616
There was a problem hiding this comment.
thanks, in fact I respect how you tend to minimize and organize code.
I am afraid if this good idea can apply during migration as I see migration tends to focus on migrating changes requied in new versions so it can be easy to review and catch errors in the process.
But, I like improvements be applied just after migration in the name of reducing code or optimizing it.
attribute_set/utils/orm.py
Outdated
| if node.get("attrs"): | ||
| modifiers.update(safe_eval(node.get("attrs"))) | ||
|
|
||
| if node.get("states"): |
There was a problem hiding this comment.
The XML attributes attrs and states are no longer used, and should be replaced by their equivalent Python expression under invisible, required and/or readonly attributes. Example: attrs="{'invisible': [('name', '=', 'red')]} is converted to invisible="name == 'red'". Full explanation and more examples at
|
I’m comparing the UI between versions
|
|
I wondered if feasible to use: 
from: |
hi @dnplkndll, do you mean replace |
|
lost display of
|
I not completely. but perhaps the similar UI to place and group them? set properties like visible in search and filter on the website. probably would just cause more required changes on upstream changes though |
|
can you have a look? |
|
is there any remaining comments? |
| "title": _("Error!"), | ||
| "message": _( |
There was a problem hiding this comment.
| "title": _("Error!"), | |
| "message": _( | |
| "title": self.env._("Error!"), | |
| "message": self.env._( |
|
|
||
| if len(placeholder) != 1: | ||
| raise ValidationError( | ||
| _( |
| <field name="model">attribute.option</field> | ||
| <field name="priority" eval="6" /> | ||
| <field name="arch" type="xml"> | ||
| <form string="Attribute Option" col="6"> |
There was a problem hiding this comment.
| <form string="Attribute Option" col="6"> | |
| <form string="Attribute Option" col="6"> |
What does the col attribute do here?
btw I see some migrations also remove the string attribute on form view, it might be an optional you can check it out here: OCA/account-financial-tools@c581dba
|
comments #192 (comment) and #192 (comment) are still pending |
what is the error in these two comments? |
|
kobros-tech
force-pushed
the
18.0-mig-attribute_set
branch
from
0a28833 to
54eef80
Compare
|
for the model_id field, thanks for clarifying I put it is its correct place! but for changing the UI and grouping fields I had to do so because of some UI errors and the fields were so many for be traced in one row. |
kobros-tech
force-pushed
the
18.0-mig-attribute_set
branch
from
54eef80 to
56d085e
Compare
kobros-tech
force-pushed
the
18.0-mig-attribute_set
branch
from
56d085e to
bac9366
Compare
|
Now I squashed all commits so someone can see changes properly, the module is huge and the changes are many. I hope we finish it with the optimum quality, thanks! |
hi, @kobros-tech In my opinion, a migration PR should focus strictly on updating the code to align with upstream changes. Any additional improvements or new features would be better handled in a separate PR or commit, so that they can be reviewed and discussed more easily. Thanks a lot for the effort you’ve put into this! |
|
maybe you are the best if you introduce a better migration PR based on this one or the previous versions as I know you are a very good developer. |
13 participants
No description provided.