[FIX] core: replace babel when grouping by week

Babel has a very long standing bug on computing ISO weeks /
year-week.

python-babel/babel#621 fixed some cases but broke others. Debian
decided to revert it[^1] rather than actually fix things up and Ubuntu
shipped that patch in Noble, which is awesome (not): test cases
embedding either behavior flip around depending whether they run or
noble or not.

python-babel/babel#887 was opened to fix the issue but left to rot
then closed when I mistakenly-ish deleted my personal fork of babel.

While technically we could monkeypatch babel and replace functions
wholesale (both versions so it works everywhere), just give up and
instead compute the weeknumber ourselves in the context of
`read_group([X:week])`:

- For ISO locales, delegate to the stdlib which does that fine.
- For non-ISO locales, assume that the week containing the first day
  of the year is the first week, and make the executive decision that
  all days in that calendar week are part of W01. The alternative
  would be to implement a split / overlapping week system where the
  same week is both W53 $YEAR and W01 $YEAR+1, and I really have no
  desire to deal with that from a UI/UX perspective.

Fix tests embedding incorrect week assignments:

- For `test_group_by_week`, redo the entire set (somewhat more
  declaratively / data driven, though not quite table-driven) to
  ensure the assignments are correct, and assert that the locales have
  the properties we assume with respect to 1dow.
- For `test_read_progress_bar`, the entire thing was a fever dream of
  wonky pseudo-split week where 2019 had no week 1.

[^1]: https://sources.debian.org/patches/python-babel/2.10.3-1/

closes odoo#170995

Signed-off-by: Xavier Morel (xmo) <[email protected]>

odoo/addons/test_read_group/tests/test_groupby_week.py

@@ -1,5 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
+import babel
+
 from odoo.tests import common
 
 
@@ -11,9 +13,46 @@ class TestGroupbyWeek(common.TransactionCase):
     def setUpClass(cls):
         super().setUpClass()
         cls.Model = cls.env['test_read_group.fill_temporal']
+        # same for all locales
+        cls.iso_weeks = {
+            52: 2,  # 2022-01-01 and 2022-01-02 (W52 of 2021)
+             1: 1,  # 2022-01-03
+            21: 3,  # 2022-05-27, 2022-05-28, 2022-05-29
+            22: 1,  # 2022-05-30
+            24: 2,  # 2022-06-18 and 2022-06-19
+            25: 1,  # 2022-06-20
+        }
+        cls.per_locale = {
+            # same as ISO
+            "fr_BE": {
+                "W52 2021": 2,
+                "W1 2022": 1,
+                "W21 2022": 3,
+                "W22 2022": 1,
+                "W24 2022": 2,
+                "W25 2022": 1
+            },
+            # non-iso, start of week = sat
+            "ar_SY": {
+                "W1 2022": 3,
+                "W21 2022": 1,
+                "W22 2022": 3,
+                "W25 2022": 3,
+            },
+            # non-iso, start of week = sun
+            "en_US": {
+                "W1 2022": 1,
+                "W2 2022": 2,
+                "W22 2022": 2,
+                "W23 2022": 2,
+                "W25 2022": 1,
+                "W26 2022": 2,
+            }
+        }
         cls.records = cls.Model.create([  # BE,  SY,  US
-            {'date': '2022-01-01'},       # W52, W52, W52
-            {'date': '2022-01-02'},       # W55, W1,  W1
+            {'date': '2022-01-01'},       # W52, W01, W01
+            {'date': '2022-01-02'},       # W52, W01, W02
+            {'date': '2022-01-03'},       # W01, W01, W02
             {'date': '2022-05-27'},       # W21, W21, W22
             {'date': '2022-05-28'},       # W21, W22, W22
             {'date': '2022-05-29'},       # W21, W22, W23
@@ -25,18 +64,16 @@ def setUpClass(cls):
 
     def test_belgium(self):
         """ fr_BE - first day of the week = Monday """
+        self.assertEqual(
+            babel.Locale.parse("fr_BE").first_week_day,
+            0,
+        )
         self.env['res.lang']._activate_lang('fr_BE')
         groups = self.Model.with_context(lang='fr_BE').read_group(
             [('id', 'in', self.records.ids)], fields=['date'], groupby=['date:week'])
         self.assertDictEqual(
             {week['date:week']: week['date_count'] for week in groups if week['date:week']},
-            {
-                'W21 2022': 3,
-                'W22 2022': 1,
-                'W24 2022': 2,
-                'W25 2022': 1,
-                'W52 2021': 2,
-            },
+            self.per_locale["fr_BE"],
             "Week groups not matching when the first day of the week is Monday"
         )
 
@@ -45,29 +82,22 @@ def test_belgium(self):
             [('id', 'in', self.records.ids)], fields=['date'], groupby=['date:iso_week_number'])
         self.assertDictEqual(
             {week['date:iso_week_number']: week['date_count'] for week in groups if week['date:iso_week_number']},
-            {
-                21: 3,  # 2022-05-27, 2022-05-28, 2022-05-29
-                22: 1,  # 2022-05-30
-                24: 2,  # 2022-06-18 and 2022-06-19
-                25: 1,  # 2022-06-20
-                52: 2,  # 2022-01-01 and 2022-01-02 (mapped to the week 52 of 2021)
-            },
+            self.iso_weeks,
             "Week groups not matching when the first day of the week is Monday"
         )
 
     def test_syria(self):
         """ ar_SY - first day of the week = Saturday """
+        self.assertEqual(
+            babel.Locale.parse("ar_SY").first_week_day,
+            5,
+        )
         self.env['res.lang']._activate_lang('ar_SY')
         groups = self.Model.with_context(lang='ar_SY').read_group(
             [('id', 'in', self.records.ids)], fields=['date'], groupby=['date:week'])
         self.assertDictEqual(
             {week['date:week']: week['date_count'] for week in groups if week['date:week']},
-            {
-                'W1 2021': 2,  # 2022-01-01 and 2022-01-02, yes this is a bug
-                'W21 2022': 1,
-                'W22 2022': 3,
-                'W25 2022': 3,
-            },
+            self.per_locale["ar_SY"],
             "Week groups not matching when the first day of the week is Saturday"
         )
 
@@ -76,30 +106,21 @@ def test_syria(self):
             [('id', 'in', self.records.ids)], fields=['date'], groupby=['date:iso_week_number'])
         self.assertDictEqual(
             {week['date:iso_week_number']: week['date_count'] for week in groups if week['date:iso_week_number']},
-            {
-                21: 3,  # 2022-05-27, 2022-05-28, 2022-05-29
-                22: 1,  # 2022-05-30
-                24: 2,  # 2022-06-18 and 2022-06-19
-                25: 1,  # 2022-06-20
-                52: 2,  # 2022-01-01 and 2022-01-02 (mapped to the week 52 of 2021)
-            },
+            self.iso_weeks,
             "Week groups not matching when the first day of the week is Saturday"
         )
 
     def test_united_states(self):
         """ en_US - first day of the week = Sunday """
+        self.assertEqual(
+            babel.Locale.parse("en_US").first_week_day,
+            6,
+        )
         groups = self.Model.with_context(lang='en_US').read_group(
             [('id', 'in', self.records.ids)], fields=['date'], groupby=['date:week'])
         self.assertDictEqual(
             {week['date:week']: week['date_count'] for week in groups if week['date:week']},
-            {
-                'W2 2021': 1,  # 2022-01-01 yes this is a bug
-                'W53 2021': 1,
-                'W22 2022': 2,
-                'W23 2022': 2,
-                'W25 2022': 1,
-                'W26 2022': 2,
-            },
+            self.per_locale["en_US"],
             "Week groups not matching when the first day of the week is Sunday"
         )
 
@@ -108,12 +129,6 @@ def test_united_states(self):
             [('id', 'in', self.records.ids)], fields=['date'], groupby=['date:iso_week_number'])
         self.assertDictEqual(
             {week['date:iso_week_number']: week['date_count'] for week in groups if week['date:iso_week_number']},
-            {
-                21: 3,  # 2022-05-27, 2022-05-28, 2022-05-29
-                22: 1,  # 2022-05-30
-                24: 2,  # 2022-06-18 and 2022-06-19
-                25: 1,  # 2022-06-20
-                52: 2,  # 2022-01-01 and 2022-01-02 (mapped to the week 52 of 2021)
-            },
+            self.iso_weeks,
             "Week groups not matching when the first day of the week is Sunday"
         )

odoo/addons/test_read_group/tests/test_read_progress_bar.py

@@ -86,7 +86,7 @@ def test_simple(self):
         c1, c2, c3 = self.env['res.country'].search([], limit=3)
 
         self.env['x_progressbar'].create([
-            # week 53 2018 / week 1 2019
+            # week 1 2019
             {'x_country_id': c1.id, 'x_date': '2019-01-01', 'x_state': 'foo'},
             {'x_country_id': c1.id, 'x_date': '2019-01-02', 'x_state': 'foo'},
             {'x_country_id': c1.id, 'x_date': '2019-01-03', 'x_state': 'foo'},
@@ -120,7 +120,7 @@ def test_simple(self):
         # check date aggregation and format
         result = self.env['x_progressbar'].read_progress_bar([], 'x_date:week', progress_bar)
         self.assertEqual(result, {
-            'W53 2018': {'foo': 3, 'bar': 1, 'baz': 1},
+            'W1 2019': {'foo': 3, 'bar': 1, 'baz': 1},
             'W2 2019': {'foo': 3, 'bar': 2, 'baz': 2},
             'W3 2019': {'foo': 0, 'bar': 0, 'baz': 3},
         })
@@ -145,4 +145,4 @@ def test_simple(self):
         }
         # It is not possible to read_progress_bar with ungroupable fields
         with self.assertRaises(ValueError):
-            result = self.env['x_progressbar'].read_progress_bar([], 'x_country_id', progress_bar)
+            self.env['x_progressbar'].read_progress_bar([], 'x_country_id', progress_bar)

odoo/models.py

@@ -25,7 +25,6 @@
 import collections
 import contextlib
 import datetime
-import dateutil
 import fnmatch
 import functools
 import inspect
@@ -2514,6 +2513,14 @@ def _read_group_format_result(self, rows_dict, lazy_groupby):
                                 value, format=READ_GROUP_DISPLAY_FORMAT[granularity],
                                 locale=locale
                             )
+                        # special case weeks because babel is broken *and*
+                        # ubuntu reverted a change so it's also inconsistent
+                        if granularity == 'week':
+                            year, week = date_utils.weeknumber(
+                                babel.Locale.parse(locale),
+                                range_start,
+                            )
+                            label = f"W{week} {year:04}"
 
                         range_start = range_start.strftime(fmt)
                         range_end = range_end.strftime(fmt)

odoo/tools/date_utils.py

@@ -1,9 +1,12 @@
 # -*- coding: utf-8 -*-
-import math
 import calendar
+import math
 from datetime import date, datetime, time
+from typing import Tuple
+
+import babel
 import pytz
-from dateutil.relativedelta import relativedelta
+from dateutil.relativedelta import relativedelta, weekdays
 
 from .func import lazy
 from odoo.loglevels import ustr
@@ -259,3 +262,43 @@ def date_range(start, end, step=relativedelta(months=1)):
     while dt <= end_dt:
         yield post_process(dt)
         dt = dt + step
+
+
+def weeknumber(locale: babel.Locale, date: date) -> Tuple[int, int]:
+    """Computes the year and weeknumber of `date`. The week number is 1-indexed
+    (so the first week is week number 1).
+
+    For ISO locales (first day of week = monday, min week days = 4) the concept
+    is clear and the Python stdlib implements it directly.
+
+    For other locales, it's basically nonsensical as there is no actual
+    definition. For now we will implement non-split first-day-of-year, that is
+    the first week of the year is the one which contains the first day of the
+    year (taking first day of week in account), and the days of the previous
+    year which are part of that week are considered to be in the next year for
+    calendaring purposes.
+
+    That is December 27, 2015 is in the first week of 2016.
+
+    An alternative is to split the week in two, so the week from December 27,
+    2015 to January 2, 2016 would be *both* W53/2015 and W01/2016.
+    """
+    if locale.first_week_day == 0 and locale.min_week_days == 4:
+        # woohoo nothing to do
+        return date.isocalendar()[:2]
+
+    # first find the first day of the first week of the next year, if the
+    # reference date is after that then it must be in the first week of the next
+    # year, remove this if we decide to implement split weeks instead
+    fdny = date.replace(year=date.year + 1, month=1, day=1) \
+       - relativedelta(weekday=weekdays[locale.first_week_day](-1))
+    if date >= fdny:
+        return date.year + 1, 1
+
+    # otherwise get the number of periods of 7 days between the first day of the
+    # first week and the reference
+    fdow = date.replace(month=1, day=1) \
+       - relativedelta(weekday=weekdays[locale.first_week_day](-1))
+    doy = (date - fdow).days
+
+    return date.year, (doy // 7 + 1)