Commit 427e7246 authored by Sean McGivern's avatar Sean McGivern Committed by Fatih Acet

Don't allow resolving invalid conflicts

An MR can only be resolved in the UI if:
- It has conflicts.
- It has valid diff_refs (in other words, it supports new diff notes).
- It has no conflicts with one side missing.
- It has no conflicts in binary files.
- It has no conflicts in files too large to display.
- It has no conflicts containing invalid conflict markers.
parent 5277239c
...@@ -132,15 +132,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -132,15 +132,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
def conflicts def conflicts
return render_404 unless @merge_request.cannot_be_merged?
respond_to do |format| respond_to do |format|
format.html { define_discussion_vars } format.html { define_discussion_vars }
format.json do format.json do
begin if @merge_request.can_resolve_conflicts_in_ui?
render json: Gitlab::Conflict::FileCollection.new(@merge_request) render json: @merge_request.conflicts
rescue Gitlab::Conflict::Parser::ParserError, Gitlab::Conflict::FileCollection::ConflictSideMissing else
render json: { render json: {
message: 'Unable to resolve conflicts in the web interface for this merge request', message: 'Unable to resolve conflicts in the web interface for this merge request',
type: 'error' type: 'error'
...@@ -452,7 +450,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -452,7 +450,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
noteable_id: @merge_request.id noteable_id: @merge_request.id
} }
@use_legacy_diff_notes = !@merge_request.support_new_diff_notes? @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?
@grouped_diff_discussions = @merge_request.notes.inc_author_project_award_emoji.grouped_diff_discussions @grouped_diff_discussions = @merge_request.notes.inc_author_project_award_emoji.grouped_diff_discussions
Banzai::NoteRenderer.render( Banzai::NoteRenderer.render(
......
...@@ -75,7 +75,7 @@ class DiffNote < Note ...@@ -75,7 +75,7 @@ class DiffNote < Note
private private
def supported? def supported?
!self.for_merge_request? || self.noteable.support_new_diff_notes? !self.for_merge_request? || self.noteable.has_complete_diff_refs?
end end
def noteable_diff_refs def noteable_diff_refs
......
...@@ -204,7 +204,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -204,7 +204,7 @@ class MergeRequest < ActiveRecord::Base
def diff_start_commit def diff_start_commit
if persisted? if persisted?
merge_request_diff.start_commit || target_branch_head merge_request_diff.start_commit
else else
target_branch_head target_branch_head
end end
...@@ -212,7 +212,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -212,7 +212,7 @@ class MergeRequest < ActiveRecord::Base
def diff_head_commit def diff_head_commit
if persisted? if persisted?
merge_request_diff.head_commit || source_branch_head merge_request_diff.head_commit
else else
source_branch_head source_branch_head
end end
...@@ -682,12 +682,12 @@ class MergeRequest < ActiveRecord::Base ...@@ -682,12 +682,12 @@ class MergeRequest < ActiveRecord::Base
merge_commit merge_commit
end end
def support_new_diff_notes? def has_complete_diff_refs?
diff_sha_refs && diff_sha_refs.complete? diff_sha_refs && diff_sha_refs.complete?
end end
def update_diff_notes_positions(old_diff_refs:, new_diff_refs:) def update_diff_notes_positions(old_diff_refs:, new_diff_refs:)
return unless support_new_diff_notes? return unless has_complete_diff_refs?
return if new_diff_refs == old_diff_refs return if new_diff_refs == old_diff_refs
active_diff_notes = self.notes.diff_notes.select do |note| active_diff_notes = self.notes.diff_notes.select do |note|
...@@ -715,4 +715,19 @@ class MergeRequest < ActiveRecord::Base ...@@ -715,4 +715,19 @@ class MergeRequest < ActiveRecord::Base
def keep_around_commit def keep_around_commit
project.repository.keep_around(self.merge_commit_sha) project.repository.keep_around(self.merge_commit_sha)
end end
def conflicts
@conflicts ||= Gitlab::Conflict::FileCollection.new(self)
end
def can_resolve_conflicts_in_ui?
return false unless cannot_be_merged?
return false unless has_complete_diff_refs?
begin
conflicts.files.each(&:lines)
rescue Gitlab::Conflict::Parser::ParserError, Gitlab::Conflict::FileCollection::ConflictSideMissing
false
end
end
end end
...@@ -4,8 +4,8 @@ ...@@ -4,8 +4,8 @@
%p %p
Please Please
= link_to conflicts_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) do - if @merge_request.can_resolve_conflicts_in_ui?
resolve these conflicts = link_to "resolve these conflicts", conflicts_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
or or
- if @merge_request.can_be_merged_via_command_line_by?(current_user) - if @merge_request.can_be_merged_via_command_line_by?(current_user)
#{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}. #{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}.
......
...@@ -69,9 +69,9 @@ module Gitlab ...@@ -69,9 +69,9 @@ module Gitlab
lines.each do |line| lines.each do |line|
if line.type == 'old' if line.type == 'old'
line.rich_text = their_highlight[line.old_line - 1].html_safe line.rich_text = their_highlight[line.old_line - 1].try(:html_safe)
else else
line.rich_text = our_highlight[line.new_line - 1].html_safe line.rich_text = our_highlight[line.new_line - 1].try(:html_safe)
end end
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