Fixes and ROADMAP

OCA · Apr 3, 2020 · 8e2c16c · 8e2c16c
1 parent ccab50e
commit 8e2c16c
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 87 deletions.
diff --git a/base_time_window/models/time_window_mixin.py b/base_time_window/models/time_window_mixin.py
@@ -9,35 +9,38 @@
 
 from odoo import _, api, fields, models
 from odoo.exceptions import ValidationError
+from odoo.tools.misc import format_time
 
 
 class TimeWindowMixin(models.AbstractModel):
 
     _name = "time.window.mixin"
     _description = "Time Window"
-    _order = "start"
+    _order = "time_window_start"
 
     # TODO patch api.constrains with field here?
-    _overlap_check_field = False
+    _time_window_overlap_check_field = False
 
-    start = fields.Float("From", required=True)
-    end = fields.Float("To", required=True)
-    weekday_ids = fields.Many2many(
+    time_window_start = fields.Float("From", required=True)
+    time_window_end = fields.Float("To", required=True)
+    time_window_weekday_ids = fields.Many2many(
         comodel_name="time.weekday", required=True
     )
 
-    @api.constrains("start", "end", "weekday_ids")
+    @api.constrains("time_window_start", "time_window_end", "time_window_weekday_ids")
     def check_window_no_overlaps(self):
-        weekdays_field = self._fields["weekday_ids"]
+        weekdays_field = self._fields["time_window_weekday_ids"]
         for record in self:
-            if record.start > record.end:
+            if record.time_window_start > record.time_window_end:
                 raise ValidationError(
                     _("%s must be > %s")
                     % (
-                        self.float_to_time_repr(record.end),
-                        self.float_to_time_repr(record.start),
+                        self.float_to_time_repr(record.time_window_end),
+                        self.float_to_time_repr(record.time_window_start),
                     )
                 )
+            if not record.time_window_weekday_ids:
+                raise ValidationError(_("At least one time.weekday is required"))
             # here we use a plain SQL query to benefit of the numrange
             # function available in PostgresSQL
             # (http://www.postgresql.org/docs/current/static/rangetypes.html)
@@ -49,7 +52,7 @@ def check_window_no_overlaps(self):
                     join %(relation)s as d
                     on d.%(relation_window_fkey)s = w.id
                 WHERE
-                    NUMRANGE(w.start::numeric, w.end::numeric) &&
+                    NUMRANGE(w.time_window_start::numeric, w.time_window_end::numeric) &&
                         NUMRANGE(%(start)s::numeric, %(end)s::numeric)
                     AND w.id != %(window_id)s
                     AND d.%(relation_week_day_fkey)s in %(weekday_ids)s
@@ -61,12 +64,12 @@ def check_window_no_overlaps(self):
                     relation=AsIs(weekdays_field.relation),
                     relation_window_fkey=AsIs(weekdays_field.column1),
                     relation_week_day_fkey=AsIs(weekdays_field.column2),
-                    start=record.start,
-                    end=record.end,
+                    start=record.time_window_start,
+                    end=record.time_window_end,
                     window_id=record.id,
-                    weekday_ids=tuple(record.weekday_ids.ids),
-                    check_field=AsIs(self._overlap_check_field),
-                    check_field_id=record[self._overlap_check_field].id,
+                    weekday_ids=tuple(record.time_window_weekday_ids.ids),
+                    check_field=AsIs(self._time_window_overlap_check_field),
+                    check_field_id=record[self._time_window_overlap_check_field].id,
                 ),
             )
             res = self.env.cr.fetchall()
@@ -77,13 +80,13 @@ def check_window_no_overlaps(self):
                     % (record.display_name, other.display_name)
                 )
 
-    @api.depends("start", "end", "weekday_ids")
+    @api.depends("time_window_start", "time_window_end", "time_window_weekday_ids")
     def _compute_display_name(self):
         for record in self:
             record.display_name = _("{days}: From {start} to {end}").format(
                 days=", ".join(record.weekday_ids.mapped("display_name")),
-                start=self.float_to_time_repr(record.start),
-                end=self.float_to_time_repr(record.end),
+                start=format_time(self.env, record.get_time_window_start_time()),
+                end=format_time(self.env, record.get_time_window_end_time()),
             )
 
     @api.model
@@ -106,8 +109,8 @@ def float_to_time(self, value):
         hour, minute = self._get_hour_min_from_value(value)
         return time(hour=hour, minute=minute)
 
-    def get_start_time(self):
-        return self.float_to_time(self.start)
+    def get_time_window_start_time(self):
+        return self.float_to_time(self.time_window_start)
 
-    def get_end_time(self):
-        return self.float_to_time(self.end)
+    def get_time_window_end_time(self):
+        return self.float_to_time(self.time_window_end)
diff --git a/base_time_window/readme/ROADMAP.rst b/base_time_window/readme/ROADMAP.rst
@@ -0,0 +1,11 @@
+* Storing times using `float_time` widget requires extra processing to ensure
+  computations are done in the right timezone, because the value is not stored
+  as UTC in the database, and must therefore be related to a `tz` field.
+
+  `float_time` in this sense should only be used for durations and not for a
+  "point in time" as this is always needs a Date for a timezone conversion to
+  be done properly. (Because a conversion from UTC to e.g. Europe/Brussels won't
+  give the same result in winter or summer because of Daylight Saving Time).
+
+  Therefore the right move would be to use a `resource.calendar` to define time
+  windows using Datetime with recurrences.
diff --git a/test_base_time_window/models/partner_time_window.py b/test_base_time_window/models/partner_time_window.py
@@ -13,7 +13,7 @@ class TestPartnerTimeWindow(models.Model):
     _inherit = "time.window.mixin"
     _description = "Test partner time Window"
 
-    _overlap_check_field = 'partner_id'
+    _time_window_overlap_check_field = "partner_id"
 
     partner_id = fields.Many2one(
         "res.partner", required=True, index=True, ondelete='cascade'
@@ -22,23 +22,3 @@ class TestPartnerTimeWindow(models.Model):
     @api.constrains("partner_id")
     def check_window_no_overlaps(self):
         return super().check_window_no_overlaps()
-
-    @api.depends("start", "end", "weekday_ids")
-    def _compute_display_name(self):
-        for record in self:
-            record.display_name = _("{days}: From {start} to {end}").format(
-                days=", ".join(record.weekday_ids.mapped("display_name")),
-                start=self.float_to_time_repr(record.start),
-                end=self.float_to_time_repr(record.end),
-            )
-
-    @api.model
-    def float_to_time_repr(self, value):
-        pattern = "%02d:%02d"
-        hour = math.floor(value)
-        min = round((value % 1) * 60)
-        if min == 60:
-            min = 0
-            hour += 1
-
-        return pattern % (hour, min)
diff --git a/test_base_time_window/models/res_partner.py b/test_base_time_window/models/res_partner.py
@@ -21,17 +21,15 @@ def get_delivery_windows(self, day_name):
         """
         Return the list of delivery windows by partner id for the given day
 
-        :param day: The day name (see delivery.week.day, ex: 0,1,2,...)
-        :return: dict partner_id:[delivery_window, ]
+        :param day: The day name (see time.weekday, ex: 0,1,2,...)
+        :return: dict partner_id:[time_window_ids, ]
         """
-        week_day_id = self.env["delivery.week.day"]._get_id_by_name(
-            day_name
-        )
+        weekday_id = self.env["time.weekday"]._get_id_by_name(day_name)
         res = defaultdict(list)
-        windows = self.env["delivery.window"].search(
+        windows = self.env["test.partner.time.window"].search(
             [
                 ("partner_id", "in", self.ids),
-                ("week_day_ids", "in", week_day_id),
+                ("time_window_weekday_ids", "in", weekday_id)
             ]
         )
         for window in windows:

diff --git a/test_base_time_window/tests/test_base_time_window.py b/test_base_time_window/tests/test_base_time_window.py
@@ -35,16 +35,16 @@ def test_00(self):
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_1.time_window_ids)
         time_window = self.partner_1.time_window_ids
-        self.assertEqual(time_window.start, 10.0)
-        self.assertEqual(time_window.end, 12.0)
-        self.assertEqual(time_window.weekday_ids, self.monday)
+        self.assertEqual(time_window.time_window_start, 10.0)
+        self.assertEqual(time_window.time_window_end, 12.0)
+        self.assertEqual(time_window.time_window_weekday_ids, self.monday)
 
     def test_01(self):
         """
@@ -62,9 +62,9 @@ def test_01(self):
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_1.time_window_ids)
@@ -89,18 +89,18 @@ def test_02(self):
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         with self.assertRaises(ValidationError):
             self.TimeWindow.create(
                 {
                     "partner_id": self.partner_1.id,
-                    "start": 11.0,
-                    "end": 13.0,
-                    "weekday_ids": [(4, self.monday.id), (4, self.sunday.id)],
+                    "time_window_start": 11.0,
+                    "time_window_end": 13.0,
+                    "time_window_weekday_ids": [(4, self.monday.id), (4, self.sunday.id)],
                 }
             )
 
@@ -119,18 +119,18 @@ def test_03(self):
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_1.time_window_ids)
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 11.0,
-                "end": 13.0,
-                "weekday_ids": [(4, self.sunday.id)],
+                "time_window_start": 11.0,
+                "time_window_end": 13.0,
+                "time_window_weekday_ids": [(4, self.sunday.id)],
             }
         )
         self.assertEquals(len(self.partner_1.time_window_ids), 2)
@@ -151,19 +151,19 @@ def test_04(self):
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_1.time_window_ids)
         self.assertFalse(self.partner_2.time_window_ids)
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_2.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_2.time_window_ids)
@@ -181,9 +181,9 @@ def test_05(self):
             self.TimeWindow.create(
                 {
                     "partner_id": self.partner_1.id,
-                    "start": 14.0,
-                    "end": 12.0,
-                    "weekday_ids": [(4, self.monday.id)],
+                    "time_window_start": 14.0,
+                    "time_window_end": 12.0,
+                    "time_window_weekday_ids": [(4, self.monday.id)],
                 }
             )
 
@@ -201,19 +201,19 @@ def test_06(self):
         p1_timewindow = self.TimeWindow.create(
             {
                 "partner_id": self.partner_1.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_1.time_window_ids)
         self.assertFalse(self.partner_2.time_window_ids)
         self.TimeWindow.create(
             {
                 "partner_id": self.partner_2.id,
-                "start": 10.0,
-                "end": 12.0,
-                "weekday_ids": [(4, self.monday.id)],
+                "time_window_start": 10.0,
+                "time_window_end": 12.0,
+                "time_window_weekday_ids": [(4, self.monday.id)],
             }
         )
         self.assertTrue(self.partner_2.time_window_ids)