Merge branch 'fix-contribution-calendar-end-date' into 'master'

Fix wrong end date in query for contributions

See merge request gitlab-org/gitlab!76077

app/helpers/time_zone_helper.rb

@@ -33,7 +33,7 @@ module TimeZoneHelper
     end
   end
 
-  def local_time_instance(timezone)
+  def local_timezone_instance(timezone)
     return Time.zone if timezone.blank?
 
     ActiveSupport::TimeZone.new(timezone) || Time.zone
@@ -42,7 +42,7 @@ module TimeZoneHelper
   def local_time(timezone)
     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")
   end
 end

app/views/users/_overview.html.haml

@@ -2,7 +2,7 @@
 
 .row.d-none.d-sm-flex
   .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
       .user-calendar-error.invisible
         = _('There was an error loading users activity calendar.')

app/views/users/calendar_activities.html.haml

@@ -7,7 +7,7 @@
       %li
         %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')
-          = 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.push_action?
             #{event.action_name} #{event.ref_type}

lib/gitlab/contributions_calendar.rb

@@ -10,7 +10,7 @@ module Gitlab
 
     def initialize(contributor, current_user = nil)
       @contributor = contributor
-      @contributor_time_instance = local_time_instance(contributor.timezone)
+      @contributor_time_instance = local_timezone_instance(contributor.timezone).now
       @current_user = current_user
       @projects = if @contributor.include_private_contributions?
                     ContributedProjectsFinder.new(@contributor).execute(@contributor)
@@ -24,18 +24,20 @@ module Gitlab
       return {} if @projects.empty?
       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
       # project_features for the (currently) 3 different contribution types
-      date_from = @contributor_time_instance.now.years_ago(1)
-      repo_events = event_created_at(date_from, :repository)
+      repo_events = events_created_between(start_time, end_time, :repository)
         .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")
-      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")
-      note_events = event_created_at(date_from, :merge_requests)
+      note_events = events_created_between(start_time, end_time, :merge_requests)
         .where(action: :commented)
 
       events = Event
@@ -54,7 +56,7 @@ module Gitlab
     def events_by_date(date)
       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)
         .where(created_at: date_in_time_zone.beginning_of_day..date_in_time_zone.end_of_day)
@@ -64,11 +66,11 @@ module Gitlab
     # rubocop: enable CodeReuse/ActiveRecord
 
     def starting_year
-      @contributor_time_instance.now.years_ago(1).year
+      @contributor_time_instance.years_ago(1).year
     end
 
     def starting_month
-      @contributor_time_instance.today.month
+      @contributor_time_instance.month
     end
 
     private
@@ -78,9 +80,7 @@ module Gitlab
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def event_created_at(date_from, feature)
-      t = Event.arel_table
-
+    def events_created_between(start_time, end_time, feature)
       # 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
       # the list will be (relatively) short
@@ -89,24 +89,22 @@ module Gitlab
       # 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
       authed_projects = if @contributor.include_private_contributions?
-                          @contributed_project_ids.join(",")
+                          @contributed_project_ids
                         else
                           ProjectFeature
                             .with_feature_available_for_user(feature, current_user)
                             .where(project_id: @contributed_project_ids)
                             .reorder(nil)
                             .select(:project_id)
-                            .to_sql
                         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)
         .select(:created_at)
-        .where(conditions)
-        .where("events.project_id in (#{authed_projects})") # rubocop:disable GitlabSecurity/SqlInjection
+        .where(
+          author_id: contributor.id,
+          created_at: start_time..end_time,
+          events: { project_id: authed_projects }
+        )
     end
     # rubocop: enable CodeReuse/ActiveRecord
   end

spec/helpers/time_zone_helper_spec.rb

@@ -125,7 +125,7 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do
     end
   end
 
-  describe '#local_time_instance' do
+  describe '#local_timezone_instance' do
     let_it_be(:timezone) { 'UTC' }
 
     before do
@@ -134,25 +134,25 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do
 
     context 'when timezone is `nil`' 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
 
     context 'when timezone is blank' 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
 
     context 'when a valid timezone is passed' 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
 
     context 'when an invalid timezone is passed' 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

spec/lib/gitlab/contributions_calendar_spec.rb

@@ -146,6 +146,7 @@ RSpec.describe Gitlab::ContributionsCalendar do
         create_event(public_project, today, 10)
         create_event(public_project, today, 16)
         create_event(public_project, today, 23)
+        create_event(public_project, tomorrow, 1)
       end
 
       it "renders correct event counts within the UTC timezone" do
@@ -158,14 +159,14 @@ RSpec.describe Gitlab::ContributionsCalendar do
       it "renders correct event counts within the Sydney timezone" do
         Time.use_zone('UTC') do
           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
 
       it "renders correct event counts within the US Central timezone" do
         Time.use_zone('UTC') do
           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