Automatically update diff note positions when MR is pushed to
This commit is contained in:
parent
8d7dc26d39
commit
710c488691
|
@ -12,7 +12,7 @@ class DiffNote < Note
|
|||
validate :positions_complete
|
||||
validate :verify_supported
|
||||
|
||||
before_validation :set_original_position, on: :create
|
||||
before_validation :set_original_position, :update_position, on: :create
|
||||
before_validation :set_line_code
|
||||
|
||||
class << self
|
||||
|
@ -71,6 +71,26 @@ class DiffNote < Note
|
|||
self.position.diff_refs == diff_refs
|
||||
end
|
||||
|
||||
def update_position
|
||||
return unless supported?
|
||||
return if for_commit?
|
||||
|
||||
return if active?
|
||||
|
||||
Notes::DiffPositionUpdateService.new(
|
||||
self.project,
|
||||
nil,
|
||||
old_diff_refs: self.position.diff_refs,
|
||||
new_diff_refs: self.noteable.diff_refs,
|
||||
paths: self.position.paths
|
||||
).execute(self)
|
||||
end
|
||||
|
||||
def update_position!
|
||||
update_position &&
|
||||
Gitlab::Timeless.timeless(self, &:save)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def supported?
|
||||
|
|
|
@ -288,14 +288,23 @@ class MergeRequest < ActiveRecord::Base
|
|||
|
||||
def update_merge_request_diff
|
||||
if source_branch_changed? || target_branch_changed?
|
||||
reload_code
|
||||
reload_diff
|
||||
end
|
||||
end
|
||||
|
||||
def reload_code
|
||||
if merge_request_diff && open?
|
||||
merge_request_diff.reload_content
|
||||
end
|
||||
def reload_diff
|
||||
return unless merge_request_diff && open?
|
||||
|
||||
old_diff_refs = self.diff_refs
|
||||
|
||||
merge_request_diff.reload_content
|
||||
|
||||
new_diff_refs = self.diff_refs
|
||||
|
||||
update_diff_notes_positions(
|
||||
old_diff_refs: old_diff_refs,
|
||||
new_diff_refs: new_diff_refs
|
||||
)
|
||||
end
|
||||
|
||||
def check_if_can_be_merged
|
||||
|
@ -646,6 +655,32 @@ class MergeRequest < ActiveRecord::Base
|
|||
diff_refs && diff_refs.complete?
|
||||
end
|
||||
|
||||
def update_diff_notes_positions(old_diff_refs:, new_diff_refs:)
|
||||
return unless support_new_diff_notes?
|
||||
return if new_diff_refs == old_diff_refs
|
||||
|
||||
active_diff_notes = self.notes.diff_notes.select do |note|
|
||||
note.new_diff_note? && note.active?(old_diff_refs)
|
||||
end
|
||||
|
||||
return if active_diff_notes.empty?
|
||||
|
||||
paths = active_diff_notes.flat_map { |n| n.diff_file.paths }.uniq
|
||||
|
||||
service = Notes::DiffPositionUpdateService.new(
|
||||
self.project,
|
||||
nil,
|
||||
old_diff_refs: old_diff_refs,
|
||||
new_diff_refs: new_diff_refs,
|
||||
paths: paths
|
||||
)
|
||||
|
||||
active_diff_notes.each do |note|
|
||||
service.execute(note)
|
||||
Gitlab::Timeless.timeless(note, &:save)
|
||||
end
|
||||
end
|
||||
|
||||
def keep_around_commit
|
||||
project.repository.keep_around(self.merge_commit_sha)
|
||||
end
|
||||
|
|
|
@ -60,7 +60,7 @@ module MergeRequests
|
|||
|
||||
merge_requests.each do |merge_request|
|
||||
if merge_request.source_branch == @branch_name || force_push?
|
||||
merge_request.reload_code
|
||||
merge_request.reload_diff
|
||||
merge_request.mark_as_unchecked
|
||||
else
|
||||
mr_commit_ids = merge_request.commits.map(&:id)
|
||||
|
@ -68,7 +68,7 @@ module MergeRequests
|
|||
matches = mr_commit_ids & push_commit_ids
|
||||
|
||||
if matches.any?
|
||||
merge_request.reload_code
|
||||
merge_request.reload_diff
|
||||
merge_request.mark_as_unchecked
|
||||
else
|
||||
merge_request.mark_as_unchecked
|
||||
|
|
|
@ -6,7 +6,7 @@ module MergeRequests
|
|||
create_note(merge_request)
|
||||
notification_service.reopen_mr(merge_request, current_user)
|
||||
execute_hooks(merge_request, 'reopen')
|
||||
merge_request.reload_code
|
||||
merge_request.reload_diff
|
||||
merge_request.mark_as_unchecked
|
||||
end
|
||||
|
||||
|
|
|
@ -0,0 +1,30 @@
|
|||
module Notes
|
||||
class DiffPositionUpdateService < BaseService
|
||||
def execute(note)
|
||||
new_position = tracer.trace(note.position)
|
||||
|
||||
# Don't update the position if the type doesn't match, since that means
|
||||
# the diff line commented on was changed, and the comment is now outdated
|
||||
old_position = note.position
|
||||
if new_position &&
|
||||
new_position != old_position &&
|
||||
new_position.type == old_position.type
|
||||
|
||||
note.position = new_position
|
||||
end
|
||||
|
||||
note
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def tracer
|
||||
@tracer ||= Gitlab::Diff::PositionTracer.new(
|
||||
repository: project.repository,
|
||||
old_diff_refs: params[:old_diff_refs],
|
||||
new_diff_refs: params[:new_diff_refs],
|
||||
paths: params[:paths]
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue