Commit 0cf76a0b authored by Douwe Maan's avatar Douwe Maan

Use Projects::CreateService to fork projects so that after-create hooks are run.

parent 62117f2f
...@@ -686,11 +686,21 @@ class Project < ActiveRecord::Base ...@@ -686,11 +686,21 @@ class Project < ActiveRecord::Base
end end
def create_repository def create_repository
if gitlab_shell.add_repository(path_with_namespace) if forked?
true if gitlab_shell.fork_repository(forked_from_project.path_with_namespace, self.namespace.path)
ensure_satellite_exists
true
else
errors.add(:base, 'Failed to fork repository')
false
end
else else
errors.add(:base, 'Failed to create repository') if gitlab_shell.add_repository(path_with_namespace)
false true
else
errors.add(:base, 'Failed to create repository')
false
end
end end
end end
......
...@@ -5,6 +5,8 @@ module Projects ...@@ -5,6 +5,8 @@ module Projects
end end
def execute def execute
forked_from_project_id = params.delete(:forked_from_project_id)
@project = Project.new(params) @project = Project.new(params)
# Make sure that the user is allowed to use the specified visibility # Make sure that the user is allowed to use the specified visibility
...@@ -45,10 +47,14 @@ module Projects ...@@ -45,10 +47,14 @@ module Projects
@project.creator = current_user @project.creator = current_user
if forked_from_project_id
@project.build_forked_project_link(forked_from_project_id: forked_from_project_id)
end
Project.transaction do Project.transaction do
@project.save @project.save
unless @project.import? if @project.persisted? && !@project.import?
unless @project.create_repository unless @project.create_repository
raise 'Failed to create repository' raise 'Failed to create repository'
end end
......
module Projects module Projects
class ForkService < BaseService class ForkService < BaseService
include Gitlab::ShellAdapter
def execute def execute
@from_project = @project new_params = {
forked_from_project_id: @project.id,
project_params = { visibility_level: @project.visibility_level,
visibility_level: @from_project.visibility_level, description: @project.description,
description: @from_project.description, name: @project.name,
path: @project.path,
namespace_id: @params[:namespace].try(:id) || current_user.namespace.id
} }
project = Project.new(project_params) if @project.avatar.present? && @project.avatar.image?
project.name = @from_project.name new_params[:avatar] = @project.avatar
project.path = @from_project.path
project.creator = @current_user
if @from_project.avatar.present? && @from_project.avatar.image?
project.avatar = @from_project.avatar
end
if namespace = @params[:namespace]
project.namespace = namespace
else
project.namespace = @current_user.namespace
end end
unless @current_user.can?(:create_projects, project.namespace) new_project = CreateService.new(current_user, new_params).execute
project.errors.add(:namespace, 'insufficient access rights')
return project
end
# If the project cannot save, we do not want to trigger the project destroy
# as this can have the side effect of deleting a repo attached to an existing
# project with the same name and namespace
if project.valid?
begin
Project.transaction do
#First save the DB entries as they can be rolled back if the repo fork fails
project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id)
if project.save
project.team << [@current_user, :master, @current_user]
end
#Now fork the repo
unless gitlab_shell.fork_repository(@from_project.path_with_namespace, project.namespace.path)
raise 'forking failed in gitlab-shell'
end
project.ensure_satellite_exists
end
if @from_project.gitlab_ci? if new_project.persisted?
ForkRegistrationWorker.perform_async(@from_project.id, project.id, @current_user.private_token) if @project.gitlab_ci?
end ForkRegistrationWorker.perform_async(@project.id, new_project.id, @current_user.private_token)
rescue => ex
project.errors.add(:base, 'Fork transaction failed.')
project.destroy
end end
else
project.errors.add(:base, 'Invalid fork destination')
end end
project new_project
end end
end end
end end
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
%a.twitter-share-button{ | %a.twitter-share-button{ |
href: "https://twitter.com/share", | href: "https://twitter.com/share", |
"data-url" => event.project.web_url, | "data-url" => event.project.web_url, |
"data-text" => "I just #{event.project.imported? ? "imported" : "created"} a new project in GitLab! GitLab is version control on your server.", | "data-text" => "I just #{event.action_name} a new project on GitLab! GitLab is version control on your server.", |
"data-size" => "medium", | "data-size" => "medium", |
"data-related" => "gitlab", | "data-related" => "gitlab", |
"data-hashtags" => "gitlab", | "data-hashtags" => "gitlab", |
......
...@@ -50,7 +50,6 @@ describe API::API, api: true do ...@@ -50,7 +50,6 @@ describe API::API, api: true do
it 'should fail if forked project exists in the user namespace' do it 'should fail 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.status).to eq(409) expect(response.status).to eq(409)
expect(json_response['message']['base']).to eq(['Invalid fork destination'])
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'])
end end
......
...@@ -27,7 +27,7 @@ describe Projects::ForkService do ...@@ -27,7 +27,7 @@ describe Projects::ForkService do
it "fails due to transaction failure" do it "fails due to transaction failure" do
@to_project = fork_project(@from_project, @to_user, false) @to_project = fork_project(@from_project, @to_user, false)
expect(@to_project.errors).not_to be_empty expect(@to_project.errors).not_to be_empty
expect(@to_project.errors[:base]).to include("Fork transaction failed.") expect(@to_project.errors[:base]).to include("Failed to fork repository")
end end
end end
...@@ -36,8 +36,8 @@ describe Projects::ForkService do ...@@ -36,8 +36,8 @@ describe Projects::ForkService do
@existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace)
@to_project = fork_project(@from_project, @to_user) @to_project = fork_project(@from_project, @to_user)
expect(@existing_project.persisted?).to be_truthy expect(@existing_project.persisted?).to be_truthy
expect(@to_project.errors[:base]).to include("Invalid fork destination") expect(@to_project.errors[:name]).to eq(['has already been taken'])
expect(@to_project.errors[:base]).not_to include("Fork transaction failed.") expect(@to_project.errors[:path]).to eq(['has already been taken'])
end end
end end
...@@ -81,7 +81,7 @@ describe Projects::ForkService do ...@@ -81,7 +81,7 @@ describe Projects::ForkService do
context 'fork project for group when user not owner' do context 'fork project for group when user not owner' do
it 'group developer should fail to fork project into the group' do it 'group developer should fail to fork project into the group' do
to_project = fork_project(@project, @developer, true, @opts) to_project = fork_project(@project, @developer, true, @opts)
expect(to_project.errors[:namespace]).to eq(['insufficient access rights']) expect(to_project.errors[:namespace]).to eq(['is not valid'])
end end
end end
...@@ -91,7 +91,6 @@ describe Projects::ForkService do ...@@ -91,7 +91,6 @@ describe Projects::ForkService do
namespace: @group) namespace: @group)
to_project = fork_project(@project, @group_owner, true, @opts) to_project = fork_project(@project, @group_owner, true, @opts)
expect(existing_project.persisted?).to be_truthy expect(existing_project.persisted?).to be_truthy
expect(to_project.errors[:base]).to eq(['Invalid fork destination'])
expect(to_project.errors[:name]).to eq(['has already been taken']) expect(to_project.errors[:name]).to eq(['has already been taken'])
expect(to_project.errors[:path]).to eq(['has already been taken']) expect(to_project.errors[:path]).to eq(['has already been taken'])
end end
...@@ -99,10 +98,7 @@ describe Projects::ForkService do ...@@ -99,10 +98,7 @@ describe Projects::ForkService do
end end
def fork_project(from_project, user, fork_success = true, params = {}) def fork_project(from_project, user, fork_success = true, params = {})
context = Projects::ForkService.new(from_project, user, params) allow_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(fork_success)
shell = double('gitlab_shell') Projects::ForkService.new(from_project, user, params).execute
shell.stub(fork_repository: fork_success)
context.stub(gitlab_shell: shell)
context.execute
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