-
-
Notifications
You must be signed in to change notification settings - Fork 195
[10.0][ADD] Module maintenance_equipment_scrap #4
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
This module improves the action of scrapping an equipment, sending a message and automatically setting the scrap date when the action is performed.
|
|
||
| To configure this module, you need to: | ||
|
|
||
| #. Go to 'Settings' -> 'Technical' -> 'Email' -> 'Templates' and create a new template you wish to use for equipment scrapping notifications |
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.
Don't you provide a default template?
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.
In my latest commit I provide it.
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.
Then change this README for telling about that.
|
|
||
| _inherit = 'maintenance.equipment' | ||
|
|
||
| asset_id = fields.Many2one('account.asset.asset', string='Asset') |
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.
Why adding this 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.
You're perfectly right. It was a mistake.
- Removed unrequired method extensions; - The Scrap Template configuration is now done in the Stock Configuration (previously on the Accounting Configuration); - Added dependency to Stock module;
| { | ||
| 'name': 'Maintenance Equipments Scrap', | ||
| 'summary': 'Enhance the functionality for Scrapping Equipments', | ||
| 'author': 'Onestein', |
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.
Add OCA
| 'summary': 'Enhance the functionality for Scrapping Equipments', | ||
| 'author': 'Onestein', | ||
| 'website': 'http://www.onestein.eu', | ||
| 'category': 'Human Resources', |
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.
Maintenance?
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 picked the same category of the Odoo base module 'maintenance', just for keeping consistency with it, but it's fine for me to change it. What do you think?
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.
OK
| equipment_scrap_template_id = fields.Many2one( | ||
| 'mail.template', | ||
| string='Equipment Scrap Email Template' | ||
| ) |
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.
What do you think about having the Template the a Equipment category level instead (or also)?
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 think it could be a good idea. Maybe the best is adding it on the Equipment category in order to be automatically proposed on the equipment when its category changes?
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, that could work.
| 'maintenance_equipment_scrap.wizard_perform_equipment_scrap_action' | ||
| ) | ||
| result = action.read()[0] | ||
| return result |
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 module is just about sending an email when you click on a button?
Don't take me wrong but it sounds shallow - a quick Automated Action can do just the same.
Shouldn't we somehow also mark the Equipment as inactive?
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 module "actively" only does two things:
- Sends an email when a scrap is performed, and only if on the equipment is selected a scrapping template;
- Writes the scrap_date field when a scrap is performed.
Still, I think that what really makes this module useful is that it provides a structure for the equipment scrapping functionality. To better explain, consider that in base Odoo, the equipment scrapping functionality just doesn't exist: as far as I can see, an equipment is considered scrapped when the user manually set a scrap date; there is no state change, no active/inactive switch, nothing else that says that a given equipment is scrapped.
Because of this lack of structure, it's quite tricky to define additional logics to be ran when an equipment is scrapped (such logic as, for exemple, to send an email when it occurs).
This module makes the scrap_date field read-only and provides a procedure for a user to scrap an equipment: it has to click the "scrap" button, to fill a form and finally to submit it. This provides, apart from the obvious functionality of sending an email, an entry-point for adding more and more logic (making the equipment inactive is a possible example, as it could be making some changes on the related asset, or whatever else we can think about, and that could be added in this or in other, more specific-purpose, modules).
What do you think about 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.
OK, thanks for the explanation. Indeed it is a good step forward.
I'm pretty sure we also need a Stage/State, but I agree that it could be left for a later PR.
In case you are willing to add that, I strongly suggest reusing the oca/server-tools base_stage_state module.
| 'depends': [ | ||
| 'maintenance', | ||
| 'mail', | ||
| 'stock', |
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 can see that you added the Stock dependency to be able to add a Settings option.
You thought well, but I think that this is a too heavy dependency.
I would prefer to have it removed, even if that also means dropping the Settings option for now.
ideally, we could have a "maintenance_settings" module, providing the missing Settings menu for the Maintenance app, and then we could add this option there.
What do you think?
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 confess I was quite doubtful about using the stock settings for this, deciding for it just because there isn't a default maintenance settings functionality to use nor I thought it was the case to add it within this module.
Therefore I'm in favor of dropping the settings option for now, looking for a further module that provides a Maintenance functionality.
- Removed settings option; - Removed dependency from module stock; - Added equipment scrap mail template on equipment category; - On equipments, when the category changes, the equipment scrap mail template is loaded from the new category; - Added test for the onchange functionality; - Fixed year in the headers;
|
@dreispt @pedrobaeza |
|
@pedrobaeza |
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.
LGTM
|
@pedrobaeza I think your comments were attended and we could move forward. |
This module improves the action of scrapping an equipment, sending a message and automatically setting the scrap date when the action is performed.