-
-
Notifications
You must be signed in to change notification settings - Fork 720
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[12.0][MIG] Migrated product_sequence to v12. #416
[12.0][MIG] Migrated product_sequence to v12. #416
Conversation
… reference. The reference (default code) is unique (SQL constraint) and required.
… each product without default_code
some minor reformatting in the process
…e reactivated when the modules are migrated
Version numbers should only have 2 digits.
In order to get visibility on https://www.odoo.com/apps the OCA board has decided to add the OCA as author of all the addons maintained as part of the association.
Add pre_init_hook to manually set nulls and '/' default codes to a unique code to greenify branch add pragma to pre_init method as it must have completed successfully if module installs
* Migration to v11 * Support sequence by Product Category
Hey @sudhir-erpharbor, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
@pedrobaeza what am I missing? |
@atchuthan that one is Odoo CLA, not OCA CLA. Did you send OCA ECLA or ICLA? There's also an issue with used emails. |
Hey @sudhir-erpharbor, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
OCA ECLA |
Hey @sudhir-erpharbor, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
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.
Tested on Odoo 12 CE and works
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.
Thanks for your contribution.
I reviewed the code (translations not reviewed) and I only have three small comments :)
Alvaro
she/he will need to write '/' on the internal reference to force the | ||
re-assignment.""" | ||
for product in self: | ||
if vals.get('default_code', '') == '/': |
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.
this if
could be out of the loop for improving the performance
for product in self: | ||
if vals.get('default_code', '') == '/': | ||
category_id = vals.get('categ_id', product.categ_id.id) | ||
category = self.env['product.category'].browse(category_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.
what if there is no category?
sequence = self.env.ref( | ||
'product_sequence.seq_product_auto') | ||
ref = sequence.next_by_id() | ||
vals['default_code'] = ref |
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.
are you sure you want to update the main dict? I would say you should update only the values to write in the record of the loop
"to be proposed." | ||
) | ||
|
||
_sql_constraints = [ |
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.
Because add sql contraints????
this functionality is added on module product_code_unique
I prefered keep into 2 modules
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.
Yes, I agree
This PR should be closed. Superseed by #507. |
Please review and give feedback.