Defect #5329

Time entries report broken on first week of year

Added by Holger Just over 7 years ago. Updated over 4 years ago.

Status:ClosedStart date:2010-04-15
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Time tracking
Target version:2.3.0
Resolution:Fixed Affected version:

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.

functional_test_for_problem.patch Magnifier (4.11 KB) Holger Just, 2010-04-15 19:56

5329_add_year_of_week_to_time_entries.diff Magnifier (6.91 KB) Holger Just, 2010-06-04 17:06

time_report.rb.patch Magnifier (863 Bytes) Filou Centrinov, 2013-02-21 23:45

time_report.rb.2.patch Magnifier - no tabs (1.04 KB) Filou Centrinov, 2013-02-22 02:23


Related issues

Related to Redmine - Defect #4953: "In the past N days" filter displays N+1 days Confirmed 2010-03-01
Duplicated by Redmine - Defect #3060: Week numbers dont match in reports and calendar popups Closed 2009-03-27
Duplicated by Redmine - Defect #2462: Incorrect Week Number in Spent Time Report Closed 2009-01-07
Duplicated by Redmine - Defect #13082: Wrong year in Spent Time Report Closed
Duplicated by Redmine - Defect #13159: Spent Time report - Column Total different for week Closed
Duplicated by Redmine - Defect #12891: Wrong week number in time entries Closed
Duplicated by Redmine - Defect #12803: Cross-year date range in time tracking report misses some... Closed

Associated revisions

Revision 11471
Added by Jean-Philippe Lang over 4 years ago

Fixed that time entries report by week breaks on edge cases (#5329).

Revision 11565
Added by Jean-Philippe Lang over 4 years ago

Merged r11471 from trunk (#5329).

History

#1 Updated by Holger Just over 7 years ago

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.

#2 Updated by Holger Just over 7 years 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.

#3 Updated by Holger Just over 7 years 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.

#4 Updated by Daniel Felix almost 5 years ago

  • Status changed from New to Needs feedback
  • Affected version (unused) deleted (devel)

This is another issue regarding the timelog. Someone who can confirm this is still able to reproduce in trunk?

#5 Updated by Alexey Verin almost 5 years ago

Please, update patch for version 2.2.1.
My Ruby skill is not enough for this (

#6 Updated by Mischa The Evil almost 5 years ago

Daniel Felix wrote:

[...] Someone who can confirm this is still able to reproduce in trunk?

I can't remember any changes that fixes this issue specifically, though I'm unable to test this on current trunk.

#7 Updated by Daniel Felix almost 5 years ago

  • Status changed from Needs feedback to Confirmed

Alexey already encountered this big again. (See linked ticket)
This is the confirmation for this issue.

#8 Updated by Filou Centrinov almost 5 years ago

Duplicate: #13159

#9 Updated by Filou Centrinov almost 5 years ago

I fixed this issue with a patch. Could we add this to Redmine 2.3?
I verfied my solution for different weeks constellations: Years with 52 and 53 weeks or first week that starts in last year and those which don't. Everthing works fine.

#11 Updated by Daniel Felix almost 5 years ago

The patch works on my testsystem too. Thanks for providing this patch!

#12 Updated by Jean-Philippe Lang almost 5 years ago

  • Target version set to Candidate for next minor release

#13 Updated by Jean-Philippe Lang over 4 years ago

  • Status changed from Confirmed to Resolved
  • Assignee set to Jean-Philippe Lang
  • Target version changed from Candidate for next minor release to 2.3.0
  • Resolution set to Fixed

Patch committed in r11471 with a test, thanks.

#14 Updated by Jean-Philippe Lang over 4 years ago

  • Subject changed from Group-by week of time entries breaks on edge cases to Time entries report broken on first week of year

#15 Updated by Jean-Philippe Lang over 4 years ago

  • Status changed from Resolved to Closed

Merged.

#16 Updated by Pavel Potcheptsov over 4 years ago

Hello, I've updated to 2.3.0.
Is it all right with:
Project -> Overview -> Report -> Details: Days, Add: User ?
What Days it should display?
In my case it displays days from 2012 year and not the last days, when time was logged in 2013.

Also available in: Atom PDF