Human Resources mailing list archives

hr@odoo-community.org

Avatar

Re: [OCA/hr] [ADD] hr_personal_equipment: added module to manage employee personal equipment (#1004)

by "Saran @ Ecosoft" <notifications@github.com> - 22/07/2021 06:14:28

@Saran440 commented on this pull request.

Code Review.


In hr_personal_equipment/__manifest__.py:

> @@ -0,0 +1,21 @@

+# Copyright 2021 Creu Blanca

+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

+

+{

+    'name': 'Hr Personal Equipment',

+    'summary': """

+        This addon allows to manage employee personal equipment""",

+    'version': '12.0.1.0.0',

+    'license': 'AGPL-3',

+    'author': 'Creu Blanca,Odoo Community Association (OCA)',

+    'website': 'www.creublanca.es',

⬇️ Suggested change
-    'website': 'www.creublanca.es',

+    'website': 'https://github.com/OCA/hr',


In hr_personal_equipment/models/hr_personal_equipment.py:

> +

+    @api.onchange('product_id')

+    def _onchange_uom_id(self):

+        if self.product_id:

+            self.product_uom_id = self.product_id.uom_id

+        return {'domain':

+                {'product_uom_id':

+                    [('category_id', '=', self.product_uom_id.category_id.id)]

+                 }

+                }

+

+    @api.depends('product_id', 'employee_id')

+    def _compute_name(self):

+        for rec in self:

+            if rec.product_id.name and rec.employee_id.name:

+                rec.name = rec.product_id.name + _(" to ") + rec.employee_id.name

⬇️ Suggested change
-                rec.name = rec.product_id.name + _(" to ") + rec.employee_id.name

+                rec.name = "{} to {}".format(rec.product_id.name, rec.employee_id.name)

I suggest use {} instead of + for python3

reference : https://realpython.com/python-string-formatting/


In hr_personal_equipment/models/hr_personal_equipment.py:

> +        return {'domain':

+                {'product_uom_id':

+                    [('category_id', '=', self.product_uom_id.category_id.id)]

+                 }

+                }

+

+    @api.depends('product_id', 'employee_id')

+    def _compute_name(self):

+        for rec in self:

+            if rec.product_id.name and rec.employee_id.name:

+                rec.name = rec.product_id.name + _(" to ") + rec.employee_id.name

+

+    def _validate_allocation_vals(self):

+        return {

+            'state': 'valid',

+            'start_date': date.today() if not self.start_date else self.start_date,

⬇️ Suggested change
-            'start_date': date.today() if not self.start_date else self.start_date,

+            'start_date': fields.Date.context_today(self) if not self.start_date else self.start_date,

fields.Date.context_today(self) support datetime with timezone.
I'm not sure that date.today() support with timezone.
If I live in Thailand (GMT +7), Is it correct?


In hr_personal_equipment/readme/CREDITS.rst:

> @@ -0,0 +1 @@

+

if you not used, please delete.


In hr_personal_equipment/readme/INSTALL.rst:

> @@ -0,0 +1 @@

+To install this module, you need to have HR module installed or it will be requested during installation.

it's not necessary to explain. please delete this file.


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.