Review: Needs Fixing
Quick eyeball review, I did not reviewed the business aspect.
dubious indentation at l. 647
837 + 'audit_ids':
fields.many2many('mgmtsystem.audit','mgmtsystem_audit_nonconformity_rel','mgmtsystem_audit_id','mgmtsystem_action_id','Related
Audits'),
Just a remark, the relation name and column names are no longer necessary, so
you can just write:
'audit_ids': fields.many2many('mgmtsystem.audit', 'Related Audits'),
No need to change that though.
1646+ 'description': fields.text('Description', translation=True),
2801+ 'description': fields.text('Description', translation=True),
typo: s/translation/translate/
The context propagation is often missing.
1795+ vals = {'evaluation_date': time.strftime('%Y-%m-%d %H:%M'),
'evaluation_date_user_id': uid }
It would be better if the date time formatting use the constant
openerp.tools.DEFAULT_SERVER_DATETIME_FORMAT
1655 +_STATES = [
1656 + ('d', _('Draft')),
1657 + ('a', _('Analysis')),
1658 + ('p', _('Pending Approval')),
1659 + ('o', _('In Progress')),
1660 + ('c', _('Closed')),
1661 + ('x', _('Cancelled')),
1662 + ]
The state codes are meaningless.
How do a developer knows what will be filtered in this domain?
[('state','=','d')]
This lack of meaning seems to be common on the fields.selection of different
classes across the merge proposal.
On the usage of namespaces, classes and instanciation of models:
2591 +from osv import fields, osv
2592 +
2593 +class mgmtsystem_nonconformity(osv.osv):
2594 + _inherit = "mgmtsystem.nonconformity"
2595 + _columns = {
2596 + 'analytic_account_id': fields.many2one('account.analytic.account',
'Contract'),
2597 + }
2598 +mgmtsystem_nonconformity()
You better have to use the complete namespace (by the way the import of osv is
now useless if we just need Model) :
from openerp.osv import fields, orm
and inherit of the new classes:
class mgmtsystem_nonconformity(orm.Model):
The instanciation (l.2598) after the class declaration is no longer required.
Unrelated to the code, the review type "Resubmit" is to be used by a reviewer
to ask you to resubmit, not the other way. You can see in the reviewer list on
top of the page that you are now part of the reviewers for your proposal, and
you ask yourself a resubmit ;-)
--
https://code.launchpad.net/~openerp-community/openerp-mgmtsystem/nc-extend/+merge/139534
Your team OpenERP Community is subscribed to branch
lp:~openerp-community/openerp-mgmtsystem/nc-extend.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-community
More help : https://help.launchpad.net/ListHelp
