Commit ff0a23b7 authored by Vladlena Shumilo's avatar Vladlena Shumilo Committed by Peter Leitzen

Fix group name bug for new purchase flow

Update the logic that assigns a default group
name for the new signup flow
parent 666187aa
...@@ -135,6 +135,10 @@ class Namespace < ApplicationRecord ...@@ -135,6 +135,10 @@ class Namespace < ApplicationRecord
uniquify.string(path) { |s| Namespace.find_by_path_or_name(s) } uniquify.string(path) { |s| Namespace.find_by_path_or_name(s) }
end end
def clean_name(value)
value.scan(Gitlab::Regex.group_name_regex_chars).join(' ')
end
def find_by_pages_host(host) def find_by_pages_host(host)
gitlab_host = "." + Settings.pages.host.downcase gitlab_host = "." + Settings.pages.host.downcase
host = host.downcase host = host.downcase
......
...@@ -44,9 +44,9 @@ class SubscriptionsController < ApplicationController ...@@ -44,9 +44,9 @@ class SubscriptionsController < ApplicationController
if params[:selected_group] if params[:selected_group]
group = current_user.manageable_groups_eligible_for_subscription.find(params[:selected_group]) group = current_user.manageable_groups_eligible_for_subscription.find(params[:selected_group])
else else
group_name = params[:setup_for_company] ? customer_params[:company] : "#{current_user.name}'s Group" name = Namespace.clean_name(params[:setup_for_company] ? customer_params[:company] : current_user.name)
path = Namespace.clean_path(group_name) path = Namespace.clean_path(name)
group = Groups::CreateService.new(current_user, name: group_name, path: path).execute group = Groups::CreateService.new(current_user, name: name, path: path).execute
return render json: group.errors.to_json unless group.persisted? return render json: group.errors.to_json unless group.persisted?
end end
......
---
title: Fix group name bug for new purchase flow
merge_request: 39915
author:
type: fixed
...@@ -114,6 +114,13 @@ RSpec.describe SubscriptionsController do ...@@ -114,6 +114,13 @@ RSpec.describe SubscriptionsController do
it 'updates the setup_for_company attribute of the current user' 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) expect { subject }.to change { user.reload.setup_for_company }.from(nil).to(true)
end 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 end
context 'when not setting up for a company' do context 'when not setting up for a company' do
...@@ -130,13 +137,14 @@ RSpec.describe SubscriptionsController do ...@@ -130,13 +137,14 @@ RSpec.describe SubscriptionsController do
it 'does not update the setup_for_company attribute of the current user' 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 } expect { subject }.not_to change { user.reload.setup_for_company }
end end
end
it 'creates a group' do 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) expect_any_instance_of(EE::Groups::CreateService).to receive(:execute)
subject subject
end end
end
context 'when an error occurs creating a group' do context 'when an error occurs creating a group' do
let(:group) { Group.new(path: 'foo') } let(:group) { Group.new(path: 'foo') }
......
...@@ -122,7 +122,11 @@ module Gitlab ...@@ -122,7 +122,11 @@ module Gitlab
end end
def group_name_regex 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 end
def group_name_regex_message def group_name_regex_message
......
...@@ -3,14 +3,19 @@ ...@@ -3,14 +3,19 @@
require 'fast_spec_helper' require 'fast_spec_helper'
RSpec.describe Gitlab::Regex do 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('GitLab CE') } it { is_expected.to match('GitLab CE') }
it { is_expected.to match('100 lines') } it { is_expected.to match('100 lines') }
it { is_expected.to match('gitlab.git') } it { is_expected.to match('gitlab.git') }
it { is_expected.to match('Český název') } it { is_expected.to match('Český název') }
it { is_expected.to match('Dash – is this') } 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('?gitlab') }
it { is_expected.not_to match("Users's something") }
end end
describe '.project_name_regex' do describe '.project_name_regex' do
...@@ -33,6 +38,16 @@ RSpec.describe Gitlab::Regex do ...@@ -33,6 +38,16 @@ RSpec.describe Gitlab::Regex do
end end
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 describe '.project_name_regex_message' do
subject { described_class.project_name_regex_message } subject { described_class.project_name_regex_message }
......
...@@ -588,6 +588,21 @@ RSpec.describe Namespace do ...@@ -588,6 +588,21 @@ RSpec.describe Namespace do
end end
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 describe "#default_branch_protection" do
let(:namespace) { create(:namespace) } let(:namespace) { create(:namespace) }
let(:default_branch_protection) { nil } let(:default_branch_protection) { nil }
......
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