-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
[REF] 8.0 mgmtsystem nonconformity #25
[REF] 8.0 mgmtsystem nonconformity #25
Conversation
527775d
to
fe04561
Compare
@@ -22,7 +22,9 @@ | |||
from openerp.tools.translate import _ | |||
from openerp import netsvc | |||
from openerp.osv import fields, orm | |||
from openerp.addons.base_status.base_state import base_state | |||
|
|||
#from openerp.addons.base_status.base_state import base_state |
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 remove this commented line
As a general rule, unless the classes are very small (case of one2many). We strongly recommend you split python/xml files by model involved. |
res = self.name_get() | ||
return dict(res) | ||
|
||
_constraints = [ |
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.
_constraint not present in mgmtsystem_nonconformity_origin.py. yet the code is almost the same. We could technically subclass both class and clone the implementation or just add a type instead of copying code.
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.
Good catch on the constraint.
I haven't done this, but I think to clone a class you can do this:
class mgmtsystem_nonconformity_origin(models.Model):
_name = "mgmtsystem.nonconformity.origin"
_inherit = "mgmtsystem.nonconformity.cause"
If that doesn't work, you can use an abstract model and then subclass from that
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.
Yes, but I'd rather subclass them as origin aren't causes and causes aren't origin. If it's fine to inherit only for the structure only then it could be fine but if there is a way to get all origins and it will catch all causes too it won't make sense.
depends on |
pep8:
|
a3bc786
to
0ef651b
Compare
Could we have this to not depend on |
@dreispt having a dependency on document_page_procedure does not necessarily mean you manage your documentation content in Odoo. We use it as a list to reference the content in another application. The day we want to put everything in Odoo, we just have to transfer the content and drop the other application. If you want to drop the dependency, you will have to create a new field to provide the list of procedures (quality), environmental aspects (environment), and other thing in other systems. I am sure you will agree with me that any Management Systems software should come with a documentation system, so why Odoo should be different here. |
0ef651b
to
c7e0c10
Compare
@@ -3,6 +3,7 @@ | |||
<data> | |||
|
|||
<record id="action_mgmtsystem_nonconformity" model="ir.actions.act_window"> | |||
<field name="name">board.mgmtsystem.nonconformity.act_window</field> |
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.
name should be a human readable text
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.
Or it can be ommited
e4a43eb
to
26e2fd9
Compare
Thanks @bwrsandman ! |
stage_id = fields.Many2one( | ||
'mgmtsystem.action.stage', | ||
'Stage', | ||
default=lambda self: self.get_default_stage() |
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 use: default=get_default_stage
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.
I use lambda so get_default_stage
can be overriden. AFAIK setting default to a member function directly does not allow it to be overriden the way that a call to self.
does.
@dreispt Most other porting PRs are dependent on this one, do you think your adjustments can be made after merging this? |
I think we added a name field on version 7 and I don't see it here. Otherwise |
@gs-clearcorp Sure! |
PR here savoirfairelinux#18 |
…y-gs 8.0 mgmtsystem nonconformity gs
@bwrsandman @max3903 Good enough today is better than perfect tomorrow, so yes, go ahead. |
👍 |
…mity Port mgmtsystem_nonconformity to 8.0
BSCOS 109 + BSCOS 108
Removed base_status dependency and reimplemented some of the methods that were missing to keep the
cases
working.depends on