Merge branch '234091-gitlab-com-new-paid-subscription-purchase-flow-throwing-error' into 'master'

Fix group name bug for new purchase flow

See merge request gitlab-org/gitlab!39915

app/models/namespace.rb

@@ -135,6 +135,10 @@ class Namespace < ApplicationRecord
       uniquify.string(path) { |s| Namespace.find_by_path_or_name(s) }
     end
 
+    def clean_name(value)
+      value.scan(Gitlab::Regex.group_name_regex_chars).join(' ')
+    end
+
     def find_by_pages_host(host)
       gitlab_host = "." + Settings.pages.host.downcase
       host = host.downcase

ee/app/controllers/subscriptions_controller.rb

@@ -44,9 +44,9 @@ class SubscriptionsController < ApplicationController
     if params[:selected_group]
       group = current_user.manageable_groups_eligible_for_subscription.find(params[:selected_group])
     else
-      group_name = params[:setup_for_company] ? customer_params[:company] : "#{current_user.name}'s Group"
-      path = Namespace.clean_path(group_name)
-      group = Groups::CreateService.new(current_user, name: group_name, path: path).execute
+      name = Namespace.clean_name(params[:setup_for_company] ? customer_params[:company] : current_user.name)
+      path = Namespace.clean_path(name)
+      group = Groups::CreateService.new(current_user, name: name, path: path).execute
       return render json: group.errors.to_json unless group.persisted?
     end
 

ee/changelogs/unreleased/234091-gitlab-com-new-paid-subscription-purchase-flow-throwing-error.yml

+---
+title: Fix group name bug for new purchase flow
+merge_request: 39915
+author:
+type: fixed

ee/spec/controllers/subscriptions_controller_spec.rb

@@ -114,6 +114,13 @@ RSpec.describe SubscriptionsController do
         it 'updates the setup_for_company attribute of the current user' do
           expect { subject }.to change { user.reload.setup_for_company }.from(nil).to(true)
         end
+
+        it 'creates a group based on the company' do
+          expect(Namespace).to receive(:clean_name).with(params.dig(:customer, :company)).and_call_original
+          expect_any_instance_of(EE::Groups::CreateService).to receive(:execute)
+
+          subject
+        end
       end
 
       context 'when not setting up for a company' do
@@ -130,12 +137,13 @@ RSpec.describe SubscriptionsController do
         it 'does not update the setup_for_company attribute of the current user' do
           expect { subject }.not_to change { user.reload.setup_for_company }
         end
-      end
 
-      it 'creates a group' do
-        expect_any_instance_of(EE::Groups::CreateService).to receive(:execute)
+        it 'creates a group based on the user' do
+          expect(Namespace).to receive(:clean_name).with(user.name).and_call_original
+          expect_any_instance_of(EE::Groups::CreateService).to receive(:execute)
 
-        subject
+          subject
+        end
       end
 
       context 'when an error occurs creating a group' do

lib/gitlab/regex.rb

@@ -122,7 +122,11 @@ module Gitlab
     end
 
     def group_name_regex
-      @group_name_regex ||= /\A[\p{Alnum}\u{00A9}-\u{1f9ff}_][\p{Alnum}\p{Pd}\u{00A9}-\u{1f9ff}_()\. ]*\z/.freeze
+      @group_name_regex ||= /\A#{group_name_regex_chars}\z/.freeze
+    end
+
+    def group_name_regex_chars
+      @group_name_regex_chars ||= /[\p{Alnum}\u{00A9}-\u{1f9ff}_][\p{Alnum}\p{Pd}\u{00A9}-\u{1f9ff}_()\. ]*/.freeze
     end
 
     def group_name_regex_message

spec/lib/gitlab/regex_spec.rb

@@ -3,14 +3,19 @@
 require 'fast_spec_helper'
 
 RSpec.describe Gitlab::Regex do
-  shared_examples_for 'project/group name regex' do
+  shared_examples_for 'project/group name chars regex' do
     it { is_expected.to match('gitlab-ce') }
     it { is_expected.to match('GitLab CE') }
     it { is_expected.to match('100 lines') }
     it { is_expected.to match('gitlab.git') }
     it { is_expected.to match('Český název') }
     it { is_expected.to match('Dash – is this') }
+  end
+
+  shared_examples_for 'project/group name regex' do
+    it_behaves_like 'project/group name chars regex'
     it { is_expected.not_to match('?gitlab') }
+    it { is_expected.not_to match("Users's something") }
   end
 
   describe '.project_name_regex' do
@@ -33,6 +38,16 @@ RSpec.describe Gitlab::Regex do
     end
   end
 
+  describe '.group_name_regex_chars' do
+    subject { described_class.group_name_regex_chars }
+
+    it_behaves_like 'project/group name chars regex'
+
+    it 'allows partial matches' do
+      is_expected.to match(',Valid name wrapped in ivalid chars&')
+    end
+  end
+
   describe '.project_name_regex_message' do
     subject { described_class.project_name_regex_message }
 

spec/models/namespace_spec.rb

@@ -588,6 +588,21 @@ RSpec.describe Namespace do
     end
   end
 
+  describe ".clean_name" do
+    context "when the name complies with the group name regex" do
+      it "returns the name as is" do
+        valid_name = "Hello - World _ (Hi.)"
+        expect(described_class.clean_name(valid_name)).to eq(valid_name)
+      end
+    end
+
+    context "when the name does not comply with the group name regex" do
+      it "sanitizes the name by replacing all invalid char sequences with a space" do
+        expect(described_class.clean_name("Green'! Test~~~")).to eq("Green Test")
+      end
+    end
+  end
+
   describe "#default_branch_protection" do
     let(:namespace) { create(:namespace) }
     let(:default_branch_protection) { nil }