diff --git a/ee/app/assets/javascripts/insights/components/insights.vue b/ee/app/assets/javascripts/insights/components/insights.vue index 5050e4f0eb388c30a2d958b99e3de5de75a154d5..2452314f17ce8855b2ed2e22b8fa91efb988f76e 100644 --- a/ee/app/assets/javascripts/insights/components/insights.vue +++ b/ee/app/assets/javascripts/insights/components/insights.vue @@ -1,11 +1,12 @@ <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 diff --git a/ee/app/assets/javascripts/insights/index.js b/ee/app/assets/javascripts/insights/index.js index e1daf076b77f780f69239bc963e9c430453b053e..7004b7581dedc3dcc6eafc259b5027eae4ce20a2 100644 --- a/ee/app/assets/javascripts/insights/index.js +++ b/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, }, }); }, diff --git a/ee/app/controllers/projects/insights_controller.rb b/ee/app/controllers/projects/insights_controller.rb index 3639493aa732daad1329884e17a2c38fab998e1d..1c87f451dbddba74fe4de6ee14301c1134f68bad 100644 --- a/ee/app/controllers/projects/insights_controller.rb +++ b/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 @@ def authorize_read_project! 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 diff --git a/ee/app/views/projects/insights/show.html.haml b/ee/app/views/projects/insights/show.html.haml index fab302648db6add8cbac8cf35468510933467793..f4bd38f04dbf7c287e3a3c06d95a0831ba03583a 100644 --- a/ee/app/views/projects/insights/show.html.haml +++ b/ee/app/views/projects/insights/show.html.haml @@ -1,3 +1,3 @@ - @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) diff --git a/ee/app/views/shared/_insights.html.haml b/ee/app/views/shared/_insights.html.haml index 72b309a2ac66a2d8928a27ec45ddbda492ff4dee..b2e77398790bdc1c9676957c972de920c01ede63 100644 --- a/ee/app/views/shared/_insights.html.haml +++ b/ee/app/views/shared/_insights.html.haml @@ -1,4 +1,4 @@ - 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] } } diff --git a/ee/changelogs/unreleased/212688-fix-insights-error-on-excluded-projects.yml b/ee/changelogs/unreleased/212688-fix-insights-error-on-excluded-projects.yml new file mode 100644 index 0000000000000000000000000000000000000000..dc7d0d601fae20cfefe520b798b48ae33fa2f213 --- /dev/null +++ b/ee/changelogs/unreleased/212688-fix-insights-error-on-excluded-projects.yml @@ -0,0 +1,5 @@ +--- +title: Fix project insights page when projects.only parameter is used +merge_request: 30988 +author: +type: fixed diff --git a/ee/lib/gitlab/insights/finders/issuable_finder.rb b/ee/lib/gitlab/insights/finders/issuable_finder.rb index 864cb8805e561166277948e6b49d3a39f190c144..7a30dd3fac7f11c728fa0a07415ec319729a0149 100644 --- a/ee/lib/gitlab/insights/finders/issuable_finder.rb +++ b/ee/lib/gitlab/insights/finders/issuable_finder.rb @@ -77,12 +77,17 @@ def finder_args }.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 @@ def entity_args end end end + # rubocop: enable CodeReuse/ActiveRecord def created_after_argument return unless query.key?(:group_by) diff --git a/ee/lib/gitlab/insights/project_insights_config.rb b/ee/lib/gitlab/insights/project_insights_config.rb new file mode 100644 index 0000000000000000000000000000000000000000..f17bc0d3bf12c5417c90d6c3f91aded6059b5fc7 --- /dev/null +++ b/ee/lib/gitlab/insights/project_insights_config.rb @@ -0,0 +1,38 @@ +# 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 diff --git a/ee/spec/frontend/insights/components/insights_spec.js b/ee/spec/frontend/insights/components/insights_spec.js index 46580cdcb910ee8f15180f7c209174cdcfa9fc48..b058ef5e4978697f1c5870822799d57678bf886e 100644 --- a/ee/spec/frontend/insights/components/insights_spec.js +++ b/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'; diff --git a/ee/spec/lib/gitlab/insights/finders/issuable_finder_spec.rb b/ee/spec/lib/gitlab/insights/finders/issuable_finder_spec.rb index 020b5018c41f6e05cb28cee610dcfa4a719fcfb7..25030e2e32ba625b2dd1a6cf40ec56697b7460f2 100644 --- a/ee/spec/lib/gitlab/insights/finders/issuable_finder_spec.rb +++ b/ee/spec/lib/gitlab/insights/finders/issuable_finder_spec.rb @@ -110,7 +110,7 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 @@ def find(entity, query:, projects: {}) 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 diff --git a/ee/spec/lib/gitlab/insights/project_insights_config_spec.rb b/ee/spec/lib/gitlab/insights/project_insights_config_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..1396b0b1037abbbbd9bfa51e3d5b658dd2ffbdbf --- /dev/null +++ b/ee/spec/lib/gitlab/insights/project_insights_config_spec.rb @@ -0,0 +1,112 @@ +# 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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 717624c29f5d4afa7c0cf71c1ea92647583ec660..48175627cdd1f2a52227a634491a4a50817c1831 100644 --- a/locale/gitlab.pot +++ b/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 ""