Commit 3a358c55 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Fix wrong end date in query for contributions

When a contributor configures a timezone that is behind UTC, the end
date is incorrectly set to the end-of-day of the previous day.

Changelog: fixed
parent 2fcace97
...@@ -33,7 +33,7 @@ module TimeZoneHelper ...@@ -33,7 +33,7 @@ module TimeZoneHelper
end end
end end
def local_time_instance(timezone) def local_timezone_instance(timezone)
return Time.zone if timezone.blank? return Time.zone if timezone.blank?
ActiveSupport::TimeZone.new(timezone) || Time.zone ActiveSupport::TimeZone.new(timezone) || Time.zone
...@@ -42,7 +42,7 @@ module TimeZoneHelper ...@@ -42,7 +42,7 @@ module TimeZoneHelper
def local_time(timezone) def local_time(timezone)
return if timezone.blank? return if timezone.blank?
time_zone_instance = local_time_instance(timezone) time_zone_instance = local_timezone_instance(timezone)
time_zone_instance.now.strftime("%-l:%M %p") time_zone_instance.now.strftime("%-l:%M %p")
end end
end end
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
.row.d-none.d-sm-flex .row.d-none.d-sm-flex
.col-12.calendar-block.gl-my-3 .col-12.calendar-block.gl-my-3
.user-calendar.light{ data: { calendar_path: user_calendar_path(@user, :json), calendar_activities_path: user_calendar_activities_path, utc_offset: local_time_instance(@user.timezone).now.utc_offset } } .user-calendar.light{ data: { calendar_path: user_calendar_path(@user, :json), calendar_activities_path: user_calendar_activities_path, utc_offset: local_timezone_instance(@user.timezone).now.utc_offset } }
.gl-spinner.gl-spinner-md.gl-my-8 .gl-spinner.gl-spinner-md.gl-my-8
.user-calendar-error.invisible .user-calendar-error.invisible
= _('There was an error loading users activity calendar.') = _('There was an error loading users activity calendar.')
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
%li %li
%span.light.js-localtime{ :data => { :datetime => event.created_at.utc.strftime('%Y-%m-%dT%H:%M:%SZ'), :toggle => 'tooltip', :placement => 'top' } } %span.light.js-localtime{ :data => { :datetime => event.created_at.utc.strftime('%Y-%m-%dT%H:%M:%SZ'), :toggle => 'tooltip', :placement => 'top' } }
= sprite_icon('clock', css_class: 'gl-vertical-align-text-bottom') = sprite_icon('clock', css_class: 'gl-vertical-align-text-bottom')
= event.created_at.to_time.in_time_zone(local_time_instance(@user.timezone)).strftime('%-I:%M%P') = event.created_at.to_time.in_time_zone(local_timezone_instance(@user.timezone)).strftime('%-I:%M%P')
- if event.visible_to_user?(current_user) - if event.visible_to_user?(current_user)
- if event.push_action? - if event.push_action?
#{event.action_name} #{event.ref_type} #{event.action_name} #{event.ref_type}
......
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
def initialize(contributor, current_user = nil) def initialize(contributor, current_user = nil)
@contributor = contributor @contributor = contributor
@contributor_time_instance = local_time_instance(contributor.timezone) @contributor_time_instance = local_timezone_instance(contributor.timezone).now
@current_user = current_user @current_user = current_user
@projects = if @contributor.include_private_contributions? @projects = if @contributor.include_private_contributions?
ContributedProjectsFinder.new(@contributor).execute(@contributor) ContributedProjectsFinder.new(@contributor).execute(@contributor)
...@@ -24,18 +24,20 @@ module Gitlab ...@@ -24,18 +24,20 @@ module Gitlab
return {} if @projects.empty? return {} if @projects.empty?
return @activity_dates if @activity_dates.present? return @activity_dates if @activity_dates.present?
date_interval = "INTERVAL '#{@contributor_time_instance.now.utc_offset} seconds'" start_time = @contributor_time_instance.years_ago(1).beginning_of_day
end_time = @contributor_time_instance.end_of_day
date_interval = "INTERVAL '#{@contributor_time_instance.utc_offset} seconds'"
# Can't use Event.contributions here because we need to check 3 different # Can't use Event.contributions here because we need to check 3 different
# project_features for the (currently) 3 different contribution types # project_features for the (currently) 3 different contribution types
date_from = @contributor_time_instance.now.years_ago(1) repo_events = events_created_between(start_time, end_time, :repository)
repo_events = event_created_at(date_from, :repository)
.where(action: :pushed) .where(action: :pushed)
issue_events = event_created_at(date_from, :issues) issue_events = events_created_between(start_time, end_time, :issues)
.where(action: [:created, :closed], target_type: "Issue") .where(action: [:created, :closed], target_type: "Issue")
mr_events = event_created_at(date_from, :merge_requests) mr_events = events_created_between(start_time, end_time, :merge_requests)
.where(action: [:merged, :created, :closed], target_type: "MergeRequest") .where(action: [:merged, :created, :closed], target_type: "MergeRequest")
note_events = event_created_at(date_from, :merge_requests) note_events = events_created_between(start_time, end_time, :merge_requests)
.where(action: :commented) .where(action: :commented)
events = Event events = Event
...@@ -54,7 +56,7 @@ module Gitlab ...@@ -54,7 +56,7 @@ module Gitlab
def events_by_date(date) def events_by_date(date)
return Event.none unless can_read_cross_project? return Event.none unless can_read_cross_project?
date_in_time_zone = date.in_time_zone(@contributor_time_instance) date_in_time_zone = date.in_time_zone(@contributor_time_instance.time_zone)
Event.contributions.where(author_id: contributor.id) Event.contributions.where(author_id: contributor.id)
.where(created_at: date_in_time_zone.beginning_of_day..date_in_time_zone.end_of_day) .where(created_at: date_in_time_zone.beginning_of_day..date_in_time_zone.end_of_day)
...@@ -64,11 +66,11 @@ module Gitlab ...@@ -64,11 +66,11 @@ module Gitlab
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def starting_year def starting_year
@contributor_time_instance.now.years_ago(1).year @contributor_time_instance.years_ago(1).year
end end
def starting_month def starting_month
@contributor_time_instance.today.month @contributor_time_instance.month
end end
private private
...@@ -78,9 +80,7 @@ module Gitlab ...@@ -78,9 +80,7 @@ module Gitlab
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def event_created_at(date_from, feature) def events_created_between(start_time, end_time, feature)
t = Event.arel_table
# re-running the contributed projects query in each union is expensive, so # re-running the contributed projects query in each union is expensive, so
# use IN(project_ids...) instead. It's the intersection of two users so # use IN(project_ids...) instead. It's the intersection of two users so
# the list will be (relatively) short # the list will be (relatively) short
...@@ -89,24 +89,22 @@ module Gitlab ...@@ -89,24 +89,22 @@ module Gitlab
# no need to check feature access of current user, if the contributor opted-in # no need to check feature access of current user, if the contributor opted-in
# to show all private events anyway - otherwise they would get filtered out again # to show all private events anyway - otherwise they would get filtered out again
authed_projects = if @contributor.include_private_contributions? authed_projects = if @contributor.include_private_contributions?
@contributed_project_ids.join(",") @contributed_project_ids
else else
ProjectFeature ProjectFeature
.with_feature_available_for_user(feature, current_user) .with_feature_available_for_user(feature, current_user)
.where(project_id: @contributed_project_ids) .where(project_id: @contributed_project_ids)
.reorder(nil) .reorder(nil)
.select(:project_id) .select(:project_id)
.to_sql
end end
conditions = t[:created_at].gteq(date_from.beginning_of_day)
.and(t[:created_at].lteq(@contributor_time_instance.today.end_of_day))
.and(t[:author_id].eq(contributor.id))
Event.reorder(nil) Event.reorder(nil)
.select(:created_at) .select(:created_at)
.where(conditions) .where(
.where("events.project_id in (#{authed_projects})") # rubocop:disable GitlabSecurity/SqlInjection author_id: contributor.id,
created_at: start_time..end_time,
events: { project_id: authed_projects }
)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
......
...@@ -125,7 +125,7 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do ...@@ -125,7 +125,7 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do
end end
end end
describe '#local_time_instance' do describe '#local_timezone_instance' do
let_it_be(:timezone) { 'UTC' } let_it_be(:timezone) { 'UTC' }
before do before do
...@@ -134,25 +134,25 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do ...@@ -134,25 +134,25 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do
context 'when timezone is `nil`' do context 'when timezone is `nil`' do
it 'returns the system timezone instance' do it 'returns the system timezone instance' do
expect(helper.local_time_instance(nil).name).to eq(timezone) expect(helper.local_timezone_instance(nil).name).to eq(timezone)
end end
end end
context 'when timezone is blank' do context 'when timezone is blank' do
it 'returns the system timezone instance' do it 'returns the system timezone instance' do
expect(helper.local_time_instance('').name).to eq(timezone) expect(helper.local_timezone_instance('').name).to eq(timezone)
end end
end end
context 'when a valid timezone is passed' do context 'when a valid timezone is passed' do
it 'returns the local time instance' do it 'returns the local time instance' do
expect(helper.local_time_instance('America/Los_Angeles').name).to eq('America/Los_Angeles') expect(helper.local_timezone_instance('America/Los_Angeles').name).to eq('America/Los_Angeles')
end end
end end
context 'when an invalid timezone is passed' do context 'when an invalid timezone is passed' do
it 'returns the system timezone instance' do it 'returns the system timezone instance' do
expect(helper.local_time_instance('Foo/Bar').name).to eq(timezone) expect(helper.local_timezone_instance('Foo/Bar').name).to eq(timezone)
end end
end end
end end
......
...@@ -146,6 +146,7 @@ RSpec.describe Gitlab::ContributionsCalendar do ...@@ -146,6 +146,7 @@ RSpec.describe Gitlab::ContributionsCalendar do
create_event(public_project, today, 10) create_event(public_project, today, 10)
create_event(public_project, today, 16) create_event(public_project, today, 16)
create_event(public_project, today, 23) create_event(public_project, today, 23)
create_event(public_project, tomorrow, 1)
end end
it "renders correct event counts within the UTC timezone" do it "renders correct event counts within the UTC timezone" do
...@@ -158,14 +159,14 @@ RSpec.describe Gitlab::ContributionsCalendar do ...@@ -158,14 +159,14 @@ RSpec.describe Gitlab::ContributionsCalendar do
it "renders correct event counts within the Sydney timezone" do it "renders correct event counts within the Sydney timezone" do
Time.use_zone('UTC') do Time.use_zone('UTC') do
contributor.timezone = 'Sydney' contributor.timezone = 'Sydney'
expect(calendar.activity_dates).to eq(today => 3, tomorrow => 2) expect(calendar.activity_dates).to eq(today => 3, tomorrow => 3)
end end
end end
it "renders correct event counts within the US Central timezone" do it "renders correct event counts within the US Central timezone" do
Time.use_zone('UTC') do Time.use_zone('UTC') do
contributor.timezone = 'Central Time (US & Canada)' contributor.timezone = 'Central Time (US & Canada)'
expect(calendar.activity_dates).to eq(yesterday => 2, today => 3) expect(calendar.activity_dates).to eq(yesterday => 2, today => 4)
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment