Add code review remarks

Change small things for better readability

5 files changed, 32 insertions, 33 deletions

diff --git a/app/models/cycle_analytics/group_level.rb b/app/models/cycle_analytics/group_level.rb
index 35efd8b8809..508cde0ca00 100644
--- a/app/models/cycle_analytics/group_level.rb
+++ b/app/models/cycle_analytics/group_level.rb
@@ -3,19 +3,20 @@
 module CycleAnalytics
   class GroupLevel
     include LevelBase
-    attr_reader :options
+    attr_reader :options, :group
 
-    def initialize(options:)
-      @options = options
+    def initialize(group:, options:)
+      @group = group
+      @options = options.merge(group: group)
     end
 
     def summary
-      @summary ||= ::Gitlab::CycleAnalytics::GroupStageSummary.new(options[:group],
+      @summary ||= ::Gitlab::CycleAnalytics::GroupStageSummary.new(group,
                                                               from: options[:from],
                                                               current_user: options[:current_user]).data
     end
 
-    def permissions(user: nil)
+    def permissions(*)
       STAGES.each_with_object({}) do |stage, obj|
         obj[stage] = true
       end
diff --git a/app/serializers/group_analytics_stage_entity.rb b/app/serializers/group_analytics_stage_entity.rb
index 019a3086f68..81be20e7dd8 100644
--- a/app/serializers/group_analytics_stage_entity.rb
+++ b/app/serializers/group_analytics_stage_entity.rb
@@ -9,7 +9,7 @@ class GroupAnalyticsStageEntity < Grape::Entity
   expose :description
 
   expose :group_median, as: :value do |stage|
-    # median returns a BatchLoader instance which we first have to unwrap by using to_f
+    # group_median returns a BatchLoader instance which we first have to unwrap by using to_f
     # we use to_f to make sure results below 1 are presented to the end-user
     stage.group_median.to_f.nonzero? ? distance_of_time_in_words(stage.group_median) : nil
   end
diff --git a/lib/gitlab/cycle_analytics/summary/group/issue.rb b/lib/gitlab/cycle_analytics/summary/group/issue.rb
index a5188056cb7..70073e6d843 100644
--- a/lib/gitlab/cycle_analytics/summary/group/issue.rb
+++ b/lib/gitlab/cycle_analytics/summary/group/issue.rb
@@ -16,7 +16,7 @@ module Gitlab
           end
 
           def value
-            @value ||= IssuesFinder.new(@current_user, group_id: @group.id, include_subgroups: true).execute.created_after(@from).count
+            @value ||= IssuesFinder.new(@current_user, group_id: @group.id, include_subgroups: true, created_after: @from).execute.count
           end
         end
       end
diff --git a/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb
index 6505fc714c4..eea4f33ccb8 100644
--- a/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb
+++ b/spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb
@@ -22,6 +22,16 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
       it "finds the number of issues created after it" do
         expect(subject.first[:value]).to eq(2)
       end
+
+      context 'with subgroups' do
+        before do
+          Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project, namespace: create(:group, parent: group))) }
+        end
+
+        it "finds issues from them" do
+          expect(subject.first[:value]).to eq(3)
+        end
+      end
     end
 
     context 'with other projects' do
@@ -35,18 +45,6 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
         expect(subject.first[:value]).to eq(2)
       end
     end
-
-    context 'with subgroups' do
-      before do
-        Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project, namespace: create(:group, parent: group))) }
-        Timecop.freeze(5.days.from_now) { create(:issue, project: project) }
-        Timecop.freeze(5.days.from_now) { create(:issue, project: project_2) }
-      end
-
-      it "finds issues from them" do
-        expect(subject.first[:value]).to eq(3)
-      end
-    end
   end
 
   describe "#deploys" do
@@ -61,29 +59,29 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
       it "finds the number of deploys made created after it" do
         expect(subject.second[:value]).to eq(2)
       end
-    end
 
-    context 'with other projects' do
-      before do
-        Timecop.freeze(5.days.from_now) do
-          create(:deployment, :success, project: create(:project, :repository, namespace: create(:group)))
+      context 'with subgroups' do
+        before do
+          Timecop.freeze(5.days.from_now) do
+            create(:deployment, :success, project: create(:project, :repository, namespace: create(:group, parent: group)))
+          end
         end
-      end
 
-      it "doesn't find deploys from them" do
-        expect(subject.second[:value]).to eq(0)
+        it "finds deploys from them" do
+          expect(subject.second[:value]).to eq(3)
+        end
       end
     end
 
-    context 'with subgroups' do
+    context 'with other projects' do
       before do
         Timecop.freeze(5.days.from_now) do
-          create(:deployment, :success, project: create(:project, :repository, namespace: create(:group, parent: group)))
+          create(:deployment, :success, project: create(:project, :repository, namespace: create(:group)))
         end
       end
 
-      it "finds deploys from them" do
-        expect(subject.second[:value]).to eq(1)
+      it "doesn't find deploys from them" do
+        expect(subject.second[:value]).to eq(0)
       end
     end
   end
diff --git a/spec/models/cycle_analytics/group_level_spec.rb b/spec/models/cycle_analytics/group_level_spec.rb
index 70c370bc39d..154c1b9c0f8 100644
--- a/spec/models/cycle_analytics/group_level_spec.rb
+++ b/spec/models/cycle_analytics/group_level_spec.rb
@@ -12,10 +12,10 @@ describe CycleAnalytics::GroupLevel do
   let(:mr) { create_merge_request_closing_issue(user, project, issue, commit_message: "References #{issue.to_reference}") }
   let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: mr.source_branch, sha: mr.source_branch_sha, head_pipeline_of: mr) }
 
-  subject { described_class.new(options: { from: from_date, group: group, current_user: user }) }
+  subject { described_class.new(group: group, options: { from: from_date, current_user: user }) }
 
   describe '#permissions' do
-    it 'returns permissions' do
+    it 'returns true for all stages' do
       expect(subject.permissions.values.uniq).to eq([true])
     end
   end