Commit 67334f00 authored by Adam Hegyi's avatar Adam Hegyi

Fix project insights with projects.only parameter

This change filters out charts where the currently visited project is
filtered out via the `projects.only` parameter.

Also fixing the finder to return always an ActiveRecord::Relation
object in order to avoid nil errors when chaining AR scopes.
parent 72788117
<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
......
......@@ -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,
},
});
},
......
......@@ -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
- @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)
- 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] } }
---
title: Fix project insights page when projects.only parameter is used
merge_request: 30988
author:
type: fixed
......@@ -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)
......
# 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
......@@ -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';
......
......@@ -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
......
# 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
......@@ -11390,6 +11390,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 ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment