Merge branch '32052-add-pa-start-date-limit' into 'master'

See merge request gitlab-org/gitlab!17933

app/models/application_setting_implementation.rb

@@ -136,7 +136,8 @@ module ApplicationSettingImplementation
         snowplow_iglu_registry_url: nil,
         custom_http_clone_url_root: nil,
         pendo_enabled: false,
-        pendo_url: nil
+        pendo_url: nil,
+        productivity_analytics_start_date: Time.now
       }
     end
 

changelogs/unreleased/32052-add-pa-start-date-limit.yml

+---
+title: Add productivity analytics merge date filtering limit
+merge_request: 32052
+author:
+type: fixed

db/migrate/20191004080818_add_productivity_analytics_start_date.rb

+# frozen_string_literal: true
+
+class AddProductivityAnalyticsStartDate < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :productivity_analytics_start_date, :datetime_with_timezone
+  end
+end

db/migrate/20191004081520_fill_productivity_analytics_start_date.rb

+# frozen_string_literal: true
+
+# Expected migration duration: 1 minute
+class FillProductivityAnalyticsStartDate < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :merge_request_metrics, :merged_at,
+                         where: "merged_at > '2019-09-01' AND commits_count IS NOT NULL",
+                         name: 'fill_productivity_analytics_start_date_tmp_index'
+
+    execute(
+      <<SQL
+        UPDATE application_settings
+        SET productivity_analytics_start_date = COALESCE((SELECT MIN(merged_at) FROM merge_request_metrics
+            WHERE merged_at > '2019-09-01' AND commits_count IS NOT NULL), NOW())
+SQL
+    )
+
+    remove_concurrent_index :merge_request_metrics, :merged_at,
+                            name: 'fill_productivity_analytics_start_date_tmp_index'
+  end
+
+  def down
+    execute('UPDATE application_settings SET productivity_analytics_start_date = NULL')
+  end
+end

db/schema.rb

@@ -350,6 +350,7 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do
     t.string "encrypted_eks_secret_access_key_iv", limit: 255
     t.text "encrypted_eks_secret_access_key"
     t.string "snowplow_app_id"
+    t.datetime_with_timezone "productivity_analytics_start_date"
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

ee/app/finders/productivity_analytics_finder.rb

@@ -37,10 +37,22 @@ class ProductivityAnalyticsFinder < MergeRequestsFinder
     return items unless params[:merged_at_after] || params[:merged_at_before]
 
     items = items.joins(:metrics)
-    items = items.where(metrics_table[:merged_at].gteq(params[:merged_at_after])) if params[:merged_at_after]
-    items = items.where(metrics_table[:merged_at].lteq(params[:merged_at_before])) if params[:merged_at_before]
+    items = items.where(metrics_table[:merged_at].gteq(merged_at_between[:from])) if merged_at_between[:from]
+    items = items.where(metrics_table[:merged_at].lteq(merged_at_between[:to])) if merged_at_between[:to]
 
     items
   end
   # rubocop: enable CodeReuse/ActiveRecord
+
+  def merged_at_between
+    @merged_at_between ||= begin
+      boundaries = { from: params[:merged_at_after], to: params[:merged_at_before] }
+
+      if ProductivityAnalytics.start_date && ProductivityAnalytics.start_date > boundaries[:from]
+        boundaries[:from] = ProductivityAnalytics.start_date
+      end
+
+      boundaries
+    end
+  end
 end

ee/app/models/productivity_analytics.rb

@@ -16,6 +16,10 @@ class ProductivityAnalytics
   METRIC_TYPES = METRIC_COLUMNS.keys.freeze
   DEFAULT_TYPE = 'days_to_merge'.freeze
 
+  def self.start_date
+    ApplicationSetting.current&.productivity_analytics_start_date
+  end
+
   def initialize(merge_requests:, sort: nil)
     @merge_requests = merge_requests.joins(:metrics)
     @sort = sort

ee/app/views/analytics/productivity_analytics/show.html.haml

@@ -5,5 +5,5 @@
     .js-group-project-select-container
     .js-search-bar.filter-container.hide
       = render 'shared/issuable/search_bar', type: :productivity_analytics
-    .js-timeframe-container
+    .js-timeframe-container{ data: { start_date: ProductivityAnalytics.start_date } }
   .js-productivity-analytics-app-container{ data: { endpoint: analytics_productivity_analytics_path, empty_state_svg_path: image_path('illustrations/productivity-analytics-empty-state.svg'), no_access_svg_path: image_path('illustrations/analytics/no-access.svg') } }

ee/spec/finders/productivity_analytics_finder_spec.rb

@@ -44,6 +44,12 @@ describe ProductivityAnalyticsFinder do
     end
 
     context 'allows to filter by merged_at' do
+      let(:pa_start_date) { 2.years.ago }
+
+      before do
+        allow(ProductivityAnalytics).to receive(:start_date).and_return(pa_start_date)
+      end
+
       around do |example|
         Timecop.freeze { example.run }
       end
@@ -76,6 +82,20 @@ describe ProductivityAnalyticsFinder do
           expect(subject.execute).to match_array([short_mr])
         end
       end
+
+      context 'with merged_at_after earlier than PA start date' do
+        let(:search_params) do
+          { merged_at_after: 3.years.ago.to_s }
+        end
+
+        it 'uses start_date as filter value' do
+          metrics_data = { merged_at: (2.years + 1.day).ago }
+          create(:merge_request, :merged, :with_productivity_metrics, created_at: 800.days.ago, metrics_data: metrics_data)
+          long_mr
+
+          expect(subject.execute).to match_array([long_mr])
+        end
+      end
     end
   end
 end

ee/spec/models/productivity_analytics_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 describe ProductivityAnalytics do
   describe 'metrics data' do
-    let(:analytics) { described_class.new(merge_requests: finder_mrs, sort: custom_sort) }
+    subject(:analytics) { described_class.new(merge_requests: finder_mrs, sort: custom_sort) }
 
     let(:finder_mrs) { ProductivityAnalyticsFinder.new(create(:admin), finder_options).execute }
     let(:finder_options) { { state: 'merged' } }
@@ -221,4 +221,17 @@ describe ProductivityAnalytics do
       end
     end
   end
+
+  describe '.start_date' do
+    subject(:start_date) { described_class.start_date }
+
+    let(:application_setting) do
+      instance_double('ApplicationSetting', productivity_analytics_start_date: 'mocked-start-date')
+    end
+
+    it 'delegates to ApplicationSetting' do
+      allow(ApplicationSetting).to receive('current').and_return(application_setting)
+      expect(start_date).to eq 'mocked-start-date'
+    end
+  end
 end

spec/migrations/fill_productivity_analytics_start_date_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191004081520_fill_productivity_analytics_start_date.rb')
+
+describe FillProductivityAnalyticsStartDate, :migration do
+  let(:settings_table) { table('application_settings') }
+  let(:metrics_table) { table('merge_request_metrics') }
+
+  before do
+    settings_table.create!
+  end
+
+  context 'with NO productivity analytics data available' do
+    it 'sets start_date to NOW' do
+      expect { migrate! }.to change {
+        settings_table.first&.productivity_analytics_start_date
+      }.to(be_like_time(Time.now))
+    end
+  end
+
+  context 'with productivity analytics data available' do
+    before do
+      ActiveRecord::Base.transaction do
+        ActiveRecord::Base.connection.execute('ALTER TABLE merge_request_metrics DISABLE TRIGGER ALL')
+        metrics_table.create!(merged_at: Time.parse('2019-09-09'), commits_count: nil, merge_request_id: 3)
+        metrics_table.create!(merged_at: Time.parse('2019-10-10'), commits_count: 5, merge_request_id: 1)
+        metrics_table.create!(merged_at: Time.parse('2019-11-11'), commits_count: 10, merge_request_id: 2)
+        ActiveRecord::Base.connection.execute('ALTER TABLE merge_request_metrics ENABLE TRIGGER ALL')
+      end
+    end
+
+    it 'set start_date to earliest merged_at value with PA data available' do
+      expect { migrate! }.to change {
+        settings_table.first&.productivity_analytics_start_date
+      }.to(be_like_time(Time.parse('2019-10-10')))
+    end
+  end
+end