Commit fd4e7ebe authored by Doug Stull's avatar Doug Stull Committed by Dmytro Zaporozhets

Move ability against pipelines into threshold

- duplicate logic and no reason for it
  to be outside
parent 253591a3
...@@ -35,9 +35,8 @@ module EE ...@@ -35,9 +35,8 @@ module EE
strong_memoize(:show_out_of_ci_minutes_notification) do strong_memoize(:show_out_of_ci_minutes_notification) do
next unless project&.persisted? || namespace&.persisted? next unless project&.persisted? || namespace&.persisted?
context = ::Ci::Minutes::Context.new(project, namespace) context = ::Ci::Minutes::Context.new(current_user, project, namespace)
threshold = ::Ci::Minutes::Threshold.new(current_user, context.level) ::Ci::Minutes::Threshold.new(context).warning_reached?
threshold.warning_reached? && context.namespace.all_pipelines.for_user(current_user).any?
end end
end end
......
...@@ -3,19 +3,30 @@ ...@@ -3,19 +3,30 @@
module Ci module Ci
module Minutes module Minutes
class Context class Context
attr_reader :namespace, :level attr_reader :namespace
delegate :full_path, to: :level delegate :shared_runners_remaining_minutes_below_threshold?,
:shared_runners_minutes_used?,
:shared_runners_minutes_limit_enabled?, to: :level
def initialize(project, namespace) def initialize(user, project, namespace)
@user = user
@project = project @project = project
@namespace = project&.shared_runners_limit_namespace || namespace @namespace = project&.shared_runners_limit_namespace || namespace
@level = project || namespace @level = project || namespace
end end
def can_see_status?
if project
user.can?(:create_pipeline, project)
else
namespace.all_pipelines.for_user(user).any?
end
end
private private
attr_reader :project attr_reader :project, :user, :level
end end
end end
end end
...@@ -3,31 +3,28 @@ ...@@ -3,31 +3,28 @@
module Ci module Ci
module Minutes module Minutes
class Threshold class Threshold
include Gitlab::Allowable include ::Gitlab::Utils::StrongMemoize
def initialize(user, context_level) def initialize(context)
@context_level = context_level @context = context
@user = user
end end
def warning_reached? def warning_reached?
show_limit? && context_level.shared_runners_remaining_minutes_below_threshold? show_limit? && context.shared_runners_remaining_minutes_below_threshold?
end end
def alert_reached? def alert_reached?
show_limit? && context_level.shared_runners_minutes_used? show_limit? && context.shared_runners_minutes_used?
end end
private private
attr_reader :user, :context_level attr_reader :context
def show_limit? def show_limit?
context_level.shared_runners_minutes_limit_enabled? && can_see_status? strong_memoize(:show_limit) do
end context.shared_runners_minutes_limit_enabled? && context.can_see_status?
end
def can_see_status?
context_level.is_a?(Namespace) || can?(user, :create_pipeline, context_level)
end end
end end
end end
......
- context = ::Ci::Minutes::Context.new(local_assigns.dig(:project), local_assigns.dig(:namespace)) - context = ::Ci::Minutes::Context.new(current_user, local_assigns.dig(:project), local_assigns.dig(:namespace))
- threshold = ::Ci::Minutes::Threshold.new(current_user, context.level) - threshold = ::Ci::Minutes::Threshold.new(context)
- if threshold.warning_reached? || threshold.alert_reached? - if threshold.warning_reached? || threshold.alert_reached?
%div{ class: ["pt-2", (classes if defined? classes)] } %div{ class: ["pt-2", (classes if defined? classes)] }
.bs-callout.shared-runner-quota-message.d-none.d-sm-block.bs-callout-danger{ data: { scope: context.full_path } } .bs-callout.shared-runner-quota-message.d-none.d-sm-block.bs-callout-danger
%p %p
= ci_usage_warning_message(context.namespace, project) = ci_usage_warning_message(context.namespace, project)
= link_to _('Purchase more minutes'), ::EE::SUBSCRIPTIONS_MORE_MINUTES_URL, class: "btn btn-danger btn-inverted" = link_to _('Purchase more minutes'), ::EE::SUBSCRIPTIONS_MORE_MINUTES_URL, class: "btn btn-danger btn-inverted"
...@@ -124,7 +124,6 @@ describe 'CI shared runner limits' do ...@@ -124,7 +124,6 @@ describe 'CI shared runner limits' do
if message if message
element = page.find('.shared-runner-quota-message') element = page.find('.shared-runner-quota-message')
expect(element).to have_content(message) expect(element).to have_content(message)
expect(element['data-scope']).to eq(project.full_path)
end end
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require "spec_helper" require "spec_helper"
describe EE::RunnersHelper do describe EE::RunnersHelper do
let_it_be(:user, reload: true) { create(:user) } let_it_be(:user) { create(:user) }
before do before do
allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:current_user).and_return(user)
...@@ -147,13 +147,10 @@ describe EE::RunnersHelper do ...@@ -147,13 +147,10 @@ describe EE::RunnersHelper do
shared_examples_for 'minutes notification' do shared_examples_for 'minutes notification' do
let_it_be(:namespace) { create(:namespace, owner: user) } let_it_be(:namespace) { create(:namespace, owner: user) }
let_it_be(:project) { create(:project, namespace: namespace) } let_it_be(:project) { create(:project, namespace: namespace) }
let(:injected_project) { project }
let(:injected_namespace) { namespace }
let(:show_warning) { true } let(:show_warning) { true }
let(:context_level) { project } let(:context_level) { project }
let(:context) { double('Ci::Minutes::Context', level: context_level, namespace: namespace) } let(:context) { double('Ci::Minutes::Context', namespace: namespace) }
let(:threshold) { double('Ci::Minutes::Threshold', warning_reached?: show_warning) } let(:threshold) { double('Ci::Minutes::Threshold', warning_reached?: show_warning) }
let!(:user_pipeline) { create(:ci_pipeline, user: user, project: project) }
before do before do
allow(::Ci::Minutes::Context).to receive(:new).and_return(context) allow(::Ci::Minutes::Context).to receive(:new).and_return(context)
...@@ -167,34 +164,34 @@ describe EE::RunnersHelper do ...@@ -167,34 +164,34 @@ describe EE::RunnersHelper do
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end end
context 'when experiment is enabled with user pipelines' do context 'when experiment is enabled' do
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
context 'without a persisted project passed' do context 'without a persisted project passed' do
let(:injected_project) { build(:project) } let(:project) { build(:project) }
let(:context_level) { namespace } let(:context_level) { namespace }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'without a persisted namespace passed' do context 'without a persisted namespace passed' do
let(:injected_namespace) { build(:namespace) } let(:namespace) { build(:namespace) }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'with neither a project nor a namespace' do context 'with neither a project nor a namespace' do
let(:injected_project) { build(:project) } let(:project) { build(:project) }
let(:injected_namespace) { build(:namespace) } let(:namespace) { build(:namespace) }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
context 'when show_ci_minutes_notification_dot? has been called before' do context 'when show_ci_minutes_notification_dot? has been called before' do
it 'does not do all the notification and query work again' do it 'does not do all the notification and query work again' do
expect(threshold).not_to receive(:warning_reached?) expect(threshold).not_to receive(:warning_reached?)
expect(injected_project).to receive(:persisted?).once expect(project).to receive(:persisted?).once
helper.show_ci_minutes_notification_dot?(injected_project, injected_namespace) helper.show_ci_minutes_notification_dot?(project, namespace)
expect(subject).to be_falsey expect(subject).to be_falsey
end end
...@@ -207,14 +204,6 @@ describe EE::RunnersHelper do ...@@ -207,14 +204,6 @@ describe EE::RunnersHelper do
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end end
context 'without user pipelines' do
before do
user.pipelines.clear # this forces us to reload user for let_it_be
end
it { is_expected.to be_falsey }
end
context 'when show_ci_minutes_notification_dot? has been called before' do context 'when show_ci_minutes_notification_dot? has been called before' do
it 'does not do all the notification and query work again' do it 'does not do all the notification and query work again' do
expect(threshold).to receive(:warning_reached?).once expect(threshold).to receive(:warning_reached?).once
...@@ -229,11 +218,11 @@ describe EE::RunnersHelper do ...@@ -229,11 +218,11 @@ describe EE::RunnersHelper do
end end
end end
context 'with pipelines' do context 'with notifications' do
let(:experiment_status) { true } let(:experiment_status) { true }
describe '.show_buy_ci_minutes?' do describe '.show_buy_ci_minutes?' do
subject { helper.show_buy_ci_minutes?(injected_project, injected_namespace) } subject { helper.show_buy_ci_minutes?(project, namespace) }
context 'when experiment is "ci_notification_dot"' do context 'when experiment is "ci_notification_dot"' do
it_behaves_like 'minutes notification' do it_behaves_like 'minutes notification' do
...@@ -255,7 +244,7 @@ describe EE::RunnersHelper do ...@@ -255,7 +244,7 @@ describe EE::RunnersHelper do
end end
describe '.show_ci_minutes_notification_dot?' do describe '.show_ci_minutes_notification_dot?' do
subject { helper.show_ci_minutes_notification_dot?(injected_project, injected_namespace) } subject { helper.show_ci_minutes_notification_dot?(project, namespace) }
it_behaves_like 'minutes notification' do it_behaves_like 'minutes notification' do
before do before do
......
...@@ -3,21 +3,16 @@ ...@@ -3,21 +3,16 @@
require 'spec_helper' require 'spec_helper'
describe Ci::Minutes::Context do describe Ci::Minutes::Context do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:project) { build(:project, namespace: group) } let(:project) { build(:project, namespace: group) }
shared_examples 'full path' do describe 'delegation' do
describe '#full_path' do subject { described_class.new(user, project, group) }
it 'shows full path' do
expect(subject.full_path).to eq context.full_path
end
end
describe '#level' do it { is_expected.to delegate_method(:shared_runners_remaining_minutes_below_threshold?).to(:level) }
it 'assigns correct level of namespace or project' do it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:level) }
expect(subject.level).to eq context it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:level) }
end
end
end end
shared_examples 'captures root namespace' do shared_examples 'captures root namespace' do
...@@ -29,22 +24,50 @@ describe Ci::Minutes::Context do ...@@ -29,22 +24,50 @@ describe Ci::Minutes::Context do
end end
context 'when at project level' do context 'when at project level' do
subject { described_class.new(project, nil) } subject { described_class.new(user, project, nil) }
it_behaves_like 'captures root namespace' it_behaves_like 'captures root namespace'
it_behaves_like 'full path' do describe '#can_see_status' do
let(:context) { project } context 'when eligible to see status' do
before do
project.add_developer(user)
end
it 'can see status' do
expect(subject.can_see_status?).to be_truthy
end
end
context 'when not eligible to see status' do
it 'cannot see status' do
expect(subject.can_see_status?).to be_falsey
end
end
end end
end end
context 'when at namespace level' do context 'when at namespace level' do
subject { described_class.new(nil, group) } subject { described_class.new(user, nil, group) }
it_behaves_like 'captures root namespace' it_behaves_like 'captures root namespace'
it_behaves_like 'full path' do describe '#can_see_status' do
let(:context) { group } context 'when eligible to see status' do
before do
create(:ci_pipeline, user: user, project: project)
end
it 'can see status' do
expect(subject.can_see_status?).to be_truthy
end
end
context 'when not eligible to see status' do
it 'cannot see status' do
expect(subject.can_see_status?).to be_falsey
end
end
end end
end end
end end
...@@ -100,13 +100,15 @@ describe Ci::Minutes::Threshold do ...@@ -100,13 +100,15 @@ describe Ci::Minutes::Threshold do
end end
context 'when at project level' do context 'when at project level' do
let(:context) { ::Ci::Minutes::Context.new(user, injected_project, nil) }
describe '#warning_reached?' do describe '#warning_reached?' do
subject do subject do
threshold = described_class.new(user, injected_project) threshold = described_class.new(context)
threshold.warning_reached? threshold.warning_reached?
end end
context 'when project member' do context 'when eligible to see warnings' do
it_behaves_like 'queries for warning being reached' do it_behaves_like 'queries for warning being reached' do
before do before do
group.add_developer(user) group.add_developer(user)
...@@ -114,18 +116,18 @@ describe Ci::Minutes::Threshold do ...@@ -114,18 +116,18 @@ describe Ci::Minutes::Threshold do
end end
end end
context 'when not a project member' do context 'when not eligible to see warnings' do
it_behaves_like 'cannot see if warning reached' it_behaves_like 'cannot see if warning reached'
end end
end end
describe '#alert_reached?' do describe '#alert_reached?' do
subject do subject do
threshold = described_class.new(user, injected_project) threshold = described_class.new(context)
threshold.alert_reached? threshold.alert_reached?
end end
context 'when project member' do context 'when eligible to see alerts' do
it_behaves_like 'queries for alert being reached' do it_behaves_like 'queries for alert being reached' do
before do before do
group.add_developer(user) group.add_developer(user)
...@@ -133,44 +135,73 @@ describe Ci::Minutes::Threshold do ...@@ -133,44 +135,73 @@ describe Ci::Minutes::Threshold do
end end
end end
context 'when not a project member' do context 'when not eligible to see alerts' do
it_behaves_like 'cannot see if alert reached' it_behaves_like 'cannot see if alert reached'
end end
end end
end end
context 'when at namespace level' do context 'when at namespace level' do
let(:context) { ::Ci::Minutes::Context.new(user, nil, injected_group) }
describe '#warning_reached?' do describe '#warning_reached?' do
subject do subject do
threshold = described_class.new(user, injected_group) threshold = described_class.new(context)
threshold.warning_reached? threshold.warning_reached?
end end
context 'with a project that has runners enabled inside namespace' do context 'when eligible to see warnings' do
it_behaves_like 'queries for warning being reached' let!(:user_pipeline) { create(:ci_pipeline, user: user, project: project) }
end
context 'with no projects that have runners enabled inside namespace' do context 'with a project that has runners enabled inside namespace' do
it_behaves_like 'cannot see if warning reached' do it_behaves_like 'queries for warning being reached'
let(:shared_runners_enabled) { false } end
context 'with no projects that have runners enabled inside namespace' do
it_behaves_like 'cannot see if warning reached' do
let(:shared_runners_enabled) { false }
end
end end
end end
context 'when not eligible to see warnings' do
it_behaves_like 'cannot see if warning reached'
end
end end
describe '#alert_reached?' do describe '#alert_reached?' do
subject do subject do
threshold = described_class.new(user, injected_group) threshold = described_class.new(context)
threshold.alert_reached? threshold.alert_reached?
end end
context 'with a project that has runners enabled inside namespace' do context 'when eligible to see warnings' do
it_behaves_like 'queries for alert being reached' let!(:user_pipeline) { create(:ci_pipeline, user: user, project: project) }
end
context 'with no projects that have runners enabled inside namespace' do context 'with a project that has runners enabled inside namespace' do
it_behaves_like 'cannot see if alert reached' do it_behaves_like 'queries for alert being reached'
let(:shared_runners_enabled) { false }
end end
context 'with no projects that have runners enabled inside namespace' do
it_behaves_like 'cannot see if alert reached' do
let(:shared_runners_enabled) { false }
end
end
end
context 'when not eligible to see warnings' do
it_behaves_like 'cannot see if warning reached'
end
end
context 'when we have already checked to see if we can show the limit' do
subject { described_class.new(context) }
it 'does not do all the verification work again' do
expect(context).to receive(:shared_runners_minutes_limit_enabled?).once
subject.warning_reached?
subject.alert_reached?
end end
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