Merge branch '217973-accept-filter-params-in-vsa-v2' into 'master'

Add finder params in Value Stream Analytics

See merge request gitlab-org/gitlab!33833

ee/app/controllers/analytics/cycle_analytics/stages_controller.rb

@@ -3,6 +3,9 @@
 module Analytics
   module CycleAnalytics
     class StagesController < Analytics::ApplicationController
+      include CycleAnalyticsParams
+      extend ::Gitlab::Utils::Override
+
       check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
 
       before_action :load_group
@@ -58,26 +61,11 @@ module Analytics
 
       private
 
-      def validate_params
-        if request_params.invalid?
-          render(
-            json: { message: 'Invalid parameters', errors: request_params.errors },
-            status: :unprocessable_entity
-          )
-        end
-      end
-
-      def request_params
-        @request_params ||= Gitlab::Analytics::CycleAnalytics::RequestParams.new(data_collector_params, current_user: current_user)
-      end
-
       def data_collector
-        @data_collector ||= Gitlab::Analytics::CycleAnalytics::DataCollector.new(stage: stage, params: {
-          current_user: current_user,
-          from: request_params.created_after,
-          to: request_params.created_before,
-          project_ids: request_params.project_ids
-        })
+        @data_collector ||= Gitlab::Analytics::CycleAnalytics::DataCollector.new(
+          stage: stage,
+          params: request_params.to_data_collector_params
+        )
       end
 
       def stage
@@ -115,8 +103,9 @@ module Analytics
         end
       end
 
-      def data_collector_params
-        params.permit(:created_before, :created_after, project_ids: [])
+      override :all_cycle_analytics_params
+      def all_cycle_analytics_params
+        super.merge({ group: @group })
       end
 
       def update_params

ee/app/controllers/analytics/cycle_analytics/summary_controller.rb

@@ -22,30 +22,7 @@ module Analytics
       private
 
       def group_level
-        @group_level ||= GroupLevel.new(group: @group, options: options(group_params))
-      end
-
-      def group_params
-        hash = { created_after: request_params.created_after, created_before: request_params.created_before }
-        hash[:project_ids] = request_params.project_ids if request_params.project_ids.any?
-        hash
-      end
-
-      def validate_params
-        if request_params.invalid?
-          render(
-            json: { message: 'Invalid parameters', errors: request_params.errors },
-            status: :unprocessable_entity
-          )
-        end
-      end
-
-      def request_params
-        @request_params ||= Gitlab::Analytics::CycleAnalytics::RequestParams.new(allowed_params, current_user: current_user)
-      end
-
-      def allowed_params
-        params.permit(:created_after, :created_before, project_ids: [])
+        @group_level ||= GroupLevel.new(group: @group, options: options(request_params.to_data_collector_params))
       end
 
       def authorize_access

ee/app/controllers/analytics/cycle_analytics_controller.rb

@@ -2,13 +2,14 @@
 
 class Analytics::CycleAnalyticsController < Analytics::ApplicationController
   include CycleAnalyticsParams
+  extend ::Gitlab::Utils::Override
 
   check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
   increment_usage_counter Gitlab::UsageDataCounters::CycleAnalyticsCounter, :views, only: :show
 
   before_action :load_group, only: :show
   before_action :load_project, only: :show
-  before_action :build_request_params, only: :show
+  before_action :request_params, only: :show
 
   before_action do
     push_frontend_feature_flag(:cycle_analytics_scatterplot_enabled, default_enabled: true)
@@ -16,15 +17,10 @@ class Analytics::CycleAnalyticsController < Analytics::ApplicationController
     push_frontend_feature_flag(:value_stream_analytics_path_navigation, @group)
   end
 
-  def build_request_params
-    @request_params ||= Gitlab::Analytics::CycleAnalytics::RequestParams.new(allowed_params.merge(group: @group), current_user: current_user)
-  end
+  private
 
-  def allowed_params
-    params.permit(
-      :created_after,
-      :created_before,
-      project_ids: []
-    )
+  override :all_cycle_analytics_params
+  def all_cycle_analytics_params
+    super.merge({ group: @group })
   end
 end

ee/app/controllers/concerns/ee/cycle_analytics_params.rb

@@ -12,8 +12,32 @@ module EE
           options[:branch] = params[:branch_name]
           options[:projects] = params[:project_ids] if params[:project_ids]
           options[:group] = params[:group_id] if params[:group_id]
+          options.merge!(params.slice(*::Gitlab::Analytics::CycleAnalytics::RequestParams::FINDER_PARAM_NAMES))
         end
       end
     end
+
+    private
+
+    def permitted_cycle_analytics_params
+      params.permit(*::Gitlab::Analytics::CycleAnalytics::RequestParams::STRONG_PARAMS_DEFINITION)
+    end
+
+    def all_cycle_analytics_params
+      permitted_cycle_analytics_params.merge(current_user: current_user)
+    end
+
+    def request_params
+      @request_params ||= ::Gitlab::Analytics::CycleAnalytics::RequestParams.new(all_cycle_analytics_params)
+    end
+
+    def validate_params
+      if request_params.invalid?
+        render(
+          json: { message: 'Invalid parameters', errors: request_params.errors },
+          status: :unprocessable_entity
+        )
+      end
+    end
   end
 end

ee/lib/ee/gitlab/analytics/cycle_analytics/base_query_builder.rb

@@ -13,6 +13,8 @@ module EE::Gitlab::Analytics::CycleAnalytics::BaseQueryBuilder
   override :build_finder_params
   def build_finder_params(params)
     super.tap do |finder_params|
+      finder_params.merge!(params.slice(*::Gitlab::Analytics::CycleAnalytics::RequestParams::FINDER_PARAM_NAMES))
+
       finder_params[:project_ids] = Array(params[:project_ids])
     end
   end

ee/lib/gitlab/analytics/cycle_analytics/request_params.rb

@@ -11,14 +11,33 @@ module Gitlab
         MAX_RANGE_DAYS = 180.days.freeze
         DEFAULT_DATE_RANGE = 29.days # 30 including Date.today
 
+        STRONG_PARAMS_DEFINITION = [
+          :created_before,
+          :created_after,
+          :author_username,
+          :milestone_title,
+          label_name: [].freeze,
+          assignee_username: [].freeze,
+          project_ids: [].freeze
+        ].freeze
+
+        FINDER_PARAM_NAMES = [
+          :assignee_username,
+          :author_username,
+          :milestone_title,
+          :label_name
+        ].freeze
+
         attr_writer :project_ids
 
         attribute :created_after, :datetime
         attribute :created_before, :datetime
+        attribute :group
+        attribute :current_user
 
-        attr_accessor :group
-
-        attr_reader :current_user
+        FINDER_PARAM_NAMES.each do |param_name|
+          attribute param_name
+        end
 
         validates :created_after, presence: true
         validates :created_before, presence: true
@@ -26,25 +45,36 @@ module Gitlab
         validate :validate_created_before
         validate :validate_date_range
 
-        def initialize(params = {}, current_user:)
+        def initialize(params = {})
           super(params)
 
           self.created_before = (self.created_before || Time.now).at_end_of_day
           self.created_after  = (created_after || default_created_after).at_beginning_of_day
-
-          @current_user = current_user
         end
 
         def project_ids
           Array(@project_ids)
         end
 
+        def to_data_collector_params
+          {
+            current_user: current_user,
+            from: created_after,
+            to: created_before,
+            project_ids: project_ids
+          }.merge(attributes.symbolize_keys.slice(*FINDER_PARAM_NAMES))
+        end
+
         def to_data_attributes
           {}.tap do |attrs|
             attrs[:group] = group_data_attributes if group
             attrs[:created_after] = created_after.to_date.iso8601
             attrs[:created_before] = created_before.to_date.iso8601
-            attrs[:projects] = group_projects(project_ids) if group && project_ids.any?
+            attrs[:projects] = group_projects(project_ids) if group && project_ids.present?
+            attrs[:labels] = label_name.to_json if label_name.present?
+            attrs[:assignees] = assignee_username.to_json if assignee_username.present?
+            attrs[:author] = author_username if author_username.present?
+            attrs[:milestone] = milestone_title if milestone_title.present?
           end
         end
 
@@ -67,10 +97,10 @@ module Gitlab
             options: { include_subgroups: true },
             project_ids_relation: project_ids
           )
-          .execute
-          .with_route
-          .map { |project| project_data_attributes(project) }
-          .to_json
+            .execute
+            .with_route
+            .map { |project| project_data_attributes(project) }
+            .to_json
         end
 
         def project_data_attributes(project)

ee/lib/gitlab/analytics/cycle_analytics/summary/group/deploy.rb

@@ -22,7 +22,7 @@ module Gitlab
             def deployments_count
               @deployments_count ||= begin
                                        deployments = Deployment.joins(:project).merge(Project.inside_path(group.full_path))
-                                       deployments = deployments.where(projects: { id: options[:projects] }) if options[:projects]
+                                       deployments = deployments.where(projects: { id: options[:projects] }) if options[:projects].present?
                                        deployments = deployments.where("deployments.created_at > ?", options[:from])
                                        deployments = deployments.where("deployments.created_at < ?", options[:to]) if options[:to]
                                        deployments.success.count

ee/lib/gitlab/analytics/cycle_analytics/summary/group/issue.rb

@@ -27,18 +27,19 @@ module Gitlab
             # rubocop: disable CodeReuse/ActiveRecord
             def issues_count
               issues = IssuesFinder.new(current_user, finder_params).execute
-              issues = issues.where(projects: { id: options[:projects] }) if options[:projects]
+              issues = issues.where(projects: { id: options[:projects] }) if options[:projects].present?
               issues.count
             end
             # rubocop: enable CodeReuse/ActiveRecord
 
             def finder_params
-              {
-                group_id: group.id,
-                include_subgroups: true,
-                created_after: options[:from],
-                created_before: options[:to]
-              }.compact
+              options.dup.tap do |hash|
+                hash.delete(:projects)
+                hash[:created_after] = hash.delete(:from)
+                hash[:created_before] = hash.delete(:to)
+                hash[:group_id] = group.id
+                hash[:include_subgroups] = true
+              end
             end
           end
         end

ee/spec/controllers/analytics/cycle_analytics/summary_controller_spec.rb

@@ -23,24 +23,6 @@ RSpec.describe Analytics::CycleAnalytics::SummaryController do
       expect(response).to match_response_schema('analytics/cycle_analytics/summary', dir: 'ee')
     end
 
-    it 'omits `projects` parameter if it is not given' do
-      expect(Analytics::CycleAnalytics::GroupLevel).to receive(:new).with(group: group, options: hash_excluding(:projects)).and_call_original
-
-      subject
-
-      expect(response).to be_successful
-    end
-
-    it 'contains `projects` parameter' do
-      params[:project_ids] = [-1]
-
-      expect(Analytics::CycleAnalytics::GroupLevel).to receive(:new).with(group: group, options: hash_including(projects: ['-1'])).and_call_original
-
-      subject
-
-      expect(response).to be_successful
-    end
-
     include_examples 'cycle analytics data endpoint examples'
     include_examples 'group permission check on the controller level'
   end

ee/spec/lib/gitlab/analytics/cycle_analytics/data_collector_spec.rb

@@ -393,10 +393,12 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::DataCollector do
       end
     end
 
-    context 'when `project_ids` parameter is given' do
-      let(:group) { create(:group) }
-      let(:project1) { create(:project, :repository, group: group) }
-      let(:project2) { create(:project, :repository, group: group) }
+    context 'when filter parameters are given' do
+      let_it_be(:group) { create(:group) }
+      let_it_be(:project1) { create(:project, :repository, group: group) }
+      let_it_be(:project2) { create(:project, :repository, group: group) }
+
+      let(:merge_request) { project2.merge_requests.first }
 
       let(:stage) do
         Analytics::CycleAnalytics::GroupStage.new(
@@ -407,12 +409,17 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::DataCollector do
         )
       end
 
-      let(:data_collector) do
-        described_class.new(stage: stage, params: {
-          from: Time.new(2019, 1, 1),
-          project_ids: [project2.id],
+      let(:data_collector_params) do
+        {
+          created_after: Time.new(2019, 1, 1),
           current_user: user
-        })
+        }
+      end
+
+      subject do
+        params = Gitlab::Analytics::CycleAnalytics::RequestParams.new(data_collector_params).to_data_collector_params
+
+        described_class.new(stage: stage, params: params).records_fetcher.serialized_records
       end
 
       before do
@@ -427,13 +434,73 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::DataCollector do
         end
       end
 
-      it 'filters for the given `project_ids`' do
-        items = data_collector.records_fetcher.serialized_records
-        expect(items.size).to eq(1)
+      shared_examples 'filter examples' do
+        it 'provides filtered results' do
+          expect(subject.size).to eq(1)
+
+          expect(subject.first[:title]).to eq(merge_request.title)
+          expect(subject.first[:iid]).to eq(merge_request.iid.to_s)
+        end
+      end
+
+      context 'when `project_ids` parameter is given' do
+        before do
+          data_collector_params[:project_ids] = [project2.id]
+        end
+
+        it_behaves_like 'filter examples'
+      end
+
+      context 'when `assignee_username` is given' do
+        let(:assignee) { create(:user) }
+
+        before do
+          merge_request.assignees << assignee
+
+          data_collector_params[:assignee_username] = [assignee.username]
+        end
+
+        it_behaves_like 'filter examples'
+      end
+
+      context 'when `author_username` is given' do
+        let(:author) { create(:user) }
+
+        before do
+          merge_request.update!(author: author)
+
+          data_collector_params[:author_username] = author.username
+        end
+
+        it_behaves_like 'filter examples'
+      end
+
+      context 'when `label_name` is given' do
+        let(:label) { create(:group_label, group: group) }
+
+        before do
+          MergeRequests::UpdateService.new(
+            merge_request.project,
+            user,
+            label_ids: [label.id]
+          ).execute(merge_request)
+
+          data_collector_params[:label_name] = [label.name]
+        end
+
+        it_behaves_like 'filter examples'
+      end
+
+      context 'when `milestone_title` is given' do
+        let(:milestone) { create(:milestone, group: group) }
+
+        before do
+          merge_request.update!(milestone: milestone)
+
+          data_collector_params[:milestone_title] = milestone.title
+        end
 
-        merge_request = project2.merge_requests.first
-        expect(items.first[:title]).to eq(merge_request.title)
-        expect(items.first[:iid]).to eq(merge_request.iid.to_s)
+        it_behaves_like 'filter examples'
       end
     end
   end

ee/spec/lib/gitlab/analytics/cycle_analytics/request_params_spec.rb

@@ -16,13 +16,16 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RequestParams do
 
   let(:project_ids) { root_group_projects.collect(&:id) }
   let(:params) do
-    { created_after: '2019-01-01',
+    {
+      created_after: '2019-01-01',
       created_before: '2019-03-01',
       project_ids: [2, 3],
-      group: root_group }
+      group: root_group,
+      current_user: user
+    }
   end
 
-  subject { described_class.new(params, current_user: user) }
+  subject { described_class.new(params) }
 
   before do
     root_group.add_owner(user)
@@ -164,4 +167,22 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RequestParams do
       it { expect(subject.group).to eq(sub_group.id) }
     end
   end
+
+  describe 'issuable filter params' do
+    before do
+      params.merge!(
+        milestone_title: 'title',
+        assignee_username: ['username1'],
+        label_name: %w[label1 label2],
+        author_username: 'author'
+      )
+    end
+
+    subject { described_class.new(params).to_data_attributes }
+
+    it { expect(subject[:milestone]).to eq('title') }
+    it { expect(subject[:assignees]).to eq('["username1"]') }
+    it { expect(subject[:labels]).to eq('["label1","label2"]') }
+    it { expect(subject[:author]).to eq('author') }
+  end
 end

ee/spec/lib/gitlab/analytics/cycle_analytics/summary/group/stage_summary_spec.rb

@@ -51,6 +51,35 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::Summary::Group::StageSummary d
         end
       end
 
+      context 'with `assignee_username` filter' do
+        let(:assignee) { create(:user) }
+
+        before do
+          issue = project.issues.last
+          issue.assignees << assignee
+        end
+
+        subject { described_class.new(group, options: { from: Time.now, current_user: user, assignee_username: [assignee.username] }).data }
+
+        it 'finds issues from those projects' do
+          expect(subject.first[:value]).to eq('1')
+        end
+      end
+
+      context 'with `author_username` filter' do
+        let(:author) { create(:user) }
+
+        before do
+          project.issues.last.update!(author: author)
+        end
+
+        subject { described_class.new(group, options: { from: Time.now, current_user: user, author_username: [author.username] }).data }
+
+        it 'finds issues from those projects' do
+          expect(subject.first[:value]).to eq('1')
+        end
+      end
+
       context 'when `from` and `to` parameters are provided' do
         subject { described_class.new(group, options: { from: 10.days.ago, to: Time.now, current_user: user }).data }