Code review comment for lp:~agilebg/account-invoice-report/adding_account_invoice_production_lot_7

Revision history for this message
Alexis de Lattre (alexis-via) wrote :

Dear Lorenzo,

Thank you for this very interesting module. I was looking for such a module for a new development I want to do to replace my "fleet_maintenance" module (lp:fleet-maintenance).

Here are my remarks, from the most important to the least important :

1) the dependancy on invoice_webkit is not a good idea. I would really like to use this module, but I use aeroo reports. That would require splitting the module in 2 :
- one generic module "account_invoice_prodlots" that adds the field prod_lot_ids on the invoice lines,
- a second module "account_invoice_prodlots_webkit" with a dependancy on invoice_webkit which writes on "formatted_note" and adds the button "Load Lines Lots".
I think it's important to keep modules that bring additionnal functionalities independant from the reporting engine as much as we can.

2) for the field 'prod_lot_ids' in the invoice lines, I would prefer to have a fields.many2many instead of a field.function. Not only for performance reasons, but also for flexibility reasons. For example, as you will see in my specifications on the replacement of the fleet_maintenance module (cf https://docs.google.com/document/d/1EH1lnvkD3-aZjOTQ2jv_fTon1ZE0MzMm7hmmMgs9dJg/edit?usp=sharing), I want to have a many2many link from the invoice lines to the production lots for the maintenance service lines, in order to know on which prodlots the maintenance service apply. For that, the fields.function 'prod_lot_ids' needs to be converted to a fields.many2many. This many2many field would have to be set:
- when creating the invoice from the picking, by an inherit of the function _prepare_invoice_line() from stock/stock.py
- when creating the invoice from the sale order, by an inherit of the function _prepare_order_line_invoice_line() from sale/sale.py

If you think it's not a good idea and you want to keep the prod_lot_ids fields as field.function, then I will have to create another field 'maintenance_prodlot_ids' in the invoice lines that only applies for the maintenance service lines.

3) on the invoice line, you add a field 'displayed_lot_id' but you don't use it anywhere. What is this field for ?

4) on the stock moves, the many2one to the prodlot is called 'prodlot_id', so I would suggest to name the field 'prodlot_ids' instead of 'prod_lot_ids' on the invoice lines

review: Needs Information

« Back to merge proposal