Merge branch '214348-move-user-pipeline-condition-into-context-class' into 'master'

Move user pipeline condition into threshold

Closes #214348

See merge request gitlab-org/gitlab!29950

ee/app/helpers/ee/runners_helper.rb

@@ -35,9 +35,8 @@ module EE
       strong_memoize(:show_out_of_ci_minutes_notification) do
         next unless project&.persisted? || namespace&.persisted?
 
-        context = ::Ci::Minutes::Context.new(project, namespace)
-        threshold = ::Ci::Minutes::Threshold.new(current_user, context.level)
-        threshold.warning_reached? && context.namespace.all_pipelines.for_user(current_user).any?
+        context = ::Ci::Minutes::Context.new(current_user, project, namespace)
+        ::Ci::Minutes::Threshold.new(context).warning_reached?
       end
     end
 

ee/app/models/ci/minutes/context.rb

@@ -3,19 +3,30 @@
 module Ci
   module Minutes
     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
         @namespace = project&.shared_runners_limit_namespace || namespace
         @level = project || namespace
       end
 
+      def can_see_status?
+        if project
+          user.can?(:create_pipeline, project)
+        else
+          namespace.all_pipelines.for_user(user).any?
+        end
+      end
+
       private
 
-      attr_reader :project
+      attr_reader :project, :user, :level
     end
   end
 end

ee/app/models/ci/minutes/threshold.rb

@@ -3,31 +3,28 @@
 module Ci
   module Minutes
     class Threshold
-      include Gitlab::Allowable
+      include ::Gitlab::Utils::StrongMemoize
 
-      def initialize(user, context_level)
-        @context_level = context_level
-        @user = user
+      def initialize(context)
+        @context = context
       end
 
       def warning_reached?
-        show_limit? && context_level.shared_runners_remaining_minutes_below_threshold?
+        show_limit? && context.shared_runners_remaining_minutes_below_threshold?
       end
 
       def alert_reached?
-        show_limit? && context_level.shared_runners_minutes_used?
+        show_limit? && context.shared_runners_minutes_used?
       end
 
       private
 
-      attr_reader :user, :context_level
+      attr_reader :context
 
       def show_limit?
-        context_level.shared_runners_minutes_limit_enabled? && can_see_status?
+        strong_memoize(:show_limit) do
+          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

ee/app/views/shared/_shared_runners_minutes_limit.html.haml

-- context = ::Ci::Minutes::Context.new(local_assigns.dig(:project), local_assigns.dig(:namespace))
-- threshold = ::Ci::Minutes::Threshold.new(current_user, context.level)
+- context = ::Ci::Minutes::Context.new(current_user, local_assigns.dig(:project), local_assigns.dig(:namespace))
+- threshold = ::Ci::Minutes::Threshold.new(context)
 
 - if threshold.warning_reached? || threshold.alert_reached?
   %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
         = ci_usage_warning_message(context.namespace, project)
       = link_to _('Purchase more minutes'), ::EE::SUBSCRIPTIONS_MORE_MINUTES_URL, class: "btn btn-danger btn-inverted"

ee/spec/features/ci_shared_runner_warnings_spec.rb

@@ -124,7 +124,6 @@ describe 'CI shared runner limits' do
     if message
       element = page.find('.shared-runner-quota-message')
       expect(element).to have_content(message)
-      expect(element['data-scope']).to eq(project.full_path)
     end
   end
 

ee/spec/helpers/ee/runners_helper_spec.rb

@@ -3,7 +3,7 @@
 require "spec_helper"
 
 describe EE::RunnersHelper do
-  let_it_be(:user, reload: true) { create(:user) }
+  let_it_be(:user) { create(:user) }
 
   before do
     allow(helper).to receive(:current_user).and_return(user)
@@ -147,13 +147,10 @@ describe EE::RunnersHelper do
   shared_examples_for 'minutes notification' do
     let_it_be(:namespace) { create(:namespace, owner: user) }
     let_it_be(:project) { create(:project, namespace: namespace) }
-    let(:injected_project) { project }
-    let(:injected_namespace) { namespace }
     let(:show_warning) { true }
     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!(:user_pipeline) { create(:ci_pipeline, user: user, project: project) }
 
     before do
       allow(::Ci::Minutes::Context).to receive(:new).and_return(context)
@@ -167,34 +164,34 @@ describe EE::RunnersHelper do
         it { is_expected.to be_falsey }
       end
 
-      context 'when experiment is enabled with user pipelines' do
+      context 'when experiment is enabled' do
         it { is_expected.to be_truthy }
 
         context 'without a persisted project passed' do
-          let(:injected_project) { build(:project) }
+          let(:project) { build(:project) }
           let(:context_level) { namespace }
 
           it { is_expected.to be_truthy }
         end
 
         context 'without a persisted namespace passed' do
-          let(:injected_namespace) { build(:namespace) }
+          let(:namespace) { build(:namespace) }
 
           it { is_expected.to be_truthy }
         end
 
         context 'with neither a project nor a namespace' do
-          let(:injected_project) { build(:project) }
-          let(:injected_namespace) { build(:namespace) }
+          let(:project) { build(:project) }
+          let(:namespace) { build(:namespace) }
 
           it { is_expected.to be_falsey }
 
           context 'when show_ci_minutes_notification_dot? has been called before' do
             it 'does not do all the notification and query work again' do
               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
             end
@@ -207,14 +204,6 @@ describe EE::RunnersHelper do
           it { is_expected.to be_falsey }
         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
           it 'does not do all the notification and query work again' do
             expect(threshold).to receive(:warning_reached?).once
@@ -229,11 +218,11 @@ describe EE::RunnersHelper do
     end
   end
 
-  context 'with pipelines' do
+  context 'with notifications' do
     let(:experiment_status) { true }
 
     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
         it_behaves_like 'minutes notification' do
@@ -255,7 +244,7 @@ describe EE::RunnersHelper do
     end
 
     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
         before do

ee/spec/models/ci/minutes/context_spec.rb

@@ -3,21 +3,16 @@
 require 'spec_helper'
 
 describe Ci::Minutes::Context do
+  let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
   let(:project) { build(:project, namespace: group) }
 
-  shared_examples 'full path' do
-    describe '#full_path' do
-      it 'shows full path' do
-        expect(subject.full_path).to eq context.full_path
-      end
-    end
+  describe 'delegation' do
+    subject { described_class.new(user, project, group) }
 
-    describe '#level' do
-      it 'assigns correct level of namespace or project' do
-        expect(subject.level).to eq context
-      end
-    end
+    it { is_expected.to delegate_method(:shared_runners_remaining_minutes_below_threshold?).to(:level) }
+    it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:level) }
+    it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:level) }
   end
 
   shared_examples 'captures root namespace' do
@@ -29,22 +24,50 @@ describe Ci::Minutes::Context do
   end
 
   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 'full path' do
-      let(:context) { project }
+    describe '#can_see_status' do
+      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
 
   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 'full path' do
-      let(:context) { group }
+    describe '#can_see_status' do
+      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

ee/spec/models/ci/minutes/threshold_spec.rb

@@ -100,13 +100,15 @@ describe Ci::Minutes::Threshold do
   end
 
   context 'when at project level' do
+    let(:context) { ::Ci::Minutes::Context.new(user, injected_project, nil) }
+
     describe '#warning_reached?' do
       subject do
-        threshold = described_class.new(user, injected_project)
+        threshold = described_class.new(context)
         threshold.warning_reached?
       end
 
-      context 'when project member' do
+      context 'when eligible to see warnings' do
         it_behaves_like 'queries for warning being reached' do
           before do
             group.add_developer(user)
@@ -114,18 +116,18 @@ describe Ci::Minutes::Threshold do
         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'
       end
     end
 
     describe '#alert_reached?' do
       subject do
-        threshold = described_class.new(user, injected_project)
+        threshold = described_class.new(context)
         threshold.alert_reached?
       end
 
-      context 'when project member' do
+      context 'when eligible to see alerts' do
         it_behaves_like 'queries for alert being reached' do
           before do
             group.add_developer(user)
@@ -133,19 +135,24 @@ describe Ci::Minutes::Threshold do
         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'
       end
     end
   end
 
   context 'when at namespace level' do
+    let(:context) { ::Ci::Minutes::Context.new(user, nil, injected_group) }
+
     describe '#warning_reached?' do
       subject do
-        threshold = described_class.new(user, injected_group)
+        threshold = described_class.new(context)
         threshold.warning_reached?
       end
 
+      context 'when eligible to see warnings' do
+        let!(:user_pipeline) { create(:ci_pipeline, user: user, project: project) }
+
         context 'with a project that has runners enabled inside namespace' do
           it_behaves_like 'queries for warning being reached'
         end
@@ -157,12 +164,20 @@ describe Ci::Minutes::Threshold do
         end
       end
 
+      context 'when not eligible to see warnings' do
+        it_behaves_like 'cannot see if warning reached'
+      end
+    end
+
     describe '#alert_reached?' do
       subject do
-        threshold = described_class.new(user, injected_group)
+        threshold = described_class.new(context)
         threshold.alert_reached?
       end
 
+      context 'when eligible to see warnings' do
+        let!(:user_pipeline) { create(:ci_pipeline, user: user, project: project) }
+
         context 'with a project that has runners enabled inside namespace' do
           it_behaves_like 'queries for alert being reached'
         end
@@ -173,5 +188,21 @@ describe Ci::Minutes::Threshold do
           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