[FIX] models: use localized first day of the week when grouping by week
Steps to reproduce:
- Make sure language preference is 'en_US'
- In accounting, in the dashboard click on bills
- Filter 'due_date' by week
Issue: The start day is Monday and should be, for 'en_US', Sunday as it
is the case in the dashboard view in accounting (see appendix).
Cause: The query uses the `date_trunc('week', date)` which in Postgres
retrieves the first day of the week as Monday (ISO week).
Solution: Create an offset in the query depending on the first day of
the locale variable.
Note: the `web/tests/test_read_progress_bar.py` has been modified: since
the default language is 'en_US' there will be an offset of one day. To
make it less confusing, I used only two anglo-saxons countries so the
day offset is not the variable tested. (for this matter, pleaser refer
to `test_read_group/tests/test_read_group_process_groupby.py`)
Appendix:
Language (english-US)
VIEW (per week) | DASHBOARD
___________________________________________________________________
W23 -> 06/05 | 05/29 -> 06/04
W24 06/06 -> 06/12 | 06/05 -> 06/11
W25 06/13 -> | 06/12 -> 06/18
(Monday - Sunday) (Sunday - Saturday)
Language (french-BE)
VIEW (per week) | DASHBOARD
___________________________________________________________________
W22 -> 06/05 | 05/30 -> 06/05
W23 06/06 -> 06/12 | 06/06 -> 06/12
W24 06/13 -> | 06/13 -> 06/19
(Monday - Sunday) (Monday - Sunday)
opw-2747066
closes odoo/odoo#93053
Related: odoo/enterprise#29539
Signed-off-by: Raphael Collet <rco@odoo.com>
This commit is contained in:
Yolann Sabaux
committed by
Raphael Collet
Raphael Collet
parent
0ef798a3db
commit
3a177c448d
@@ -196,7 +196,13 @@ class Base(models.AbstractModel):
|
||||
# Again, imitating what _read_group_format_result and _read_group_prepare_data do
|
||||
if group_by_value and field_type in ['date', 'datetime']:
|
||||
locale = get_lang(self.env).code
|
||||
group_by_value = date_utils.start_of(fields.Datetime.to_datetime(group_by_value), group_by_modifier)
|
||||
group_by_value = fields.Datetime.to_datetime(group_by_value)
|
||||
if group_by_modifier != 'week':
|
||||
# start_of(v, 'week') does not take into account the locale
|
||||
# to determine the first day of the week; this part is not
|
||||
# necessary, since the formatting below handles the locale
|
||||
# as expected, and outputs correct results
|
||||
group_by_value = date_utils.start_of(group_by_value, group_by_modifier)
|
||||
group_by_value = pytz.timezone('UTC').localize(group_by_value)
|
||||
tz_info = None
|
||||
if field_type == 'datetime' and self._context.get('tz') in pytz.all_timezones:
|
||||
|
||||
@@ -86,24 +86,24 @@ class TestReadProgressBar(common.TransactionCase):
|
||||
c1, c2, c3 = self.env['res.country'].search([], limit=3)
|
||||
|
||||
self.env['x_progressbar'].create([
|
||||
# week 21
|
||||
{'x_country_id': c1.id, 'x_date': '2021-05-20', 'x_state': 'foo'},
|
||||
{'x_country_id': c1.id, 'x_date': '2021-05-21', 'x_state': 'foo'},
|
||||
{'x_country_id': c1.id, 'x_date': '2021-05-22', 'x_state': 'foo'},
|
||||
{'x_country_id': c1.id, 'x_date': '2021-05-23', 'x_state': 'bar'},
|
||||
# week 22
|
||||
{'x_country_id': c1.id, 'x_date': '2021-05-24', 'x_state': 'baz'},
|
||||
{'x_country_id': c2.id, 'x_date': '2021-05-25', 'x_state': 'foo'},
|
||||
{'x_country_id': c2.id, 'x_date': '2021-05-26', 'x_state': 'bar'},
|
||||
{'x_country_id': c2.id, 'x_date': '2021-05-27', 'x_state': 'bar'},
|
||||
{'x_country_id': c2.id, 'x_date': '2021-05-28', 'x_state': 'baz'},
|
||||
{'x_country_id': c2.id, 'x_date': '2021-05-29', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2021-05-30', 'x_state': 'foo'},
|
||||
# week 23
|
||||
{'x_country_id': c3.id, 'x_date': '2021-05-31', 'x_state': 'foo'},
|
||||
{'x_country_id': c3.id, 'x_date': '2021-06-01', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2021-06-02', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2021-06-03', 'x_state': 'baz'},
|
||||
# week 53 2018 / 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'},
|
||||
{'x_country_id': c1.id, 'x_date': '2019-01-04', 'x_state': 'bar'},
|
||||
{'x_country_id': c1.id, 'x_date': '2019-01-05', 'x_state': 'baz'},
|
||||
# week 2 2019
|
||||
{'x_country_id': c2.id, 'x_date': '2019-01-06', 'x_state': 'foo'},
|
||||
{'x_country_id': c2.id, 'x_date': '2019-01-07', 'x_state': 'bar'},
|
||||
{'x_country_id': c2.id, 'x_date': '2019-01-08', 'x_state': 'bar'},
|
||||
{'x_country_id': c2.id, 'x_date': '2019-01-09', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2019-01-10', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2019-01-11', 'x_state': 'foo'},
|
||||
{'x_country_id': c3.id, 'x_date': '2019-01-12', 'x_state': 'foo'},
|
||||
# week 3 2019
|
||||
{'x_country_id': c3.id, 'x_date': '2019-01-13', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2019-01-14', 'x_state': 'baz'},
|
||||
{'x_country_id': c3.id, 'x_date': '2019-01-15', 'x_state': 'baz'},
|
||||
])
|
||||
|
||||
progress_bar = {
|
||||
@@ -113,16 +113,16 @@ class TestReadProgressBar(common.TransactionCase):
|
||||
result = self.env['x_progressbar'].read_progress_bar([], 'x_country_id', progress_bar)
|
||||
self.assertEqual(result, {
|
||||
c1.display_name: {'foo': 3, 'bar': 1, 'baz': 1},
|
||||
c2.display_name: {'foo': 1, 'bar': 2, 'baz': 2},
|
||||
c3.display_name: {'foo': 2, 'bar': 0, 'baz': 3},
|
||||
c2.display_name: {'foo': 1, 'bar': 2, 'baz': 1},
|
||||
c3.display_name: {'foo': 2, 'bar': 0, 'baz': 4},
|
||||
})
|
||||
|
||||
# check date aggregation and format
|
||||
result = self.env['x_progressbar'].read_progress_bar([], 'x_date:week', progress_bar)
|
||||
self.assertEqual(result, {
|
||||
'W21 2021': {'foo': 3, 'bar': 1, 'baz': 0},
|
||||
'W22 2021': {'foo': 2, 'bar': 2, 'baz': 3},
|
||||
'W23 2021': {'foo': 1, 'bar': 0, 'baz': 3},
|
||||
'W53 2018': {'foo': 3, 'bar': 1, 'baz': 1},
|
||||
'W2 2019': {'foo': 3, 'bar': 2, 'baz': 2},
|
||||
'W3 2019': {'foo': 0, 'bar': 0, 'baz': 3},
|
||||
})
|
||||
|
||||
# add a computed field on model
|
||||
@@ -146,13 +146,14 @@ class TestReadProgressBar(common.TransactionCase):
|
||||
result = self.env['x_progressbar'].read_progress_bar([], 'x_country_id', progress_bar)
|
||||
self.assertEqual(result, {
|
||||
c1.display_name: {'foo': 3, 'bar': 1, 'baz': 1},
|
||||
c2.display_name: {'foo': 1, 'bar': 2, 'baz': 2},
|
||||
c3.display_name: {'foo': 2, 'bar': 0, 'baz': 3},
|
||||
c2.display_name: {'foo': 1, 'bar': 2, 'baz': 1},
|
||||
c3.display_name: {'foo': 2, 'bar': 0, 'baz': 4},
|
||||
})
|
||||
|
||||
result = self.env['x_progressbar'].read_progress_bar([], 'x_date:week', progress_bar)
|
||||
self.assertEqual(result, {
|
||||
'W21 2021': {'foo': 3, 'bar': 1, 'baz': 0},
|
||||
'W22 2021': {'foo': 2, 'bar': 2, 'baz': 3},
|
||||
'W23 2021': {'foo': 1, 'bar': 0, 'baz': 3},
|
||||
# first week is not the same as above, but that seems acceptable...
|
||||
'W1 2019': {'foo': 3, 'bar': 1, 'baz': 1},
|
||||
'W2 2019': {'foo': 3, 'bar': 2, 'baz': 2},
|
||||
'W3 2019': {'foo': 0, 'bar': 0, 'baz': 3},
|
||||
})
|
||||
|
||||
@@ -6,3 +6,4 @@ from . import test_fill_temporal
|
||||
from . import test_auto_join
|
||||
from . import test_m2m_grouping
|
||||
from . import test_date_range
|
||||
from . import test_groupby_week
|
||||
|
||||
@@ -232,6 +232,58 @@ class TestFillTemporal(common.TransactionCase):
|
||||
|
||||
self.assertEqual(groups, expected)
|
||||
|
||||
def test_date_range_groupby_week(self):
|
||||
"""Test data with weeks starting on Sunday."""
|
||||
self.Model.create([
|
||||
{'date': '1916-08-19', 'value': 4}, # saturday W34
|
||||
{'date': '1916-08-20', 'value': 13}, # sunday W35
|
||||
{'date': '1916-09-10', 'value': 5}, # sunday W38
|
||||
{'date': '1916-08-18', 'value': 3}, # friday W34
|
||||
{'date': '1916-09-11', 'value': 4}, # monday W38
|
||||
{'date': '1916-09-12', 'value': 11}, # tuesday W38
|
||||
])
|
||||
|
||||
expected = [{
|
||||
'__domain': ['&', ('date', '>=', '1916-08-13'), ('date', '<', '1916-08-20')],
|
||||
'__range': {'date:week': {'from': '1916-08-13', 'to': '1916-08-20'}},
|
||||
'date:week': 'W34 1916',
|
||||
'date_count': 2,
|
||||
'value': 7,
|
||||
}, {
|
||||
'__domain': ['&', ('date', '>=', '1916-08-20'), ('date', '<', '1916-08-27')],
|
||||
'__range': {'date:week': {'from': '1916-08-20', 'to': '1916-08-27'}},
|
||||
'date:week': 'W35 1916',
|
||||
'date_count': 1,
|
||||
'value': 13,
|
||||
}, {
|
||||
'__domain': ['&', ('date', '>=', '1916-08-27'), ('date', '<', '1916-09-03')],
|
||||
'__range': {'date:week': {'from': '1916-08-27', 'to': '1916-09-03'}},
|
||||
'date:week': 'W36 1916',
|
||||
'date_count': 0,
|
||||
'value': 0,
|
||||
}, {
|
||||
'__domain': ['&', ('date', '>=', '1916-09-03'), ('date', '<', '1916-09-10')],
|
||||
'__range': {'date:week': {'from': '1916-09-03', 'to': '1916-09-10'}},
|
||||
'date:week': 'W37 1916',
|
||||
'date_count': 0,
|
||||
'value': 0,
|
||||
}, {
|
||||
'__domain': ['&', ('date', '>=', '1916-09-10'), ('date', '<', '1916-09-17')],
|
||||
'__range': {'date:week': {'from': '1916-09-10', 'to': '1916-09-17'}},
|
||||
'date:week': 'W38 1916',
|
||||
'date_count': 3,
|
||||
'value': 20,
|
||||
}]
|
||||
|
||||
groups = self.Model.read_group([], fields=['date', 'value'], groupby=['date:week'])
|
||||
|
||||
self.assertEqual(groups, [group for group in expected if group['date_count']])
|
||||
|
||||
model_fill = self.Model.with_context(fill_temporal=True)
|
||||
groups = model_fill.read_group([], fields=['date', 'value'], groupby=['date:week'])
|
||||
|
||||
self.assertEqual(groups, expected)
|
||||
|
||||
def test_order_date_desc(self):
|
||||
"""Test if changing Model._order has influence on the result."""
|
||||
self.Model.create({'date': '1916-08-18', 'value': 3})
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
from odoo.tests import common
|
||||
|
||||
|
||||
class TestGroupbyWeek(common.TransactionCase):
|
||||
""" Test for read_group() with group by week: the first day of the week
|
||||
depends on the language.
|
||||
"""
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
super().setUpClass()
|
||||
cls.Model = cls.env['res.partner']
|
||||
cls.records = cls.Model.create([ # BE, SY, US
|
||||
{'date': '2022-05-27', 'name': 'May twenty-seven'}, # W21, W21, W22
|
||||
{'date': '2022-05-28', 'name': 'May twenty-eight'}, # W21, W22, W22
|
||||
{'date': '2022-05-29', 'name': 'May twenty-nine'}, # W21, W22, W23
|
||||
{'date': '2022-05-30', 'name': 'May thirty'}, # W22, W22, W23
|
||||
{'date': '2022-06-18', 'name': 'June eighteen'}, # W24, W25, W25
|
||||
{'date': '2022-06-19', 'name': 'June nineteen'}, # W24, W25, W26
|
||||
{'date': '2022-06-20', 'name': 'June twenty'}, # W25, W25, W26
|
||||
])
|
||||
|
||||
def test_belgium(self):
|
||||
""" fr_BE - first day of the week = Monday """
|
||||
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', 'name'], 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,
|
||||
},
|
||||
"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.env['res.lang']._activate_lang('ar_SY')
|
||||
groups = self.Model.with_context(lang='ar_SY').read_group(
|
||||
[('id', 'in', self.records.ids)], fields=['date', 'name'], groupby=['date:week'])
|
||||
self.assertDictEqual(
|
||||
{week['date:week']: week['date_count'] for week in groups if week['date:week']},
|
||||
{
|
||||
'W21 2022': 1,
|
||||
'W22 2022': 3,
|
||||
'W25 2022': 3,
|
||||
},
|
||||
"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 """
|
||||
groups = self.Model.with_context(lang='en_US').read_group(
|
||||
[('id', 'in', self.records.ids)], fields=['date', 'name'], groupby=['date:week'])
|
||||
self.assertDictEqual(
|
||||
{week['date:week']: week['date_count'] for week in groups if week['date:week']},
|
||||
{
|
||||
'W22 2022': 2,
|
||||
'W23 2022': 2,
|
||||
'W25 2022': 1,
|
||||
'W26 2022': 2,
|
||||
},
|
||||
"Week groups not matching when the first day of the week is Sunday"
|
||||
)
|
||||
+12
-5
@@ -43,6 +43,7 @@ from contextlib import closing
|
||||
from inspect import getmembers, currentframe
|
||||
from operator import attrgetter, itemgetter
|
||||
|
||||
import babel
|
||||
import babel.dates
|
||||
import dateutil.relativedelta
|
||||
import psycopg2
|
||||
@@ -1975,7 +1976,7 @@ class BaseModel(metaclass=MetaModel):
|
||||
if not field:
|
||||
raise ValueError("Invalid field %r on model %r" % (split[0], self._name))
|
||||
field_type = field.type
|
||||
gb_function = split[1] if len(split) == 2 else None
|
||||
gb_function = split[1] if len(split) == 2 else 'month'
|
||||
temporal = field_type in ('date', 'datetime')
|
||||
tz_convert = field_type == 'datetime' and self._context.get('tz') in pytz.all_timezones
|
||||
qualified_field = self._inherits_join_calc(self._table, split[0], query)
|
||||
@@ -2007,16 +2008,22 @@ class BaseModel(metaclass=MetaModel):
|
||||
}
|
||||
if tz_convert:
|
||||
qualified_field = "timezone('%s', timezone('UTC',%s))" % (self._context.get('tz', 'UTC'), qualified_field)
|
||||
qualified_field = "date_trunc('%s', %s::timestamp)" % (gb_function or 'month', qualified_field)
|
||||
if gb_function == 'week':
|
||||
# first_week_day: 0=Monday, 1=Tuesday, ...
|
||||
first_week_day = int(get_lang(self.env).week_start) - 1
|
||||
days_offset = first_week_day and 7 - first_week_day
|
||||
qualified_field = f"date_trunc('{gb_function}', {qualified_field}::timestamp - INTERVAL '-{days_offset} DAY') + INTERVAL '-{days_offset} DAY'"
|
||||
else:
|
||||
qualified_field = f"date_trunc('{gb_function}', {qualified_field}::timestamp)"
|
||||
if field_type == 'boolean':
|
||||
qualified_field = "coalesce(%s,false)" % qualified_field
|
||||
return {
|
||||
'field': split[0],
|
||||
'groupby': gb,
|
||||
'type': field_type,
|
||||
'display_format': display_formats[gb_function or 'month'] if temporal else None,
|
||||
'interval': time_intervals[gb_function or 'month'] if temporal else None,
|
||||
'granularity': gb_function or 'month' if temporal else None,
|
||||
'display_format': display_formats[gb_function] if temporal else None,
|
||||
'interval': time_intervals[gb_function] if temporal else None,
|
||||
'granularity': gb_function if temporal else None,
|
||||
'tz_convert': tz_convert,
|
||||
'qualified_field': qualified_field,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user