diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 70fbcb4c4e9..881ae5d1cad 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -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? diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 82e0f5a0b53..4624e9d36e9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -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 diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index de79c024428..21490ac77ea 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -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 diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index 8279ad2001b..eb88ae9d11c 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -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 diff --git a/app/services/notes/diff_position_update_service.rb b/app/services/notes/diff_position_update_service.rb new file mode 100644 index 00000000000..0cb731f5bc3 --- /dev/null +++ b/app/services/notes/diff_position_update_service.rb @@ -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