diff --git a/app/models/namespace_onboarding_action.rb b/app/models/namespace_onboarding_action.rb index bf4df7de13ffa9673ccaf3899375cb5502305e29..43dd872673c6d8f87ad8b3f0282a3086c9dcd8a4 100644 --- a/app/models/namespace_onboarding_action.rb +++ b/app/models/namespace_onboarding_action.rb @@ -9,7 +9,8 @@ class NamespaceOnboardingAction < ApplicationRecord subscription_created: 1, git_write: 2, merge_request_created: 3, - git_read: 4 + git_read: 4, + user_added: 6 }.freeze enum action: ACTIONS diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 088e6f031c84b0af753a71cdf9c01c97d5802d69..3588cda180fcd6c004c4f2b0b28c820460661b97 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -38,6 +38,8 @@ def execute(source) end end + enqueue_onboarding_progress_action(source) if members.size > errors.size + return success unless errors.any? error(errors.to_sentence) @@ -50,6 +52,10 @@ def user_limit limit && limit < 0 ? nil : limit end + + def enqueue_onboarding_progress_action(source) + Namespaces::OnboardingUserAddedWorker.perform_async(source.id) + end end end diff --git a/app/services/onboarding_progress_service.rb b/app/services/onboarding_progress_service.rb index c45edcaaf3340a5469e813ece72baa44f020ae1c..ebe7caabdefce624c4a567a6f53fd82e83b62b2b 100644 --- a/app/services/onboarding_progress_service.rb +++ b/app/services/onboarding_progress_service.rb @@ -2,7 +2,7 @@ class OnboardingProgressService def initialize(namespace) - @namespace = namespace + @namespace = namespace.root_ancestor end def execute(action:) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 098d883744d693b4d847929347e2ec5497afce0a..1f2e8213b64aa5fd3431e3bc66ae9fc97b707607 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1787,6 +1787,14 @@ :weight: 1 :idempotent: :tags: [] +- :name: namespaces_onboarding_user_added + :feature_category: :users + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: new_issue :feature_category: :issue_tracking :has_external_dependencies: diff --git a/app/workers/namespaces/onboarding_user_added_worker.rb b/app/workers/namespaces/onboarding_user_added_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..02608268d6fe021d77e78dc9c6d0f2b4473c817f --- /dev/null +++ b/app/workers/namespaces/onboarding_user_added_worker.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Namespaces + class OnboardingUserAddedWorker + include ApplicationWorker + + feature_category :users + urgency :low + + idempotent! + + def perform(namespace_id) + namespace = Namespace.find(namespace_id) + OnboardingProgressService.new(namespace).execute(action: :user_added) + end + end +end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 44a5d69dbdebdffa0b3eeea43995fa01f81fdd4b..6f8c0b352fd84abea042aa8087752a9131fc45d5 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -198,6 +198,8 @@ - 1 - - namespaceless_project_destroy - 1 +- - namespaces_onboarding_user_added + - 1 - - new_epic - 2 - - new_issue diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 00b5ff59e48f7894c1dd061ada068b3b4dc1754a..e8a4a798b20cb0457adeae288da8eb1536485f26 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -3,59 +3,68 @@ require 'spec_helper' RSpec.describe Members::CreateService do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:project_user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:project_user) { create(:user) } + let_it_be(:user_ids) { project_user.id.to_s } + let_it_be(:access_level) { Gitlab::Access::GUEST } + let(:params) { { user_ids: user_ids, access_level: access_level } } + + subject(:execute_service) { described_class.new(user, params).execute(project) } before do project.add_maintainer(user) + allow(Namespaces::OnboardingUserAddedWorker).to receive(:idempotent?).and_return(false) end - it 'adds user to members' do - params = { user_ids: project_user.id.to_s, access_level: Gitlab::Access::GUEST } - result = described_class.new(user, params).execute(project) - - expect(result[:status]).to eq(:success) - expect(project.users).to include project_user + context 'when passing valid parameters' do + it 'adds a user to members' do + expect(execute_service[:status]).to eq(:success) + expect(project.users).to include project_user + expect(Namespaces::OnboardingUserAddedWorker.jobs.last['args'][0]).to eq(project.id) + end end - it 'adds no user to members' do - params = { user_ids: '', access_level: Gitlab::Access::GUEST } - result = described_class.new(user, params).execute(project) + context 'when passing no user ids' do + let(:user_ids) { '' } - expect(result[:status]).to eq(:error) - expect(result[:message]).to be_present - expect(project.users).not_to include project_user + it 'does not add a member' do + expect(execute_service[:status]).to eq(:error) + expect(execute_service[:message]).to be_present + expect(project.users).not_to include project_user + expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0) + end end - it 'limits the number of users to 100' do - user_ids = 1.upto(101).to_a.join(',') - params = { user_ids: user_ids, access_level: Gitlab::Access::GUEST } + context 'when passing many user ids' do + let(:user_ids) { 1.upto(101).to_a.join(',') } - result = described_class.new(user, params).execute(project) - - expect(result[:status]).to eq(:error) - expect(result[:message]).to be_present - expect(project.users).not_to include project_user + it 'limits the number of users to 100' do + expect(execute_service[:status]).to eq(:error) + expect(execute_service[:message]).to be_present + expect(project.users).not_to include project_user + expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0) + end end - it 'does not add an invalid member' do - params = { user_ids: project_user.id.to_s, access_level: -1 } - result = described_class.new(user, params).execute(project) + context 'when passing an invalid access level' do + let(:access_level) { -1 } - expect(result[:status]).to eq(:error) - expect(result[:message]).to include("#{project_user.username}: Access level is not included in the list") - expect(project.users).not_to include project_user + it 'does not add a member' do + expect(execute_service[:status]).to eq(:error) + expect(execute_service[:message]).to include("#{project_user.username}: Access level is not included in the list") + expect(project.users).not_to include project_user + expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0) + end end - it 'does not add a member with an existing invite' do - invited_member = create(:project_member, :invited, project: project) - - params = { user_ids: invited_member.invite_email, - access_level: Gitlab::Access::GUEST } - result = described_class.new(user, params).execute(project) + context 'when passing an existing invite user id' do + let(:user_ids) { create(:project_member, :invited, project: project).invite_email } - expect(result[:status]).to eq(:error) - expect(result[:message]).to eq('Invite email has already been taken') + it 'does not add a member' do + expect(execute_service[:status]).to eq(:error) + expect(execute_service[:message]).to eq('Invite email has already been taken') + expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0) + end end end diff --git a/spec/services/onboarding_progress_service_spec.rb b/spec/services/onboarding_progress_service_spec.rb index edf40dfeed19a38735402cae218bae7bc9ef9672..59b6083d38aaa3eafe880ce4a0d783b6daabfbbd 100644 --- a/spec/services/onboarding_progress_service_spec.rb +++ b/spec/services/onboarding_progress_service_spec.rb @@ -4,16 +4,30 @@ RSpec.describe OnboardingProgressService do describe '#execute' do - let_it_be(:namespace) { build(:namespace) } - let(:action) { :namespace_action } + let(:namespace) { create(:namespace, parent: root_namespace) } - subject(:execute_service) { described_class.new(namespace).execute(action: action) } + subject(:execute_service) { described_class.new(namespace).execute(action: :subscription_created) } - it 'records a namespace onboarding progress action' do - expect(NamespaceOnboardingAction).to receive(:create_action) - .with(namespace, :namespace_action) + context 'when the namespace is a root' do + let(:root_namespace) { nil } - subject + it 'records a namespace onboarding progress action for the given namespace' do + expect(NamespaceOnboardingAction).to receive(:create_action) + .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) } + + 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 + + expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1) + end end end end diff --git a/spec/workers/namespaces/onboarding_user_added_worker_spec.rb b/spec/workers/namespaces/onboarding_user_added_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..03c668259f85dc11fecbad85eaebd7b689797f04 --- /dev/null +++ b/spec/workers/namespaces/onboarding_user_added_worker_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::OnboardingUserAddedWorker, '#perform' do + include AfterNextHelpers + + let_it_be(:group) { create(:group) } + + it 'records the event' do + expect_next(OnboardingProgressService, group) + .to receive(:execute).with(action: :user_added).and_call_original + + expect { subject.perform(group.id) }.to change(NamespaceOnboardingAction, :count).by(1) + end +end