diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 238e8f7077890d7723080efeec5cb33b9ad16e80..3a1ae2c3ef6f2af7450705acfdf5423ef7f29338 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -438,6 +438,10 @@ def shared_runners_setting_higher_than?(other_setting) end end + def root? + !has_parent? + end + private def all_projects_with_pages diff --git a/app/models/namespace_onboarding_action.rb b/app/models/namespace_onboarding_action.rb index 43dd872673c6d8f87ad8b3f0282a3086c9dcd8a4..2b0d6cad02d1b938741bfe8e32b7a54316b074bc 100644 --- a/app/models/namespace_onboarding_action.rb +++ b/app/models/namespace_onboarding_action.rb @@ -10,9 +10,15 @@ class NamespaceOnboardingAction < ApplicationRecord git_write: 2, merge_request_created: 3, git_read: 4, + pipeline_created: 5, user_added: 6 }.freeze + # The monitoring window prevents the recording of a namespace_onboarding_action if a namespace is created before this + # time span. We are not interested in older namspaces, because the purpose of this table is to monitor and act on the + # progress of newly created namespaces or namespaces that already have at least one recorded action. + MONITORING_WINDOW = 90.days + enum action: ACTIONS class << self @@ -21,6 +27,9 @@ def completed?(namespace, action) end def create_action(namespace, action) + return unless namespace.root? + return if namespace.created_at < MONITORING_WINDOW.ago && !namespace.namespace_onboarding_actions.exists? + NamespaceOnboardingAction.safe_find_or_create_by(namespace: namespace, action: action) end end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 44cf8b53d0db65b88aba1c656b53f9459a23ac98..19936cefbb885d9874feb5639cfc5b4e52ac1e1a 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -84,6 +84,7 @@ def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request if pipeline.persisted? schedule_head_pipeline_update record_conversion_event + create_namespace_onboarding_action end # If pipeline is not persisted, try to recover IID @@ -123,6 +124,10 @@ def record_conversion_event Experiments::RecordConversionEventWorker.perform_async(:ci_syntax_templates, current_user.id) end + def create_namespace_onboarding_action + Namespaces::OnboardingPipelineCreatedWorker.perform_async(project.namespace_id) + end + def extra_options(content: nil, dry_run: false) { content: content, dry_run: dry_run } end diff --git a/app/services/onboarding_progress_service.rb b/app/services/onboarding_progress_service.rb index ebe7caabdefce624c4a567a6f53fd82e83b62b2b..fe00c96ed4c101a57f3401d99ed84c12e6d317bf 100644 --- a/app/services/onboarding_progress_service.rb +++ b/app/services/onboarding_progress_service.rb @@ -2,10 +2,12 @@ class OnboardingProgressService def initialize(namespace) - @namespace = namespace.root_ancestor + @namespace = namespace&.root_ancestor end def execute(action:) + return unless @namespace + NamespaceOnboardingAction.create_action(@namespace, action) end end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index c0b49a8624f3ae4b1f0a052382533623b1e568b9..ee82f9ee3c7c1354f32341f2176c43bfa544319d 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1795,6 +1795,14 @@ :weight: 1 :idempotent: :tags: [] +- :name: namespaces_onboarding_pipeline_created + :feature_category: :subgroups + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: namespaces_onboarding_user_added :feature_category: :users :has_external_dependencies: diff --git a/app/workers/namespaces/onboarding_pipeline_created_worker.rb b/app/workers/namespaces/onboarding_pipeline_created_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..e1de6d0046baf2d4bd02dd351a1a9e8e0b7f16e4 --- /dev/null +++ b/app/workers/namespaces/onboarding_pipeline_created_worker.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Namespaces + class OnboardingPipelineCreatedWorker + include ApplicationWorker + + feature_category :subgroups + urgency :low + + deduplicate :until_executing + idempotent! + + def perform(namespace_id) + namespace = Namespace.find_by_id(namespace_id) + return unless namespace + + OnboardingProgressService.new(namespace).execute(action: :pipeline_created) + end + end +end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index e0c64e4c2b77792390ad28bd22bd04440e990417..0fac249dee2423ba5f44e9601a65af931e0d3304 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -200,6 +200,8 @@ - 1 - - namespaceless_project_destroy - 1 +- - namespaces_onboarding_pipeline_created + - 1 - - namespaces_onboarding_user_added - 1 - - new_epic diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 43d64c41a289fe7c90c5928affe25aec02a7b6cd..010cb2dfe9c215cc3338a07b1275901b641ac13a 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -245,10 +245,6 @@ def ci_minutes_quota @ci_minutes_quota ||= ::Ci::Minutes::Quota.new(self) end - def root? - !has_parent? - end - # The same method name is used also at project and job level def shared_runners_minutes_limit_enabled? ci_minutes_quota.enabled? diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index fff48acc6f84c59ac1fb75679ce41d37aa864c9d..df4b1b3146d216c56655395f3794ad5aa854b213 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -646,26 +646,6 @@ end end - describe '#root?' do - subject { namespace.root? } - - context 'when is subgroup' do - before do - namespace.parent = build(:group) - end - - it 'returns false' do - is_expected.to eq(false) - end - end - - context 'when is root' do - it 'returns true' do - is_expected.to eq(true) - end - end - end - describe '#shared_runners_minutes_limit_enabled?' do subject { namespace.shared_runners_minutes_limit_enabled? } diff --git a/spec/models/namespace_onboarding_action_spec.rb b/spec/models/namespace_onboarding_action_spec.rb index 70dcb989b323f4c3becce318d3b0c13cb01487c6..aba6b2061c39ee1ec500f43b9ddfee67551436c0 100644 --- a/spec/models/namespace_onboarding_action_spec.rb +++ b/spec/models/namespace_onboarding_action_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe NamespaceOnboardingAction do - let(:namespace) { build(:namespace) } + let(:namespace) { create(:namespace) } describe 'associations' do it { is_expected.to belong_to(:namespace).required } @@ -46,6 +46,32 @@ expect { create_action }.to change { count_namespace_actions }.by(0) end + context 'when the namespace is created outside the monitoring window' do + let(:namespace) { create(:namespace, created_at: (NamespaceOnboardingAction::MONITORING_WINDOW + 1.day).ago) } + + it 'does not create an action for the namespace' do + expect { create_action }.not_to change { count_namespace_actions } + end + + context 'when an action has already been recorded for the namespace' do + before do + described_class.create!(namespace: namespace, action: :git_write) + end + + it 'creates an action for the namespace' do + expect { create_action }.to change { count_namespace_actions }.by(1) + end + end + end + + context 'when the namespace is not a root' do + let(:namespace) { create(:namespace, parent: build(:namespace)) } + + it 'does not create an action for the namespace' do + expect { create_action }.not_to change { count_namespace_actions } + end + end + def count_namespace_actions described_class.where(namespace: namespace, action: action).count end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 0130618d004e7bade17f784131d5c542a58ffec7..f86669d60b71b833d6b8874b33fe443153ab107d 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1500,4 +1500,24 @@ def project_rugged(project) end end end + + describe '#root?' do + subject { namespace.root? } + + context 'when is subgroup' do + before do + namespace.parent = build(:group) + end + + it 'returns false' do + is_expected.to eq(false) + end + end + + context 'when is root' do + it 'returns true' do + is_expected.to eq(true) + end + end + end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index de52f8df31b12c1241f13d783d28df99d2377725..c8200f5eab46ea99f73345b39ce089a8528fc88f 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -489,6 +489,7 @@ def previous_commit_sha_from_ref(ref) expect(execute_service).not_to be_persisted expect(Ci::Pipeline.count).to eq(0) + expect(Namespaces::OnboardingPipelineCreatedWorker).not_to receive(:perform_async) end shared_examples 'a failed pipeline' do @@ -1426,6 +1427,13 @@ def previous_commit_sha_from_ref(ref) pipeline end + it 'schedules a namespace onboarding create action worker' do + expect(Namespaces::OnboardingPipelineCreatedWorker) + .to receive(:perform_async).with(project.namespace_id) + + pipeline + end + context 'when target sha is specified' do let(:target_sha) { merge_request.target_branch_sha } diff --git a/spec/services/onboarding_progress_service_spec.rb b/spec/services/onboarding_progress_service_spec.rb index 59b6083d38aaa3eafe880ce4a0d783b6daabfbbd..de4c0d2a3ba415a069a554fdacbe7e2b22973a75 100644 --- a/spec/services/onboarding_progress_service_spec.rb +++ b/spec/services/onboarding_progress_service_spec.rb @@ -5,29 +5,39 @@ RSpec.describe OnboardingProgressService do describe '#execute' do let(:namespace) { create(:namespace, parent: root_namespace) } + let(:root_namespace) { nil } + let(:action) { :namespace_action } subject(:execute_service) { described_class.new(namespace).execute(action: :subscription_created) } context 'when the namespace is a root' do - let(:root_namespace) { nil } - - it 'records a namespace onboarding progress action for the given namespace' do + it 'records a namespace onboarding progress action fot the given namespace' do expect(NamespaceOnboardingAction).to receive(:create_action) - .with(namespace, :subscription_created).and_call_original + .with(namespace, :subscription_created).and_call_original expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1) end end context 'when the namespace is not the root' do - let_it_be(:root_namespace) { build(:namespace) } + let(:root_namespace) { build(:namespace) } it 'records a namespace onboarding progress action for the root namespace' do expect(NamespaceOnboardingAction).to receive(:create_action) - .with(root_namespace, :subscription_created).and_call_original + .with(root_namespace, :subscription_created).and_call_original expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1) end end + + context 'when no namespace is passed' do + let(:namespace) { nil } + + it 'does not record a namespace onboarding progress action' do + expect(NamespaceOnboardingAction).not_to receive(:create_action) + + execute_service + end + end end end diff --git a/spec/workers/namespaces/onboarding_pipeline_created_worker_spec.rb b/spec/workers/namespaces/onboarding_pipeline_created_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..240a0b698349a83c225775cc035234213e091056 --- /dev/null +++ b/spec/workers/namespaces/onboarding_pipeline_created_worker_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::OnboardingPipelineCreatedWorker, '#perform' do + include AfterNextHelpers + + let_it_be(:ci_pipeline) { create(:ci_pipeline) } + + it 'records the event' do + expect_next(OnboardingProgressService, ci_pipeline.project.namespace) + .to receive(:execute).with(action: :pipeline_created).and_call_original + + expect do + subject.perform(ci_pipeline.project.namespace_id) + end.to change(NamespaceOnboardingAction, :count).by(1) + end + + context "when a namespace doesn't exist" do + it "does nothing" do + expect_next(OnboardingProgressService, ci_pipeline.project.namespace).not_to receive(:execute) + + expect { subject.perform(nil) }.not_to change(NamespaceOnboardingAction, :count) + end + end +end