Ensure Insights charts show all periods even if there are no data

Signed-off-by: Rémy Coutable <remy@rymai.me>

ee/app/controllers/concerns/insights_actions.rb

@@ -36,25 +36,31 @@ module InsightsActions
   end
 
   def insights_json
-    issuables = find_issuables(params[:query])
-    insights = reduce(issuables, params[:chart_type], params[:query])
+    issuables_finder = finder(params[:query])
+    issuables = issuables_finder.find
+    insights = reduce(
+      issuables: issuables,
+      chart_type: params[:chart_type],
+      period: params[:query][:group_by],
+      period_limit: issuables_finder.period_limit,
+      labels: params[:query][:collection_labels])
     serializer(params[:chart_type]).present(insights)
   end
 
-  def find_issuables(query)
-    Gitlab::Insights::Finders::IssuableFinder
-      .new(insights_entity, current_user, query).find
-  end
-
-  def reduce(issuables, chart_type, query)
+  def reduce(issuables:, chart_type:, period:, period_limit:, labels: nil)
     case chart_type
     when 'stacked-bar', 'line'
-      Gitlab::Insights::Reducers::LabelCountPerPeriodReducer.reduce(issuables, period: query[:group_by], labels: query[:collection_labels])
+      Gitlab::Insights::Reducers::LabelCountPerPeriodReducer.reduce(issuables, period: period, period_limit: period_limit, labels: labels)
     when 'bar'
-      Gitlab::Insights::Reducers::CountPerPeriodReducer.reduce(issuables, period: query[:group_by])
+      Gitlab::Insights::Reducers::CountPerPeriodReducer.reduce(issuables, period: period, period_limit: period_limit)
     end
   end
 
+  def finder(query)
+    Gitlab::Insights::Finders::IssuableFinder
+      .new(insights_entity, current_user, query)
+  end
+
   def serializer(chart_type)
     case chart_type
     when 'stacked-bar'

ee/changelogs/unreleased/11096-insights-charts-display-all-periods.yml

+---
+title: Ensure Insights charts show all periods even if there are no data
+merge_request: 10733
+author:
+type: fixed

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

@@ -36,6 +36,19 @@ module Gitlab
           relation
         end
 
+        def period_limit
+          @period_limit ||=
+            if opts.key?(:period_limit)
+              begin
+                Integer(opts[:period_limit])
+              rescue ArgumentError
+                raise InvalidPeriodLimitError, "Invalid `:period_limit` option: `#{opts[:period_limit]}`. Expected an integer!"
+              end
+            else
+              PERIODS.dig(period, :default)
+            end
+        end
+
         private
 
         attr_reader :entity, :current_user, :opts
@@ -86,19 +99,6 @@ module Gitlab
               period
             end
         end
-
-        def period_limit
-          @period_limit ||=
-            if opts.key?(:period_limit)
-              begin
-                Integer(opts[:period_limit])
-              rescue ArgumentError
-                raise InvalidPeriodLimitError, "Invalid `:period_limit` option: `#{opts[:period_limit]}`. Expected an integer!"
-              end
-            else
-              PERIODS.dig(period, :default)
-            end
-        end
       end
     end
   end

ee/lib/gitlab/insights/reducers/count_per_period_reducer.rb

@@ -6,13 +6,15 @@ module Gitlab
       class CountPerPeriodReducer < BaseReducer
         InvalidPeriodError = Class.new(BaseReducerError)
         InvalidPeriodFieldError = Class.new(BaseReducerError)
+        InvalidPeriodLimitError = Class.new(BaseReducerError)
 
         VALID_PERIOD = %w[day week month].freeze
         VALID_PERIOD_FIELD = %i[created_at].freeze
 
-        def initialize(issuables, period:, period_field: :created_at)
+        def initialize(issuables, period:, period_limit:, period_field: :created_at)
           super(issuables)
           @period = period.to_s.singularize
+          @period_limit = period_limit.to_i
           @period_field = period_field
 
           validate!
@@ -25,14 +27,15 @@ module Gitlab
         #     'March 2019' => 1
         #   }
         def reduce
-          issuables_grouped_by_normalized_period.each_with_object({}) do |(period, issuables), hash|
-            hash[period.strftime(period_format)] = value_for_period(issuables)
+          (0...period_limit).reverse_each.each_with_object({}) do |period_ago, hash|
+            period_time = normalized_time(period_ago.public_send(period).ago) # rubocop:disable GitlabSecurity/PublicSend
+            hash[period_time.strftime(period_format)] = value_for_period(issuables_grouped_by_normalized_period.fetch(period_time, []))
           end
         end
 
         private
 
-        attr_reader :period, :period_field
+        attr_reader :period, :period_limit, :period_field
 
         def validate!
           unless VALID_PERIOD.include?(period)
@@ -42,6 +45,10 @@ module Gitlab
           unless VALID_PERIOD_FIELD.include?(period_field)
             raise InvalidPeriodFieldError, "Invalid value for `period_field`: `#{period_field}`. Allowed values are #{VALID_PERIOD_FIELD}!"
           end
+
+          unless period_limit > 0
+            raise InvalidPeriodLimitError, "Invalid value for `period_limit`: `#{period_limit}`. Value must be greater than 0!"
+          end
         end
 
         # Returns a hash { period => [array of issuables] }, e.g.
@@ -51,11 +58,15 @@ module Gitlab
         #     #<Fri, 01 Mar 2019 00:00:00 UTC +00:00> => [#<Issue id:3 namespace1/project1#3>]
         #   }
         def issuables_grouped_by_normalized_period
-          issuables.group_by do |issuable|
-            issuable.public_send(period_field).public_send(period_normalizer) # rubocop:disable GitlabSecurity/PublicSend
+          @issuables_grouped_by_normalized_period ||= issuables.group_by do |issuable|
+            normalized_time(issuable.public_send(period_field)) # rubocop:disable GitlabSecurity/PublicSend
           end
         end
 
+        def normalized_time(time)
+          time.public_send(period_normalizer) # rubocop:disable GitlabSecurity/PublicSend
+        end
+
         def period_normalizer
           :"beginning_of_#{period}"
         end

ee/lib/gitlab/insights/reducers/label_count_per_period_reducer.rb

@@ -17,8 +17,8 @@ module Gitlab
       #     }
       #   }
       class LabelCountPerPeriodReducer < CountPerPeriodReducer
-        def initialize(issuables, labels:, period:, period_field: :created_at)
-          super(issuables, period: period, period_field: period_field)
+        def initialize(issuables, labels:, period:, period_limit:, period_field: :created_at)
+          super(issuables, period: period, period_limit: period_limit, period_field: period_field)
           @labels = labels
         end
 

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

@@ -7,6 +7,14 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
     Timecop.freeze(Time.utc(2019, 3, 5)) { example.run }
   end
 
+  let(:base_opts) do
+    {
+      state: 'opened',
+      group_by: 'months'
+    }
+  end
+
+  describe '#find' do
     def find(entity, opts)
       described_class.new(entity, nil, opts).find
     end
@@ -40,13 +48,10 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
       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]) }
       let!(:issuable4) { create(:"labeled_#{issuable_type}", :opened, created_at: Time.utc(2019, 3, 5), labels: [label_bug, label_quality], project_association_key => project, **extra_issuable_attrs[4]) }
       let(:opts) do
-      {
-        state: 'opened',
+        base_opts.merge(
           issuable_type: issuable_type,
           filter_labels: [label_bug.title],
-        collection_labels: [label_manage.title, label_plan.title, label_create.title],
-        group_by: 'months'
-      }
+          collection_labels: [label_manage.title, label_plan.title, label_create.title])
       end
 
       subject { find(entity, opts) }
@@ -193,4 +198,53 @@ RSpec.describe Gitlab::Insights::Finders::IssuableFinder do
         end
       end
     end
+  end
+
+  describe '#period_limit' do
+    subject { described_class.new(create(:project, :public), nil, opts).period_limit }
+
+    describe 'default values' do
+      context 'with group_by: "day"' do
+        let(:opts) { base_opts.merge!(group_by: 'day') }
+
+        it 'returns 30' do
+          expect(subject).to eq(30)
+        end
+      end
+
+      context 'with group_by: "week"' do
+        let(:opts) { base_opts.merge!(group_by: 'week') }
+
+        it 'returns 4' do
+          expect(subject).to eq(4)
+        end
+      end
+
+      context 'with group_by: "month"' do
+        let(:opts) { base_opts.merge!(group_by: 'month') }
+
+        it 'returns 12' do
+          expect(subject).to eq(12)
+        end
+      end
+    end
+
+    describe 'custom values' do
+      context 'with period_limit: 42' do
+        let(:opts) { base_opts.merge!(period_limit: 42) }
+
+        it 'returns 42' do
+          expect(subject).to eq(42)
+        end
+      end
+
+      context 'with an invalid period_limit' do
+        let(:opts) { base_opts.merge!(period_limit: 'many') }
+
+        it 'raises an error' do
+          expect { subject }.to raise_error(described_class::InvalidPeriodLimitError, "Invalid `:period_limit` option: `many`. Expected an integer!")
+        end
+      end
+    end
+  end
 end

ee/spec/lib/gitlab/insights/reducers/count_per_label_reducers_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerLabelReducer do
       filter_labels: [label_bug.title],
       collection_labels: [label_manage.title, label_plan.title],
       group_by: 'month',
-      period_limit: 2
+      period_limit: 5
     }
   end
   let(:issuable_relation) { find_issuables(project, opts) }

ee/spec/lib/gitlab/insights/reducers/count_per_period_reducers_spec.rb

@@ -9,8 +9,8 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
     Gitlab::Insights::Finders::IssuableFinder.new(project, nil, opts).find
   end
 
-  def reduce(issuable_relation, period, period_field = :created_at)
-    described_class.reduce(issuable_relation, period: period, period_field: period_field)
+  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
@@ -19,7 +19,7 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
       issuable_type: 'issue',
       filter_labels: [label_bug.title],
       group_by: 'month',
-      period_limit: 3
+      period_limit: 5
     }
   end
   let(:issuable_relation) { find_issuables(project, opts) }
@@ -29,8 +29,10 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
   let(:expected) do
     {
       'January 2019' => 1,
-      'February 2019' => 1,
-      'March 2019' => 1
+      'February 2019' => 0,
+      'March 2019' => 1,
+      'April 2019' => 1,
+      'May 2019' => 0
     }
   end
 
@@ -39,7 +41,11 @@ RSpec.describe Gitlab::Insights::Reducers::CountPerPeriodReducer do
   end
 
   it 'raises an error for an unknown :period_field option' do
-    expect { reduce(issuable_relation, 'month', :foo) }.to raise_error(described_class::InvalidPeriodFieldError, "Invalid value for `period_field`: `foo`. Allowed values are #{described_class::VALID_PERIOD_FIELD}!")
+    expect { reduce(issuable_relation, 'month', 5, :foo) }.to raise_error(described_class::InvalidPeriodFieldError, "Invalid value for `period_field`: `foo`. Allowed values are #{described_class::VALID_PERIOD_FIELD}!")
+  end
+
+  it 'raises an error for an unknown :period_limit option' do
+    expect { reduce(issuable_relation, 'month', -1) }.to raise_error(described_class::InvalidPeriodLimitError, "Invalid value for `period_limit`: `-1`. Value must be greater than 0!")
   end
 
   it 'returns issuables with only the needed fields' do

ee/spec/lib/gitlab/insights/reducers/label_count_per_period_reducers_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
   end
 
   def reduce(issuable_relation, period, labels)
-    described_class.reduce(issuable_relation, period: period, labels: labels)
+    described_class.reduce(issuable_relation, period: period, period_limit: 5, labels: labels)
   end
 
   let(:opts) do
@@ -20,7 +20,7 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
       filter_labels: [label_bug.title],
       collection_labels: [label_manage.title, label_plan.title],
       group_by: 'month',
-      period_limit: 3
+      period_limit: 5
     }
   end
   let(:issuable_relation) { find_issuables(project, opts) }
@@ -35,14 +35,24 @@ RSpec.describe Gitlab::Insights::Reducers::LabelCountPerPeriodReducer do
         Gitlab::Insights::UNCATEGORIZED => 1
       },
       'February 2019' => {
-        label_manage.title => 1,
+        label_manage.title => 0,
         label_plan.title => 0,
         Gitlab::Insights::UNCATEGORIZED => 0
       },
       'March 2019' => {
+        label_manage.title => 1,
+        label_plan.title => 0,
+        Gitlab::Insights::UNCATEGORIZED => 0
+      },
+      'April 2019' => {
         label_manage.title => 0,
         label_plan.title => 1,
         Gitlab::Insights::UNCATEGORIZED => 0
+      },
+      'May 2019' => {
+        label_manage.title => 0,
+        label_plan.title => 0,
+        Gitlab::Insights::UNCATEGORIZED => 0
       }
     }
   end

ee/spec/support/shared_contexts/lib/gitlab/insights/reducers/reducers_shared_contexts.rb

@@ -2,7 +2,7 @@
 
 RSpec.shared_context 'Insights reducers context' do
   around do |example|
-    Timecop.freeze(Time.utc(2019, 3, 5)) { example.run }
+    Timecop.freeze(Time.utc(2019, 5, 5)) { example.run }
   end
 
   let(:project) { create(:project, :public) }
@@ -11,6 +11,6 @@ RSpec.shared_context 'Insights reducers context' do
   let(:label_plan) { create(:label, project: project, name: 'Plan') }
   let!(:issuable0) { create(:labeled_issue, :opened, created_at: Time.utc(2019, 1, 5), project: project) }
   let!(:issuable1) { create(:labeled_issue, :opened, created_at: Time.utc(2019, 1, 5), labels: [label_bug], project: project) }
-  let!(:issuable2) { create(:labeled_issue, :opened, created_at: Time.utc(2019, 2, 5), labels: [label_bug, label_manage, label_plan], project: project) }
-  let!(:issuable3) { create(:labeled_issue, :opened, created_at: Time.utc(2019, 3, 5), labels: [label_bug, label_plan], project: project) }
+  let!(:issuable2) { create(:labeled_issue, :opened, created_at: Time.utc(2019, 3, 5), labels: [label_bug, label_manage, label_plan], project: project) }
+  let!(:issuable3) { create(:labeled_issue, :opened, created_at: Time.utc(2019, 4, 5), labels: [label_bug, label_plan], project: project) }
 end