diff --git a/changelogs/unreleased/19966-api-call-to-move-project-to-different-group-fails-when-using-group-and-project-names-instead-of-id.yml b/changelogs/unreleased/19966-api-call-to-move-project-to-different-group-fails-when-using-group-and-project-names-instead-of-id.yml
new file mode 100644
index 0000000000000000000000000000000000000000..61cf047026b993d61a023101a8b5d6097a58f0a6
--- /dev/null
+++ b/changelogs/unreleased/19966-api-call-to-move-project-to-different-group-fails-when-using-group-and-project-names-instead-of-id.yml
@@ -0,0 +1,4 @@
+---
+title: Allow group and project paths when transferring projects via the API
+merge_request:
+author:
diff --git a/doc/api/groups.md b/doc/api/groups.md
index bc737bff8eec9fa2a660da4345b67dc761449d91..f7807390e6862f0b67b0acf6eed2d66a82a25296 100644
--- a/doc/api/groups.md
+++ b/doc/api/groups.md
@@ -335,7 +335,7 @@ POST /groups/:id/projects/:project_id
Parameters:
- `id` (required) - The ID or path of a group
-- `project_id` (required) - The ID of a project
+- `project_id` (required) - The ID or path of a project
## Update group
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index e04d2e40fb6f6ed2886d84fe962f2261939f530e..7682d2868665f08fd93812fe5d74cf909a9d05cb 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -156,12 +156,12 @@ module API
success Entities::GroupDetail
end
params do
- requires :project_id, type: String, desc: 'The ID of the project'
+ requires :project_id, type: String, desc: 'The ID or path of the project'
end
post ":id/projects/:project_id" do
authenticated_as_admin!
- group = Group.find_by(id: params[:id])
- project = Project.find(params[:project_id])
+ group = find_group!(params[:id])
+ project = find_project!(params[:project_id])
result = ::Projects::TransferService.new(project, current_user).execute(group)
if result
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 0e8d6faea2731eabcecb541c7292e04bdfc13f1d..e355d5e28bcc9006d4c8b08a33d22c79dac4171a 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -23,6 +23,7 @@ describe API::Groups, api: true do
context "when unauthenticated" do
it "returns authentication error" do
get api("/groups")
+
expect(response).to have_http_status(401)
end
end
@@ -30,6 +31,7 @@ describe API::Groups, api: true do
context "when authenticated as user" do
it "normal user: returns an array of groups of user1" do
get api("/groups", user1)
+
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
@@ -48,6 +50,7 @@ describe API::Groups, api: true do
context "when authenticated as admin" do
it "admin: returns an array of all groups" do
get api("/groups", admin)
+
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(2)
@@ -94,6 +97,7 @@ describe API::Groups, api: true do
it "returns all groups you have access to" do
public_group = create :group, :public
+
get api("/groups", user1), all_available: true
expect(response).to have_http_status(200)
@@ -140,6 +144,7 @@ describe API::Groups, api: true do
context 'when unauthenticated' do
it 'returns authentication error' do
get api('/groups/owned')
+
expect(response).to have_http_status(401)
end
end
@@ -147,6 +152,7 @@ describe API::Groups, api: true do
context 'when authenticated as group owner' do
it 'returns an array of groups the user owns' do
get api('/groups/owned', user2)
+
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.first['name']).to eq(group2.name)
@@ -179,6 +185,7 @@ describe API::Groups, api: true do
it "does not return a non existing group" do
get api("/groups/1328", user1)
+
expect(response).to have_http_status(404)
end
@@ -192,12 +199,14 @@ describe API::Groups, api: true do
context "when authenticated as admin" do
it "returns any existing group" do
get api("/groups/#{group2.id}", admin)
+
expect(response).to have_http_status(200)
expect(json_response['name']).to eq(group2.name)
end
it "does not return a non existing group" do
get api("/groups/1328", admin)
+
expect(response).to have_http_status(404)
end
end
@@ -205,12 +214,14 @@ describe API::Groups, api: true do
context 'when using group path in URL' do
it 'returns any existing group' do
get api("/groups/#{group1.path}", admin)
+
expect(response).to have_http_status(200)
expect(json_response['name']).to eq(group1.name)
end
it 'does not return a non existing group' do
get api('/groups/unknown', admin)
+
expect(response).to have_http_status(404)
end
@@ -302,6 +313,7 @@ describe API::Groups, api: true do
it "does not return a non existing group" do
get api("/groups/1328/projects", user1)
+
expect(response).to have_http_status(404)
end
@@ -325,6 +337,7 @@ describe API::Groups, api: true do
context "when authenticated as admin" do
it "should return any existing group" do
get api("/groups/#{group2.id}/projects", admin)
+
expect(response).to have_http_status(200)
expect(json_response.length).to eq(1)
expect(json_response.first['name']).to eq(project2.name)
@@ -332,6 +345,7 @@ describe API::Groups, api: true do
it "should not return a non existing group" do
get api("/groups/1328/projects", admin)
+
expect(response).to have_http_status(404)
end
end
@@ -347,6 +361,7 @@ describe API::Groups, api: true do
it 'does not return a non existing group' do
get api('/groups/unknown/projects', admin)
+
expect(response).to have_http_status(404)
end
@@ -362,6 +377,7 @@ describe API::Groups, api: true do
context "when authenticated as user without group permissions" do
it "does not create group" do
post api("/groups", user1), attributes_for(:group)
+
expect(response).to have_http_status(403)
end
end
@@ -371,6 +387,7 @@ describe API::Groups, api: true do
group = attributes_for(:group, { request_access_enabled: false })
post api("/groups", user3), group
+
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(group[:name])
@@ -380,17 +397,20 @@ describe API::Groups, api: true do
it "does not create group, duplicate" do
post api("/groups", user3), { name: 'Duplicate Test', path: group2.path }
+
expect(response).to have_http_status(400)
expect(response.message).to eq("Bad Request")
end
it "returns 400 bad request error if name not given" do
post api("/groups", user3), { path: group2.path }
+
expect(response).to have_http_status(400)
end
it "returns 400 bad request error if path not given" do
post api("/groups", user3), { name: 'test' }
+
expect(response).to have_http_status(400)
end
end
@@ -400,18 +420,22 @@ describe API::Groups, api: true do
context "when authenticated as user" do
it "removes group" do
delete api("/groups/#{group1.id}", user1)
+
expect(response).to have_http_status(200)
end
it "does not remove a group if not an owner" do
user4 = create(:user)
group1.add_master(user4)
+
delete api("/groups/#{group1.id}", user3)
+
expect(response).to have_http_status(403)
end
it "does not remove a non existing group" do
delete api("/groups/1328", user1)
+
expect(response).to have_http_status(404)
end
@@ -425,11 +449,13 @@ describe API::Groups, api: true do
context "when authenticated as admin" do
it "removes any existing group" do
delete api("/groups/#{group2.id}", admin)
+
expect(response).to have_http_status(200)
end
it "does not remove a non existing group" do
delete api("/groups/1328", admin)
+
expect(response).to have_http_status(404)
end
end
@@ -437,15 +463,17 @@ describe API::Groups, api: true do
describe "POST /groups/:id/projects/:project_id" do
let(:project) { create(:project) }
+ let(:project_path) { "#{project.namespace.path}%2F#{project.path}" }
+
before(:each) do
allow_any_instance_of(Projects::TransferService).
to receive(:execute).and_return(true)
- allow(Project).to receive(:find).and_return(project)
end
context "when authenticated as user" do
it "does not transfer project to group" do
post api("/groups/#{group1.id}/projects/#{project.id}", user2)
+
expect(response).to have_http_status(403)
end
end
@@ -453,8 +481,45 @@ describe API::Groups, api: true do
context "when authenticated as admin" do
it "transfers project to group" do
post api("/groups/#{group1.id}/projects/#{project.id}", admin)
+
expect(response).to have_http_status(201)
end
+
+ context 'when using project path in URL' do
+ context 'with a valid project path' do
+ it "transfers project to group" do
+ post api("/groups/#{group1.id}/projects/#{project_path}", admin)
+
+ expect(response).to have_http_status(201)
+ end
+ end
+
+ context 'with a non-existent project path' do
+ it "does not transfer project to group" do
+ post api("/groups/#{group1.id}/projects/nogroup%2Fnoproject", admin)
+
+ expect(response).to have_http_status(404)
+ end
+ end
+ end
+
+ context 'when using a group path in URL' do
+ context 'with a valid group path' do
+ it "transfers project to group" do
+ post api("/groups/#{group1.path}/projects/#{project_path}", admin)
+
+ expect(response).to have_http_status(201)
+ end
+ end
+
+ context 'with a non-existent group path' do
+ it "does not transfer project to group" do
+ post api("/groups/noexist/projects/#{project_path}", admin)
+
+ expect(response).to have_http_status(404)
+ end
+ end
+ end
end
end
end