-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
[ADD] hr fleet : claim report and cars history for one employee #28077
Conversation
addons/hr_fleet/models/employee.py
Outdated
def action_open_employee_cars(self): | ||
self.ensure_one() | ||
Fleet = self.env['fleet.vehicle.assignation.log'] | ||
car_line = Fleet.search([('driver_id','=',self.user_id.partner_id.id)]).mapped('vehicle_id').mapped('id') |
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 replace by
car_line = Fleet.search([('driver_id','=',self.user_id.partner_id.id)]).mapped('vehicle_id')
(remove mapped('id')
)
and put "domain": [("id", "in", car_line.ids)],
(add .ids
)
addons/hr_fleet/models/employee.py
Outdated
def _compute_employee_cars_count(self): | ||
Fleet = self.env['fleet.vehicle.assignation.log'].sudo() | ||
for employee in self: | ||
employee.employee_cars_count = Fleet.search_count([ |
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.
use read_group
instead search_count
(only sql request for all employee)
3290589
to
499e7d8
Compare
3290589
to
765944e
Compare
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 comments, mostly linting/guidelines issues. Some chunks of code could be improved too.
Globally it's a good work ! Thanks.
addons/hr_fleet/controllers/main.py
Outdated
import base64 | ||
import io | ||
|
||
from odoo import http, _ |
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.
_ is imported but unused. Could be removed.
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.
Could be from odoo.http import request, route, Controller
. More compact.
addons/hr_fleet/controllers/main.py
Outdated
@http.route(["/hrfleet/claimreport/<int:employee_id>"], type='http', auth='user') | ||
def get_claim_report_user(self, employee_id, **post): | ||
|
||
user_rec = http.request.env['res.users'].sudo().search([('id', '=', http.request.session.uid)]) |
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 could use browse
instead of searching id = uid
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.
Is the sudo really needed ?
addons/hr_fleet/controllers/main.py
Outdated
if not user_rec.has_group('fleet.fleet_group_manager'): | ||
return http.request.not_found() | ||
|
||
Fleet = http.request.env['fleet.vehicle.assignation.log'] |
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.
Your models are used only once or twice in your method. No need to store them into variables.
addons/hr_fleet/controllers/main.py
Outdated
Attachment = http.request.env['ir.attachment'] | ||
Employee = http.request.env['hr.employee'] | ||
|
||
employee = Employee.search([('id','=',employee_id)],limit=1) |
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.
Missing space in [('id', '=', employee_id)], limit=1
You could use a linter to avoid those kind of issues. If you're using sublime-text, pylinter is a good one.
addons/hr_fleet/controllers/main.py
Outdated
|
||
partner_id = employee.user_id.partner_id.id | ||
if not partner_id: | ||
return http.request.not_found() |
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.
Could be factored into:
employee = Employee.search([('id', '=', employee_id)], limit=1)
partner_id = employee.user_id.partner_id
if not employee or not partner:
return http.request.not_found()
addons/hr_fleet/models/employee.py
Outdated
|
||
def _compute_employee_cars_count(self): | ||
Fleet = self.env['fleet.vehicle.assignation.log'].sudo() | ||
driver_id = self.mapped('user_id').mapped('partner_id').ids |
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 is the same as self.mapped('user_id.partner_id').ids
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.
And driver_id
could be a list of ids, so you could rename it into driver_ids
addons/hr_fleet/models/employee.py
Outdated
fleet_data = Fleet.read_group(domain=[('driver_id', 'in', driver_id)], fields=['driver_id'], groupby=['driver_id']) | ||
mapped_data = dict([(m['driver_id'][0], m['driver_id_count']) for m in fleet_data]) | ||
for employee in self: | ||
employee.employee_cars_count = mapped_data.get(employee.user_id.partner_id.id) |
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.
It's great you used read_group
instead of search_count
. This is a good practice!
addons/hr_fleet/models/employee.py
Outdated
'name': 'Claim Report', | ||
'type': 'ir.actions.act_url', | ||
'url': '/hrfleet/claimreport/%(employee_id)s' % {'employee_id': self.id}, | ||
} |
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.
Linting issue (missing end of file line)
addons/hr_fleet/controllers/main.py
Outdated
|
||
|
||
class HrFleet(http.Controller): | ||
@http.route(["/hrfleet/claimreport/<int:employee_id>"], type='http', auth='user') |
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.
The route could be renamed into /fleet/print_claim_report/<int:employee_id>
from odoo import api, fields, models | ||
|
||
|
||
class FleetVehicleAssignationLog(models.Model): |
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 know that the model name is not sexy, but according to the guidelines, the file should be named fleet_vehicle_assignation_log
765944e
to
57ed957
Compare
Purpose ======= When an employee leaves the company, the fleet manager must ask to the insurance company a claim report for this employee. But in oddo, there is no easy way to have an history of the drivers. Specification ============= 1/ On the employee form, add a stat button: "x cars". The button shouldn't be displayed if the count = 0. Make a bridge (auto_install: True in the manifest) depending on hr (hr.employee) and fleet (fleet.vehicle.assignation.log) - Add a field employee_cars_count (computed) to count the amount of cars the employee drove. - Add an method action_open_employee_cars, that return an action (ir.actions.act_window) that open on the fleet.vehicle kanban filtered on cars the employee had driven. 2/ In this bridge module, add on the fleet.vehicle.assignation.logs a button (like in expense) to upload documents about the car for a particular employee (PV, rapport d'assurance, blabla). On the employee form view, add an action button (with groups fleet_manager) to print all the related documents (all the attachements on the fleet.vehicle.assignation.logs for the employee).
57ed957
to
167dc68
Compare
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 small comments.
addons/hr_fleet/controllers/main.py
Outdated
@http.route(["/fleet/print_claim_report/<int:employee_id>"], type='http', auth='user') | ||
def get_claim_report_user(self, employee_id, **post): | ||
|
||
user_rec = http.request.env['res.users'].browse(http.request.session.uid) |
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 get user from request.env.user
. No need to browse it.
addons/hr_fleet/controllers/main.py
Outdated
import base64 | ||
import io | ||
|
||
from odoo import http, _ |
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.
Could be from odoo.http import request, route, Controller
. More compact.
@robodoo r+ |
<header> | ||
<button name="action_get_claim_report" string="Claim Report" | ||
type="object" groups="fleet.fleet_group_manager" | ||
attrs="{'invisible': [('employee_cars_count','=', 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.
are you sur it work ?, because employee_cars_count
is not prevent in the view
<field name='employee_cars_count' invisible="1"/>
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.
@fmdl If you look, the field is present inside the second button ;)
Purpose ======= When an employee leaves the company, the fleet manager must ask to the insurance company a claim report for this employee. But in oddo, there is no easy way to have an history of the drivers. Specification ============= 1/ On the employee form, add a stat button: "x cars". The button shouldn't be displayed if the count = 0. Make a bridge (auto_install: True in the manifest) depending on hr (hr.employee) and fleet (fleet.vehicle.assignation.log) - Add a field employee_cars_count (computed) to count the amount of cars the employee drove. - Add an method action_open_employee_cars, that return an action (ir.actions.act_window) that open on the fleet.vehicle kanban filtered on cars the employee had driven. 2/ In this bridge module, add on the fleet.vehicle.assignation.logs a button (like in expense) to upload documents about the car for a particular employee (PV, rapport d'assurance, blabla). On the employee form view, add an action button (with groups fleet_manager) to print all the related documents (all the attachements on the fleet.vehicle.assignation.logs for the employee). closes #28077
Merged, thanks! |
Add a field employee_cars_count (computed) to count the amount of cars the employee drove on employee page. This button open the kanban view with all car drove by this employee.
On the fleet.vehicle.assignation.logs a button (like in expense) to upload documents about the car for a particular employee. On the employee form view, add an action button (with groups fleet_manager) to print all the documents merged in one file. On each document, there is a header to show the vehicle and the dove period.
id=1892039
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr