From 75c63315169c0a4c6c92403690c54d606680593b Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 27 Jun 2024 10:39:16 +0200 Subject: [PATCH] [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/odoo#170995 Signed-off-by: Xavier Morel (xmo) --- .../tests/test_groupby_week.py | 103 ++++++++++-------- .../tests/test_read_progress_bar.py | 6 +- odoo/models.py | 9 +- odoo/tools/date_utils.py | 47 +++++++- 4 files changed, 115 insertions(+), 50 deletions(-) diff --git a/odoo/addons/test_read_group/tests/test_groupby_week.py b/odoo/addons/test_read_group/tests/test_groupby_week.py index b1fa287c6e071..e29e4d9482599 100644 --- a/odoo/addons/test_read_group/tests/test_groupby_week.py +++ b/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" ) diff --git a/odoo/addons/test_read_group/tests/test_read_progress_bar.py b/odoo/addons/test_read_group/tests/test_read_progress_bar.py index 032225b7656ae..764c28cdf9cae 100644 --- a/odoo/addons/test_read_group/tests/test_read_progress_bar.py +++ b/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) diff --git a/odoo/models.py b/odoo/models.py index 6e95ba316a165..552fa39b0078f 100644 --- a/odoo/models.py +++ b/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) diff --git a/odoo/tools/date_utils.py b/odoo/tools/date_utils.py index fba7910bed362..8e811c980f398 100644 --- a/odoo/tools/date_utils.py +++ b/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)