Improved code quality on API fork namespace feature
This commit is contained in:
		
							parent
							
								
									bad3fb895c
								
							
						
					
					
						commit
						3aa40153e0
					
				|  | @ -199,18 +199,21 @@ module API | ||||||
|       post 'fork/:id' do |       post 'fork/:id' do | ||||||
|         attrs = {} |         attrs = {} | ||||||
|         namespace_id = params[:namespace] |         namespace_id = params[:namespace] | ||||||
|  | 
 | ||||||
|         if namespace_id.present? |         if namespace_id.present? | ||||||
|           namespace = Namespace.find_by(id: namespace_id) || Namespace.find_by_path_or_name(namespace_id) |           namespace = Namespace.find_by(id: namespace_id) || Namespace.find_by_path_or_name(namespace_id) | ||||||
|           if namespace.nil? | 
 | ||||||
|             not_found!('Target Namespace') |           not_found!('Target Namespace') unless namespace | ||||||
|           end |  | ||||||
|           authorize! :create_projects, namespace |           authorize! :create_projects, namespace | ||||||
|  | 
 | ||||||
|           attrs[:namespace] = namespace |           attrs[:namespace] = namespace | ||||||
|         end |         end | ||||||
|  | 
 | ||||||
|         @forked_project = |         @forked_project = | ||||||
|           ::Projects::ForkService.new(user_project, |           ::Projects::ForkService.new(user_project, | ||||||
|                                       current_user, |                                       current_user, | ||||||
|                                       attrs).execute |                                       attrs).execute | ||||||
|  | 
 | ||||||
|         if @forked_project.errors.any? |         if @forked_project.errors.any? | ||||||
|           conflict!(@forked_project.errors.messages) |           conflict!(@forked_project.errors.messages) | ||||||
|         else |         else | ||||||
|  |  | ||||||
|  | @ -28,6 +28,7 @@ describe API::API, api: true  do | ||||||
|     context 'when authenticated' do |     context 'when authenticated' do | ||||||
|       it 'forks if user has sufficient access to project' do |       it 'forks if user has sufficient access to project' do | ||||||
|         post api("/projects/fork/#{project.id}", user2) |         post api("/projects/fork/#{project.id}", user2) | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['name']).to eq(project.name) |         expect(json_response['name']).to eq(project.name) | ||||||
|         expect(json_response['path']).to eq(project.path) |         expect(json_response['path']).to eq(project.path) | ||||||
|  | @ -38,6 +39,7 @@ describe API::API, api: true  do | ||||||
| 
 | 
 | ||||||
|       it 'forks if user is admin' do |       it 'forks if user is admin' do | ||||||
|         post api("/projects/fork/#{project.id}", admin) |         post api("/projects/fork/#{project.id}", admin) | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['name']).to eq(project.name) |         expect(json_response['name']).to eq(project.name) | ||||||
|         expect(json_response['path']).to eq(project.path) |         expect(json_response['path']).to eq(project.path) | ||||||
|  | @ -48,12 +50,14 @@ describe API::API, api: true  do | ||||||
| 
 | 
 | ||||||
|       it 'fails on missing project access for the project to fork' do |       it 'fails on missing project access for the project to fork' do | ||||||
|         post api("/projects/fork/#{project.id}", user3) |         post api("/projects/fork/#{project.id}", user3) | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(404) |         expect(response).to have_http_status(404) | ||||||
|         expect(json_response['message']).to eq('404 Project Not Found') |         expect(json_response['message']).to eq('404 Project Not Found') | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'fails if forked project exists in the user namespace' do |       it 'fails if forked project exists in the user namespace' do | ||||||
|         post api("/projects/fork/#{project.id}", user) |         post api("/projects/fork/#{project.id}", user) | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(409) |         expect(response).to have_http_status(409) | ||||||
|         expect(json_response['message']['name']).to eq(['has already been taken']) |         expect(json_response['message']['name']).to eq(['has already been taken']) | ||||||
|         expect(json_response['message']['path']).to eq(['has already been taken']) |         expect(json_response['message']['path']).to eq(['has already been taken']) | ||||||
|  | @ -61,52 +65,61 @@ describe API::API, api: true  do | ||||||
| 
 | 
 | ||||||
|       it 'fails if project to fork from does not exist' do |       it 'fails if project to fork from does not exist' do | ||||||
|         post api('/projects/fork/424242', user) |         post api('/projects/fork/424242', user) | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(404) |         expect(response).to have_http_status(404) | ||||||
|         expect(json_response['message']).to eq('404 Project Not Found') |         expect(json_response['message']).to eq('404 Project Not Found') | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'forks with explicit own user namespace id' do |       it 'forks with explicit own user namespace id' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{user2.namespace.id}", user2) |         post api("/projects/fork/#{project.id}", user2), namespace: user2.namespace.id | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['owner']['id']).to eq(user2.id) |         expect(json_response['owner']['id']).to eq(user2.id) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'forks with explicit own user name as namespace' do |       it 'forks with explicit own user name as namespace' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{user2.username}", user2) |         post api("/projects/fork/#{project.id}", user2), namespace: user2.username | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['owner']['id']).to eq(user2.id) |         expect(json_response['owner']['id']).to eq(user2.id) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'forks to another user when admin' do |       it 'forks to another user when admin' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{user2.username}", admin) |         post api("/projects/fork/#{project.id}", admin), namespace: user2.username | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['owner']['id']).to eq(user2.id) |         expect(json_response['owner']['id']).to eq(user2.id) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'fails if trying to fork to another user when not admin' do |       it 'fails if trying to fork to another user when not admin' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{admin.namespace.id}", user2) |         post api("/projects/fork/#{project.id}", user2), namespace: admin.namespace.id | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(403) |         expect(response).to have_http_status(403) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'fails if trying to fork to non-existent namespace' do |       it 'fails if trying to fork to non-existent namespace' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=42424242", user2) |         post api("/projects/fork/#{project.id}", user2), namespace: 42424242 | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(404) |         expect(response).to have_http_status(404) | ||||||
|         expect(json_response['message']).to eq('404 Target Namespace Not Found') |         expect(json_response['message']).to eq('404 Target Namespace Not Found') | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'forks to owned group' do |       it 'forks to owned group' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{group2.name}", user2) |         post api("/projects/fork/#{project.id}", user2), namespace: group2.name | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['namespace']['name']).to eq(group2.name) |         expect(json_response['namespace']['name']).to eq(group2.name) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'fails to fork to not owned group' do |       it 'fails to fork to not owned group' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{group.name}", user2) |         post api("/projects/fork/#{project.id}", user2), namespace: group.name | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(403) |         expect(response).to have_http_status(403) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'forks to not owned group when admin' do |       it 'forks to not owned group when admin' do | ||||||
|         post api("/projects/fork/#{project.id}?namespace=#{group.name}", admin) |         post api("/projects/fork/#{project.id}", admin), namespace: group.name | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(201) |         expect(response).to have_http_status(201) | ||||||
|         expect(json_response['namespace']['name']).to eq(group.name) |         expect(json_response['namespace']['name']).to eq(group.name) | ||||||
|       end |       end | ||||||
|  | @ -115,6 +128,7 @@ describe API::API, api: true  do | ||||||
|     context 'when unauthenticated' do |     context 'when unauthenticated' do | ||||||
|       it 'returns authentication error' do |       it 'returns authentication error' do | ||||||
|         post api("/projects/fork/#{project.id}") |         post api("/projects/fork/#{project.id}") | ||||||
|  | 
 | ||||||
|         expect(response).to have_http_status(401) |         expect(response).to have_http_status(401) | ||||||
|         expect(json_response['message']).to eq('401 Unauthorized') |         expect(json_response['message']).to eq('401 Unauthorized') | ||||||
|       end |       end | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue