Commit ecbb690b authored by Jay's avatar Jay Committed by Dmitry Gruzd

Flag namespaces created during onboarding

parent afbd063f
......@@ -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)
......
......@@ -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
......
......@@ -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)
......
......@@ -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
......@@ -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?
......
......@@ -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
......
# 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
......@@ -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' } }
......
......@@ -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
......
......@@ -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
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment