-
-
Notifications
You must be signed in to change notification settings - Fork 788
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
[MIG] purchase_supplierinfo_discount: migrate to version 9.0 #272
[MIG] purchase_supplierinfo_discount: migrate to version 9.0 #272
Conversation
'product_id', 'product_qty', 'product_uom', 'partner_id', | ||
'date_order', 'date_planned', 'name', 'price_unit', 'state') | ||
def _compute_discount(self): | ||
discount = 0 |
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.
Don't change discount if not product_id.
else: | ||
break | ||
return res | ||
@api.onchange( |
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.
Why do you need all these onchange fields?
@api.onchange( | ||
'product_id', 'product_qty', 'product_uom', 'partner_id', | ||
'date_order', 'date_planned', 'name', 'price_unit', 'state') | ||
def _compute_discount(self): |
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 is not a compute field, so don't use it this nomenclature. Start with onchange_
.
@@ -19,49 +19,30 @@ | |||
# along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
# | |||
############################################################################## | |||
from openerp import models, api | |||
from openerp import models, api, fields |
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.
Sort this by alphabetic order
"author": "Serv. Tecnol. Avanzados - Pedro M. Baeza, " | ||
"Antiun Ingeniería S.L., " | ||
"Julien Weste" |
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.
Comma at the end
@@ -22,9 +22,10 @@ | |||
|
|||
{ | |||
"name": "Discounts in product supplier info", | |||
"version": "8.0.1.0.0", | |||
"version": "9.0.1.0.0", | |||
"author": "Serv. Tecnol. Avanzados - Pedro M. Baeza, " |
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 switch these two companies (Serv. Tecn... and Antiun) to the new one we have made: Tecnativa
if self.product_id: | ||
# Look for a possible discount | ||
qty_in_product_uom = self.product_qty | ||
tday = fields.Date.today() |
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.
Call it today
, as you only save one letter and it's less understandable
qty_in_product_uom = self.product_qty | ||
tday = fields.Date.today() | ||
sinfos = self.env['product.supplierinfo'].search( | ||
[('product_tmpl_id', '=', self.product_id.product_tmpl_id.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.
Use self.product_id.seller_ids.filtered()
instead, and you will propitiate a better inheritability (for example, there's other module that adds the supplierinfo at product level, other that use supplierinfo for customers). With this form, all of them will be compatible.
'|', ('date_start', '=', False), ('date_start', '<=', tday), | ||
'|', ('date_end', '=', False), ('date_end', '>=', tday), | ||
], order="min_qty desc") | ||
for sinfo in sinfos: |
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.
You can include this criteria in the filtered sentence, sort by min_qty, and select the last one ([:-1]
).
Test folder is for testing that the module works correctly, so it's needed. If Travis is red, you will need to adapt it to the new data layout. |
Thanks for the review! Here are some changes. |
Here are the tests. They are running ok on my side. |
I didn't see that there were a previous migration of the module (#272), and it's currently working, so I close this one. You should check next time first before making the effort. |
I didn't touch the "test" folder as I'm not sure at all how to deal with it.