openerp-community team mailing list archive

[Pedro Manuel Baeza <pedro.baeza@xxxxxxxxx>]

Review: Needs Fixing code review and test

Hi, Joël and Romain,

Thank you very much for the effort you are making to get this module in the best way. I have tested it and view the code, and these are my remarks:

- We agreed about renaming to product_price_historization_by_company or it can be only product_price_historization, are we?
- You can put in the description that is multi-company aware and avoid it on the module name.
- Put product_multi_company_purchase_demo.yml on test folder and add it to tests on manifest file.
- Create at least the pot translation template file.
- Rename 'price.history' model to 'product.price.history' to be consistent.
- In security/ir.model.access.csv, there are references to groups installed by "mrp", "sale" or "hr"modules, but these modules are not declared as dependences. BTW, it doesn't make any sense to have it, because you already have enough permissions with groups like base.group_sale_manager or base.group_user.
- It would be very interesting to have any screen to query past prices.

Regards.
-- 
https://code.launchpad.net/~camptocamp/openerp-product-attributes/port-add-product_multi_company_7.0-bis-jge/+merge/192872
Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes.