Commit 8b7b4cf0 authored by Mike Greiling's avatar Mike Greiling

Merge branch '10904-insights-include-projects' into 'master'

Limit group insights query in a subset of projects

Closes #10904

See merge request gitlab-org/gitlab!15930
parents 3e2a830d 6f256428
......@@ -51,14 +51,21 @@ module Routable
# Klass.where_full_path_in(%w{gitlab-org/gitlab-foss gitlab-org/gitlab})
#
# Returns an ActiveRecord::Relation.
def where_full_path_in(paths)
def where_full_path_in(paths, use_includes: true)
return none if paths.empty?
wheres = paths.map do |path|
"(LOWER(routes.path) = LOWER(#{connection.quote(path)}))"
end
includes(:route).where(wheres.join(' OR ')).references(:routes)
route =
if use_includes
includes(:route).references(:routes)
else
joins(:route)
end
route.where(wheres.join(' OR '))
end
end
......
---
title: Add projects.only option to Insights
merge_request: 15930
author:
type: added
......@@ -58,9 +58,9 @@ For example, here's a single definition for Insights that will display one page
```yaml
bugsCharts:
title: 'Charts for Bugs'
title: "Charts for bugs"
charts:
- title: Monthly Bugs Created (bar)
- title: "Monthly bugs created"
type: bar
query:
issuable_type: issue
......@@ -76,7 +76,7 @@ Each chart definition is made up of a hash composed of key-value pairs.
For example, here's single chart definition:
```yaml
- title: Monthly Bugs Created (bar)
- title: "Monthly bugs created"
type: bar
query:
issuable_type: issue
......@@ -111,7 +111,7 @@ For example:
```yaml
monthlyBugsCreated:
title: Monthly Bugs Created (bar)
title: "Monthly bugs created"
```
### `type`
......@@ -122,7 +122,7 @@ For example:
```yaml
monthlyBugsCreated:
title: Monthly Bugs Created (bar)
title: "Monthly bugs created"
type: bar
```
......@@ -145,7 +145,7 @@ Example:
```yaml
monthlyBugsCreated:
title: Monthly Bugs Created (bar)
title: "Monthly bugs created"
type: bar
query:
issuable_type: issue
......@@ -174,7 +174,7 @@ Supported values are:
Filter by the state of the queried "issuable".
If you omit it, the `opened` state filter will be applied.
By default, the `opened` state filter will be applied.
Supported values are:
......@@ -188,14 +188,14 @@ Supported values are:
Filter by labels applied to the queried "issuable".
If you omit it, no labels filter will be applied. All the defined labels must be
By default, no labels filter will be applied. All the defined labels must be
applied to the "issuable" in order for it to be selected.
Example:
```yaml
monthlyBugsCreated:
title: Monthly regressions Created (bar)
title: "Monthly regressions created"
type: bar
query:
issuable_type: issue
......@@ -209,14 +209,14 @@ monthlyBugsCreated:
Group "issuable" by the configured labels.
If you omit it, no grouping will be done. When using this keyword, you need to
By default, no grouping will be done. When using this keyword, you need to
set `type` to either `line` or `stacked-bar`.
Example:
```yaml
weeklyBugsBySeverity:
title: Weekly Bugs By Severity (stacked bar)
title: "Weekly bugs by severity"
type: stacked-bar
query:
issuable_type: issue
......@@ -248,7 +248,7 @@ The unit is related to the `query.group_by` you defined. For instance if you
defined `query.group_by: 'day'` then `query.period_limit: 365` would mean
"Gather and display data for the last 365 days".
If you omit it, default values will be applied depending on the `query.group_by`
By default, default values will be applied depending on the `query.group_by`
you defined.
| `query.group_by` | Default value |
......@@ -257,14 +257,63 @@ you defined.
| `week` | 4 |
| `month` | 12 |
### `projects`
> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/10904) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.4.
You can limit where the "issuables" can be queried from:
- If `.gitlab/insights.yml` is used for a [group's insights](../../group/insights/index.md#configure-your-insights), with `projects`, you can limit the projects to be queried. By default, all projects under the group will be used.
- If `.gitlab/insights.yml` is used for a project's insights, specifying any other projects will yield no results. By default, the project itself will be used.
#### `projects.only`
The `projects.only` option specifies the projects which the "issuables"
should be queried from.
Projects listed here will be ignored when:
- They don't exist.
- The current user doesn't have sufficient permissions to read them.
- They are outside of the group.
In the following `insights.yml` example, we specify the projects
the queries will be used on. This example is useful when setting
a group's insights:
```yaml
monthlyBugsCreated:
title: "Monthly bugs created"
type: bar
query:
issuable_type: issue
issuable_state: opened
filter_labels:
- bug
projects:
only:
- 3 # You can use the project ID
- groupA/projectA # Or full project path
- groupA/subgroupB/projectC # Projects in subgroups can be included
- groupB/project # Projects outside the group will be ignored
```
## Complete example
```yaml
.projectsOnly: &projectsOnly
projects:
only:
- 3
- groupA/projectA
- groupA/subgroupB/projectC
bugsCharts:
title: 'Charts for Bugs'
title: "Charts for bugs"
charts:
- title: Monthly Bugs Created (bar)
- title: "Monthly bugs created"
type: bar
<<: *projectsOnly
query:
issuable_type: issue
issuable_state: opened
......@@ -272,8 +321,10 @@ bugsCharts:
- bug
group_by: month
period_limit: 24
- title: Weekly Bugs By Severity (stacked bar)
- title: "Weekly bugs by severity"
type: stacked-bar
<<: *projectsOnly
query:
issuable_type: issue
issuable_state: opened
......@@ -286,8 +337,10 @@ bugsCharts:
- S4
group_by: week
period_limit: 104
- title: Monthly Bugs By Team (line)
- title: "Monthly bugs by team"
type: line
<<: *projectsOnly
query:
issuable_type: merge_request
issuable_state: opened
......
......@@ -30,10 +30,7 @@ export const receiveChartDataError = ({ commit }, { chart, error }) =>
export const fetchChartData = ({ dispatch }, { endpoint, chart }) =>
axios
.post(endpoint, {
query: chart.query,
chart_type: chart.type,
})
.post(endpoint, chart)
.then(({ data }) => dispatch('receiveChartDataSuccess', { chart, data }))
.catch(error => {
let message = `${__('There was an error gathering the chart data')}`;
......
......@@ -39,8 +39,8 @@ module InsightsActions
Gitlab::Insights::Validators::ParamsValidator.new(params).validate!
end
def chart_type_param
@chart_type_param ||= params[:chart_type]
def type_param
@type_param ||= params[:type]
end
def query_param
......@@ -51,6 +51,10 @@ module InsightsActions
@period_param ||= query_param[:group_by]
end
def projects_param
@projects_param ||= params[:projects] || {}
end
def collection_labels_param
@collection_labels_param ||= query_param[:collection_labels]
end
......@@ -62,7 +66,7 @@ module InsightsActions
end
def reduce(issuables:, period_limit: nil)
case chart_type_param
case type_param
when 'stacked-bar', 'line'
Gitlab::Insights::Reducers::LabelCountPerPeriodReducer.reduce(issuables, period: period_param, period_limit: period_limit, labels: collection_labels_param)
when 'bar', 'pie'
......@@ -77,11 +81,12 @@ module InsightsActions
def finder
@finder ||=
Gitlab::Insights::Finders::IssuableFinder
.new(insights_entity, current_user, query_param)
.new(insights_entity, current_user,
query: query_param, projects: projects_param)
end
def serializer
case chart_type_param
case type_param
when 'stacked-bar'
Gitlab::Insights::Serializers::Chartjs::MultiSeriesSerializer
when 'bar', 'pie'
......
......@@ -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)
......@@ -20,10 +22,11 @@ module Gitlab
months: { default: 12 }
}.with_indifferent_access.freeze
def initialize(entity, current_user, opts)
def initialize(entity, current_user, query: {}, projects: {})
@entity = entity
@current_user = current_user
@opts = opts
@query = query
@projects = projects
end
# Returns an Active Record relation of issuables.
......@@ -31,18 +34,18 @@ module Gitlab
relation = finder
.new(current_user, finder_args)
.execute
relation = relation.preload(:labels) if opts.key?(:collection_labels) # rubocop:disable CodeReuse/ActiveRecord
relation = relation.preload(:labels) if query.key?(:collection_labels) # rubocop:disable CodeReuse/ActiveRecord
relation
end
def period_limit
@period_limit ||=
if opts.key?(:period_limit)
if query.key?(:period_limit)
begin
Integer(opts[:period_limit])
Integer(query[:period_limit])
rescue ArgumentError
raise InvalidPeriodLimitError, "Invalid `:period_limit` option: `#{opts[:period_limit]}`. Expected an integer!"
raise InvalidPeriodLimitError, "Invalid `:period_limit` option: `#{query[:period_limit]}`. Expected an integer!"
end
else
PERIODS.dig(period, :default)
......@@ -51,49 +54,54 @@ module Gitlab
private
attr_reader :entity, :current_user, :opts
attr_reader :entity, :current_user, :query, :projects
def finder
issuable_type = opts[:issuable_type]&.to_sym
issuable_type = query[:issuable_type]&.to_sym
FINDERS[issuable_type] ||
raise(InvalidIssuableTypeError, "Invalid `:issuable_type` option: `#{opts[:issuable_type]}`. Allowed values are #{FINDERS.keys}!")
raise(InvalidIssuableTypeError, "Invalid `:issuable_type` option: `#{query[:issuable_type]}`. Allowed values are #{FINDERS.keys}!")
end
def finder_args
{
include_subgroups: true,
state: opts[:issuable_state] || 'opened',
label_name: opts[:filter_labels],
state: query[:issuable_state] || 'opened',
label_name: query[:filter_labels],
sort: 'created_asc',
created_after: created_after_argument
}.merge(entity_key => entity.id)
created_after: created_after_argument,
projects: finder_projects
}.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
end
def created_after_argument
return unless opts.key?(:group_by)
return unless query.key?(:group_by)
Time.zone.now.advance(period => -period_limit)
end
def period
@period ||=
if opts.key?(:group_by)
period = opts[:group_by].to_s.pluralize.to_sym
if query.key?(:group_by)
period = query[:group_by].to_s.pluralize.to_sym
unless PERIODS.key?(period)
raise InvalidGroupByError, "Invalid `:group_by` option: `#{opts[:group_by]}`. Allowed values are #{PERIODS.keys}!"
raise InvalidGroupByError, "Invalid `:group_by` option: `#{query[:group_by]}`. Allowed values are #{PERIODS.keys}!"
end
period
......@@ -101,6 +109,43 @@ module Gitlab
:days
end
end
def finder_projects
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
Project.id_in(finder_projects_options[:ids]).select(:id)
end
def finder_projects_paths
Project.where_full_path_in(
finder_projects_options[:paths], use_includes: false
).select(:id)
end
def finder_projects_options
@finder_projects_options ||= projects[:only]&.group_by do |item|
case item
when Integer
:ids
when String
:paths
else
:unknown
end
end || {}
end
end
end
end
......
......@@ -5,17 +5,28 @@ module Gitlab
module Validators
class ParamsValidator
ParamsValidatorError = Class.new(StandardError)
InvalidChartTypeError = Class.new(ParamsValidatorError)
InvalidTypeError = Class.new(ParamsValidatorError)
InvalidProjectsError = Class.new(ParamsValidatorError)
SUPPORTER_CHART_TYPES = %w[bar line stacked-bar pie].freeze
SUPPORTER_TYPES = %w[bar line stacked-bar pie].freeze
def initialize(params)
@params = params
end
def validate!
unless SUPPORTER_CHART_TYPES.include?(params[:chart_type])
raise InvalidChartTypeError, "Invalid `:chart_type`: `#{params[:chart_type]}`. Allowed values are #{SUPPORTER_CHART_TYPES}!"
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) || params[:projects].is_a?(ActionController::Parameters)
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
......
......@@ -8,7 +8,8 @@ describe Groups::InsightsController do
set(:project) { create(:project, :private) }
set(:insight) { create(:insight, group: parent_group, project: project) }
set(:user) { create(:user) }
let(:query_params) { { chart_type: 'bar', query: { issuable_type: 'issue', collection_labels: ['bug'] } } }
let(:query_params) { { type: 'bar', query: { issuable_type: 'issue', collection_labels: ['bug'] }, projects: projects_params } }
let(:projects_params) { { only: [project.id, project.full_path] } }
before do
stub_licensed_features(insights: true)
......
......@@ -174,12 +174,7 @@ describe('Insights store actions', () => {
describe('successful request', () => {
beforeEach(() => {
mock
.onPost(`${gl.TEST_HOST}/query`, {
query: chart.query,
chart_type: chart.type,
})
.reply(200, chartData);
mock.onPost(`${gl.TEST_HOST}/query`, chart).reply(200, chartData);
});
it('calls receiveChartDataSuccess with chart data', done => {
......@@ -202,12 +197,7 @@ describe('Insights store actions', () => {
describe('failed request', () => {
beforeEach(() => {
mock
.onPost(`${gl.TEST_HOST}/query`, {
query: chart.query,
chart_type: chart.type,
})
.reply(500);
mock.onPost(`${gl.TEST_HOST}/query`, chart).reply(500);
});
it('calls receiveChartDataError with error message', done => {
......
......@@ -5,15 +5,15 @@ require 'spec_helper'
RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
include_context 'Insights reducers context'
def find_issuables(project, opts)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
def find_issuables(project, query)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, query: query).find
end
def reduce(issuable_relation, labels)
described_class.reduce(issuable_relation, labels: labels)
end
let(:opts) do
let(:query) do
{
state: 'opened',
issuable_type: 'issue',
......@@ -23,9 +23,9 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
period_limit: 5
}
end
let(:issuable_relation) { find_issuables(project, opts) }
let(:issuable_relation) { find_issuables(project, query) }
subject { reduce(issuable_relation, opts[:collection_labels]) }
subject { reduce(issuable_relation, query[:collection_labels]) }
let(:expected) do
{
......@@ -47,6 +47,6 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
control_queries = ActiveRecord::QueryRecorder.new { subject }
create(:labeled_issue, :opened, labels: [label_bug], project: project)
expect { reduce(find_issuables(project, opts), opts[:collection_labels]) }.not_to exceed_query_limit(control_queries)
expect { reduce(find_issuables(project, query), query[:collection_labels]) }.not_to exceed_query_limit(control_queries)
end
end
......@@ -5,15 +5,15 @@ require 'spec_helper'
RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
include_context 'Insights reducers context'
def find_issuables(project, opts)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
def find_issuables(project, query)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, query: query).find
end
def reduce(issuable_relation, period, period_limit = 5, period_field = :created_at)
described_class.reduce(issuable_relation, period: period, period_limit: period_limit, period_field: period_field)
end
let(:opts) do
let(:query) do
{
state: 'opened',
issuable_type: 'issue',
......@@ -22,9 +22,9 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
period_limit: 5
}
end
let(:issuable_relation) { find_issuables(project, opts) }
let(:issuable_relation) { find_issuables(project, query) }
subject { reduce(issuable_relation, opts[:group_by]) }
subject { reduce(issuable_relation, query[:group_by]) }
let(:expected) do
{
......@@ -56,6 +56,6 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
control_queries = ActiveRecord::QueryRecorder.new { subject }
create(:labeled_issue, :opened, created_at: Time.utc(2019, 2, 5), labels: [label_bug], project: project)
expect { reduce(find_issuables(project, opts), opts[:group_by]) }.not_to exceed_query_limit(control_queries)
expect { reduce(find_issuables(project, query), query[:group_by]) }.not_to exceed_query_limit(control_queries)
end
end
......@@ -5,15 +5,15 @@ require 'spec_helper'
RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
include_context 'Insights reducers context'
def find_issuables(project, opts)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
def find_issuables(project, query)
Gitlab::Insights::Finders::IssuableFinder.new(project, nil, query: query).find
end
def reduce(issuable_relation, period, labels)
described_class.reduce(issuable_relation, period: period, period_limit: 5, labels: labels)
end
let(:opts) do
let(:query) do
{
state: 'opened',
issuable_type: 'issue',
......@@ -23,9 +23,9 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
period_limit: 5
}
end
let(:issuable_relation) { find_issuables(project, opts) }
let(:issuable_relation) { find_issuables(project, query) }
subject { reduce(issuable_relation, opts[:group_by], opts[:collection_labels]) }
subject { reduce(issuable_relation, query[:group_by], query[:collection_labels]) }
let(:expected) do
{
......@@ -65,6 +65,6 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
control_queries = ActiveRecord::QueryRecorder.new { subject }
create(:labeled_issue, :opened, created_at: Time.utc(2019, 2, 5), labels: [label_bug], project: project)
expect { reduce(find_issuables(project, opts), opts[:group_by], opts[:collection_labels]) }.not_to exceed_query_limit(control_queries)
expect { reduce(find_issuables(project, query), query[:group_by], query[:collection_labels]) }.not_to exceed_query_limit(control_queries)
end
end
# frozen_string_literal: true
require 'spec_helper'
require 'fast_spec_helper'
RSpec.describe Gitlab::Insights::Validators::ParamsValidator do
subject { described_class.new(params).validate! }
describe ':chart_type' do
described_class::SUPPORTER_CHART_TYPES.each do |chart_type|
context "with chart_type: '#{chart_type}'" do
describe ':type' do
described_class::SUPPORTER_TYPES.each do |type|
context "with type: '#{type}'" do
let(:params) do
{ chart_type: chart_type }
{ type: type }
end
it 'does not raise an error' do
......@@ -18,13 +18,47 @@ RSpec.describe Gitlab::Insights::Validators::ParamsValidator do
end
end
context 'with an invalid :chart_type' do
context 'with an invalid :type' do
let(:params) do
{ chart_type: 'unknown' }
{ type: 'unknown' }
end
it 'raises an error' do
expect { subject }.to raise_error(described_class::InvalidChartTypeError, "Invalid `:chart_type`: `unknown`. Allowed values are #{described_class::SUPPORTER_CHART_TYPES}!")
expect { subject }.to raise_error(described_class::InvalidTypeError, "Invalid `:type`: `unknown`. Allowed values are #{described_class::SUPPORTER_TYPES}!")
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
......
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