-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
[16.0][MIG] product_attribute_set #142
Conversation
ddc4f56
to
d50bd05
Compare
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.
Thank you @sbejaoui LGTM (Code review + functional tests)
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.
Migration LGTM
self.attribute_set_id = self.categ_id.attribute_set_id | ||
|
||
|
||
# TODO : add the 'attribute.set.owner.mixin' to product.product in order to display |
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.
TODO still valid?
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.
ping :)
@sbejaoui just a reminder: |
/ocabot migration product_attribute_set |
@sbejaoui Could you attend to simahawk's comments and rebase PR ? Ty in advance |
ec39862
to
a061c05
Compare
2ff4c3e
to
9333148
Compare
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.
LG overall. Cleaning up commits as per #141 would be nice ;)
self.attribute_set_id = self.categ_id.attribute_set_id | ||
|
||
|
||
# TODO : add the 'attribute.set.owner.mixin' to product.product in order to display |
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.
ping :)
# Fill Category's products with Category's default attribute_set_id if empty | ||
super().write(vals) | ||
for record in self: | ||
if vals.get("attribute_set_id"): |
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.
can we have any test here?
I've added a dependency on #171 to remove domain from view (as it causes access rights problems for normal users) |
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 [ADD] icon.png
courtesy of <olivier@naya-tec.com> Apply dotfiles [UPD] Update product_attribute_set.pot
[UPD] README.rst
[UPD] Update product_attribute_set.pot
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: odoo-pim-14.0/odoo-pim-14.0-product_attribute_set Translate-URL: https://translation.odoo-community.org/projects/odoo-pim-14-0/odoo-pim-14-0-product_attribute_set/
Currently translated at 100.0% (27 of 27 strings) Translation: odoo-pim-14.0/odoo-pim-14.0-product_attribute_set Translate-URL: https://translation.odoo-community.org/projects/odoo-pim-14-0/odoo-pim-14-0-product_attribute_set/es_AR/
49f623f
to
1072cc5
Compare
@rousseldenis #171 is merged.. This one is ready to be merged |
/ocabot merge minor |
This PR looks fantastic, let's merge it! |
This PR has the |
Congratulations, your PR was merged at e8446f8. Thanks a lot for contributing to OCA. ❤️ |
No description provided.