Merge branch '212688-fix-insights-error-on-excluded-projects' into 'master'

Fix project insights with projects.only parameter

Closes #212688

See merge request gitlab-org/gitlab!30988

ee/app/assets/javascripts/insights/components/insights.vue

 <script>
 import { mapActions, mapState } from 'vuex';
-import { GlDropdown, GlDropdownItem, GlLoadingIcon } from '@gitlab/ui';
+import { GlAlert, GlDropdown, GlDropdownItem, GlLoadingIcon } from '@gitlab/ui';
 import InsightsPage from './insights_page.vue';
 import InsightsConfigWarning from './insights_config_warning.vue';
 
 export default {
   components: {
+    GlAlert,
     GlLoadingIcon,
     InsightsPage,
     InsightsConfigWarning,
@@ -21,6 +22,11 @@ export default {
       type: String,
       required: true,
     },
+    notice: {
+      type: String,
+      default: '',
+      required: false,
+    },
   },
   computed: {
     ...mapState('insights', [
@@ -54,6 +60,9 @@ export default {
         isActive: this.activeTab === key,
       }));
     },
+    allItemsAreFilteredOut() {
+      return this.configPresent && Object.keys(this.configData).length === 0;
+    },
     configPresent() {
       return !this.configLoading && this.configData != null;
     },
@@ -88,6 +97,15 @@ export default {
     <div v-if="configLoading" class="insights-config-loading text-center">
       <gl-loading-icon :inline="true" size="lg" />
     </div>
+    <div v-else-if="allItemsAreFilteredOut" class="insights-wrapper">
+      <gl-alert>
+        {{
+          s__(
+            'Insights|This project is filtered out in the insights.yml file (see the projects.only config for more information).',
+          )
+        }}
+      </gl-alert>
+    </div>
     <div v-else-if="configPresent" class="insights-wrapper">
       <gl-dropdown
         class="js-insights-dropdown w-100"
@@ -105,6 +123,9 @@ export default {
           >{{ page.name }}</gl-dropdown-item
         >
       </gl-dropdown>
+      <gl-alert v-if="notice != ''">
+        {{ notice }}
+      </gl-alert>
       <insights-page :query-endpoint="queryEndpoint" :page-config="activePage" />
     </div>
     <insights-config-warning

ee/app/assets/javascripts/insights/index.js

@@ -5,7 +5,7 @@ import store from './stores';
 
 export default () => {
   const el = document.querySelector('#js-insights-pane');
-  const { endpoint, queryEndpoint } = el.dataset;
+  const { endpoint, queryEndpoint, notice } = el.dataset;
   const router = createRouter(endpoint);
 
   if (!el) return null;
@@ -22,6 +22,7 @@ export default () => {
         props: {
           endpoint,
           queryEndpoint,
+          notice,
         },
       });
     },

ee/app/controllers/projects/insights_controller.rb

@@ -3,6 +3,8 @@
 class Projects::InsightsController < Projects::ApplicationController
   include InsightsActions
 
+  helper_method :project_insights_config
+
   before_action :authorize_read_project!
 
   private
@@ -14,4 +16,12 @@ class Projects::InsightsController < Projects::ApplicationController
   def insights_entity
     project
   end
+
+  def config_data
+    project_insights_config.filtered_config
+  end
+
+  def project_insights_config
+    @project_insights_config ||= Gitlab::Insights::ProjectInsightsConfig.new(project: project, insights_config: insights_entity.insights_config)
+  end
 end

ee/app/views/projects/insights/show.html.haml

 - @no_container = true
 
-= render('shared/insights', endpoint: namespace_project_insights_path(@project.namespace, @project, format: :json), query_endpoint: query_namespace_project_insights_path(@project.namespace, @project))
+= render('shared/insights', endpoint: namespace_project_insights_path(@project.namespace, @project, format: :json), query_endpoint: query_namespace_project_insights_path(@project.namespace, @project), notice: project_insights_config.notice_text)

ee/app/views/shared/_insights.html.haml

 - page_title _('Insights')
 
 %div{ class: container_class }
-  #js-insights-pane{ data: { endpoint: endpoint, query_endpoint: query_endpoint } }
+  #js-insights-pane{ data: { endpoint: endpoint, query_endpoint: query_endpoint, notice: local_assigns[:notice] } }

ee/changelogs/unreleased/212688-fix-insights-error-on-excluded-projects.yml

+---
+title: Fix project insights page when projects.only parameter is used
+merge_request: 30988
+author:
+type: fixed

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

@@ -77,12 +77,17 @@ module Gitlab
           }.merge(entity_args)
         end
 
+        # rubocop: disable CodeReuse/ActiveRecord
         def entity_args
           strong_memoize(:entity_args) do
             case entity
             when ::Project
               if finder_projects
-                { project_id: entity.id } if finder_projects.exists?(entity.id) # rubocop: disable CodeReuse/ActiveRecord
+                if finder_projects.exists?(entity.id)
+                  { project_id: entity.id }
+                else
+                  { projects: ::Project.none }
+                end
               else
                 { project_id: entity.id }
               end
@@ -93,6 +98,7 @@ module Gitlab
             end
           end
         end
+        # rubocop: enable CodeReuse/ActiveRecord
 
         def created_after_argument
           return unless query.key?(:group_by)

ee/lib/gitlab/insights/project_insights_config.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Insights
+    class ProjectInsightsConfig
+      attr_reader :project, :insights_config
+
+      def initialize(project:, insights_config:)
+        @project = project
+        @insights_config = insights_config.deep_dup
+      end
+
+      def filtered_config
+        @filtered_config ||= insights_config.each_with_object({}) do |(page_identifier, page_config), new_config|
+          charts = Array(page_config[:charts]).map do |chart|
+            project_ids_or_paths = Array(chart.dig(:projects, :only))
+
+            chart if project_ids_or_paths.empty? || includes_project?(project_ids_or_paths)
+          end.compact
+
+          new_config[page_identifier] = page_config.merge(charts: charts) if charts.any?
+        end
+      end
+
+      def notice_text
+        if filtered_config != insights_config
+          s_('Insights|Some items are not visible beacuse the project was filtered out in the insights.yml file (see the projects.only config for more information).')
+        end
+      end
+
+      private
+
+      def includes_project?(project_ids_or_paths)
+        project_ids_or_paths.any? { |item| item == project.id || item == project.full_path }
+      end
+    end
+  end
+end

ee/spec/frontend/insights/components/insights_spec.js

@@ -107,6 +107,21 @@ describe('Insights component', () => {
     });
   });
 
+  describe('filtered out items', () => {
+    beforeEach(() => {
+      vm.$store.state.insights.configLoading = false;
+      vm.$store.state.insights.configData = {};
+    });
+
+    it('it displays a warning', () => {
+      return vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.gl-alert-body').innerText.trim()).toContain(
+          'This project is filtered out in the insights.yml file',
+        );
+      });
+    });
+  });
+
   describe('hash fragment present', () => {
     const defaultKey = 'issues';
     const selectedKey = 'mrs';

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

@@ -110,7 +110,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
 
           it 'returns issuable created after 30 days ago' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -120,7 +120,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
 
           it 'returns issuable created after one day ago' do
-            expect(subject.to_a).to eq([issuable4])
+            expect(subject).to eq([issuable4])
           end
         end
 
@@ -130,7 +130,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
 
           it 'returns issuable created after 12 weeks ago' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -140,7 +140,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
 
           it 'returns issuable created after one week ago' do
-            expect(subject.to_a).to eq([issuable4])
+            expect(subject).to eq([issuable4])
           end
         end
 
@@ -150,7 +150,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
 
           it 'returns issuable created after 12 months ago' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -160,7 +160,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           end
 
           it 'returns issuable created after one month ago' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
       end
@@ -188,7 +188,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [project.id] } }
 
           it 'returns issuables for that project' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -202,7 +202,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
               expected.shift(2) # Those are from other_project
             end
 
-            expect(subject.to_a).to eq(expected)
+            expect(subject).to eq(expected)
           end
         end
 
@@ -210,7 +210,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [0] } }
 
           it 'returns nothing' do
-            expect(subject.to_a).to be_empty
+            expect(subject).to be_empty
           end
         end
 
@@ -218,7 +218,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [0, project.id] } }
 
           it 'returns issuables for good project' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -226,7 +226,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [project.full_path] } }
 
           it 'returns issuables for that project' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -240,7 +240,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
               expected.shift(2) # Those are from other_project
             end
 
-            expect(subject.to_a).to eq(expected)
+            expect(subject).to eq(expected)
           end
         end
 
@@ -248,7 +248,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [project.id, project.full_path] } }
 
           it 'returns issuables for that project without duplicated issuables' do
-            expect(subject.to_a).to eq([issuable2, issuable3, issuable4])
+            expect(subject).to eq([issuable2, issuable3, issuable4])
           end
         end
 
@@ -256,7 +256,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [project.full_path.reverse] } }
 
           it 'returns nothing' do
-            expect(subject.to_a).to be_empty
+            expect(subject).to be_empty
           end
         end
 
@@ -264,7 +264,7 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
           let(:projects) { { only: [create(:project, :public).id] } }
 
           it 'returns nothing' do
-            expect(subject.to_a).to be_empty
+            expect(subject).to be_empty
           end
         end
       end

ee/spec/lib/gitlab/insights/project_insights_config_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Insights::ProjectInsightsConfig do
+  let_it_be(:project) { create(:project) }
+  let(:chart1) { { title: 'chart 1', description: 'description 1' } }
+  let(:chart2) { { title: 'chart 2', description: 'description 2' } }
+
+  let(:config) do
+    {
+      item1: {
+        title: 'item 1',
+        charts: [
+          chart1,
+          chart2
+        ]
+      },
+      item2: {
+        title: 'item 3',
+        charts: [
+          {
+            title: 'chart 3',
+            description: 'description 3'
+          }
+        ]
+      }
+    }
+  end
+
+  subject { described_class.new(project: project, insights_config: config) }
+
+  context 'when no projects.only filter present' do
+    it 'does not change the config' do
+      expect(subject.filtered_config).to eq(config)
+    end
+
+    it 'clones the original config' do
+      expect(subject.filtered_config.object_id).not_to eq(config.object_id)
+    end
+  end
+
+  context 'when not included in the projcts.only filter' do
+    context 'by project id' do
+      before do
+        chart = config[:item1][:charts].last
+        chart[:projects] = { only: [-1] }
+      end
+
+      it 'filters out the chart' do
+        expect(subject.filtered_config[:item1][:charts]).to eq([chart1])
+      end
+
+      it 'has notice text' do
+        expect(subject.notice_text).not_to eq(nil)
+      end
+    end
+
+    context 'by project full path' do
+      before do
+        chart = config[:item1][:charts].last
+        chart[:projects] = { only: ['some/full/path'] }
+      end
+
+      it 'filters out the chart' do
+        expect(subject.filtered_config[:item1][:charts]).to eq([chart1])
+      end
+    end
+  end
+
+  context 'when included in projects.only filter' do
+    context 'by project id' do
+      before do
+        chart = config[:item1][:charts].last
+        chart[:projects] = { only: [project.id] }
+      end
+
+      it 'includes the chart' do
+        expect(subject.filtered_config[:item1][:charts]).to eq([chart1, chart2])
+      end
+
+      it 'does not have notice text' do
+        expect(subject.notice_text).to eq(nil)
+      end
+    end
+
+    context 'by project full path' do
+      before do
+        chart = config[:item1][:charts].last
+        chart[:projects] = { only: [project.full_path] }
+      end
+
+      it 'filters out the chart' do
+        expect(subject.filtered_config[:item1][:charts]).to eq([chart1, chart2])
+      end
+    end
+  end
+
+  context 'when all charts are excluded' do
+    before do
+      config.each do |key, item|
+        item[:charts].each do |chart|
+          chart[:projects] = { only: [-1] }
+        end
+      end
+    end
+
+    it 'returns an empty hash' do
+      expect(subject.filtered_config).to eq({})
+    end
+  end
+end

locale/gitlab.pot

@@ -11387,6 +11387,12 @@ msgstr ""
 msgid "Insights"
 msgstr ""
 
+msgid "Insights|Some items are not visible beacuse the project was filtered out in the insights.yml file (see the projects.only config for more information)."
+msgstr ""
+
+msgid "Insights|This project is filtered out in the insights.yml file (see the projects.only config for more information)."
+msgstr ""
+
 msgid "Install"
 msgstr ""