Merge branch 'fix/namespace-existence-check' into 'master'

Fix namespace validation (unique path) on group creation

See merge request gitlab-org/gitlab!57563

app/assets/javascripts/group.js

@@ -16,9 +16,7 @@ export default class Group {
       if (groupName.value === '') {
         groupName.addEventListener('keyup', this.updateHandler);
 
-        if (!this.parentId.value) {
-          groupName.addEventListener('blur', this.updateGroupPathSlugHandler);
-        }
+        groupName.addEventListener('blur', this.updateGroupPathSlugHandler);
       }
     });
 
@@ -53,7 +51,7 @@ export default class Group {
     const slug = this.groupPaths[0]?.value || slugify(value);
     if (!slug) return;
 
-    fetchGroupPathAvailability(slug)
+    fetchGroupPathAvailability(slug, this.parentId?.value)
       .then(({ data }) => data)
       .then(({ exists, suggests }) => {
         if (exists && suggests.length) {

app/assets/javascripts/pages/groups/new/fetch_group_path_availability.js

+import { buildApiUrl } from '~/api/api_utils';
 import axios from '~/lib/utils/axios_utils';
 
-const rootUrl = gon.relative_url_root;
+const NAMESPACE_EXISTS_PATH = '/api/:version/namespaces/:id/exists';
 
-export default function fetchGroupPathAvailability(groupPath) {
-  return axios.get(`${rootUrl}/users/${groupPath}/suggests`);
+export default function fetchGroupPathAvailability(groupPath, parentId) {
+  const url = buildApiUrl(NAMESPACE_EXISTS_PATH).replace(':id', encodeURIComponent(groupPath));
+
+  return axios.get(url, {
+    params: { parent_id: parentId },
+  });
 }

app/assets/javascripts/pages/groups/new/group_path_validator.js

@@ -8,6 +8,7 @@ import fetchGroupPathAvailability from './fetch_group_path_availability';
 const debounceTimeoutDuration = 1000;
 const invalidInputClass = 'gl-field-error-outline';
 const successInputClass = 'gl-field-success-outline';
+const parentIdSelector = 'group_parent_id';
 const successMessageSelector = '.validation-success';
 const pendingMessageSelector = '.validation-pending';
 const unavailableMessageSelector = '.validation-error';
@@ -20,9 +21,10 @@ export default class GroupPathValidator extends InputValidator {
 
     const container = opts.container || '';
     const validateElements = document.querySelectorAll(`${container} .js-validate-group-path`);
+    const parentIdElement = document.getElementById(parentIdSelector);
 
     this.debounceValidateInput = debounce((inputDomElement) => {
-      GroupPathValidator.validateGroupPathInput(inputDomElement);
+      GroupPathValidator.validateGroupPathInput(inputDomElement, parentIdElement);
     }, debounceTimeoutDuration);
 
     validateElements.forEach((element) =>
@@ -37,13 +39,14 @@ export default class GroupPathValidator extends InputValidator {
     this.debounceValidateInput(inputDomElement);
   }
 
-  static validateGroupPathInput(inputDomElement) {
+  static validateGroupPathInput(inputDomElement, parentIdElement) {
     const groupPath = inputDomElement.value;
+    const parentId = parentIdElement.value;
 
     if (inputDomElement.checkValidity() && groupPath.length > 1) {
       GroupPathValidator.setMessageVisibility(inputDomElement, pendingMessageSelector);
 
-      fetchGroupPathAvailability(groupPath)
+      fetchGroupPathAvailability(groupPath, parentId)
         .then(({ data }) => data)
         .then((data) => {
           GroupPathValidator.setInputState(inputDomElement, !data.exists);

app/assets/javascripts/pages/groups/new/index.js

@@ -5,10 +5,8 @@ import Group from '~/group';
 import LinkedTabs from '~/lib/utils/bootstrap_linked_tabs';
 import GroupPathValidator from './group_path_validator';
 
-const parentId = $('#group_parent_id');
-if (!parentId.val()) {
-  new GroupPathValidator(); // eslint-disable-line no-new
-}
+new GroupPathValidator(); // eslint-disable-line no-new
+
 BindInOut.initAll();
 initFilePickers();
 

app/models/namespace.rb

@@ -92,6 +92,8 @@ class Namespace < ApplicationRecord
   scope :for_user, -> { where('type IS NULL') }
   scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) }
   scope :include_route, -> { includes(:route) }
+  scope :by_parent, -> (parent) { where(parent_id: parent) }
+  scope :filter_by_path, -> (query) { where('lower(path) = :query', query: query.downcase) }
 
   scope :with_statistics, -> do
     joins('LEFT JOIN project_statistics ps ON ps.namespace_id = namespaces.id')

changelogs/unreleased/fix-namespace-existence-check.yml

+---
+title: Fix namespace validation (unique path) on group creation
+merge_request: 57563
+author: Jonas Wälter @wwwjon
+type: fixed

doc/api/namespaces.md

@@ -225,3 +225,33 @@ Example response:
   "trial": false
 }
 ```
+
+## Get existence of a namespace
+
+Get existence of a namespace by path. Suggests a new namespace path that does not already exist.
+
+```plaintext
+GET /namespaces/:namespace/exists
+```
+
+| Attribute   | Type    | Required | Description |
+| ----------- | ------- | -------- | ----------- |
+| `namespace` | string  | yes      | Namespace's path. |
+| `parent_id` | integer | no       | The ID of the parent namespace. If no ID is specified, only top-level namespaces are considered. |
+
+Example request:
+
+```shell
+curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/namespaces/my-group/exists?parent_id=1"
+```
+
+Example response:
+
+```json
+{
+    "exists": true,
+    "suggests": [
+        "my-group1"
+    ]
+}
+```

lib/api/entities/namespace_existence.rb

+# frozen_string_literal: true
+
+module API
+  module Entities
+    class NamespaceExistence < Grape::Entity
+      expose :exists, :suggests
+    end
+  end
+end

lib/api/namespaces.rb

@@ -56,6 +56,23 @@ module API
 
         present user_namespace, with: Entities::Namespace, current_user: current_user
       end
+
+      desc 'Get existence of a namespace including alternative suggestions' do
+        success Entities::NamespaceExistence
+      end
+      params do
+        requires :namespace, type: String, desc: "Namespace's path"
+        optional :parent_id, type: Integer, desc: "The ID of the parent namespace. If no ID is specified, only top-level namespaces are considered."
+      end
+      get ':namespace/exists', requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+        namespace_path = params[:namespace]
+
+        exists = Namespace.by_parent(params[:parent_id]).filter_by_path(namespace_path).exists?
+        suggestions = exists ? [Namespace.clean_path(namespace_path)] : []
+
+        present :exists, exists
+        present :suggests, suggestions
+      end
     end
   end
 end

spec/features/groups_spec.rb

@@ -230,6 +230,29 @@ RSpec.describe 'Group' do
         end
       end
     end
+
+    describe 'real-time group url validation', :js do
+      let_it_be(:subgroup) { create(:group, path: 'sub', parent: group) }
+
+      before do
+        group.add_owner(user)
+        visit new_group_path(parent_id: group.id)
+      end
+
+      it 'shows a message if group url is available' do
+        fill_in 'Group URL', with: group.path
+        wait_for_requests
+
+        expect(page).to have_content('Group path is available')
+      end
+
+      it 'shows an error if group url is taken' do
+        fill_in 'Group URL', with: subgroup.path
+        wait_for_requests
+
+        expect(page).to have_content('Group path is already taken')
+      end
+    end
   end
 
   it 'checks permissions to avoid exposing groups by parent_id' do

spec/models/namespace_spec.rb

@@ -155,6 +155,33 @@ RSpec.describe Namespace do
     end
   end
 
+  describe 'scopes' do
+    let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') }
+    let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') }
+    let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace1) }
+    let_it_be(:namespace2sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace2) }
+
+    describe '.by_parent' do
+      it 'includes correct namespaces' do
+        expect(described_class.by_parent(namespace1.id)).to eq([namespace1sub])
+        expect(described_class.by_parent(namespace2.id)).to eq([namespace2sub])
+        expect(described_class.by_parent(nil)).to match_array([namespace, namespace1, namespace2])
+      end
+    end
+
+    describe '.filter_by_path' do
+      it 'includes correct namespaces' do
+        expect(described_class.filter_by_path(namespace1.path)).to eq([namespace1])
+        expect(described_class.filter_by_path(namespace2.path)).to eq([namespace2])
+        expect(described_class.filter_by_path('sub-namespace')).to match_array([namespace1sub, namespace2sub])
+      end
+
+      it 'filters case-insensitive' do
+        expect(described_class.filter_by_path(namespace1.path.upcase)).to eq([namespace1])
+      end
+    end
+  end
+
   describe 'delegate' do
     it { is_expected.to delegate_method(:name).to(:owner).with_prefix.with_arguments(allow_nil: true) }
     it { is_expected.to delegate_method(:avatar_url).to(:owner).with_arguments(allow_nil: true) }

spec/requests/api/namespaces_spec.rb

@@ -216,4 +216,77 @@ RSpec.describe API::Namespaces do
       end
     end
   end
+
+  describe 'GET /namespaces/:namespace/exists' do
+    let!(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') }
+    let!(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') }
+    let!(:namespace1sub) { create(:group, name: 'Sub Namespace 1', path: 'sub-namespace-1', parent: namespace1) }
+    let!(:namespace2sub) { create(:group, name: 'Sub Namespace 2', path: 'sub-namespace-2', parent: namespace2) }
+
+    context 'when unauthenticated' do
+      it 'returns authentication error' do
+        get api("/namespaces/#{namespace1.path}/exists")
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+
+    context 'when authenticated' do
+      it 'returns JSON indicating the namespace exists and a suggestion' do
+        get api("/namespaces/#{namespace1.path}/exists", user)
+
+        expected_json = { exists: true, suggests: ["#{namespace1.path}1"] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+
+      it 'returns JSON indicating the namespace does not exist without a suggestion' do
+        get api("/namespaces/non-existing-namespace/exists", user)
+
+        expected_json = { exists: false, suggests: [] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+
+      it 'checks the existence of a namespace in case-insensitive manner' do
+        get api("/namespaces/#{namespace1.path.upcase}/exists", user)
+
+        expected_json = { exists: true, suggests: ["#{namespace1.path.upcase}1"] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+
+      it 'checks the existence within the parent namespace only' do
+        get api("/namespaces/#{namespace1sub.path}/exists", user), params: { parent_id: namespace1.id }
+
+        expected_json = { exists: true, suggests: ["#{namespace1sub.path}1"] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+
+      it 'ignores nested namespaces when checking for top-level namespace' do
+        get api("/namespaces/#{namespace1sub.path}/exists", user)
+
+        expected_json = { exists: false, suggests: [] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+
+      it 'ignores top-level namespaces when checking with parent_id' do
+        get api("/namespaces/#{namespace1.path}/exists", user), params: { parent_id: namespace1.id }
+
+        expected_json = { exists: false, suggests: [] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+
+      it 'ignores namespaces of other parent namespaces when checking with parent_id' do
+        get api("/namespaces/#{namespace2sub.path}/exists", user), params: { parent_id: namespace1.id }
+
+        expected_json = { exists: false, suggests: [] }.to_json
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.body).to eq(expected_json)
+      end
+    end
+  end
 end