Commit 2f88963a authored by Jonas Wälter's avatar Jonas Wälter Committed by Mike Jang

Fix namespace validation (unique path) on group creation

parent 69f6e347
...@@ -16,10 +16,8 @@ export default class Group { ...@@ -16,10 +16,8 @@ export default class Group {
if (groupName.value === '') { if (groupName.value === '') {
groupName.addEventListener('keyup', this.updateHandler); groupName.addEventListener('keyup', this.updateHandler);
if (!this.parentId.value) {
groupName.addEventListener('blur', this.updateGroupPathSlugHandler); groupName.addEventListener('blur', this.updateGroupPathSlugHandler);
} }
}
}); });
this.groupPaths.forEach((groupPath) => { this.groupPaths.forEach((groupPath) => {
...@@ -53,7 +51,7 @@ export default class Group { ...@@ -53,7 +51,7 @@ export default class Group {
const slug = this.groupPaths[0]?.value || slugify(value); const slug = this.groupPaths[0]?.value || slugify(value);
if (!slug) return; if (!slug) return;
fetchGroupPathAvailability(slug) fetchGroupPathAvailability(slug, this.parentId?.value)
.then(({ data }) => data) .then(({ data }) => data)
.then(({ exists, suggests }) => { .then(({ exists, suggests }) => {
if (exists && suggests.length) { if (exists && suggests.length) {
......
import { buildApiUrl } from '~/api/api_utils';
import axios from '~/lib/utils/axios_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) { export default function fetchGroupPathAvailability(groupPath, parentId) {
return axios.get(`${rootUrl}/users/${groupPath}/suggests`); const url = buildApiUrl(NAMESPACE_EXISTS_PATH).replace(':id', encodeURIComponent(groupPath));
return axios.get(url, {
params: { parent_id: parentId },
});
} }
...@@ -8,6 +8,7 @@ import fetchGroupPathAvailability from './fetch_group_path_availability'; ...@@ -8,6 +8,7 @@ import fetchGroupPathAvailability from './fetch_group_path_availability';
const debounceTimeoutDuration = 1000; const debounceTimeoutDuration = 1000;
const invalidInputClass = 'gl-field-error-outline'; const invalidInputClass = 'gl-field-error-outline';
const successInputClass = 'gl-field-success-outline'; const successInputClass = 'gl-field-success-outline';
const parentIdSelector = 'group_parent_id';
const successMessageSelector = '.validation-success'; const successMessageSelector = '.validation-success';
const pendingMessageSelector = '.validation-pending'; const pendingMessageSelector = '.validation-pending';
const unavailableMessageSelector = '.validation-error'; const unavailableMessageSelector = '.validation-error';
...@@ -20,9 +21,10 @@ export default class GroupPathValidator extends InputValidator { ...@@ -20,9 +21,10 @@ export default class GroupPathValidator extends InputValidator {
const container = opts.container || ''; const container = opts.container || '';
const validateElements = document.querySelectorAll(`${container} .js-validate-group-path`); const validateElements = document.querySelectorAll(`${container} .js-validate-group-path`);
const parentIdElement = document.getElementById(parentIdSelector);
this.debounceValidateInput = debounce((inputDomElement) => { this.debounceValidateInput = debounce((inputDomElement) => {
GroupPathValidator.validateGroupPathInput(inputDomElement); GroupPathValidator.validateGroupPathInput(inputDomElement, parentIdElement);
}, debounceTimeoutDuration); }, debounceTimeoutDuration);
validateElements.forEach((element) => validateElements.forEach((element) =>
...@@ -37,13 +39,14 @@ export default class GroupPathValidator extends InputValidator { ...@@ -37,13 +39,14 @@ export default class GroupPathValidator extends InputValidator {
this.debounceValidateInput(inputDomElement); this.debounceValidateInput(inputDomElement);
} }
static validateGroupPathInput(inputDomElement) { static validateGroupPathInput(inputDomElement, parentIdElement) {
const groupPath = inputDomElement.value; const groupPath = inputDomElement.value;
const parentId = parentIdElement.value;
if (inputDomElement.checkValidity() && groupPath.length > 1) { if (inputDomElement.checkValidity() && groupPath.length > 1) {
GroupPathValidator.setMessageVisibility(inputDomElement, pendingMessageSelector); GroupPathValidator.setMessageVisibility(inputDomElement, pendingMessageSelector);
fetchGroupPathAvailability(groupPath) fetchGroupPathAvailability(groupPath, parentId)
.then(({ data }) => data) .then(({ data }) => data)
.then((data) => { .then((data) => {
GroupPathValidator.setInputState(inputDomElement, !data.exists); GroupPathValidator.setInputState(inputDomElement, !data.exists);
......
...@@ -5,10 +5,8 @@ import Group from '~/group'; ...@@ -5,10 +5,8 @@ import Group from '~/group';
import LinkedTabs from '~/lib/utils/bootstrap_linked_tabs'; import LinkedTabs from '~/lib/utils/bootstrap_linked_tabs';
import GroupPathValidator from './group_path_validator'; import GroupPathValidator from './group_path_validator';
const parentId = $('#group_parent_id'); new GroupPathValidator(); // eslint-disable-line no-new
if (!parentId.val()) {
new GroupPathValidator(); // eslint-disable-line no-new
}
BindInOut.initAll(); BindInOut.initAll();
initFilePickers(); initFilePickers();
......
...@@ -92,6 +92,8 @@ class Namespace < ApplicationRecord ...@@ -92,6 +92,8 @@ class Namespace < ApplicationRecord
scope :for_user, -> { where('type IS NULL') } scope :for_user, -> { where('type IS NULL') }
scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) } scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) }
scope :include_route, -> { includes(:route) } 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 scope :with_statistics, -> do
joins('LEFT JOIN project_statistics ps ON ps.namespace_id = namespaces.id') joins('LEFT JOIN project_statistics ps ON ps.namespace_id = namespaces.id')
......
---
title: Fix namespace validation (unique path) on group creation
merge_request: 57563
author: Jonas Wälter @wwwjon
type: fixed
...@@ -225,3 +225,33 @@ Example response: ...@@ -225,3 +225,33 @@ Example response:
"trial": false "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"
]
}
```
# frozen_string_literal: true
module API
module Entities
class NamespaceExistence < Grape::Entity
expose :exists, :suggests
end
end
end
...@@ -56,6 +56,23 @@ module API ...@@ -56,6 +56,23 @@ module API
present user_namespace, with: Entities::Namespace, current_user: current_user present user_namespace, with: Entities::Namespace, current_user: current_user
end 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 end
end end
...@@ -230,6 +230,29 @@ RSpec.describe 'Group' do ...@@ -230,6 +230,29 @@ RSpec.describe 'Group' do
end end
end 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 end
it 'checks permissions to avoid exposing groups by parent_id' do it 'checks permissions to avoid exposing groups by parent_id' do
......
...@@ -155,6 +155,33 @@ RSpec.describe Namespace do ...@@ -155,6 +155,33 @@ RSpec.describe Namespace do
end end
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 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(: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) } it { is_expected.to delegate_method(:avatar_url).to(:owner).with_arguments(allow_nil: true) }
......
...@@ -216,4 +216,77 @@ RSpec.describe API::Namespaces do ...@@ -216,4 +216,77 @@ RSpec.describe API::Namespaces do
end end
end 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 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