From 8bbb35d90ba84cd2dd08857674ba5c7c06eaa10c Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Sat, 26 Mar 2016 12:11:08 +0800 Subject: [PATCH 1/2] Replace Date.today with User.current.today Depending on the offset between a user's configured timezone and the server timezone, Date.today may be more or less often wrong from the user's perspective, leading to things like issues marked as overdue too early or too late, or yesterday / tomorrow being displayed / selected where 'today' is intended. A test case illustrating the problem with Issue#overdue? is included --- app/controllers/activities_controller.rb | 2 +- app/controllers/calendars_controller.rb | 4 ++-- app/controllers/issues_controller.rb | 2 +- app/controllers/repositories_controller.rb | 5 +++-- app/helpers/application_helper.rb | 2 +- app/helpers/my_helper.rb | 2 +- app/helpers/settings_helper.rb | 2 +- app/models/issue.rb | 4 ++-- app/models/mail_handler.rb | 2 +- app/models/project.rb | 2 +- app/models/query.rb | 14 +++++++------- app/models/user.rb | 1 + app/models/version.rb | 6 +++--- app/views/activities/index.html.erb | 2 +- app/views/common/_calendar.html.erb | 2 +- app/views/gantts/show.html.erb | 4 ++-- app/views/my/blocks/_calendar.html.erb | 2 +- app/views/my/blocks/_timelog.html.erb | 2 +- lib/redmine/export/pdf/issues_pdf_helper.rb | 4 ++-- lib/redmine/export/pdf/wiki_pdf_helper.rb | 4 ++-- lib/redmine/helpers/gantt.rb | 14 +++++++------- lib/redmine/helpers/time_report.rb | 4 ++-- test/unit/issue_test.rb | 26 ++++++++++++++++++++++++++ 23 files changed, 70 insertions(+), 42 deletions(-) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index ce3f600..32df00f 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -27,7 +27,7 @@ class ActivitiesController < ApplicationController begin; @date_to = params[:from].to_date + 1; rescue; end end - @date_to ||= Date.today + 1 + @date_to ||= User.current.today + 1 @date_from = @date_to - @days @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1') if params[:user_id].present? diff --git a/app/controllers/calendars_controller.rb b/app/controllers/calendars_controller.rb index 756d62b..276d798 100644 --- a/app/controllers/calendars_controller.rb +++ b/app/controllers/calendars_controller.rb @@ -35,8 +35,8 @@ class CalendarsController < ApplicationController @month = params[:month].to_i end end - @year ||= Date.today.year - @month ||= Date.today.month + @year ||= User.current.today.year + @month ||= User.current.today.month @calendar = Redmine::Helpers::Calendar.new(Date.civil(@year, @month, 1), current_language, :month) retrieve_query diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 5199954..2fb5c2e 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -442,7 +442,7 @@ class IssuesController < ApplicationController @issue.project ||= @issue.allowed_target_projects.first end @issue.author ||= User.current - @issue.start_date ||= Date.today if Setting.default_issue_start_date_to_creation_date? + @issue.start_date ||= User.current.today if Setting.default_issue_start_date_to_creation_date? attrs = (params[:issue] || {}).deep_dup if action_name == 'new' && params[:was_default_status] == attrs[:status_id] diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index ded172a..1b394de 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -350,7 +350,7 @@ class RepositoriesController < ApplicationController end def graph_commits_per_month(repository) - @date_to = Date.today + @date_to = User.current.today @date_from = @date_to << 11 @date_from = Date.civil(@date_from.year, @date_from.month, 1) commits_by_day = Changeset. @@ -369,7 +369,8 @@ class RepositoriesController < ApplicationController changes_by_day.each {|c| changes_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last } fields = [] - 12.times {|m| fields << month_name(((Date.today.month - 1 - m) % 12) + 1)} + today = User.current.today + 12.times {|m| fields << month_name(((today.month - 1 - m) % 12) + 1)} graph = SVG::Graph::Bar.new( :height => 300, diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f091037..55c0ad5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -252,7 +252,7 @@ module ApplicationHelper def due_date_distance_in_words(date) if date - l((date < Date.today ? :label_roadmap_overdue : :label_roadmap_due_in), distance_of_date_in_words(Date.today, date)) + l((date < User.current.today ? :label_roadmap_overdue : :label_roadmap_due_in), distance_of_date_in_words(User.current.today, date)) end end diff --git a/app/helpers/my_helper.rb b/app/helpers/my_helper.rb index 504bd0b..ef1dbb8 100644 --- a/app/helpers/my_helper.rb +++ b/app/helpers/my_helper.rb @@ -65,7 +65,7 @@ module MyHelper def timelog_items TimeEntry. - where("#{TimeEntry.table_name}.user_id = ? AND #{TimeEntry.table_name}.spent_on BETWEEN ? AND ?", User.current.id, Date.today - 6, Date.today). + where("#{TimeEntry.table_name}.user_id = ? AND #{TimeEntry.table_name}.spent_on BETWEEN ? AND ?", User.current.id, User.current.today - 6, User.current.today). joins(:activity, :project). references(:issue => [:tracker, :status]). includes(:issue => [:tracker, :status]). diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index 43b2c70..2c657f0 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -182,7 +182,7 @@ module SettingsHelper # Returns the options for the date_format setting def date_format_setting_options(locale) Setting::DATE_FORMATS.map do |f| - today = ::I18n.l(Date.today, :locale => locale, :format => f) + today = ::I18n.l(User.current.today, :locale => locale, :format => f) format = f.gsub('%', '').gsub(/[dmY]/) do {'d' => 'dd', 'm' => 'mm', 'Y' => 'yyyy'}[$&] end diff --git a/app/models/issue.rb b/app/models/issue.rb index 5bf845d..3012aeb 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -806,14 +806,14 @@ class Issue < ActiveRecord::Base # Returns true if the issue is overdue def overdue? - due_date.present? && (due_date < Date.today) && !closed? + due_date.present? && (due_date < User.current.today) && !closed? end # Is the amount of work done less than it should for the due date def behind_schedule? return false if start_date.nil? || due_date.nil? done_date = start_date + ((due_date - start_date + 1) * done_ratio / 100).floor - return done_date <= Date.today + return done_date <= User.current.today end # Does this issue have children? diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index d5332e2..40bd02a 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -206,7 +206,7 @@ class MailHandler < ActionMailer::Base issue.subject = '(no subject)' end issue.description = cleaned_up_text_body - issue.start_date ||= Date.today if Setting.default_issue_start_date_to_creation_date? + issue.start_date ||= User.current.today if Setting.default_issue_start_date_to_creation_date? issue.is_private = (handler_options[:issue][:is_private] == '1') # add To and Cc as watchers before saving so the watchers can reply to Redmine diff --git a/app/models/project.rb b/app/models/project.rb index 2e3a106..dee2e6d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -594,7 +594,7 @@ class Project < ActiveRecord::Base end def overdue? - active? && !due_date.nil? && (due_date < Date.today) + active? && !due_date.nil? && (due_date < User.current.today) end # Returns the percent completed for this project, based on the diff --git a/app/models/query.rb b/app/models/query.rb index d0a54d6..76e8cf1 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -869,32 +869,32 @@ class Query < ActiveRecord::Base when "w" # = this week first_day_of_week = l(:general_first_day_of_week).to_i - day_of_week = Date.today.cwday + day_of_week = User.current.today.cwday days_ago = (day_of_week >= first_day_of_week ? day_of_week - first_day_of_week : day_of_week + 7 - first_day_of_week) sql = relative_date_clause(db_table, db_field, - days_ago, - days_ago + 6, is_custom_filter) when "lw" # = last week first_day_of_week = l(:general_first_day_of_week).to_i - day_of_week = Date.today.cwday + day_of_week = User.current.today.cwday days_ago = (day_of_week >= first_day_of_week ? day_of_week - first_day_of_week : day_of_week + 7 - first_day_of_week) sql = relative_date_clause(db_table, db_field, - days_ago - 7, - days_ago - 1, is_custom_filter) when "l2w" # = last 2 weeks first_day_of_week = l(:general_first_day_of_week).to_i - day_of_week = Date.today.cwday + day_of_week = User.current.today.cwday days_ago = (day_of_week >= first_day_of_week ? day_of_week - first_day_of_week : day_of_week + 7 - first_day_of_week) sql = relative_date_clause(db_table, db_field, - days_ago - 14, - days_ago - 1, is_custom_filter) when "m" # = this month - date = Date.today + date = User.current.today sql = date_clause(db_table, db_field, date.beginning_of_month, date.end_of_month, is_custom_filter) when "lm" # = last month - date = Date.today.prev_month + date = User.current.today.prev_month sql = date_clause(db_table, db_field, date.beginning_of_month, date.end_of_month, is_custom_filter) when "y" # = this year - date = Date.today + date = User.current.today sql = date_clause(db_table, db_field, date.beginning_of_year, date.end_of_year, is_custom_filter) when "~" sql = sql_contains("#{db_table}.#{db_field}", value.first) @@ -994,7 +994,7 @@ class Query < ActiveRecord::Base # Returns a SQL clause for a date or datetime field using relative dates. def relative_date_clause(table, field, days_from, days_to, is_custom_filter) - date_clause(table, field, (days_from ? Date.today + days_from : nil), (days_to ? Date.today + days_to : nil), is_custom_filter) + date_clause(table, field, (days_from ? User.current.today + days_from : nil), (days_to ? User.current.today + days_to : nil), is_custom_filter) end # Returns a Date or Time from the given filter value diff --git a/app/models/user.rb b/app/models/user.rb index 1eb2c32..c5a0773 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -510,6 +510,7 @@ class User < Principal if time_zone.nil? Date.today else + # TODO replace with time_zone.today Time.now.in_time_zone(time_zone).to_date end end diff --git a/app/models/version.rb b/app/models/version.rb index ad3d816..bc71b72 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -104,7 +104,7 @@ class Version < ActiveRecord::Base # Returns true if the version is completed: closed or due date reached and no open issues def completed? - closed? || (effective_date && (effective_date < Date.today) && (open_issues_count == 0)) + closed? || (effective_date && (effective_date < User.current.today) && (open_issues_count == 0)) end def behind_schedule? @@ -112,7 +112,7 @@ class Version < ActiveRecord::Base return false elsif due_date && start_date done_date = start_date + ((due_date - start_date+1)* completed_percent/100).floor - return done_date <= Date.today + return done_date <= User.current.today else false # No issues so it's not late end @@ -141,7 +141,7 @@ class Version < ActiveRecord::Base # Returns true if the version is overdue: due date reached and some open issues def overdue? - effective_date && (effective_date < Date.today) && (open_issues_count > 0) + effective_date && (effective_date < User.current.today) && (open_issues_count > 0) end # Returns assigned issues count diff --git a/app/views/activities/index.html.erb b/app/views/activities/index.html.erb index 830f408..58664d1 100644 --- a/app/views/activities/index.html.erb +++ b/app/views/activities/index.html.erb @@ -31,7 +31,7 @@ <%= link_to_content_update(l(:label_next) + " \xc2\xbb", params.merge(:from => @date_to + @days - 1), :title => l(:label_date_from_to, :start => format_date(@date_to), :end => format_date(@date_to + @days - 1)), - :accesskey => accesskey(:next)) unless @date_to >= Date.today %> + :accesskey => accesskey(:next)) unless @date_to >= User.current.today %>   <% other_formats_links do |f| %> diff --git a/app/views/common/_calendar.html.erb b/app/views/common/_calendar.html.erb index d950749..eb830b6 100644 --- a/app/views/common/_calendar.html.erb +++ b/app/views/common/_calendar.html.erb @@ -7,7 +7,7 @@ <% day = calendar.startdt while day <= calendar.enddt %> <%= ("#{(day+(11-day.cwday)%7).cweek}".html_safe) if day.cwday == calendar.first_wday %> - +

<%= day.day %>

<% calendar.events_on(day).each do |i| %> <% if i.is_a? Issue %> diff --git a/app/views/gantts/show.html.erb b/app/views/gantts/show.html.erb index 070a0de..34ee14c 100644 --- a/app/views/gantts/show.html.erb +++ b/app/views/gantts/show.html.erb @@ -302,9 +302,9 @@ <%= @gantt.lines.html_safe %> <% ###### Today red line (excluded from cache) ###### %> -<% if Date.today >= @gantt.date_from and Date.today <= @gantt.date_to %> +<% if User.current.today >= @gantt.date_from and User.current.today <= @gantt.date_to %> <% - today_left = (((Date.today - @gantt.date_from + 1) * zoom).floor() - 1).to_i + today_left = (((User.current.today - @gantt.date_from + 1) * zoom).floor() - 1).to_i style = "" style += "position: absolute;" style += "height: #{g_height}px;" diff --git a/app/views/my/blocks/_calendar.html.erb b/app/views/my/blocks/_calendar.html.erb index 41ca18a..89dde0e 100644 --- a/app/views/my/blocks/_calendar.html.erb +++ b/app/views/my/blocks/_calendar.html.erb @@ -1,6 +1,6 @@

<%= l(:label_calendar) %>

-<% calendar = Redmine::Helpers::Calendar.new(Date.today, current_language, :week) +<% calendar = Redmine::Helpers::Calendar.new(User.current.today, current_language, :week) calendar.events = calendar_items(calendar.startdt, calendar.enddt) %> <%= render :partial => 'common/calendar', :locals => {:calendar => calendar } %> diff --git a/app/views/my/blocks/_timelog.html.erb b/app/views/my/blocks/_timelog.html.erb index 7fcac4a..14a0711 100644 --- a/app/views/my/blocks/_timelog.html.erb +++ b/app/views/my/blocks/_timelog.html.erb @@ -29,7 +29,7 @@ entries_by_day = entries.group_by(&:spent_on) <% entries_by_day.keys.sort.reverse.each do |day| %> - <%= day == Date.today ? l(:label_today).titleize : format_date(day) %> + <%= day == User.current.today ? l(:label_today).titleize : format_date(day) %> <%= html_hours("%.2f" % entries_by_day[day].sum(&:hours).to_f) %> diff --git a/lib/redmine/export/pdf/issues_pdf_helper.rb b/lib/redmine/export/pdf/issues_pdf_helper.rb index a9ee230..75f63d2 100644 --- a/lib/redmine/export/pdf/issues_pdf_helper.rb +++ b/lib/redmine/export/pdf/issues_pdf_helper.rb @@ -26,7 +26,7 @@ module Redmine pdf = ITCPDF.new(current_language) pdf.set_title("#{issue.project} - #{issue.tracker} ##{issue.id}") pdf.alias_nb_pages - pdf.footer_date = format_date(Date.today) + pdf.footer_date = format_date(User.current.today) pdf.add_page pdf.SetFontStyle('B',11) buf = "#{issue.project} - #{issue.tracker} ##{issue.id}" @@ -246,7 +246,7 @@ module Redmine title = "#{project} - #{title}" if project pdf.set_title(title) pdf.alias_nb_pages - pdf.footer_date = format_date(Date.today) + pdf.footer_date = format_date(User.current.today) pdf.set_auto_page_break(false) pdf.add_page("L") diff --git a/lib/redmine/export/pdf/wiki_pdf_helper.rb b/lib/redmine/export/pdf/wiki_pdf_helper.rb index 37dbc1d..aea7b7e 100644 --- a/lib/redmine/export/pdf/wiki_pdf_helper.rb +++ b/lib/redmine/export/pdf/wiki_pdf_helper.rb @@ -26,7 +26,7 @@ module Redmine pdf = Redmine::Export::PDF::ITCPDF.new(current_language) pdf.set_title(project.name) pdf.alias_nb_pages - pdf.footer_date = format_date(Date.today) + pdf.footer_date = format_date(User.current.today) pdf.add_page pdf.SetFontStyle('B',11) pdf.RDMMultiCell(190,5, project.name) @@ -43,7 +43,7 @@ module Redmine pdf = ITCPDF.new(current_language) pdf.set_title("#{project} - #{page.title}") pdf.alias_nb_pages - pdf.footer_date = format_date(Date.today) + pdf.footer_date = format_date(User.current.today) pdf.add_page pdf.SetFontStyle('B',11) pdf.RDMMultiCell(190,5, diff --git a/lib/redmine/helpers/gantt.rb b/lib/redmine/helpers/gantt.rb index af78d9e..683d201 100644 --- a/lib/redmine/helpers/gantt.rb +++ b/lib/redmine/helpers/gantt.rb @@ -59,8 +59,8 @@ module Redmine @month_from = 1 end else - @month_from ||= Date.today.month - @year_from ||= Date.today.year + @month_from ||= User.current.today.month + @year_from ||= User.current.today.year end zoom = (options[:zoom] || User.current.pref[:gantt_zoom]).to_i @zoom = (zoom > 0 && zoom < 5) ? zoom : 2 @@ -428,9 +428,9 @@ module Redmine lines(:image => gc, :top => top, :zoom => zoom, :subject_width => subject_width, :format => :image) # today red line - if Date.today >= @date_from and Date.today <= date_to + if User.current.today >= @date_from and User.current.today <= date_to gc.stroke('red') - x = (Date.today - @date_from + 1) * zoom + subject_width + x = (User.current.today - @date_from + 1) * zoom + subject_width gc.line(x, headers_height, x, headers_height + g_height - 1) end gc.draw(imgl) @@ -442,7 +442,7 @@ module Redmine pdf = ::Redmine::Export::PDF::ITCPDF.new(current_language) pdf.SetTitle("#{l(:label_gantt)} #{project}") pdf.alias_nb_pages - pdf.footer_date = format_date(Date.today) + pdf.footer_date = format_date(User.current.today) pdf.AddPage("L") pdf.SetFontStyle('B', 12) pdf.SetX(15) @@ -592,8 +592,8 @@ module Redmine coords[:bar_progress_end] = self.date_to - self.date_from + 1 end end - if progress_date < Date.today - late_date = [Date.today, end_date].min + if progress_date < User.current.today + late_date = [User.current.today, end_date].min if late_date > self.date_from && late_date > start_date if late_date < self.date_to coords[:bar_late_end] = late_date - self.date_from + 1 diff --git a/lib/redmine/helpers/time_report.rb b/lib/redmine/helpers/time_report.rb index 884d00c..c9ce70e 100644 --- a/lib/redmine/helpers/time_report.rb +++ b/lib/redmine/helpers/time_report.rb @@ -70,10 +70,10 @@ module Redmine end min = @hours.collect {|row| row['spent_on']}.min - @from = min ? min.to_date : Date.today + @from = min ? min.to_date : User.current.today max = @hours.collect {|row| row['spent_on']}.max - @to = max ? max.to_date : Date.today + @to = max ? max.to_date : User.current.today @total_hours = @hours.inject(0) {|s,k| s = s + k['hours'].to_f} diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 3daca71..f70e53e 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2745,4 +2745,30 @@ class IssueTest < ActiveSupport::TestCase issue.reload.assigned_to = nil assert_equal group, issue.assigned_to_was end + + def test_issue_overdue_should_respect_user_timezone + user_in_europe = users(:users_001) + user_in_europe.pref.update_attribute :time_zone, 'UTC' + + user_in_asia = users(:users_002) + user_in_asia.pref.update_attribute :time_zone, 'Hongkong' + + issue = Issue.generate! :due_date => Date.parse('2016-03-20') + + # server time is UTC + time = Time.parse '2016-03-20 20:00 UTC' + Time.stubs(:now).returns(time) + Date.stubs(:today).returns(time.to_date) + + # for a user in the same time zone as the server the issue is not overdue + # yet + User.current = user_in_europe + assert !issue.overdue? + + # at the same time, a user in East Asia looks at the issue - it's already + # March 21st and the issue should be marked overdue + User.current = user_in_asia + assert issue.overdue? + + end end -- 2.1.4