Defect #5329
Group-by week of time entries breaks on edge cases
| Status: | New | Start date: | 2010-04-15 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: | - | % Done: | 0% |
|
| Category: | Time tracking | |||
| Target version: | - | |||
| Affected version: | devel | Resolution: |
Description
Redmine stores some additional columns time entries:
- tmonth ->
date.month - tyear ->
date.year - tweek ->
date.cweek
These are used to support group by of the time entries in the reports view. However, there is an edge case which breaks the code.
Consider the following 2 time entry:
| field | Time Entry 1 | Time Entry 2 |
|---|---|---|
| spent_on | 2009-12-12 | 2010-01-01 |
| tmonth | 12 | 01 |
| tyear | 2009 | 2010 |
| tweek | 53 | 53 |
Both are in the same week, but in different years. This leads to the second entry to be not displayed often, and if then in the wrong week, i.e. 2010-53. For the group-by week, instead of date.year, date.cwyear should be used. It results in 2009 for the second example entry.
The attached patch creates a test for that. It also adds the two time entries above as fixtures. Thus, some other tests had to be adapted.
Related issues
History
Updated by Holger Just over 1 year ago
- File 5329_add_year_of_week_to_time_entries.diff added
The attached patch fixes the edge case.
It introduces a new field named twyear to the time entries. It contains the year of the week of the entry. So for the above example, its content for both entries would be 2009. That way, it is possible to properly group based on weeks.
The included tests run in a current trunk.
Updated by Holger Just over 1 year ago
Just verified that patch again on a current trunk.
It is clean and does what it advertises. I'm just not 100% sure if I managed to find all usages of the week stuff in time_entries. But I'm rather confident, I did.
Updated by Holger Just over 1 year ago
Okay, after thinking a bit more about it (and fixing some other bugs :)), I conclude that the whole concept of denormalizing the week into the database for grouping can not properly work.
This is because the week boundaries expected by a user are actually not fixed but depend on her settings. So the actual data set in a group depends on user settings. To always show the correct result, we would have to
- either save at least two variants of the tweek + twyear (for week starting sunday or monday)
- or calculate the week using SQL-functions.
I would prefer the second method. Unfortunately, these functions are not fully standardized, so there would be a differentiation needed for different database engines.
Until then, the attached patch fixes the behavior for the standard commercial week starting on Mondays.