Merge branch 'namespace-onboarding-action-add-user' into 'master'

Add user added namespace onboarding action

See merge request gitlab-org/gitlab!49383

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

app/services/members/create_service.rb

@@ -38,6 +38,8 @@ module Members
         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 @@ module Members
 
       limit && limit < 0 ? nil : limit
     end
+
+    def enqueue_onboarding_progress_action(source)
+      Namespaces::OnboardingUserAddedWorker.perform_async(source.id)
+    end
   end
 end
 

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:)

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:

app/workers/namespaces/onboarding_user_added_worker.rb

+# 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

config/sidekiq_queues.yml

@@ -198,6 +198,8 @@
   - 1
 - - namespaceless_project_destroy
   - 1
+- - namespaces_onboarding_user_added
+  - 1
 - - new_epic
   - 2
 - - new_issue

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

spec/services/onboarding_progress_service_spec.rb

@@ -4,16 +4,30 @@ require 'spec_helper'
 
 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

spec/workers/namespaces/onboarding_user_added_worker_spec.rb

+# 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