Merge branch 'jswain_flag_onboarding_namespaces' into 'master'

Flag namespaces created during onboarding

See merge request gitlab-org/gitlab!71509

app/services/groups/create_service.rb

@@ -6,6 +6,7 @@ module Groups
       @current_user = user
       @params = params.dup
       @chat_team = @params.delete(:create_chat_team)
+      @create_event = @params.delete(:create_event)
     end
 
     def execute
@@ -42,15 +43,23 @@ module Groups
         end
       end
 
+      after_create_hook
+
       @group
     end
 
     private
 
+    attr_reader :create_event
+
     def after_build_hook(group, params)
       # overridden in EE
     end
 
+    def after_create_hook
+      # overridden in EE
+    end
+
     def remove_unallowed_params
       params.delete(:default_branch_protection) unless can?(current_user, :create_group_with_default_branch_protection)
       params.delete(:allow_mfa_for_subgroups)

config/sidekiq_queues.yml

@@ -175,6 +175,8 @@
   - 1
 - - group_wikis_git_garbage_collect
   - 1
+- - groups_create_event
+  - 1
 - - groups_export_memberships
   - 1
 - - groups_schedule_bulk_repository_shard_moves

ee/app/controllers/registrations/groups_controller.rb

@@ -17,7 +17,7 @@ module Registrations
     end
 
     def create
-      @group = Groups::CreateService.new(current_user, group_params).execute
+      @group = Groups::CreateService.new(current_user, group_params.merge(create_event: true)).execute
 
       if @group.persisted?
         experiment(:combined_registration, user: current_user).track(:create_group, namespace: @group)

ee/app/controllers/registrations/groups_projects_controller.rb

@@ -80,12 +80,12 @@ module Registrations
 
     def modified_group_params
       group_name = params.dig(:group, :name)
-
+      modifed_group_params = group_params
       if group_name.present? && params.dig(:group, :path).blank?
-        group_params.compact_blank.with_defaults(path: Namespace.clean_path(group_name))
-      else
-        group_params
+        modifed_group_params = modifed_group_params.compact_blank.with_defaults(path: Namespace.clean_path(group_name))
       end
+
+      modifed_group_params.merge(create_event: true)
     end
   end
 end

ee/app/services/ee/groups/create_service.rb

@@ -24,6 +24,13 @@ module EE
         group.repository_size_limit = ::Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
       end
 
+      override :after_create_hook
+      def after_create_hook
+        if group.persisted? && create_event
+          ::Groups::CreateEventWorker.perform_async(group.id, current_user.id, :created)
+        end
+      end
+
       override :remove_unallowed_params
       def remove_unallowed_params
         unless current_user&.admin?

ee/app/workers/all_queues.yml

@@ -993,6 +993,15 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: groups_create_event
+  :worker_name: Groups::CreateEventWorker
+  :feature_category: :onboarding
+  :has_external_dependencies:
+  :urgency: :throttled
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: groups_export_memberships
   :worker_name: Groups::ExportMembershipsWorker
   :feature_category: :compliance_management

ee/app/workers/groups/create_event_worker.rb

+# frozen_string_literal: true
+
+module Groups
+  class CreateEventWorker
+    include ApplicationWorker
+
+    deduplicate :until_executing, including_scheduled: true
+    data_consistency :delayed
+
+    idempotent!
+    feature_category :onboarding
+    urgency :throttled
+
+    def perform(group_id, current_user_id, action)
+      Event.create!({
+        group_id: group_id,
+        action: action,
+        author_id: current_user_id
+      })
+    end
+  end
+end

ee/spec/controllers/registrations/groups_controller_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Registrations::GroupsController do
     let_it_be(:trial_onboarding_flow_params) { {} }
 
     let(:dev_env_or_com) { true }
-    let(:group_params) { { name: 'Group name', path: 'group-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE, emails: ['', ''] } }
+    let(:group_params) { { name: 'Group name', path: 'group-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s } }
     let(:params) do
       { group: group_params }.merge(glm_params).merge(trial_form_params).merge(trial_onboarding_flow_params)
     end
@@ -40,6 +40,12 @@ RSpec.describe Registrations::GroupsController do
             expect { post_create }.to change { Group.count }.by(1)
           end
 
+          it 'passes create_event to Groups::CreateService' do
+            expect(Groups::CreateService).to receive(:new).with(user, ActionController::Parameters.new(group_params.merge(create_event: true)).permit!).and_call_original
+
+            post_create
+          end
+
           context 'when in trial onboarding  - apply_trial_for_trial_onboarding_flow' do
             let_it_be(:group) { create(:group) }
             let_it_be(:trial_onboarding_flow_params) { { trial_onboarding_flow: true, glm_source: 'about.gitlab.com', glm_content: 'content' } }

ee/spec/controllers/registrations/groups_projects_controller_spec.rb

@@ -32,11 +32,11 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
   end
 
   describe 'POST #create' do
-    subject { post :create, params: params }
+    subject(:post_create) { post :create, params: params }
 
     let(:params) { { group: group_params, project: project_params }.merge(extra_params) }
     let(:extra_params) { {} }
-    let(:group_params) { { name: 'Group name', path: 'group-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE, emails: ['', ''] } }
+    let(:group_params) { { name: 'Group name', path: 'group-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s } }
     let(:project_params) { { name: 'New project', path: 'project-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
     let(:dev_env_or_com) { true }
 
@@ -61,12 +61,18 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
             expect(controller).to receive(:record_experiment_user).with(:remove_known_trial_form_fields_welcoming, namespace_id: anything)
             expect(controller).to receive(:record_experiment_conversion_event).with(:remove_known_trial_form_fields_welcoming)
 
-            subject
+            post_create
           end
         end
 
         it 'creates a group' do
-          expect { subject }.to change { Group.count }.by(1)
+          expect { post_create }.to change { Group.count }.by(1)
+        end
+
+        it 'passes create_event: true to the Groups::CreateService' do
+          expect(Groups::CreateService).to receive(:new).with(user, ActionController::Parameters.new(group_params.merge(create_event: true)).permit!).and_call_original
+
+          post_create
         end
 
         it 'tracks create events for the combined_registration experiment' do
@@ -79,7 +85,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
             expect(e).to receive(:track).with(:create_project, namespace: an_instance_of(Group))
           end
 
-          subject
+          post_create
         end
       end
 
@@ -95,7 +101,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
         let(:group_params) { { name: '', path: '' } }
 
         it 'does not create a group', :aggregate_failures do
-          expect { subject }.not_to change { Group.count }
+          expect { post_create }.not_to change { Group.count }
           expect(assigns(:group).errors).not_to be_blank
         end
 
@@ -105,11 +111,11 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
             expect(e).not_to receive(:track).with(:create_project)
           end
 
-          subject
+          post_create
         end
 
         it 'the project is not disgarded completely' do
-          subject
+          post_create
 
           expect(assigns(:project).name).to eq('New project')
         end
@@ -122,8 +128,8 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
         let(:project_params) { { name: '', path: '', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
 
         it 'does not create a project', :aggregate_failures do
-          expect { subject }.to change { Group.count }
-          expect { subject }.not_to change { Project.count }
+          expect { post_create }.to change { Group.count }
+          expect { post_create }.not_to change { Project.count }
           expect(assigns(:project).errors).not_to be_blank
         end
 
@@ -133,7 +139,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
             expect(e).not_to receive(:track).with(:create_project)
           end
 
-          subject
+          post_create
         end
 
         it { is_expected.to have_gitlab_http_status(:ok) }
@@ -148,8 +154,8 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
         let(:group_params) { { id: group.id } }
 
         it 'creates a project and not another group', :aggregate_failures do
-          expect { subject }.to change { Project.count }
-          expect { subject }.not_to change { Group.count }
+          expect { post_create }.to change { Project.count }
+          expect { post_create }.not_to change { Group.count }
         end
 
         it 'selectively tracks events for the combined_registration experiment' do
@@ -162,7 +168,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
             expect(e).to receive(:track).with(:create_project, namespace: an_instance_of(Group))
           end
 
-          subject
+          post_create
         end
 
         context 'it redirects' do
@@ -193,7 +199,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
         allow(controller).to receive(:record_experiment_conversion_event).and_call_original
 
         allow(Groups::CreateService).to receive(:new).and_call_original
-        allow(Groups::CreateService).to receive(:new).with(user, ActionController::Parameters.new(group_params).permit!).and_return(create_service)
+        allow(Groups::CreateService).to receive(:new).with(user, ActionController::Parameters.new(group_params.merge(create_event: true)).permit!).and_return(create_service)
         allow(create_service).to receive(:execute).and_return(namespace)
       end
     end
@@ -214,10 +220,10 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
   end
 
   describe 'POST #import' do
-    subject { post :import, params: params }
+    subject(:post_import) { post :import, params: params }
 
     let(:params) { { group: group_params, import_url: new_import_github_path } }
-    let(:group_params) { { name: 'Group name', path: 'group-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE, emails: ['', ''] } }
+    let(:group_params) { { name: 'Group name', path: 'group-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s } }
     let(:dev_env_or_com) { true }
 
     context 'with an unauthenticated user' do
@@ -243,7 +249,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
         it "doesn't track for the combined_registration experiment" do
           expect(experiment(:combined_registration)).not_to track(:create_group)
 
-          subject
+          post_import
         end
 
         it { is_expected.to render_template(:new) }
@@ -260,14 +266,20 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
 
       context 'when group can be created' do
         it 'creates a group' do
-          expect { subject }.to change { Group.count }.by(1)
+          expect { post_import }.to change { Group.count }.by(1)
+        end
+
+        it 'passes create_event: true to the Groups::CreateService' do
+          expect(Groups::CreateService).to receive(:new).with(user, ActionController::Parameters.new(group_params.merge(create_event: true)).permit!).and_call_original
+
+          post_import
         end
 
         it 'tracks an event for the combined_registration experiment' do
           expect(experiment(:combined_registration)).to track(:create_group, namespace: an_instance_of(Group))
                                                     .on_next_instance
 
-          subject
+          post_import
         end
 
         it 'redirects to the import url with a namespace_id parameter' do
@@ -275,7 +287,7 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
             allow(service).to receive(:execute).and_return(group)
           end
 
-          expect(subject).to redirect_to(new_import_github_url(namespace_id: group.id))
+          expect(post_import).to redirect_to(new_import_github_url(namespace_id: group.id))
         end
       end
     end

ee/spec/services/groups/create_service_spec.rb

@@ -131,6 +131,38 @@ RSpec.describe Groups::CreateService, '#execute' do
     end
   end
 
+  context 'when create_event is true' do
+    subject(:execute) { described_class.new(user, group_params.merge(create_event: true)).execute }
+
+    it 'enqueues a create event worker' do
+      expect(Groups::CreateEventWorker).to receive(:perform_async).with(anything, user.id, :created)
+
+      execute
+    end
+
+    context 'when user can not create a group' do
+      before do
+        user.update_attribute(:can_create_group, false)
+      end
+
+      it "doesn't enqueue a create event worker" do
+        expect(Groups::CreateEventWorker).not_to receive(:perform_async)
+
+        execute
+      end
+    end
+  end
+
+  context 'when create_event is NOT true' do
+    subject(:execute) { described_class.new(user, group_params).execute }
+
+    it "doesn't enqueue a create event worker" do
+      expect(Groups::CreateEventWorker).not_to receive(:perform_async)
+
+      execute
+    end
+  end
+
   def create_group(user, opts)
     described_class.new(user, opts).execute
   end

ee/spec/workers/groups/create_event_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::CreateEventWorker do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+
+  subject(:worker) { described_class.new }
+
+  it 'creats an event' do
+    expect do
+      worker.perform(group.id, user.id, :created)
+    end.to change(Event, :count).by(1)
+  end
+
+  it 'passes the correct arguments' do
+    expect(Event).to receive(:create!).with(
+      group_id: group.id,
+      action: :created,
+      author_id: user.id
+    )
+
+    worker.perform(group.id, user.id, :created)
+  end
+end