Merge branch '21636-deleting-source-project-with-existing-fork-closes-all-related-merge-requests' into 'master'
Deleting source project with existing fork link should closes all related merge requests, and does not render `invalid` template for merge requests with deleted source project. The merge request without source project can be edited but can't be reopened. Closes #21636 See merge request !6177
This commit is contained in:
commit
404f438f45
8 changed files with 127 additions and 21 deletions
|
@ -127,6 +127,7 @@ v 8.12.0 (unreleased)
|
|||
- Allow bulk update merge requests from merge requests index page
|
||||
- Add notification_settings API calls !5632 (mahcsig)
|
||||
- Remove duplication between project builds and admin builds view !5680 (Katarzyna Kobierska Ula Budziszewska)
|
||||
- Deleting source project with existing fork link will close all related merge requests !6177 (Katarzyna Kobierska Ula Budziszeska)
|
||||
|
||||
v 8.11.6 (unreleased)
|
||||
- Fix an error where we were unable to create a CommitStatus for running state
|
||||
|
|
|
@ -428,17 +428,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
|||
end
|
||||
|
||||
def validates_merge_request
|
||||
# If source project was removed (Ex. mr from fork to origin)
|
||||
return invalid_mr unless @merge_request.source_project
|
||||
|
||||
# Show git not found page
|
||||
# if there is no saved commits between source & target branch
|
||||
if @merge_request.commits.blank?
|
||||
# and if target branch doesn't exist
|
||||
return invalid_mr unless @merge_request.target_branch_exists?
|
||||
|
||||
# or if source branch doesn't exist
|
||||
return invalid_mr unless @merge_request.source_branch_exists?
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -316,6 +316,10 @@ class MergeRequest < ActiveRecord::Base
|
|||
closed? && forked_source_project_missing?
|
||||
end
|
||||
|
||||
def closed_without_source_project?
|
||||
closed? && !source_project
|
||||
end
|
||||
|
||||
def forked_source_project_missing?
|
||||
return false unless for_fork?
|
||||
return true unless source_project
|
||||
|
@ -323,6 +327,12 @@ class MergeRequest < ActiveRecord::Base
|
|||
!source_project.forked_from?(target_project)
|
||||
end
|
||||
|
||||
def reopenable?
|
||||
return false if closed_without_fork? || closed_without_source_project? || merged?
|
||||
|
||||
closed?
|
||||
end
|
||||
|
||||
def ensure_merge_request_diff
|
||||
merge_request_diff || create_merge_request_diff
|
||||
end
|
||||
|
|
|
@ -27,6 +27,8 @@ module Projects
|
|||
# Git data (e.g. a list of branch names).
|
||||
flush_caches(project, wiki_path)
|
||||
|
||||
Projects::UnlinkForkService.new(project, current_user).execute
|
||||
|
||||
Project.transaction do
|
||||
project.destroy!
|
||||
|
||||
|
|
|
@ -2,7 +2,7 @@
|
|||
- if can?(current_user, :update_merge_request, @merge_request)
|
||||
- if @merge_request.open?
|
||||
= link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"}
|
||||
- if @merge_request.closed?
|
||||
- if @merge_request.reopenable?
|
||||
= link_to 'Reopen merge request', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request", data: {original_text: "Reopen merge request", alternative_text: "Comment & reopen merge request"}
|
||||
%comment-and-resolve-btn{ "inline-template" => true, ":discussion-id" => "" }
|
||||
%button.btn.btn-nr.btn-default.append-right-10.js-comment-resolve-button{ "v-if" => "showButton", type: "submit", data: { namespace_path: "#{@merge_request.project.namespace.path}", project_path: "#{@merge_request.project.path}" } }
|
||||
|
|
|
@ -29,6 +29,7 @@
|
|||
%ul.dropdown-menu.dropdown-menu-align-right
|
||||
%li= link_to "Email Patches", merge_request_path(@merge_request, format: :patch)
|
||||
%li= link_to "Plain Diff", merge_request_path(@merge_request, format: :diff)
|
||||
- unless @merge_request.closed_without_fork?
|
||||
.normal
|
||||
%span Request to merge
|
||||
%span.label-branch= source_branch_with_namespace(@merge_request)
|
||||
|
@ -38,6 +39,7 @@
|
|||
- if @merge_request.open? && @merge_request.diverged_from_target_branch?
|
||||
%span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind)
|
||||
|
||||
- unless @merge_request.closed_without_source_project?
|
||||
= render "projects/merge_requests/show/how_to_merge"
|
||||
= render "projects/merge_requests/widget/show.html.haml"
|
||||
|
||||
|
@ -53,6 +55,7 @@
|
|||
= link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do
|
||||
Discussion
|
||||
%span.badge= @merge_request.mr_and_commit_notes.user.count
|
||||
- unless @merge_request.closed_without_source_project?
|
||||
%li.commits-tab
|
||||
= link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do
|
||||
Commits
|
||||
|
|
|
@ -181,6 +181,25 @@ describe ProjectsController do
|
|||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(dashboard_projects_path)
|
||||
end
|
||||
|
||||
context "when the project is forked" do
|
||||
let(:project) { create(:project) }
|
||||
let(:fork_project) { create(:project, forked_from_project: project) }
|
||||
let(:merge_request) do
|
||||
create(:merge_request,
|
||||
source_project: fork_project,
|
||||
target_project: project)
|
||||
end
|
||||
|
||||
it "closes all related merge requests" do
|
||||
project.merge_requests << merge_request
|
||||
sign_in(admin)
|
||||
|
||||
delete :destroy, namespace_id: fork_project.namespace.path, id: fork_project.path
|
||||
|
||||
expect(merge_request.reload.state).to eq('closed')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "POST #toggle_star" do
|
||||
|
|
|
@ -1038,4 +1038,81 @@ describe MergeRequest, models: true do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#closed_without_source_project?' do
|
||||
let(:project) { create(:project) }
|
||||
let(:user) { create(:user) }
|
||||
let(:fork_project) { create(:project, forked_from_project: project, namespace: user.namespace) }
|
||||
let(:destroy_service) { Projects::DestroyService.new(fork_project, user) }
|
||||
|
||||
context 'when the merge request is closed' do
|
||||
let(:closed_merge_request) do
|
||||
create(:closed_merge_request,
|
||||
source_project: fork_project,
|
||||
target_project: project)
|
||||
end
|
||||
|
||||
it 'returns false if the source project exists' do
|
||||
expect(closed_merge_request.closed_without_source_project?).to be_falsey
|
||||
end
|
||||
|
||||
it 'returns true if the source project does not exist' do
|
||||
destroy_service.execute
|
||||
closed_merge_request.reload
|
||||
|
||||
expect(closed_merge_request.closed_without_source_project?).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the merge request is open' do
|
||||
it 'returns false' do
|
||||
expect(subject.closed_without_source_project?).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#reopenable?' do
|
||||
context 'when the merge request is closed' do
|
||||
it 'returns true' do
|
||||
subject.close
|
||||
|
||||
expect(subject.reopenable?).to be_truthy
|
||||
end
|
||||
|
||||
context 'forked project' do
|
||||
let(:project) { create(:project) }
|
||||
let(:user) { create(:user) }
|
||||
let(:fork_project) { create(:project, forked_from_project: project, namespace: user.namespace) }
|
||||
let(:merge_request) do
|
||||
create(:closed_merge_request,
|
||||
source_project: fork_project,
|
||||
target_project: project)
|
||||
end
|
||||
|
||||
it 'returns false if unforked' do
|
||||
Projects::UnlinkForkService.new(fork_project, user).execute
|
||||
|
||||
expect(merge_request.reload.reopenable?).to be_falsey
|
||||
end
|
||||
|
||||
it 'returns false if the source project is deleted' do
|
||||
Projects::DestroyService.new(fork_project, user).execute
|
||||
|
||||
expect(merge_request.reload.reopenable?).to be_falsey
|
||||
end
|
||||
|
||||
it 'returns false if the merge request is merged' do
|
||||
merge_request.update_attributes(state: 'merged')
|
||||
|
||||
expect(merge_request.reload.reopenable?).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the merge request is opened' do
|
||||
it 'returns false' do
|
||||
expect(subject.reopenable?).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue