diff options
author | Małgorzata Ksionek <mksionek@gitlab.com> | 2019-07-16 17:17:21 +0200 |
---|---|---|
committer | Małgorzata Ksionek <mksionek@gitlab.com> | 2019-07-16 18:45:15 +0200 |
commit | 18535eb411887ac8283cc24b8ed1e384d3aabcc1 (patch) | |
tree | 7c08d43d993c463f7999099d31569175b86b1cf0 | |
parent | e6c61c89527eb1a8f0f8affff5a76c81af656d8e (diff) | |
download | gitlab-ce-18535eb411887ac8283cc24b8ed1e384d3aabcc1.tar.gz |
Add code review remarks
Change small things for better readability
-rw-r--r-- | app/models/cycle_analytics/group_level.rb | 11 | ||||
-rw-r--r-- | app/serializers/group_analytics_stage_entity.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/cycle_analytics/summary/group/issue.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/cycle_analytics/group_stage_summary_spec.rb | 46 | ||||
-rw-r--r-- | spec/models/cycle_analytics/group_level_spec.rb | 4 |
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 |