diff --git a/app/models/project.rb b/app/models/project.rb index 293ee04f228c76c89d5b6284c86bb556ab3ad717..397232e98d8a46c0d8ebf57e5ad86b0c2f04a307 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -686,11 +686,21 @@ class Project < ActiveRecord::Base end def create_repository - if gitlab_shell.add_repository(path_with_namespace) - true + if forked? + 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 - errors.add(:base, 'Failed to create repository') - false + if gitlab_shell.add_repository(path_with_namespace) + true + else + errors.add(:base, 'Failed to create repository') + false + end end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index a7afcf8f64b94420ad2b82fa617ed92e1d5c8a45..011f6f6145e18c81072ffa4fb45caa059d996b0d 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -5,6 +5,8 @@ module Projects end def execute + forked_from_project_id = params.delete(:forked_from_project_id) + @project = Project.new(params) # Make sure that the user is allowed to use the specified visibility @@ -45,10 +47,14 @@ module Projects @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.save - unless @project.import? + if @project.persisted? && !@project.import? unless @project.create_repository raise 'Failed to create repository' end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 1e4deb6ed39babde42725d703f7206fd2c14011a..50f208b11d15c396c3ecb1f253e6e28d7e52245f 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -1,66 +1,28 @@ module Projects class ForkService < BaseService - include Gitlab::ShellAdapter - def execute - @from_project = @project - - project_params = { - visibility_level: @from_project.visibility_level, - description: @from_project.description, + new_params = { + forked_from_project_id: @project.id, + visibility_level: @project.visibility_level, + description: @project.description, + name: @project.name, + path: @project.path, + namespace_id: @params[:namespace].try(:id) || current_user.namespace.id } - project = Project.new(project_params) - project.name = @from_project.name - 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 + if @project.avatar.present? && @project.avatar.image? + new_params[:avatar] = @project.avatar end - unless @current_user.can?(:create_projects, project.namespace) - 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 + new_project = CreateService.new(current_user, new_params).execute - if @from_project.gitlab_ci? - ForkRegistrationWorker.perform_async(@from_project.id, project.id, @current_user.private_token) - end - rescue => ex - project.errors.add(:base, 'Fork transaction failed.') - project.destroy + if new_project.persisted? + if @project.gitlab_ci? + ForkRegistrationWorker.perform_async(@project.id, new_project.id, @current_user.private_token) end - else - project.errors.add(:base, 'Invalid fork destination') end - project + new_project end end end diff --git a/app/views/events/event/_created_project.html.haml b/app/views/events/event/_created_project.html.haml index 552525f4a0783257f881e7dcd125ccdfc986301c..c2577a249824d165272c23eb4d9d3ccf891c89f1 100644 --- a/app/views/events/event/_created_project.html.haml +++ b/app/views/events/event/_created_project.html.haml @@ -18,7 +18,7 @@ %a.twitter-share-button{ | href: "https://twitter.com/share", | "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-related" => "gitlab", | "data-hashtags" => "gitlab", | diff --git a/spec/requests/api/fork_spec.rb b/spec/requests/api/fork_spec.rb index fb3ff552c8d412440c4b4b6263d748cecb08ec77..7a784796031a63c3092e976f1e2e9a290f532675 100644 --- a/spec/requests/api/fork_spec.rb +++ b/spec/requests/api/fork_spec.rb @@ -50,7 +50,6 @@ describe API::API, api: true do it 'should fail if forked project exists in the user namespace' do post api("/projects/fork/#{project.id}", user) 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']['path']).to eq(['has already been taken']) end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index c9025bdf133087619ec0999f79e543db7d51b97e..f158ac87e2b724eacbb1bd587cbd3600c0560fd7 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -27,7 +27,7 @@ describe Projects::ForkService do it "fails due to transaction failure" do @to_project = fork_project(@from_project, @to_user, false) 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 @@ -36,8 +36,8 @@ describe Projects::ForkService do @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) @to_project = fork_project(@from_project, @to_user) expect(@existing_project.persisted?).to be_truthy - expect(@to_project.errors[:base]).to include("Invalid fork destination") - expect(@to_project.errors[:base]).not_to include("Fork transaction failed.") + expect(@to_project.errors[:name]).to eq(['has already been taken']) + expect(@to_project.errors[:path]).to eq(['has already been taken']) end end @@ -81,7 +81,7 @@ describe Projects::ForkService do context 'fork project for group when user not owner' do it 'group developer should fail to fork project into the group' do 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 @@ -91,7 +91,6 @@ describe Projects::ForkService do namespace: @group) to_project = fork_project(@project, @group_owner, true, @opts) 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[:path]).to eq(['has already been taken']) end @@ -99,10 +98,7 @@ describe Projects::ForkService do end def fork_project(from_project, user, fork_success = true, params = {}) - context = Projects::ForkService.new(from_project, user, params) - shell = double('gitlab_shell') - shell.stub(fork_repository: fork_success) - context.stub(gitlab_shell: shell) - context.execute + allow_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(fork_success) + Projects::ForkService.new(from_project, user, params).execute end end