-
-
Notifications
You must be signed in to change notification settings - Fork 195
[ADD] maintenance_plan module #2
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
Conversation
|
LGTM; please fix TravisCI Suggestions/questions
|
dddec62 to
07a2f54
Compare
|
@dreispt
IMO both could be nice addition to the module, but I don't have time to spend on these features at the moment. |
|
@grindtildeath That's why I mentioned them as suggestion ideas. |
|
Does this have any conflict with the standard module's planned maintenance generation, if both are configured? Or from a user PoV: |
|
@dreispt |
dd33d59 to
06d8d54
Compare
c7bfb04 to
63e332e
Compare
63e332e to
e35b7e6
Compare
4d41b53 to
5fa2521
Compare
5fa2521 to
c2b47a8
Compare
dreispt
left a comment
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.
LGTM but made a few comements
| }) | ||
|
|
||
| @api.model | ||
| def _cron_generate_requests(self): |
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.
If this is to be used by a Cron it is an external API, so the name method should not start with an underscore.
Furthermore, I can see this used in a Button somewhere, so I suggest renaming it to "do_generate_requests" and expand the docstring to explain that ii can bu used by a cron job.
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.
Thanks for your review.
However, as this method is called by standard ir.cron from maintenance module, I suppose it's better to leave its name as it is, so we don't have to update the existing ir.cron :
https://github.com/odoo/odoo/blob/10.0/addons/maintenance/data/maintenance_cron.xml#L10
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 see, OK then.
| <record id="maintenance_kind_view_search" model="ir.ui.view"> | ||
| <field name="name">maintenance.kind.search</field> | ||
| <field name="model">maintenance.kind</field> | ||
| <field name="arch" type="xml"> |
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.
This view is redundant - Odoo does name searches by default.
| def _create_new_request(self, maintenance_plan): | ||
| self.ensure_one() | ||
| self.env['maintenance.request'].create({ | ||
| 'name': _('Preventive Maintenance (%s) - %s') % |
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 extract the dict creation into it's own _prepare_new_maintenance_request method, so that it is easy to extend it.
elicoidal
left a comment
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.
some details and one question on contraint
| # Copyright 2017 Camptocamp SA | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
| {'name': 'Maintenance Plan', | ||
| 'version': '10.0.1.0.0', |
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.
a summary would be welcome
maintenance_plan/hooks.py
Outdated
| "You have multiple preventive maintenance requests on an " | ||
| "equipment's next action date. Please leave only one " | ||
| "request as preventive on the date of equipment's next " | ||
| "action to install the module.")) |
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.
Nice to have: indicate which record are faulty
|
|
||
| _sql_constraints = [ | ||
| ('name_uniq', 'unique (name)', | ||
| "Maintenance kind name already exists !")] |
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.
remove exclamation mark
| date_gap = fields.Date.from_string( | ||
| next_maintenance_todo.request_date) - \ | ||
| fields.Date.from_string(last_maintenance_done.close_date) | ||
| # If the gap between the last_maintenance_done and the |
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.
This look like specific rule that might not apply everywhere
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.
@elicoidal Thanks for your review.
This code is an adaptation from odoo to keep the behaviour as close as possible to standard module : https://github.com/odoo/odoo/blob/10.0/addons/maintenance/models/maintenance.py#L165
| _sql_constraints = [ | ||
| ('equipment_kind_uniq', 'unique (equipment_id, maintenance_kind_id)', | ||
| "You cannot define multiple times the same maintenance kind on an " | ||
| "equipment maintenance plan!")] |
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 exclamation mark.
|
@elicoidal can you review the changes please? |
================
Maintenance Plan
This module extends the functionality of Odoo Maintenance module by allowing
an equipment to have different preventive maintenance kinds.
Installation
Install the module.
Usage
Instead of defining a period and duration for only one preventive maintenance
per equipment, you can define multiple preventive maintenance kind for each
equipment.
Maintenance Kinds have to be defined through the configuration menu. Their name
have to be unique and can be set as active or inactive, should these not be
used anymore.
On any equipment over the maintenance tab, the maintenance plan will appear
as an embedded list view, allowing to add different maintenance kind with their
own period and duration. The next maintenance date will then be computed
automatically according to today's date and the period defined, but the
maintenance request won't be created automatically as is the case in Odoo's
Maintenance module.
Instead, this module uses the original Cron job of Odoo's Maintenance module
to generate maintenance requests, should there not be any requests which is not
done, at the request date matching the maintenance plan next_maintenance_date
for this equipment and this maintenance kind !