Handle the case where projects is provided

For project's insights. Also add validations and tests for input.

ee/lib/gitlab/insights/finders/issuable_finder.rb

@@ -4,6 +4,8 @@ module Gitlab
   module Insights
     module Finders
       class IssuableFinder
+        include Gitlab::Utils::StrongMemoize
+
         IssuableFinderError = Class.new(StandardError)
         InvalidIssuableTypeError = Class.new(IssuableFinderError)
         InvalidGroupByError = Class.new(IssuableFinderError)
@@ -69,15 +71,19 @@ module Gitlab
             sort: 'created_asc',
             created_after: created_after_argument,
             projects: finder_projects
-          }.merge(entity_key => entity.id)
+          }.merge(entity_arg)
         end
 
-        def entity_key
+        def entity_arg
           case entity
           when ::Project
-            :project_id
+            if finder_projects
+              {} # We just rely on projects argument
+            else
+              { project_id: entity.id }
+            end
           when ::Namespace
-            :group_id
+            { group_id: entity.id }
           else
             raise InvalidEntityError, "Entity class `#{entity.class}` is not supported. Supported classes are Project and Namespace!"
           end
@@ -105,9 +111,17 @@ module Gitlab
         end
 
         def finder_projects
-          return if projects.empty?
-
-          Project.from_union([finder_projects_ids, finder_projects_paths])
+          strong_memoize(:finder_projects) do
+            if projects.empty?
+              nil
+            elsif finder_projects_options[:ids] && finder_projects_options[:paths]
+              Project.from_union([finder_projects_ids, finder_projects_paths])
+            elsif finder_projects_options[:ids]
+              finder_projects_ids
+            elsif finder_projects_options[:paths]
+              finder_projects_paths
+            end
+          end
         end
 
         def finder_projects_ids

ee/lib/gitlab/insights/validators/params_validator.rb

@@ -6,6 +6,7 @@ module Gitlab
       class ParamsValidator
         ParamsValidatorError = Class.new(StandardError)
         InvalidTypeError = Class.new(ParamsValidatorError)
+        InvalidProjectsError = Class.new(ParamsValidatorError)
 
         SUPPORTER_TYPES = %w[bar line stacked-bar pie].freeze
 
@@ -17,6 +18,16 @@ module Gitlab
           unless SUPPORTER_TYPES.include?(params[:type])
             raise InvalidTypeError, "Invalid `:type`: `#{params[:type]}`. Allowed values are #{SUPPORTER_TYPES}!"
           end
+
+          if params[:projects]
+            unless params[:projects].is_a?(Hash)
+              raise InvalidProjectsError, "Invalid `:projects`: `#{params[:projects]}`. It should be a hash."
+            end
+
+            unless params.dig(:projects, :only).is_a?(Array)
+              raise InvalidProjectsError, "Invalid `:projects`.`only`: `#{params.dig(:projects, :only)}`. It should be an array."
+            end
+          end
         end
 
         private

ee/spec/lib/gitlab/insights/finders/issuable_finder_spec.rb

@@ -15,20 +15,26 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
   end
 
   describe '#find' do
-    def find(entity, query)
-      described_class.new(entity, nil, query: query).find
+    def find(entity, query:, projects: {})
+      described_class.new(entity, nil, query: query, projects: projects).find
     end
 
     it 'raises an error for an invalid :issuable_type option' do
-      expect { find(build(:project), issuable_type: 'foo') }.to raise_error(described_class::InvalidIssuableTypeError, "Invalid `:issuable_type` option: `foo`. Allowed values are #{described_class::FINDERS.keys}!")
+      expect do
+        find(build(:project), query: { issuable_type: 'foo' })
+      end.to raise_error(described_class::InvalidIssuableTypeError, "Invalid `:issuable_type` option: `foo`. Allowed values are #{described_class::FINDERS.keys}!")
     end
 
     it 'raises an error for an invalid entity object' do
-      expect { find(build(:user), issuable_type: 'issue') }.to raise_error(described_class::InvalidEntityError, 'Entity class `User` is not supported. Supported classes are Project and Namespace!')
+      expect do
+        find(build(:user), query: { issuable_type: 'issue' })
+      end.to raise_error(described_class::InvalidEntityError, 'Entity class `User` is not supported. Supported classes are Project and Namespace!')
     end
 
     it 'raises an error for an invalid :group_by option' do
-      expect { find(build(:project), issuable_type: 'issue', group_by: 'foo') }.to raise_error(described_class::InvalidGroupByError, "Invalid `:group_by` option: `foo`. Allowed values are #{described_class::PERIODS.keys}!")
+      expect do
+        find(build(:project), query: { issuable_type: 'issue', group_by: 'foo' })
+      end.to raise_error(described_class::InvalidGroupByError, "Invalid `:group_by` option: `foo`. Allowed values are #{described_class::PERIODS.keys}!")
     end
 
     it 'defaults to the "days" period if no :group_by is given' do
@@ -36,7 +42,9 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
     end
 
     it 'raises an error for an invalid :period_limit option' do
-      expect { find(build(:project), issuable_type: 'issue', group_by: 'months', period_limit: 'many') }.to raise_error(described_class::InvalidPeriodLimitError, "Invalid `:period_limit` option: `many`. Expected an integer!")
+      expect do
+        find(build(:project), query: { issuable_type: 'issue', group_by: 'months', period_limit: 'many' })
+      end.to raise_error(described_class::InvalidPeriodLimitError, "Invalid `:period_limit` option: `many`. Expected an integer!")
     end
 
     shared_examples_for "insights issuable finder" do
@@ -46,7 +54,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
       let(:label_create) { create(label_type, label_entity_association_key => entity, name: 'Create') }
       let(:label_quality) { create(label_type, label_entity_association_key => entity, name: 'Quality') }
       let(:extra_issuable_attrs) { [{}, {}, {}, {}, {}, {}] }
-      let!(:issuable0) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2018, 2, 1), project_association_key => project, **extra_issuable_attrs[0]) }
+      let!(:issuable0) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2018, 1, 1), project_association_key => project, **extra_issuable_attrs[0]) }
       let!(:issuable1) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2018, 2, 1), labels: [label_bug, label_manage], project_association_key => project, **extra_issuable_attrs[1]) }
       let!(:issuable2) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 2, 6), labels: [label_bug, label_plan], project_association_key => project, **extra_issuable_attrs[2]) }
       let!(:issuable3) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 2, 20), labels: [label_bug, label_create], project_association_key => project, **extra_issuable_attrs[3]) }
@@ -57,17 +65,20 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           filter_labels: [label_bug.title],
           collection_labels: [label_manage.title, label_plan.title, label_create.title])
       end
+      let(:projects) { {} }
 
-      subject { find(entity, query) }
+      subject { find(entity, query: query, projects: projects) }
 
       it 'avoids N + 1 queries' do
         control_queries = ActiveRecord::QueryRecorder.new { subject.map { |issuable| issuable.labels.map(&:title) } }
         create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 3, 5), labels: [label_bug], project_association_key => project, **extra_issuable_attrs[5])
 
-        expect { find(entity, query).map { |issuable| issuable.labels.map(&:title) } }.not_to exceed_query_limit(control_queries)
+        expect do
+          find(entity, query: query).map { |issuable| issuable.labels.map(&:title) }
+        end.not_to exceed_query_limit(control_queries)
       end
 
-      context ':period_limit option' do
+      context ':period_limit query' do
         context 'with group_by: "day"' do
           before do
             query.merge!(group_by: 'day')
@@ -128,6 +139,84 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
         end
       end
+
+      context ':projects option' do
+        let(:query) do
+          { issuable_type: issuable_type }
+        end
+
+        before do
+          # For merge requests we need to update both projects
+          attributes =
+            Hash[
+              [
+                [:project, other_project],
+                [project_association_key, other_project]
+              ].uniq
+            ]
+
+          issuable0.update!(attributes)
+          issuable1.update!(attributes)
+        end
+
+        context 'when `projects.only` are specified by one id' do
+          let(:projects) { { only: [project.id] } }
+
+          it 'returns issuables for that project' do
+            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+          end
+        end
+
+        context 'when `projects.only` are specified by two ids' do
+          let(:projects) { { only: [project.id, other_project.id] } }
+
+          it 'returns issuables for all projects' do
+            expect(subject.to_a)
+              .to eq([issuable0, issuable1, issuable2, issuable3, issuable4])
+          end
+        end
+
+        context 'when `projects.only` are specified by bad id' do
+          let(:projects) { { only: [0] } }
+
+          it 'returns nothing' do
+            expect(subject.to_a).to be_empty
+          end
+        end
+
+        context 'when `projects.only` are specified by bad id and good id' do
+          let(:projects) { { only: [0, other_project.id] } }
+
+          it 'returns issuables for good project' do
+            expect(subject.to_a).to eq([issuable0, issuable1])
+          end
+        end
+
+        context 'when `projects.only` are specified by one project full path' do
+          let(:projects) { { only: [project.full_path] } }
+
+          it 'returns issuables for that project' do
+            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+          end
+        end
+
+        context 'when `projects.only` are specified by project full path and id' do
+          let(:projects) { { only: [project.id, other_project.full_path] } }
+
+          it 'returns issuables for all projects' do
+            expect(subject.to_a)
+              .to eq([issuable0, issuable1, issuable2, issuable3, issuable4])
+          end
+        end
+
+        context 'when `projects.only` are specified by bad project path' do
+          let(:projects) { { only: [project.full_path.reverse] } }
+
+          it 'returns nothing' do
+            expect(subject.to_a).to be_empty
+          end
+        end
+      end
     end
 
     shared_examples_for 'group tests' do
@@ -163,17 +252,20 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
     context 'for a group' do
       include_examples 'group tests' do
         let(:project) { create(:project, :public, group: entity) }
+        let(:other_project) { create(:project, :public, group: entity) }
       end
     end
 
     context 'for a group with subgroups' do
       include_examples 'group tests' do
         let(:project) { create(:project, :public, group: create(:group, parent: entity)) }
+        let(:other_project) { create(:project, :public, group: entity) }
       end
     end
 
     context 'for a project' do
       let(:project) { create(:project, :public) }
+      let(:other_project) { create(:project, :public) }
       let(:entity) { project }
       let(:label_type) { :label }
       let(:label_entity_association_key) { :project }

ee/spec/lib/gitlab/insights/validators/params_validator_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 
 RSpec.describe Gitlab::Insights::Validators::ParamsValidator do
   subject { described_class.new(params).validate! }
@@ -28,4 +28,38 @@ RSpec.describe Gitlab::Insights::Validators::ParamsValidator do
       end
     end
   end
+
+  describe ':projects' do
+    let(:base_params) { { type: described_class::SUPPORTER_TYPES.first } }
+
+    context 'when projects is an array' do
+      let(:params) do
+        base_params.merge(projects: [])
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(described_class::InvalidProjectsError, "Invalid `:projects`: `[]`. It should be a hash.")
+      end
+    end
+
+    context 'when projects is a hash, having `only` with an integer' do
+      let(:params) do
+        base_params.merge(projects: { only: 1 })
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(described_class::InvalidProjectsError, "Invalid `:projects`.`only`: `1`. It should be an array.")
+      end
+    end
+
+    context 'when projects is a hash, having `only` with an array' do
+      let(:params) do
+        base_params.merge(projects: { only: [] })
+      end
+
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
+  end
 end