From b8e918d52b0fb28f48e5332d3a4314062f514508 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Wed, 4 Sep 2024 13:32:15 +0200 Subject: [PATCH 1/3] Remove organization validation exception from GitlabSubscriptions::Trials::CreateService --- .../gitlab_subscriptions/trials/create_service.rb | 5 ++--- .../trials/create_service_spec.rb | 13 ++++++++++--- .../disable_namespace_organization_validation.yml | 1 - 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ee/app/services/gitlab_subscriptions/trials/create_service.rb b/ee/app/services/gitlab_subscriptions/trials/create_service.rb index fb37fc79b45fb6..c56fddcfd81538 100644 --- a/ee/app/services/gitlab_subscriptions/trials/create_service.rb +++ b/ee/app/services/gitlab_subscriptions/trials/create_service.rb @@ -28,9 +28,8 @@ def create_group_flow name = ActionController::Base.helpers.sanitize(trial_params[:new_group_name]) path = Namespace.clean_path(name.parameterize) create_service_params = { name: name, path: path, organization_id: trial_params[:organization_id] } - response = Namespace.with_disabled_organization_validation do - Groups::CreateService.new(user, create_service_params).execute - end + response = Groups::CreateService.new(user, create_service_params).execute + @namespace = response[:group] if response.success? diff --git a/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb b/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb index 5d474c64ff4c62..ab8064a4aee147 100644 --- a/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb @@ -6,6 +6,7 @@ include TrialHelpers let_it_be(:user, reload: true) { create(:user) } + let_it_be(:organization) { create(:organization, users: [user]) } let(:step) { described_class::LEAD } describe '#execute', :saas do @@ -32,7 +33,9 @@ context 'when in the create group flow' do let(:step) { described_class::TRIAL } let(:extra_params) { { trial_entity: '_entity_' } } - let(:trial_params) { { new_group_name: 'gitlab', namespace_id: '0' }.merge(extra_params) } + let(:trial_params) do + { new_group_name: 'gitlab', namespace_id: '0', organization_id: organization.id }.merge(extra_params) + end context 'when group is successfully created' do context 'when trial creation is successful' do @@ -49,6 +52,8 @@ end context 'when trial creation fails' do + let(:extra_params) { super().merge(organization_id: organization.id) } + it 'returns an error indicating trial failed' do stub_apply_trial( user, namespace_id: anything, success: false, extra_params: extra_params.merge(new_group_attrs) @@ -62,6 +67,8 @@ end context 'when group name needs sanitized' do + let(:extra_params) { super().merge(organization_id: organization.id) } + it 'return success with the namespace path sanitized for duplication' do create(:group_with_plan, plan: :free_plan, name: 'gitlab') @@ -94,7 +101,7 @@ context 'when group creation had an error' do context 'when there are invalid characters used' do - let(:trial_params) { { new_group_name: ' _invalid_ ', namespace_id: '0' } } + let(:trial_params) { { new_group_name: ' _invalid_ ', namespace_id: '0', organization_id: organization.id } } it 'returns namespace_create_failed' do expect(apply_trial_service_class).not_to receive(:new) @@ -108,7 +115,7 @@ end context 'when name is entered with blank spaces' do - let(:trial_params) { { new_group_name: ' ', namespace_id: '0' } } + let(:trial_params) { { new_group_name: ' ', namespace_id: '0', organization_id: organization.id } } it 'returns namespace_create_failed' do expect(apply_trial_service_class).not_to receive(:new) diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index dcc1fb0ea06f1e..852457c3122c72 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -12,7 +12,6 @@ - ee/spec/services/ee/users/create_service_spec.rb - ee/spec/services/users/service_accounts/create_service_spec.rb - ee/spec/services/epics/update_dates_service_spec.rb -- ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb - spec/controllers/admin/groups_controller_spec.rb - spec/controllers/admin/users_controller_spec.rb - spec/controllers/groups_controller_spec.rb -- GitLab From fb0ac5960d2248fcfc9083f21cdb3e5f3b51ed81 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 5 Sep 2024 16:23:47 +0200 Subject: [PATCH 2/3] Simplify test setup --- .../gitlab_subscriptions/trials/create_service_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb b/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb index ab8064a4aee147..4397a772de7edc 100644 --- a/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb @@ -32,7 +32,7 @@ context 'when in the create group flow' do let(:step) { described_class::TRIAL } - let(:extra_params) { { trial_entity: '_entity_' } } + let(:extra_params) { { trial_entity: '_entity_', organization_id: organization.id } } let(:trial_params) do { new_group_name: 'gitlab', namespace_id: '0', organization_id: organization.id }.merge(extra_params) end @@ -52,8 +52,6 @@ end context 'when trial creation fails' do - let(:extra_params) { super().merge(organization_id: organization.id) } - it 'returns an error indicating trial failed' do stub_apply_trial( user, namespace_id: anything, success: false, extra_params: extra_params.merge(new_group_attrs) @@ -67,8 +65,6 @@ end context 'when group name needs sanitized' do - let(:extra_params) { super().merge(organization_id: organization.id) } - it 'return success with the namespace path sanitized for duplication' do create(:group_with_plan, plan: :free_plan, name: 'gitlab') -- GitLab From c9eabb83930f2aae7a162f7c96483ccd30def0aa Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 6 Sep 2024 09:35:42 +0200 Subject: [PATCH 3/3] Remove redundant organization_id key --- .../services/gitlab_subscriptions/trials/create_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb b/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb index 4397a772de7edc..e72c48f0d6657b 100644 --- a/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/trials/create_service_spec.rb @@ -34,7 +34,7 @@ let(:step) { described_class::TRIAL } let(:extra_params) { { trial_entity: '_entity_', organization_id: organization.id } } let(:trial_params) do - { new_group_name: 'gitlab', namespace_id: '0', organization_id: organization.id }.merge(extra_params) + { new_group_name: 'gitlab', namespace_id: '0' }.merge(extra_params) end context 'when group is successfully created' do -- GitLab