From cf8fac7f374eb08fcdbe1f99833e1372adc0f252 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Taymans?= <remytaymans@gmail.com>
Date: Tue, 28 May 2019 18:47:04 +0200
Subject: [PATCH] [FIX] b_shift: Properly resigning worker

This fix the case where a worker is unsubscribed from a shift where he
is supercooperator.

In the function unsubscribed_from_today, when removing the supercoop,
the field is_regular was also set to False. I seams that this is not
correct as this field is related to the worker_id not to the supercoop.

Also, I found other issues as when removing a worker_id the fields
is_regular and is_compensation was not reset.

This let me think about other cases where worker_id is emptied, the
is_regular and is_compensation would be never been reset if the
programmer doesn't think to reset it.

So I modified the constrains on these field in order to have these field
reset properly when worker_id is changed.
---
 beesdoo_shift/__openerp__.py |  2 +-
 beesdoo_shift/models/task.py | 51 ++++++++++++++++++++++++------------
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/beesdoo_shift/__openerp__.py b/beesdoo_shift/__openerp__.py
index baa4ec3..b1cf8e3 100644
--- a/beesdoo_shift/__openerp__.py
+++ b/beesdoo_shift/__openerp__.py
@@ -13,7 +13,7 @@
     'website': "https://github.com/beescoop/Obeesdoo",
 
     'category': 'Cooperative management',
-    'version': '9.0.1.0.1',
+    'version': '9.0.1.0.2',
 
     'depends': ['beesdoo_base'],
 
diff --git a/beesdoo_shift/models/task.py b/beesdoo_shift/models/task.py
index 5ce5175..ac81bfa 100644
--- a/beesdoo_shift/models/task.py
+++ b/beesdoo_shift/models/task.py
@@ -47,23 +47,41 @@ class Task(models.Model):
     revert_info = fields.Text(copy=False)
     working_mode = fields.Selection(related='worker_id.working_mode')
 
-    @api.constrains('is_regular', 'is_compensation', 'worker_id')
+    def _compensation_validation(self, task):
+        """
+        Raise a validation error if the fields is_regular and
+        is_compensation are not properly set.
+        """
+        if (task.is_regular == task.is_compensation
+                or not (task.is_regular or task.is_compensation)):
+            raise ValidationError(
+                "You must choose between Regular Shift or "
+                "Compensation Shift."
+            )
+
+    @api.constrains('is_regular', 'is_compensation')
     def _check_compensation(self):
         for task in self:
             if task.working_mode == 'regular':
-                if (task.is_regular == task.is_compensation
-                        or not (task.is_regular or task.is_compensation)):
-                    raise ValidationError(
-                        "You must choose between Regular Shift or "
-                        "Compensation Shift."
-                    )
-
-    @api.onchange('worker_id')
-    def _onchange_worker_id(self):
+                self._compensation_validation(task)
+
+    @api.constrains('worker_id')
+    def _check_worker_id(self):
+        """
+        When worker_id changes we need to check whether is_regular
+        and is_compensation are set correctly.
+        When worker_id is set to a worker that doesn't need field
+        is_regular and is_compensation, these two fields are set to
+        False.
+        """
         for task in self:
-            if task.working_mode != 'regular':
-                task.is_regular = False
-                task.is_compensation = False
+            if task.working_mode == 'regular':
+                self._compensation_validation(task)
+            else:
+                task.write({
+                    'is_regular': False,
+                    'is_compensation': False,
+                })
 
     def message_auto_subscribe(self, updated_fields, values=None):
         self._add_follower(values)
@@ -97,11 +115,11 @@ class Task(models.Model):
             date_domain.append(('end_time', '<=', end_date))
         to_unsubscribe = self.search([('worker_id', 'in', worker_ids)] + date_domain)
 
-        to_unsubscribe.write({'worker_id': False, 'is_regular': False})
+        to_unsubscribe.write({'worker_id': False})
         # What about replacement ?
         # Remove worker, replaced_id and regular
         to_unsubscribe_replace = self.search([('replaced_id', 'in', worker_ids)] + date_domain)
-        to_unsubscribe_replace.write({'worker_id': False, 'is_regular': False, 'replaced_id': False})
+        to_unsubscribe_replace.write({'worker_id': False, 'replaced_id': False})
 
         # If worker is Super cooperator, remove it from planning
         super_coop_ids = self.env['res.users'].search(
@@ -110,8 +128,7 @@ class Task(models.Model):
         if super_coop_ids:
             to_unsubscribe_super_coop = self.search(
                 [('super_coop_id', 'in', super_coop_ids)] + date_domain)
-            to_unsubscribe_super_coop.write({'super_coop_id': False,
-                                             'is_regular': False})
+            to_unsubscribe_super_coop.write({'super_coop_id': False})
 
     @api.multi
     def write(self, vals):