Defect #30288

Groups are incorrect when grouping by date without user timezone set

Added by Mizuki ISHIKAWA 6 months ago. Updated 1 day ago.

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

0%

Category:Issues filter
Target version:-
Resolution:Fixed Affected version:

Description

The following problems may occur when grouping by date (TimeWithZone).
  1. group count is not displayed
  2. The displayed created_on and group name date are different
Precondition:
  1. Use PostgreSQL or MySQL
  2. Database time zone is JST (It is not just JST)
  3. User's time zone is unset (User.current.time_zone is nil)

pry(main)> Issue.find(3).created_on
Wed, 19 Jul 2006 19:07:27 UTC +00:00

pry(main)> Issue.find(2).created_on
Wed, 19 Jul 2006 19:04:21 UTC +00:00
pry(main)> Issue.find(2).created_on.localtime.to_date # JST
Thu, 20 Jul 2006

pry(main)> Issue.find(2).created_on.to_date # UTC
Wed, 19 Jul 2006

In environments where the above conditions apply, the test of "IssuesControllerTest#test_index_grouped_by_created_on [test/functional/issues_controller_test.rb:365]" fails.

related: #13803

grouping_issues.png (73.4 KB) Mizuki ISHIKAWA, 2018-12-28 07:21

fix-30288-v1.patch Magnifier (4.15 KB) Mizuki ISHIKAWA, 2019-01-04 07:35


Related issues

Related to Redmine - Feature #13803: Implement grouping issues by date (start, due, creation, ... Closed
Related to Redmine - Defect #31620: ActivitiesControllerTest#test_previous_project_index fail... Closed

Associated revisions

Revision 18264
Added by Jean-Philippe Lang 6 days ago

Groups are incorrect when grouping by date without user timezone set.

Revision 18284
Added by Jean-Philippe Lang 6 days ago

Test failure with mysql (#30288).

Revision 18315
Added by Go MAEDA 1 day ago

test_page_with_activity fails depending on the current time and zone (#30288).

Patch by Mizuki ISHIKAWA.

Revision 18316
Added by Go MAEDA about 24 hours ago

test_previous_project_index fails depending on the current time and zone (#30288, #31620).

Patch by Mizuki ISHIKAWA.

History

#1 Updated by Go MAEDA 6 months ago

  • Related to Feature #13803: Implement grouping issues by date (start, due, creation, update, closing dates) added

#2 Updated by Marius BALTEANU 6 months ago

  • Description updated (diff)

#3 Updated by Marius BALTEANU 6 months ago

Do you think that it can have any connection with the issue from #16482?

#4 Updated by Mizuki ISHIKAWA 6 months ago

Marius BALTEANU wrote:

Do you think that it can have any connection with the issue from #16482?

I think that this problem is a different problem from #16482.

source:/trunk/app/helpers/queries_helper.rb#L139 returned nil when the test failed.
At that time, result_count_by_group was {Thu, 20 Jul 2006=>2} and group was "Wed, 19 Jul 2006".
Issue 's created_on was converted to different time zones, JST and UTC, so they were on different dates.

I think that time zones when converting time to date should be unified, but the default time zone when changing Redmine's time to date is not unified.
method name time zone source
i18n#format_time localtime (Database time zone) source:/trunk/lib/redmine/i18n.rb#L81
User#time_to_date UTC source:/trunk/app/models/user.rb#L543

I attached a patch with the default timezone of User#time_to_date change to localtime (Database time zone).
In my environment, applying this patch solves the problem.

#5 Updated by Marius BALTEANU 5 months ago

  • Assignee set to Jean-Philippe Lang
  • Target version set to 4.1.0

#6 Updated by Go MAEDA 5 months ago

  • Category set to Issues filter

#7 Updated by Jean-Philippe Lang 6 days ago

  • Subject changed from Group name and group count are incorrect when grouping by date (TimeWithZone) to Groups are incorrect when grouping by date without user timezone set
  • Status changed from New to Closed
  • Resolution set to Fixed

Patch committed, thanks!

#8 Updated by Go MAEDA 6 days ago

  • Target version deleted (4.1.0)

Removing the target version. This fix is a part of #13803 which will be delivered in upcoming 4.1.0, so it is not necessary to publish in the changelog.

#9 Updated by Go MAEDA 2 days ago

  • Status changed from Closed to Reopened

r18264 broke a test. Probably it fails depending on the current time.

$ date -R ; bin/rails test test/functional/my_controller_test.rb:204
Mon, 24 Jun 2019 08:29:07 +0900
Run options: --seed 30392

# Running:

F

Failure:
MyControllerTest#test_page_with_activity [/Users/maeda/redmines/redmine-trunk/test/functional/my_controller_test.rb:212]:
Expected at least 1 element matching "a[href="/activity?from=2019-06-23&user_id=2"]", found 0..
Expected 0 to be >= 1.

bin/rails test test/functional/my_controller_test.rb:204

#10 Updated by Mizuki ISHIKAWA 2 days ago

Go MAEDA wrote:

r18264 broke a test. Probably it fails depending on the current time.

[...]

I think that the test succeeds by changing as below.

diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb
index 069cfdc336..81df1d56c4 100644
--- a/test/functional/my_controller_test.rb
+++ b/test/functional/my_controller_test.rb
@@ -205,13 +205,14 @@ class MyControllerTest < Redmine::ControllerTest
     user = User.find(2)
     user.pref.my_page_layout = {'top' => ['activity']}
     user.pref.save!
+    latest_issue = Issue.generate!

     get :page
     assert_response :success

     assert_select 'div#block-activity' do
       assert_select 'h3' do
-        assert_select 'a[href=?]', activity_path(from: Date.current, user_id: user.id),  :text => 'Activity'
+        assert_select 'a[href=?]', activity_path(from: user.time_to_date(latest_issue.created_on), user_id: user.id), :text => 'Activity'
       end
       assert_select 'div#activity' do
         assert_select 'dt', 10

#11 Updated by Mizuki ISHIKAWA 2 days ago

This change may be better than the #30288#note-10 change.

diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb
index 069cfdc336..42a028d407 100644
--- a/test/functional/my_controller_test.rb
+++ b/test/functional/my_controller_test.rb
@@ -204,6 +204,7 @@ class MyControllerTest < Redmine::ControllerTest
   def test_page_with_activity
     user = User.find(2)
     user.pref.my_page_layout = {'top' => ['activity']}
+    user.pref.time_zone = 'UTC'
     user.pref.save!

     get :page
@@ -211,7 +212,7 @@ class MyControllerTest < Redmine::ControllerTest

     assert_select 'div#block-activity' do
       assert_select 'h3' do
-        assert_select 'a[href=?]', activity_path(from: Date.current, user_id: user.id),  :text => 'Activity'
+        assert_select 'a[href=?]', activity_path(from: User.current.today, user_id: user.id),  :text => 'Activity'
       end
       assert_select 'div#activity' do
         assert_select 'dt', 10

#12 Updated by Go MAEDA 1 day ago

  • Status changed from Reopened to Closed

Go MAEDA wrote:

r18264 broke a test. Probably it fails depending on the current time.

[...]

Committed the fix written by Mizuki Ishikawa in r18315.

#13 Updated by Go MAEDA about 24 hours ago

  • Related to Defect #31620: ActivitiesControllerTest#test_previous_project_index fails depending on the current time and zone added

Also available in: Atom PDF