From 28eba3d6db3bda04538f2c72beb88f86bf8978a0 Mon Sep 17 00:00:00 2001
From: Kevin Baptiste <kba@odoo.com>
Date: Fri, 18 Jun 2021 12:42:31 +0000
Subject: [PATCH] [IMP] hr_skills: improve UX

- Reset skill levels on skill/skill type change
- Improve form view
- Add a sequence on hr.skill

closes odoo/odoo#73305

Taskid: 2575351
Signed-off-by: Kevin Baptiste <kba@odoo.com>
---
 addons/hr/views/hr_views.xml                  | 17 ++----
 addons/hr_skills/models/hr_skills.py          | 56 ++++++++++++++++++-
 .../hr_skills/static/src/css/hr_skills.scss   | 48 +++++++++-------
 .../hr_skills/static/src/js/resume_widget.js  | 21 +++++++
 addons/hr_skills/views/hr_views.xml           | 43 ++++++++------
 5 files changed, 135 insertions(+), 50 deletions(-)

diff --git a/addons/hr/views/hr_views.xml b/addons/hr/views/hr_views.xml
index f9744dfb1f2..84d6c9c0a61 100644
--- a/addons/hr/views/hr_views.xml
+++ b/addons/hr/views/hr_views.xml
@@ -63,25 +63,18 @@
                 sequence="1"/>
 
             <menuitem
-                id="menu_human_resources_configuration_employee"
-                name="Employee"
+                id="menu_view_employee_category_form"
+                name="Tags"
+                action="open_view_categ_form"
                 parent="menu_human_resources_configuration"
                 groups="base.group_no_one"
-                sequence="1"/>
-
-                <menuitem
-                    id="menu_view_employee_category_form"
-                    name="Tags"
-                    action="open_view_categ_form"
-                    parent="menu_human_resources_configuration_employee"
-                    groups="base.group_no_one"
-                    sequence="1"/>
+                sequence="2"/>
 
             <menuitem
                 id="menu_hr_department_tree"
                 action="hr_department_tree_action"
                 parent="menu_human_resources_configuration"
-                sequence="2"
+                sequence="4"
                 groups="group_hr_user"/>
 
             <menuitem
diff --git a/addons/hr_skills/models/hr_skills.py b/addons/hr_skills/models/hr_skills.py
index 759607a8e72..29c031dde84 100644
--- a/addons/hr_skills/models/hr_skills.py
+++ b/addons/hr_skills/models/hr_skills.py
@@ -8,8 +8,10 @@ from odoo.exceptions import ValidationError
 class Skill(models.Model):
     _name = 'hr.skill'
     _description = "Skill"
+    _order = "sequence"
 
     name = fields.Char(required=True)
+    sequence = fields.Integer(default=10)
     skill_type_id = fields.Many2one('hr.skill.type', ondelete='cascade')
 
 
@@ -20,8 +22,8 @@ class EmployeeSkill(models.Model):
     _order = "skill_level_id"
 
     employee_id = fields.Many2one('hr.employee', required=True, ondelete='cascade')
-    skill_id = fields.Many2one('hr.skill', required=True)
-    skill_level_id = fields.Many2one('hr.skill.level', required=True)
+    skill_id = fields.Many2one('hr.skill', compute='_compute_skill_id', store=True, domain="[('skill_type_id', '=', skill_type_id)]", readonly=False, required=True)
+    skill_level_id = fields.Many2one('hr.skill.level', compute='_compute_skill_level_id', domain="[('skill_type_id', '=', skill_type_id)]", store=True, readonly=False, required=True)
     skill_type_id = fields.Many2one('hr.skill.type', required=True)
     level_progress = fields.Integer(related='skill_level_id.level_progress')
 
@@ -41,6 +43,21 @@ class EmployeeSkill(models.Model):
             if record.skill_level_id not in record.skill_type_id.skill_level_ids:
                 raise ValidationError(_("The skill level %(level)s is not valid for skill type: %(type)s", level=record.skill_level_id.name, type=record.skill_type_id.name))
 
+    @api.depends('skill_type_id')
+    def _compute_skill_id(self):
+        for record in self:
+            if record.skill_id.skill_type_id != record.skill_type_id:
+                record.skill_id = False
+
+    @api.depends('skill_id')
+    def _compute_skill_level_id(self):
+        for record in self:
+            if not record.skill_id:
+                record.skill_level_id = False
+            else:
+                skill_levels = record.skill_type_id.skill_level_ids
+                record.skill_level_id = skill_levels.filtered('default_level') or skill_levels[0] if skill_levels else False
+
 
 class SkillLevel(models.Model):
     _name = 'hr.skill.level'
@@ -50,7 +67,34 @@ class SkillLevel(models.Model):
     skill_type_id = fields.Many2one('hr.skill.type', ondelete='cascade')
     name = fields.Char(required=True)
     level_progress = fields.Integer(string="Progress", help="Progress from zero knowledge (0%) to fully mastered (100%).")
+    default_level = fields.Boolean(help="If checked, this level will be the default one selected when choosing this skill.")
 
+    def create(self, vals_list):
+        levels = super().create(vals_list)
+        levels.skill_type_id._set_default_level()
+        return levels
+
+    def write(self, values):
+        levels = super().write(values)
+        self.skill_type_id._set_default_level()
+        return levels
+
+    def unlink(self):
+        skill_types = self.skill_type_id
+        res = super().unlink()
+        skill_types._set_default_level()
+        return res
+
+    @api.constrains('default_level', 'skill_type_id')
+    def _constrains_default_level(self):
+        for skill_type in set(self.mapped('skill_type_id')):
+            if len(skill_type.skill_level_ids.filtered('default_level')) > 1:
+                raise ValidationError(_('Only one default level is allowed per skill type.'))
+
+    def action_set_default(self):
+        self.ensure_one()
+        self.skill_type_id.skill_level_ids.with_context(no_skill_level_check=True).default_level = False
+        self.default_level = True
 
 class SkillType(models.Model):
     _name = 'hr.skill.type'
@@ -59,3 +103,11 @@ class SkillType(models.Model):
     name = fields.Char(required=True)
     skill_ids = fields.One2many('hr.skill', 'skill_type_id', string="Skills")
     skill_level_ids = fields.One2many('hr.skill.level', 'skill_type_id', string="Levels")
+
+    def _set_default_level(self):
+        if self.env.context.get('no_skill_level_check'):
+            return
+
+        for types in self:
+            if not types.skill_level_ids.filtered('default_level'):
+                types.skill_level_ids[:1].default_level = True
diff --git a/addons/hr_skills/static/src/css/hr_skills.scss b/addons/hr_skills/static/src/css/hr_skills.scss
index 04517c59d35..725d46184bc 100644
--- a/addons/hr_skills/static/src/css/hr_skills.scss
+++ b/addons/hr_skills/static/src/css/hr_skills.scss
@@ -122,27 +122,35 @@
 
 // Editing mode
 // =========================================
-.o_form_view.o_form_editable .o_form_sheet .o_hr_skills_group {
-    .o_group_name {
-        background-color: gray('200');
+.o_form_view.o_form_editable {
+    .o_form_sheet .o_hr_skills_group {
+        .o_group_name {
+            background-color: gray('200');
+        }
+
+        .o_resume_group_header .btn {
+            margin-top: .25rem;
+            margin-right: .4rem;
+        }
+
+        &.o_group_skills .o_group_name {
+            padding-top: .4em;
+            padding-bottom: .4rem;
+        }
+
+        .o_group_name, .o_skill_cell {
+            padding-left: .5rem;
+        }
+
+        &.o_group_skills .o_group_name > b, .o_horizontal_separator {
+            color: color-yiq(gray('200'));
+            font-style: italic;
+        }
     }
 
-    .o_resume_group_header .btn {
-        margin-top: .25rem;
-        margin-right: .4rem;
-    }
-
-    &.o_group_skills .o_group_name {
-        padding-top: .4em;
-        padding-bottom: .4rem;
-    }
-
-    .o_group_name, .o_skill_cell {
-        padding-left: .5rem;
-    }
-
-    &.o_group_skills .o_group_name > b, .o_horizontal_separator {
-        color: color-yiq(gray('200'));
-        font-style: italic;
+    .o_hr_skills_progress {
+        .o_progressbar_value {
+            width: auto;
+        }
     }
 }
diff --git a/addons/hr_skills/static/src/js/resume_widget.js b/addons/hr_skills/static/src/js/resume_widget.js
index df1b92f0643..dee17c1c188 100644
--- a/addons/hr_skills/static/src/js/resume_widget.js
+++ b/addons/hr_skills/static/src/js/resume_widget.js
@@ -219,6 +219,27 @@ var SkillsRenderer = AbstractGroupedOne2ManyRenderer.extend({
             self.$el.find('table').toggleClass('table-striped');
         });
     },
+
+    _onAddRecord: function(ev) {
+        ev.preventDefault();
+        ev.stopPropagation();
+
+        let ctx = _.extend(JSON.parse(ev.currentTarget.dataset.context || '{}'), this.state.getContext());
+        this._rpc({
+            model: 'hr.skill.type',
+            method: 'search_count',
+            args: [[
+                    ['skill_ids', '!=', false],
+                    ['skill_level_ids', '!=', false]
+                ]],
+        }).then(count => {
+            if (count) {
+                this.unselectRow().then(() => this.trigger_up('add_record', {context: [ctx]}));
+            } else {
+                this.do_action('hr_skills.hr_skill_type_action');
+            }
+        });
+    }
 });
 
 
diff --git a/addons/hr_skills/views/hr_views.xml b/addons/hr_skills/views/hr_views.xml
index cd0c96fa18e..1e2c3e12f84 100644
--- a/addons/hr_skills/views/hr_views.xml
+++ b/addons/hr_skills/views/hr_views.xml
@@ -76,7 +76,7 @@
                         </div>
                         <div class="o_hr_skills_group o_group_skills col-lg-5 d-flex flex-column">
                             <separator string="Skills"/>
-                            <field mode="tree" nolabel="1" name="employee_skill_ids"  widget="hr_skills">
+                            <field mode="tree" nolabel="1" name="employee_skill_ids" widget="hr_skills" context="{'default_employee_id': id}">
                                 <tree>
                                     <field name="skill_type_id" invisible="1"/>
                                     <field name="skill_id"/>
@@ -206,6 +206,12 @@
 
     <!-- Skills -->
 
+    <record id="hr_skill_type_action" model="ir.actions.act_window">
+        <field name="name">Skill Types</field>
+        <field name="res_model">hr.skill.type</field>
+        <field name="view_mode">list,form</field>
+    </record>
+
     <record id="employee_skill_level_view_tree" model="ir.ui.view">
         <field name="name">hr.skill.level.tree</field>
         <field name="model">hr.skill.level</field>
@@ -213,6 +219,8 @@
             <tree string="Skill Levels">
                 <field name="name"/>
                 <field name="level_progress" widget="progressbar"/>
+                <field name="default_level"/>
+                <button string="Set Default" type="object" name="action_set_default" attrs="{'invisible': [('default_level', '=', True)]}"/>
             </tree>
         </field>
     </record>
@@ -250,15 +258,23 @@
                 <sheet>
                     <group>
                         <group>
-                            <field name="skill_type_id"/>
-                            <field
-                                name="skill_id"
-                                domain="[('skill_type_id', '=', skill_type_id)]"
-                                options="{'no_create_edit':True}"/>
+                            <field name="employee_id" invisible="1"/>
+                            <field name="skill_type_id" widget="radio" />
                         </group>
                         <group>
-                            <field name="skill_level_id" domain="[('skill_type_id', '=', skill_type_id)]"/>
-                            <field name="level_progress" widget="progressbar"/>
+                            <field name="skill_id" options="{'no_open': True, 'no_create_edit': True}"
+                                    context="{'default_skill_type_id': skill_type_id}"
+                                    domain="[('skill_type_id', '=', skill_type_id)]"
+                                    attrs="{'readonly': [('skill_type_id', '=', False)]}"/>
+                            <label for="skill_level_id" />
+                            <div class="o_row">
+                                <span class="col-4 pl-0">
+                                    <field name="skill_level_id" attrs="{'readonly': [('skill_id', '=', False)]}" />
+                                </span>
+                                <span class="col-8">
+                                    <field name="level_progress" widget="progressbar" class="o_hr_skills_progress" attrs="{'invisible': [('skill_level_id', '=', False)]}" />
+                                </span>
+                            </div>
                         </group>
                     </group>
                 </sheet>
@@ -305,6 +321,7 @@
                     <group string="Skills">
                         <field name="skill_ids" nolabel="1" context="{'default_skill_type_id': id}">
                             <tree editable="bottom">
+                                <field name="sequence" widget="handle" />
                                 <field name="name"/>
                             </tree>
                         </field>
@@ -317,17 +334,11 @@
         </field>
     </record>
 
-    <record id="hr_skill_type_action" model="ir.actions.act_window">
-        <field name="name">Skill Types</field>
-        <field name="res_model">hr.skill.type</field>
-        <field name="view_mode">tree,form</field>
-    </record>
-
     <menuitem
         id="hr_skill_type_menu"
         name="Skill Types"
         action="hr_skill_type_action"
-        parent="hr.menu_human_resources_configuration_employee"
+        parent="hr.menu_human_resources_configuration"
         sequence="3"
-        groups="base.group_no_one"/>
+        groups="hr.group_hr_user"/>
 </odoo>