Commit e19f54ca authored by Douwe Maan's avatar Douwe Maan

Merge branch 'bvl-unlink-fixes' into 'master'

This fixes some bugs related to forked projects of which the source was deleted.

Closes #39667

See merge request gitlab-org/gitlab-ce!15150
parents 9f070338 0f1d6402
...@@ -94,10 +94,9 @@ module LfsRequest ...@@ -94,10 +94,9 @@ module LfsRequest
@storage_project ||= begin @storage_project ||= begin
result = project result = project
loop do # TODO: Make this go to the fork_network root immeadiatly
break unless result.forked? # dependant on the discussion in: https://gitlab.com/gitlab-org/gitlab-ce/issues/39769
result = result.forked_from_project result = result.fork_source while result.forked?
end
result result
end end
......
...@@ -110,7 +110,15 @@ module ProjectsHelper ...@@ -110,7 +110,15 @@ module ProjectsHelper
def remove_fork_project_message(project) def remove_fork_project_message(project)
_("You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?") % _("You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?") %
{ forked_from_project: @project.forked_from_project.name_with_namespace } { forked_from_project: fork_source_name(project) }
end
def fork_source_name(project)
if @project.fork_source
@project.fork_source.full_name
else
@project.fork_network&.deleted_root_project_name
end
end end
def project_nav_tabs def project_nav_tabs
...@@ -140,8 +148,8 @@ module ProjectsHelper ...@@ -140,8 +148,8 @@ module ProjectsHelper
def can_change_visibility_level?(project, current_user) def can_change_visibility_level?(project, current_user)
return false unless can?(current_user, :change_visibility_level, project) return false unless can?(current_user, :change_visibility_level, project)
if project.forked? if project.fork_source
project.forked_from_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE project.fork_source.visibility_level > Gitlab::VisibilityLevel::PRIVATE
else else
true true
end end
......
...@@ -12,4 +12,8 @@ class ForkNetwork < ActiveRecord::Base ...@@ -12,4 +12,8 @@ class ForkNetwork < ActiveRecord::Base
def find_forks_in(other_projects) def find_forks_in(other_projects)
projects.where(id: other_projects) projects.where(id: other_projects)
end end
def merge_requests
MergeRequest.where(target_project: projects)
end
end end
...@@ -1040,6 +1040,10 @@ class Project < ActiveRecord::Base ...@@ -1040,6 +1040,10 @@ class Project < ActiveRecord::Base
!(forked_project_link.nil? || forked_project_link.forked_from_project.nil?) !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
end end
def fork_source
forked_from_project || fork_network&.root_project
end
def personal? def personal?
!group !group
end end
......
...@@ -3,18 +3,24 @@ module Projects ...@@ -3,18 +3,24 @@ module Projects
def execute def execute
return unless @project.forked? return unless @project.forked?
@project.forked_from_project.lfs_objects.find_each do |lfs_object| if fork_source = @project.fork_source
fork_source.lfs_objects.find_each do |lfs_object|
lfs_object.projects << @project lfs_object.projects << @project
end end
merge_requests = @project.forked_from_project.merge_requests.opened.from_project(@project) refresh_forks_count(fork_source)
end
merge_requests = @project.fork_network
.merge_requests
.opened
.where.not(target_project: @project)
.from_project(@project)
merge_requests.each do |mr| merge_requests.each do |mr|
::MergeRequests::CloseService.new(@project, @current_user).execute(mr) ::MergeRequests::CloseService.new(@project, @current_user).execute(mr)
end end
refresh_forks_count(@project.forked_from_project)
@project.fork_network_member.destroy @project.fork_network_member.destroy
@project.forked_project_link.destroy @project.forked_project_link.destroy
end end
......
- empty_repo = @project.empty_repo? - empty_repo = @project.empty_repo?
- fork_network = @project.fork_network - fork_network = @project.fork_network
- forked_from_project = @project.forked_from_project || fork_network&.root_project
.project-home-panel.text-center{ class: ("empty-project" if empty_repo) } .project-home-panel.text-center{ class: ("empty-project" if empty_repo) }
.limit-container-width{ class: container_class } .limit-container-width{ class: container_class }
.avatar-container.s70.project-avatar .avatar-container.s70.project-avatar
...@@ -16,13 +15,13 @@ ...@@ -16,13 +15,13 @@
- if @project.forked? - if @project.forked?
%p %p
- if forked_from_project - if @project.fork_source
#{ s_('ForkedFromProjectPath|Forked from') } #{ s_('ForkedFromProjectPath|Forked from') }
= link_to project_path(forked_from_project) do = link_to project_path(@project.fork_source) do
= forked_from_project.full_name = fork_source_name(@project)
- else - else
- deleted_message = s_('ForkedFromProjectPath|Forked from %{project_name} (deleted)') - deleted_message = s_('ForkedFromProjectPath|Forked from %{project_name} (deleted)')
= deleted_message % { project_name: fork_network.deleted_root_project_name } = deleted_message % { project_name: fork_source_name(@project) }
.project-repo-buttons .project-repo-buttons
.count-buttons .count-buttons
......
...@@ -173,7 +173,10 @@ ...@@ -173,7 +173,10 @@
%p %p
This will remove the fork relationship to source project This will remove the fork relationship to source project
= succeed "." do = succeed "." do
= link_to @project.forked_from_project.name_with_namespace, project_path(@project.forked_from_project) - if @project.fork_source
= link_to(fork_source_name(@project), project_path(@project.fork_source))
- else
= fork_source_name(@project)
= form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_project_path(@project), method: :delete, remote: true, html: { class: 'transfer-project' }) do |f| = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_project_path(@project), method: :delete, remote: true, html: { class: 'transfer-project' }) do |f|
%p %p
%strong Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source. %strong Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source.
......
---
title: Fix issues with forked projects of which the source was deleted
merge_request: 15150
author:
type: fixed
require 'spec_helper'
describe LfsRequest do
include ProjectForksHelper
controller(Projects::GitHttpClientController) do
# `described_class` is not available in this context
include LfsRequest # rubocop:disable RSpec/DescribedClass
def show
storage_project
render nothing: true
end
def project
@project ||= Project.find(params[:id])
end
def download_request?
true
end
def ci?
false
end
end
let(:project) { create(:project, :public) }
before do
stub_lfs_setting(enabled: true)
end
describe '#storage_project' do
it 'assigns the project as storage project' do
get :show, id: project.id
expect(assigns(:storage_project)).to eq(project)
end
it 'assigns the source of a forked project' do
forked_project = fork_project(project)
get :show, id: forked_project.id
expect(assigns(:storage_project)).to eq(project)
end
end
end
require 'spec_helper'
feature 'Settings for a forked project', :js do
include ProjectForksHelper
let(:user) { create(:user) }
let(:original_project) { create(:project) }
let(:forked_project) { fork_project(original_project, user) }
before do
original_project.add_master(user)
forked_project.add_master(user)
sign_in(user)
end
shared_examples 'project settings for a forked projects' do
it 'allows deleting the link to the forked project' do
visit edit_project_path(forked_project)
click_button 'Remove fork relationship'
wait_for_requests
fill_in('confirm_name_input', with: forked_project.name)
click_button('Confirm')
expect(page).to have_content('The fork relationship has been removed.')
expect(forked_project.reload.forked?).to be_falsy
end
end
it_behaves_like 'project settings for a forked projects'
context 'when the original project is deleted' do
before do
original_project.destroy!
end
it_behaves_like 'project settings for a forked projects'
end
end
...@@ -24,6 +24,16 @@ describe ForkNetwork do ...@@ -24,6 +24,16 @@ describe ForkNetwork do
end end
end end
describe '#merge_requests' do
it 'finds merge requests within the fork network' do
project = create(:project)
forked_project = fork_project(project)
merge_request = create(:merge_request, source_project: forked_project, target_project: project)
expect(project.fork_network.merge_requests).to include(merge_request)
end
end
context 'for a deleted project' do context 'for a deleted project' do
it 'keeps the fork network' do it 'keeps the fork network' do
project = create(:project, :public) project = create(:project, :public)
......
...@@ -1923,6 +1923,20 @@ describe Project do ...@@ -1923,6 +1923,20 @@ describe Project do
expect(forked_project.in_fork_network_of?(other_project)).to be_falsy expect(forked_project.in_fork_network_of?(other_project)).to be_falsy
end end
end end
describe '#fork_source' do
let!(:second_fork) { fork_project(forked_project) }
it 'returns the direct source if it exists' do
expect(second_fork.fork_source).to eq(forked_project)
end
it 'returns the root of the fork network when the directs source was deleted' do
forked_project.destroy
expect(second_fork.fork_source).to eq(project)
end
end
end end
describe '#pushes_since_gc' do describe '#pushes_since_gc' do
......
...@@ -12,6 +12,9 @@ describe Projects::UnlinkForkService do ...@@ -12,6 +12,9 @@ describe Projects::UnlinkForkService do
context 'with opened merge request on the source project' do context 'with opened merge request on the source project' do
let(:merge_request) { create(:merge_request, source_project: forked_project, target_project: fork_link.forked_from_project) } let(:merge_request) { create(:merge_request, source_project: forked_project, target_project: fork_link.forked_from_project) }
let(:merge_request2) { create(:merge_request, source_project: forked_project, target_project: fork_project(project)) }
let(:merge_request_in_fork) { create(:merge_request, source_project: forked_project, target_project: forked_project) }
let(:mr_close_service) { MergeRequests::CloseService.new(forked_project, user) } let(:mr_close_service) { MergeRequests::CloseService.new(forked_project, user) }
before do before do
...@@ -22,9 +25,14 @@ describe Projects::UnlinkForkService do ...@@ -22,9 +25,14 @@ describe Projects::UnlinkForkService do
it 'close all pending merge requests' do it 'close all pending merge requests' do
expect(mr_close_service).to receive(:execute).with(merge_request) expect(mr_close_service).to receive(:execute).with(merge_request)
expect(mr_close_service).to receive(:execute).with(merge_request2)
subject.execute subject.execute
end end
it 'does not close merge requests for the project being unlinked' do
expect(mr_close_service).not_to receive(:execute).with(merge_request_in_fork)
end
end end
it 'remove fork relation' do it 'remove fork relation' do
...@@ -53,4 +61,14 @@ describe Projects::UnlinkForkService do ...@@ -53,4 +61,14 @@ describe Projects::UnlinkForkService do
expect(source.forks_count).to be_zero expect(source.forks_count).to be_zero
end end
context 'when the original project was deleted' do
it 'does not fail when the original project is deleted' do
source = forked_project.forked_from_project
source.destroy
forked_project.reload
expect { subject.execute }.not_to raise_error
end
end
end end
...@@ -38,6 +38,10 @@ module StubConfiguration ...@@ -38,6 +38,10 @@ module StubConfiguration
allow(Gitlab.config.backup).to receive_messages(to_settings(messages)) allow(Gitlab.config.backup).to receive_messages(to_settings(messages))
end end
def stub_lfs_setting(messages)
allow(Gitlab.config.lfs).to receive_messages(to_settings(messages))
end
def stub_storage_settings(messages) def stub_storage_settings(messages)
# Default storage is always required # Default storage is always required
messages['default'] ||= Gitlab.config.repositories.storages.default messages['default'] ||= Gitlab.config.repositories.storages.default
......
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