Add code review remarks

Add cr remarks

Improve specs according to the review

Fix schema

Add cr remarks

Fix naming

Add cr remarks

app/models/cycle_analytics/group_level.rb

@@ -11,10 +11,7 @@ module CycleAnalytics
     end
 
     def summary
-      @summary ||= ::Gitlab::CycleAnalytics::GroupStageSummary.new(group,
-                                                              from: options[:from],
-                                                              current_user: options[:current_user],
-                                                              options: options).data
+      @summary ||= ::Gitlab::CycleAnalytics::GroupStageSummary.new(group, options: options).data
     end
 
     def permissions(*)

lib/gitlab/cycle_analytics/base_event_fetcher.rb

@@ -4,7 +4,7 @@ module Gitlab
   module CycleAnalytics
     class BaseEventFetcher
       include BaseQuery
-      include BaseDataExtraction
+      include GroupProjectsProvider
 
       attr_reader :projections, :query, :stage, :order, :options
 

lib/gitlab/cycle_analytics/base_stage.rb

@@ -4,7 +4,7 @@ module Gitlab
   module CycleAnalytics
     class BaseStage
       include BaseQuery
-      include BaseDataExtraction
+      include GroupProjectsProvider
 
       attr_reader :options
 

lib/gitlab/cycle_analytics/base_data_extraction.rb → lib/gitlab/cycle_analytics/group_projects_provider.rb

@@ -2,11 +2,9 @@
 
 module Gitlab
   module CycleAnalytics
-    module BaseDataExtraction
-      private
-
+    module GroupProjectsProvider
       def projects
-        group ? extract_projects(options) : [project]
+        group ? projects_for_group : [project]
       end
 
       def group
@@ -17,7 +15,9 @@ module Gitlab
         @project ||= options.fetch(:project, nil)
       end
 
-      def extract_projects(options)
+      private
+
+      def projects_for_group
         projects = Project.inside_path(group.full_path)
         projects = projects.where(id: options[:projects]) if options[:projects]
         projects

lib/gitlab/cycle_analytics/group_stage_summary.rb

@@ -3,16 +3,18 @@
 module Gitlab
   module CycleAnalytics
     class GroupStageSummary
-      def initialize(group, from:, current_user:, options:)
+      attr_reader :group, :from, :current_user, :options
+
+      def initialize(group, options:)
         @group = group
-        @from = from
-        @current_user = current_user
+        @from = options[:from]
+        @current_user = options[:current_user]
         @options = options
       end
 
       def data
-        [serialize(Summary::Group::Issue.new(group: @group, from: @from, current_user: @current_user, options: @options)),
-         serialize(Summary::Group::Deploy.new(group: @group, from: @from, options: @options))]
+        [serialize(Summary::Group::Issue.new(group: group, from: from, current_user: current_user, options: options)),
+         serialize(Summary::Group::Deploy.new(group: group, from: from, options: options))]
       end
 
       private

lib/gitlab/cycle_analytics/summary/group/base.rb

@@ -5,6 +5,8 @@ module Gitlab
     module Summary
       module Group
         class Base
+          attr_reader :group, :from, :options
+
           def initialize(group:, from:, options:)
             @group = group
             @from = from

lib/gitlab/cycle_analytics/summary/group/deploy.rb

@@ -5,6 +5,8 @@ module Gitlab
     module Summary
       module Group
         class Deploy < Group::Base
+          include GroupProjectsProvider
+
           def title
             n_('Deploy', 'Deploys', value)
           end
@@ -17,15 +19,10 @@ module Gitlab
 
           def find_deployments
             deployments = Deployment.joins(:project)
-              .where(projects: { id: projects })
-              .where("deployments.created_at > ?", @from)
-            deployments = deployments.where(projects: { id: @options[:projects] }) if @options[:projects]
+              .where(projects: { id: projects.ids })
+              .where("deployments.created_at > ?", from)
             deployments.success.count
           end
-
-          def projects
-            Project.inside_path(@group.full_path).ids
-          end
         end
       end
     end

lib/gitlab/cycle_analytics/summary/group/issue.rb

@@ -5,6 +5,8 @@ module Gitlab
     module Summary
       module Group
         class Issue < Group::Base
+          attr_reader :group, :from, :current_user, :options
+
           def initialize(group:, from:, current_user:, options:)
             @group = group
             @from = from
@@ -23,8 +25,8 @@ module Gitlab
           private
 
           def find_issues
-            issues = IssuesFinder.new(@current_user, group_id: @group.id, include_subgroups: true, created_after: @from).execute
-            issues = issues.where(projects: { id: @options[:projects] }) if @options[:projects]
+            issues = IssuesFinder.new(current_user, group_id: group.id, include_subgroups: true, created_after: from).execute
+            issues = issues.where(projects: { id: options[:projects] }) if options[:projects]
             issues.count
           end
         end

spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb

@@ -8,7 +8,7 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
   let(:from) { 1.day.ago }
   let(:user) { create(:user, :admin) }
 
-  subject { described_class.new(group, from: Time.now, current_user: user, options: {}).data }
+  subject { described_class.new(group, options: { from: Time.now, current_user: user }).data }
 
   describe "#new_issues" do
     context 'with from date' do
@@ -38,7 +38,7 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
           Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project, namespace: group)) }
         end
 
-        subject { described_class.new(group, from: Time.now, current_user: user, options: { projects: [project.id, project_2.id] }).data }
+        subject { described_class.new(group, options: { from: Time.now, current_user: user, projects: [project.id, project_2.id] }).data }
 
         it 'finds issues from those projects' do
           expect(subject.first[:value]).to eq(2)
@@ -91,7 +91,7 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
           end
         end
 
-        subject { described_class.new(group, from: Time.now, current_user: user, options: { projects: [project.id, project_2.id] }).data }
+        subject { described_class.new(group, options: { from: Time.now, current_user: user, projects: [project.id, project_2.id] }).data }
 
         it 'shows deploys from those projects' do
           expect(subject.second[:value]).to eq(2)
@@ -110,6 +110,5 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
         expect(subject.second[:value]).to eq(0)
       end
     end
-
   end
 end

spec/lib/gitlab/cycle_analytics/issue_stage_spec.rb

@@ -85,11 +85,11 @@ describe Gitlab::CycleAnalytics::IssueStage do
       end
 
       describe '#events' do
-        it 'exposes merge requests that close issues' do
-          result = stage.events
+        subject { stage.events }
 
-          expect(result.count).to eq(1)
-          expect(result.map { |event| event[:title] }).to contain_exactly(issue_2_1.title)
+        it 'exposes merge requests that close issues' do
+          expect(subject.count).to eq(1)
+          expect(subject.map { |event| event[:title] }).to contain_exactly(issue_2_1.title)
         end
       end
     end