Code review comment for lp:~lfreeke/therp-addons/7.0-res_partner_fiscal_position
Revision history for this message
| Stefan Rijnhart (Opener) (stefan-opener) wrote | # |
review:
Needs Fixing
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
Thanks! Looks really useful. Some comments:
If I understand correctly, name_get() was modified to display the company of the fiscal position, because in a superior company the fiscal positions of the underlying companies are accessible. This could help the user not to pick a fiscal position of any other company than the one it is working for at that moment, but it does not prevent a mixup of company data. I would prefer to filter the fiscal positions on the current user's company. You may have to create a function field on res.country to be able to access this piece of data though.
In the manifest, please explain what is different in this module from the module that it is based on.
The object instantiation in line number 150 is not needed in recent versions of Odoo and should be removed.
Please remove commented code in ll.136,137