Merge branch '13216-handle-date-range-params-in-ca' into 'master'

Handle date range params in CA

See merge request gitlab-org/gitlab!18025

app/controllers/concerns/cycle_analytics_params.rb

@@ -3,8 +3,20 @@
 module CycleAnalyticsParams
   extend ActiveSupport::Concern
 
+  def cycle_analytics_project_params
+    return {} unless params[:cycle_analytics].present?
+
+    params[:cycle_analytics].permit(:start_date, :created_after, :created_before, :branch_name)
+  end
+
+  def cycle_analytics_group_params
+    return {} unless params[:cycle_analytics].present?
+
+    params[:cycle_analytics].permit(:start_date, :created_after, :created_before, project_ids: [])
+  end
+
   def options(params)
-    @options ||= { from: start_date(params), current_user: current_user }
+    @options ||= { from: start_date(params), current_user: current_user }.merge(date_range(params))
   end
 
   def start_date(params)
@@ -17,6 +29,17 @@ module CycleAnalyticsParams
       90.days.ago
     end
   end
+
+  def date_range(params)
+    {}.tap do |date_range_params|
+      date_range_params[:from] = to_utc_time(params[:created_after]).beginning_of_day if params[:created_after]
+      date_range_params[:to] = to_utc_time(params[:created_before]).end_of_day if params[:created_before]
+    end.compact
+  end
+
+  def to_utc_time(field)
+    Date.parse(field).to_time.utc
+  end
 end
 
 CycleAnalyticsParams.prepend_if_ee('EE::CycleAnalyticsParams')

app/controllers/projects/cycle_analytics/events_controller.rb

@@ -23,7 +23,7 @@ module Projects
       end
 
       def test
-        options(cycle_analytics_params)[:branch] = cycle_analytics_params[:branch_name]
+        options(cycle_analytics_project_params)[:branch] = cycle_analytics_project_params[:branch_name]
 
         render_events(cycle_analytics[:test].events)
       end
@@ -50,13 +50,7 @@ module Projects
       end
 
       def cycle_analytics
-        @cycle_analytics ||= ::CycleAnalytics::ProjectLevel.new(project, options: options(cycle_analytics_params))
-      end
-
-      def cycle_analytics_params
-        return {} unless params[:cycle_analytics].present?
-
-        params[:cycle_analytics].permit(:start_date, :branch_name)
+        @cycle_analytics ||= ::CycleAnalytics::ProjectLevel.new(project, options: options(cycle_analytics_project_params))
       end
     end
   end

app/controllers/projects/cycle_analytics_controller.rb

@@ -9,7 +9,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
   before_action :authorize_read_cycle_analytics!
 
   def show
-    @cycle_analytics = ::CycleAnalytics::ProjectLevel.new(@project, options: options(cycle_analytics_params))
+    @cycle_analytics = ::CycleAnalytics::ProjectLevel.new(@project, options: options(cycle_analytics_project_params))
 
     @cycle_analytics_no_data = @cycle_analytics.no_stats?
 
@@ -27,12 +27,6 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
 
   private
 
-  def cycle_analytics_params
-    return {} unless params[:cycle_analytics].present?
-
-    params[:cycle_analytics].permit(:start_date)
-  end
-
   def cycle_analytics_json
     {
       summary: @cycle_analytics.summary,

app/models/cycle_analytics/project_level.rb

@@ -13,6 +13,7 @@ module CycleAnalytics
     def summary
       @summary ||= ::Gitlab::CycleAnalytics::StageSummary.new(project,
                                                               from: options[:from],
+                                                              to: options[:to],
                                                               current_user: options[:current_user]).data
     end
 

ee/app/controllers/groups/cycle_analytics/events_controller.rb

@@ -45,13 +45,7 @@ class Groups::CycleAnalytics::EventsController < Groups::ApplicationController
   end
 
   def cycle_analytics
-    @cycle_analytics ||= ::CycleAnalytics::GroupLevel.new(group: group, options: options(cycle_analytics_params))
-  end
-
-  def cycle_analytics_params
-    return {} unless params[:cycle_analytics].present?
-
-    params[:cycle_analytics].permit(:start_date, :branch_name, project_ids: [])
+    @cycle_analytics ||= ::CycleAnalytics::GroupLevel.new(group: group, options: options(cycle_analytics_group_params))
   end
 
   def authorize_group_cycle_analytics!

ee/app/controllers/groups/cycle_analytics_controller.rb

@@ -16,12 +16,6 @@ class Groups::CycleAnalyticsController < Groups::ApplicationController
 
   private
 
-  def cycle_analytics_params
-    return {} unless params[:cycle_analytics].present?
-
-    params[:cycle_analytics].permit(:start_date, project_ids: [])
-  end
-
   def cycle_analytics_json
     {
       summary: cycle_analytics_stats.summary,
@@ -31,7 +25,7 @@ class Groups::CycleAnalyticsController < Groups::ApplicationController
   end
 
   def cycle_analytics_stats
-    @cycle_analytics_stats ||= ::CycleAnalytics::GroupLevel.new(group: group, options: options(cycle_analytics_params))
+    @cycle_analytics_stats ||= ::CycleAnalytics::GroupLevel.new(group: group, options: options(cycle_analytics_group_params))
   end
 
   def whitelist_query_limiting

ee/spec/requests/groups/cycle_analytics_events_spec.rb

@@ -24,6 +24,40 @@ describe 'cycle analytics events' do
       login_as(user)
     end
 
+    context 'when date range parameters are given' do
+      it 'filter by `created_after`' do
+        params = { cycle_analytics: { created_after: issue.created_at - 5.days } }
+
+        get group_cycle_analytics_issue_path(group, params: params, format: :json)
+
+        expect(json_response['events']).not_to be_empty
+      end
+
+      it 'filters by `created_after` where no events should be found' do
+        params = { cycle_analytics: { created_after: issue.created_at + 5.days } }
+
+        get group_cycle_analytics_issue_path(group, params: params, format: :json)
+
+        expect(json_response['events']).to be_empty
+      end
+
+      it 'filter by `created_after` and `created_before`' do
+        params = { cycle_analytics: { created_after: issue.created_at - 5.days, created_before: issue.created_at + 5.days } }
+
+        get group_cycle_analytics_issue_path(group, params: params, format: :json)
+
+        expect(json_response['events']).not_to be_empty
+      end
+
+      it 'raises error when date cannot be parsed' do
+        params = { cycle_analytics: { created_after: 'invalid' } }
+
+        expect do
+          get group_cycle_analytics_issue_path(group, params: params, format: :json)
+        end.to raise_error(ArgumentError)
+      end
+    end
+
     it 'lists the issue events' do
       get group_cycle_analytics_issue_path(group, format: :json)
 

lib/gitlab/cycle_analytics/base_query.rb

@@ -23,6 +23,7 @@ module Gitlab
           .project(routes_table[:path].as("namespace_path"))
 
         query = limit_query(query, project_ids)
+        query = limit_query_by_date_range(query)
 
         # Load merge_requests
 
@@ -34,7 +35,12 @@ module Gitlab
       def limit_query(query, project_ids)
         query.where(issue_table[:project_id].in(project_ids))
           .where(routes_table[:source_type].eq('Namespace'))
-          .where(issue_table[:created_at].gteq(options[:from]))
+      end
+
+      def limit_query_by_date_range(query)
+        query = query.where(issue_table[:created_at].gteq(options[:from]))
+        query = query.where(issue_table[:created_at].lteq(options[:to])) if options[:to]
+        query
       end
 
       def load_merge_requests(query)

lib/gitlab/cycle_analytics/issue_helper.rb

@@ -12,14 +12,12 @@ module Gitlab
           .project(routes_table[:path].as("namespace_path"))
 
         query = limit_query(query, project_ids)
-
-        query
+        limit_query_by_date_range(query)
       end
 
       def limit_query(query, project_ids)
         query.where(issue_table[:project_id].in(project_ids))
           .where(routes_table[:source_type].eq('Namespace'))
-          .where(issue_table[:created_at].gteq(options[:from]))
           .where(issue_metrics_table[:first_added_to_board_at].not_eq(nil).or(issue_metrics_table[:first_associated_with_milestone_at].not_eq(nil)))
       end
     end

lib/gitlab/cycle_analytics/plan_helper.rb

@@ -14,12 +14,11 @@ module Gitlab
           .where(routes_table[:source_type].eq('Namespace'))
         query = limit_query(query)
 
-        query
+        limit_query_by_date_range(query)
       end
 
       def limit_query(query)
-        query.where(issue_table[:created_at].gteq(options[:from]))
-          .where(issue_metrics_table[:first_added_to_board_at].not_eq(nil).or(issue_metrics_table[:first_associated_with_milestone_at].not_eq(nil)))
+        query.where(issue_metrics_table[:first_added_to_board_at].not_eq(nil).or(issue_metrics_table[:first_associated_with_milestone_at].not_eq(nil)))
           .where(issue_metrics_table[:first_mentioned_in_commit_at].not_eq(nil))
       end
     end

lib/gitlab/cycle_analytics/stage_summary.rb

@@ -3,16 +3,17 @@
 module Gitlab
   module CycleAnalytics
     class StageSummary
-      def initialize(project, from:, current_user:)
+      def initialize(project, from:, to: nil, current_user:)
         @project = project
         @from = from
+        @to = to
         @current_user = current_user
       end
 
       def data
-        [serialize(Summary::Issue.new(project: @project, from: @from, current_user: @current_user)),
-         serialize(Summary::Commit.new(project: @project, from: @from)),
-         serialize(Summary::Deploy.new(project: @project, from: @from))]
+        [serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user)),
+         serialize(Summary::Commit.new(project: @project, from: @from, to: @to)),
+         serialize(Summary::Deploy.new(project: @project, from: @from, to: @to))]
       end
 
       private

lib/gitlab/cycle_analytics/summary/base.rb

@@ -4,9 +4,10 @@ module Gitlab
   module CycleAnalytics
     module Summary
       class Base
-        def initialize(project:, from:)
+        def initialize(project:, from:, to: nil)
           @project = project
           @from = from
+          @to = to
         end
 
         def title

lib/gitlab/cycle_analytics/summary/commit.rb

@@ -21,7 +21,7 @@ module Gitlab
         def count_commits
           return unless ref
 
-          gitaly_commit_client.commit_count(ref, after: @from)
+          gitaly_commit_client.commit_count(ref, after: @from, before: @to)
         end
 
         def gitaly_commit_client

lib/gitlab/cycle_analytics/summary/deploy.rb

@@ -4,12 +4,18 @@ module Gitlab
   module CycleAnalytics
     module Summary
       class Deploy < Base
+        include Gitlab::Utils::StrongMemoize
+
         def title
           n_('Deploy', 'Deploys', value)
         end
 
         def value
-          @value ||= @project.deployments.where("created_at > ?", @from).count
+          strong_memoize(:value) do
+            query = @project.deployments.where("created_at >= ?", @from)
+            query = query.where("created_at <= ?", @to) if @to
+            query.count
+          end
         end
       end
     end

lib/gitlab/cycle_analytics/summary/issue.rb

@@ -4,9 +4,10 @@ module Gitlab
   module CycleAnalytics
     module Summary
       class Issue < Base
-        def initialize(project:, from:, current_user:)
+        def initialize(project:, from:, to: nil, current_user:)
           @project = project
           @from = from
+          @to = to
           @current_user = current_user
         end
 
@@ -15,7 +16,7 @@ module Gitlab
         end
 
         def value
-          @value ||= IssuesFinder.new(@current_user, project_id: @project.id).execute.created_after(@from).count
+          @value ||= IssuesFinder.new(@current_user, project_id: @project.id, created_after: @from, created_before: @to).execute.count
         end
       end
     end

spec/lib/gitlab/cycle_analytics/code_stage_spec.rb

@@ -12,7 +12,8 @@ describe Gitlab::CycleAnalytics::CodeStage do
   let(:issue_3) { create(:issue, project: project, created_at: 60.minutes.ago) }
   let(:mr_1) { create(:merge_request, source_project: project, created_at: 15.minutes.ago) }
   let(:mr_2) { create(:merge_request, source_project: project, created_at: 10.minutes.ago, source_branch: 'A') }
-  let(:stage) { described_class.new(options: { from: 2.days.ago, current_user: project.creator, project: project }) }
+  let(:stage_options) { { from: 2.days.ago, current_user: project.creator, project: project } }
+  let(:stage) { described_class.new(options: stage_options) }
 
   before do
     issue_1.metrics.update!(first_associated_with_milestone_at: 60.minutes.ago, first_mentioned_in_commit_at: 45.minutes.ago)
@@ -33,6 +34,8 @@ describe Gitlab::CycleAnalytics::CodeStage do
     it 'counts median from issues with metrics' do
       expect(stage.project_median).to eq(ISSUES_MEDIAN)
     end
+
+    include_examples 'calculate #median with date range'
   end
 
   describe '#events' do

spec/lib/gitlab/cycle_analytics/issue_stage_spec.rb

@@ -10,7 +10,8 @@ describe Gitlab::CycleAnalytics::IssueStage do
   let(:issue_2) { create(:issue, project: project, created_at: 60.minutes.ago) }
   let(:issue_3) { create(:issue, project: project, created_at: 30.minutes.ago) }
   let!(:issue_without_milestone) { create(:issue, project: project, created_at: 1.minute.ago) }
-  let(:stage) { described_class.new(options: { from: 2.days.ago, current_user: project.creator, project: project }) }
+  let(:stage_options) { { from: 2.days.ago, current_user: project.creator, project: project } }
+  let(:stage) { described_class.new(options: stage_options) }
 
   before do
     issue_1.metrics.update!(first_associated_with_milestone_at: 60.minutes.ago )
@@ -28,6 +29,8 @@ describe Gitlab::CycleAnalytics::IssueStage do
     it 'counts median from issues with metrics' do
       expect(stage.project_median).to eq(ISSUES_MEDIAN)
     end
+
+    include_examples 'calculate #median with date range'
   end
 
   describe '#events' do

spec/lib/gitlab/cycle_analytics/plan_stage_spec.rb

@@ -10,7 +10,8 @@ describe Gitlab::CycleAnalytics::PlanStage do
   let!(:issue_2) { create(:issue, project: project, created_at: 60.minutes.ago) }
   let!(:issue_3) { create(:issue, project: project, created_at: 30.minutes.ago) }
   let!(:issue_without_milestone) { create(:issue, project: project, created_at: 1.minute.ago) }
-  let(:stage) { described_class.new(options: { from: 2.days.ago, current_user: project.creator, project: project }) }
+  let(:stage_options) { { from: 2.days.ago, current_user: project.creator, project: project } }
+  let(:stage) { described_class.new(options: stage_options) }
 
   before do
     issue_1.metrics.update!(first_associated_with_milestone_at: 60.minutes.ago, first_mentioned_in_commit_at: 10.minutes.ago)
@@ -28,6 +29,8 @@ describe Gitlab::CycleAnalytics::PlanStage do
     it 'counts median from issues with metrics' do
       expect(stage.project_median).to eq(ISSUES_MEDIAN)
     end
+
+    include_examples 'calculate #median with date range'
   end
 
   describe '#events' do

spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb

@@ -32,3 +32,23 @@ shared_examples 'base stage' do
     expect(stage.events).not_to be_nil
   end
 end
+
+shared_examples 'calculate #median with date range' do
+  context 'when valid date range is given' do
+    before do
+      stage_options[:from] = 5.days.ago
+      stage_options[:to] = 5.days.from_now
+    end
+
+    it { expect(stage.project_median).to eq(ISSUES_MEDIAN) }
+  end
+
+  context 'when records are out of the date range' do
+    before do
+      stage_options[:from] = 2.years.ago
+      stage_options[:to] = 1.year.ago
+    end
+
+    it { expect(stage.project_median).to eq(nil) }
+  end
+end

spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb

@@ -4,52 +4,98 @@ require 'spec_helper'
 
 describe Gitlab::CycleAnalytics::StageSummary do
   let(:project) { create(:project, :repository) }
-  let(:from) { 1.day.ago }
+  let(:options) { { from: 1.day.ago, current_user: user } }
   let(:user) { create(:user, :admin) }
-  subject { described_class.new(project, from: Time.now, current_user: user).data }
+  let(:stage_summary) { described_class.new(project, options).data }
 
   describe "#new_issues" do
+    subject { stage_summary.first[:value] }
+
     it "finds the number of issues created after the 'from date'" do
       Timecop.freeze(5.days.ago) { create(:issue, project: project) }
       Timecop.freeze(5.days.from_now) { create(:issue, project: project) }
 
-      expect(subject.first[:value]).to eq(1)
+      expect(subject).to eq(1)
     end
 
     it "doesn't find issues from other projects" do
       Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project)) }
 
-      expect(subject.first[:value]).to eq(0)
+      expect(subject).to eq(0)
+    end
+
+    context 'when `to` parameter is given' do
+      before do
+        Timecop.freeze(5.days.ago) { create(:issue, project: project) }
+        Timecop.freeze(5.days.from_now) { create(:issue, project: project) }
+      end
+
+      it "doesn't find any record" do
+        options[:to] = Time.now
+
+        expect(subject).to eq(0)
+      end
+
+      it "finds records created between `from` and `to` range" do
+        options[:from] = 10.days.ago
+        options[:to] = 10.days.from_now
+
+        expect(subject).to eq(2)
+      end
     end
   end
 
   describe "#commits" do
+    subject { stage_summary.second[:value] }
+
     it "finds the number of commits created after the 'from date'" do
       Timecop.freeze(5.days.ago) { create_commit("Test message", project, user, 'master') }
       Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master') }
 
-      expect(subject.second[:value]).to eq(1)
+      expect(subject).to eq(1)
     end
 
     it "doesn't find commits from other projects" do
       Timecop.freeze(5.days.from_now) { create_commit("Test message", create(:project, :repository), user, 'master') }
 
-      expect(subject.second[:value]).to eq(0)
+      expect(subject).to eq(0)
     end
 
     it "finds a large (> 100) snumber of commits if present" do
       Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master', count: 100) }
 
-      expect(subject.second[:value]).to eq(100)
+      expect(subject).to eq(100)
+    end
+
+    context 'when `to` parameter is given' do
+      before do
+        Timecop.freeze(5.days.ago) { create_commit("Test message", project, user, 'master') }
+        Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master') }
+      end
+
+      it "doesn't find any record" do
+        options[:to] = Time.now
+
+        expect(subject).to eq(0)
+      end
+
+      it "finds records created between `from` and `to` range" do
+        options[:from] = 10.days.ago
+        options[:to] = 10.days.from_now
+
+        expect(subject).to eq(2)
+      end
     end
   end
 
   describe "#deploys" do
+    subject { stage_summary.third[:value] }
+
     it "finds the number of deploys made created after the 'from date'" do
       Timecop.freeze(5.days.ago) { create(:deployment, :success, project: project) }
       Timecop.freeze(5.days.from_now) { create(:deployment, :success, project: project) }
 
-      expect(subject.third[:value]).to eq(1)
+      expect(subject).to eq(1)
     end
 
     it "doesn't find commits from other projects" do
@@ -57,7 +103,27 @@ describe Gitlab::CycleAnalytics::StageSummary do
         create(:deployment, :success, project: create(:project, :repository))
       end
 
-      expect(subject.third[:value]).to eq(0)
+      expect(subject).to eq(0)
+    end
+
+    context 'when `to` parameter is given' do
+      before do
+        Timecop.freeze(5.days.ago) { create(:deployment, :success, project: project) }
+        Timecop.freeze(5.days.from_now) { create(:deployment, :success, project: project) }
+      end
+
+      it "doesn't find any record" do
+        options[:to] = Time.now
+
+        expect(subject).to eq(0)
+      end
+
+      it "finds records created between `from` and `to` range" do
+        options[:from] = 10.days.ago
+        options[:to] = 10.days.from_now
+
+        expect(subject).to eq(2)
+      end
     end
   end
 end

spec/lib/gitlab/cycle_analytics/staging_stage_spec.rb

@@ -16,7 +16,8 @@ describe Gitlab::CycleAnalytics::StagingStage do
   let(:build_1) { create(:ci_build, project: project) }
   let(:build_2) { create(:ci_build, project: project) }
 
-  let(:stage) { described_class.new(options: { from: 2.days.ago, current_user: project.creator, project: project }) }
+  let(:stage_options) { { from: 2.days.ago, current_user: project.creator, project: project } }
+  let(:stage) { described_class.new(options: stage_options) }
 
   before do
     mr_1.metrics.update!(merged_at: 80.minutes.ago, first_deployed_to_production_at: 50.minutes.ago, pipeline_id: build_1.commit_id)
@@ -38,6 +39,8 @@ describe Gitlab::CycleAnalytics::StagingStage do
     it 'counts median from issues with metrics' do
       expect(stage.project_median).to eq(ISSUES_MEDIAN)
     end
+
+    it_behaves_like 'calculate #median with date range'
   end
 
   describe '#events' do

spec/lib/gitlab/cycle_analytics/test_stage_spec.rb

@@ -6,7 +6,8 @@ require 'lib/gitlab/cycle_analytics/shared_stage_spec'
 describe Gitlab::CycleAnalytics::TestStage do
   let(:stage_name) { :test }
   let(:project) { create(:project) }
-  let(:stage) { described_class.new(options: { from: 2.days.ago, current_user: project.creator, project: project }) }
+  let(:stage_options) { { from: 2.days.ago, current_user: project.creator, project: project } }
+  let(:stage) { described_class.new(options: stage_options) }
 
   it_behaves_like 'base stage'
 
@@ -40,5 +41,7 @@ describe Gitlab::CycleAnalytics::TestStage do
     it 'counts median from issues with metrics' do
       expect(stage.project_median).to eq(ISSUES_MEDIAN)
     end
+
+    include_examples 'calculate #median with date range'
   end
 end