Merge PR #1338 into 14.0

Signed-off-by etobella

hr_job_category/models/hr.py

@@ -23,16 +23,36 @@ class HRJob(models.Model):
 class HRContract(models.Model):
     _inherit = "hr.contract"
 
-    def _tag_employees(self, job_id):
-        if job_id:
+    def _remove_tags(self, job_id):
+        if isinstance(job_id, int):
             job = self.env["hr.job"].browse(job_id)
-            self.mapped("employee_id").write(
-                {"category_ids": [(6, 0, job.category_ids.ids)]}
+        else:
+            job = job_id
+        for employee in self.mapped("employee_id"):
+            _logger.debug(
+                "Removing employee tags if tags exist on contract job: %s",
+                employee.category_ids,
             )
+            tags_to_remove = [
+                (3, tag.id) for tag in job.category_ids & employee.category_ids
+            ]
+            if tags_to_remove:
+                employee.write({"category_ids": tags_to_remove})
+
+    def _tag_employees(self, job_id):
+        if isinstance(job_id, int):
+            job = self.env["hr.job"].browse(job_id)
         else:
-            for contract in self:
-                categories = contract.job_id and contract.job_id.category_ids.ids or []
-                contract.employee_id.write({"category_ids": [(6, 0, categories)]})
+            job = job_id
+        _logger.debug(
+            "Adding employee tags if job tags doesn't exist: %s", job.category_ids
+        )
+        for employee in self.mapped("employee_id"):
+            tags_to_add = [
+                (4, tag.id) for tag in job.category_ids - employee.category_ids
+            ]
+            if tags_to_add:
+                employee.write({"category_ids": tags_to_add})
 
     @api.model
     def create(self, vals):
@@ -42,13 +62,29 @@ def create(self, vals):
         return res
 
     def write(self, vals):
-        if "employee_id" in vals:
-            self.mapped("employee_id").write({"category_ids": [(5,)]})
+        prev_data = {
+            res["id"]: res["job_id"][0]
+            for res in self.read(["job_id"])
+            if res["job_id"]
+        }
+        for record in self:
+            if "employee_id" in vals and record.employee_id != vals.get("employee_id"):
+                record._remove_tags(record.job_id)
         res = super().write(vals)
-        if "job_id" in vals or ("employee_id" in vals and vals["employee_id"]):
-            self._tag_employees(vals.get("job_id"))
+        # Go through each record and delete tags associated with the previous
+        # job, then add the tags of the new job.
+        #
+        if "job_id" in vals:
+            for contract in self:
+                job_id = prev_data.get(contract.id)
+                if job_id:
+                    contract._remove_tags(job_id)
+                self._tag_employees(contract.job_id)
         return res
 
     def unlink(self):
-        self.mapped("employee_id").write({"category_ids": [(5,)]})
+        # Go through each record and delete tags associated with the previous job
+        for contract in self:
+            if contract.job_id:
+                contract._remove_tags(contract.job_id)
         return super().unlink()

hr_job_category/tests/test_hr_job_categories.py

@@ -17,10 +17,13 @@ def setUp(self):
         self.employee_id_1 = self.employee_model.create({"name": "Employee 1"})
         self.employee_id_2 = self.employee_model.create({"name": "Employee 2"})
 
-        # Create two employee categories
+        # Create two employee categories for job positions
         self.categ_id = self.employee_categ_model.create({"name": "Category 1"})
         self.categ_2_id = self.employee_categ_model.create({"name": "Category 2"})
 
+        # Create an employee category to be used out of job positions
+        self.categ_3_id = self.employee_categ_model.create({"name": "Category 3"})
+
         # Create two jobs
         self.job_id = self.job_model.create(
             {"name": "Job 1", "category_ids": [(6, 0, [self.categ_id.id])]}
@@ -81,3 +84,61 @@ def test_write_computes_with_normal_args(self):
 
         self.contract_id.unlink()
         self.assertFalse(self.employee_id_2.category_ids)
+
+    def test_add_new_tags_with_already_present_tags(self):
+        """
+        When a tag is manually added, adding new tags from a contract shouldn't remove
+        them
+        """
+        self.employee_id_1.write({"category_ids": self.categ_3_id})
+        # We have added manually a tag
+        self.assertEqual(len(self.employee_id_1.category_ids.ids), 1)
+        self.assertEqual(self.employee_id_1.category_ids.ids[0], self.categ_3_id.id)
+        # We are now adding contract with 1 job category
+        # The employee should now have two tags
+        self.contract_id.write({"job_id": self.job_id.id})
+        self.contract_id.refresh()
+        self.assertEqual(len(self.employee_id_1.category_ids.ids), 2)
+        self.assertIn(self.categ_3_id.id, self.employee_id_1.category_ids.ids)
+        self.assertIn(
+            self.job_id.category_ids.ids[0], self.employee_id_1.category_ids.ids
+        )
+
+    def test_remove_tags_from_previous_job(self):
+        """Changing the job position removes previous tags and add the new ones"""
+        self.employee_id_1.write({"category_ids": self.categ_3_id})
+        self.contract_id.write({"job_id": self.job_id.id})
+        self.contract_id.refresh()
+
+        # We have two tags (from job and the manual added one)
+        self.assertEqual(len(self.employee_id_1.category_ids.ids), 2)
+
+        # We change the contract of the employe
+        # We should now have the tag
+        self.contract_id.write({"job_id": self.job_2_id.id})
+        self.contract_id.flush()
+
+        self.assertEqual(len(self.employee_id_1.category_ids.ids), 2)
+        self.assertIn(self.categ_3_id.id, self.employee_id_1.category_ids.ids)
+        self.assertNotIn(
+            self.job_id.category_ids.ids[0], self.employee_id_1.category_ids.ids
+        )
+        self.assertIn(
+            self.job_2_id.category_ids.ids[0], self.employee_id_1.category_ids.ids
+        )
+
+    def test_unlink_contract(self):
+        """When we unlink a contract, it should remove only the tags related to it"""
+        self.employee_id_1.write({"category_ids": self.categ_3_id})
+        self.contract_id.write({"job_id": self.job_id.id})
+        self.contract_id.refresh()
+
+        # We have two tags (from job and the manual added one)
+        self.assertEqual(len(self.employee_id_1.category_ids.ids), 2)
+
+        self.contract_id.unlink()
+        self.assertEqual(len(self.employee_id_1.category_ids.ids), 1)
+        self.assertIn(self.categ_3_id.id, self.employee_id_1.category_ids.ids)
+        self.assertNotIn(
+            self.job_id.category_ids.ids[0], self.employee_id_1.category_ids.ids
+        )