Commit 18535eb4 authored by Małgorzata Ksionek's avatar Małgorzata Ksionek

Add code review remarks

Change small things for better readability
parent e6c61c89
...@@ -3,19 +3,20 @@ ...@@ -3,19 +3,20 @@
module CycleAnalytics module CycleAnalytics
class GroupLevel class GroupLevel
include LevelBase include LevelBase
attr_reader :options attr_reader :options, :group
def initialize(options:) def initialize(group:, options:)
@options = options @group = group
@options = options.merge(group: group)
end end
def summary def summary
@summary ||= ::Gitlab::CycleAnalytics::GroupStageSummary.new(options[:group], @summary ||= ::Gitlab::CycleAnalytics::GroupStageSummary.new(group,
from: options[:from], from: options[:from],
current_user: options[:current_user]).data current_user: options[:current_user]).data
end end
def permissions(user: nil) def permissions(*)
STAGES.each_with_object({}) do |stage, obj| STAGES.each_with_object({}) do |stage, obj|
obj[stage] = true obj[stage] = true
end end
......
...@@ -9,7 +9,7 @@ class GroupAnalyticsStageEntity < Grape::Entity ...@@ -9,7 +9,7 @@ class GroupAnalyticsStageEntity < Grape::Entity
expose :description expose :description
expose :group_median, as: :value do |stage| 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 # 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 stage.group_median.to_f.nonzero? ? distance_of_time_in_words(stage.group_median) : nil
end end
......
...@@ -16,7 +16,7 @@ module Gitlab ...@@ -16,7 +16,7 @@ module Gitlab
end end
def value 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 end
end end
......
...@@ -22,6 +22,16 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do ...@@ -22,6 +22,16 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
it "finds the number of issues created after it" do it "finds the number of issues created after it" do
expect(subject.first[:value]).to eq(2) 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))) }
end
it "finds issues from them" do
expect(subject.first[:value]).to eq(3)
end
end
end end
context 'with other projects' do context 'with other projects' do
...@@ -35,18 +45,6 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do ...@@ -35,18 +45,6 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
expect(subject.first[:value]).to eq(2) expect(subject.first[:value]).to eq(2)
end end
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 end
describe "#deploys" do describe "#deploys" do
...@@ -61,29 +59,29 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do ...@@ -61,29 +59,29 @@ describe Gitlab::CycleAnalytics::GroupStageSummary do
it "finds the number of deploys made created after it" do it "finds the number of deploys made created after it" do
expect(subject.second[:value]).to eq(2) expect(subject.second[:value]).to eq(2)
end end
end
context 'with other projects' do context 'with subgroups' do
before do before do
Timecop.freeze(5.days.from_now) do Timecop.freeze(5.days.from_now) do
create(:deployment, :success, project: create(:project, :repository, namespace: create(:group))) create(:deployment, :success, project: create(:project, :repository, namespace: create(:group, parent: group)))
end
end end
end
it "doesn't find deploys from them" do it "finds deploys from them" do
expect(subject.second[:value]).to eq(0) expect(subject.second[:value]).to eq(3)
end
end end
end end
context 'with subgroups' do context 'with other projects' do
before do before do
Timecop.freeze(5.days.from_now) 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
end end
it "finds deploys from them" do it "doesn't find deploys from them" do
expect(subject.second[:value]).to eq(1) expect(subject.second[:value]).to eq(0)
end end
end end
end end
......
...@@ -12,10 +12,10 @@ describe CycleAnalytics::GroupLevel do ...@@ -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(: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) } 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 describe '#permissions' do
it 'returns permissions' do it 'returns true for all stages' do
expect(subject.permissions.values.uniq).to eq([true]) expect(subject.permissions.values.uniq).to eq([true])
end 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