diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-06-01 17:00:00 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-06-01 17:00:00 +0300 |
commit | 0218a0bd2308eaa1b968cc0e0c68fdb88788df56 (patch) | |
tree | 9c69bde0abe23fc0ac6a64726cdefac5fd0807ad /app | |
parent | f07aee72bef4604312e11a43fce3a47865bce100 (diff) | |
parent | 09838ac626df739f0ab9852e3c7d862c7fd707e5 (diff) |
Merge branch 'dm-update-discussion-diff-position' into 'master'
Update diff discussion position per discussion instead of per note
Closes #33157
See merge request !11833
Diffstat (limited to 'app')
-rw-r--r-- | app/models/diff_discussion.rb | 1 | ||||
-rw-r--r-- | app/models/diff_note.rb | 18 | ||||
-rw-r--r-- | app/models/merge_request.rb | 22 | ||||
-rw-r--r-- | app/services/discussions/update_diff_position_service.rb | 41 | ||||
-rw-r--r-- | app/services/notes/diff_position_update_service.rb | 33 |
5 files changed, 64 insertions, 51 deletions
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 800574d8be0..07c4846e2ac 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -10,6 +10,7 @@ class DiffDiscussion < Discussion delegate :position, :original_position, + :change_position, to: :first_note diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 6cd0502069a..20ef1378500 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -95,13 +95,21 @@ class DiffNote < Note return if active? - Notes::DiffPositionUpdateService.new( - self.project, - nil, + tracer = Gitlab::Diff::PositionTracer.new( + project: self.project, old_diff_refs: self.position.diff_refs, - new_diff_refs: noteable.diff_refs, + new_diff_refs: self.noteable.diff_refs, paths: self.position.paths - ).execute(self) + ) + + result = tracer.trace(self.position) + return unless result + + if result[:outdated] + self.change_position = result[:position] + else + self.position = result[:position] + end end def verify_supported diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4bccda8a732..dd155252ad5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -421,7 +421,7 @@ class MergeRequest < ActiveRecord::Base MergeRequests::MergeRequestDiffCacheService.new.execute(self) new_diff_refs = self.diff_refs - update_diff_notes_positions( + update_diff_discussion_positions( old_diff_refs: old_diff_refs, new_diff_refs: new_diff_refs, current_user: current_user @@ -853,19 +853,18 @@ class MergeRequest < ActiveRecord::Base diff_refs && diff_refs.complete? end - def update_diff_notes_positions(old_diff_refs:, new_diff_refs:, current_user: nil) + def update_diff_discussion_positions(old_diff_refs:, new_diff_refs:, current_user: nil) return unless has_complete_diff_refs? return if new_diff_refs == old_diff_refs - active_diff_notes = self.notes.new_diff_notes.select do |note| - note.active?(old_diff_refs) + active_diff_discussions = self.notes.new_diff_notes.discussions.select do |discussion| + discussion.active?(old_diff_refs) end + return if active_diff_discussions.empty? - return if active_diff_notes.empty? + paths = active_diff_discussions.flat_map { |n| n.diff_file.paths }.uniq - paths = active_diff_notes.flat_map { |n| n.diff_file.paths }.uniq - - service = Notes::DiffPositionUpdateService.new( + service = Discussions::UpdateDiffPositionService.new( self.project, current_user, old_diff_refs: old_diff_refs, @@ -873,11 +872,8 @@ class MergeRequest < ActiveRecord::Base paths: paths ) - transaction do - active_diff_notes.each do |note| - service.execute(note) - Gitlab::Timeless.timeless(note, &:save) - end + active_diff_discussions.each do |discussion| + service.execute(discussion) end end diff --git a/app/services/discussions/update_diff_position_service.rb b/app/services/discussions/update_diff_position_service.rb new file mode 100644 index 00000000000..1ef8d9edbe1 --- /dev/null +++ b/app/services/discussions/update_diff_position_service.rb @@ -0,0 +1,41 @@ +module Discussions + class UpdateDiffPositionService < BaseService + def execute(discussion) + result = tracer.trace(discussion.position) + return unless result + + position = result[:position] + outdated = result[:outdated] + + discussion.notes.each do |note| + if outdated + note.change_position = position + else + note.position = position + note.change_position = nil + end + end + + Note.transaction do + discussion.notes.each do |note| + Gitlab::Timeless.timeless(note, &:save) + end + + if outdated && current_user + SystemNoteService.diff_discussion_outdated(discussion, project, current_user, position) + end + end + end + + private + + def tracer + @tracer ||= Gitlab::Diff::PositionTracer.new( + project: project, + old_diff_refs: params[:old_diff_refs], + new_diff_refs: params[:new_diff_refs], + paths: params[:paths] + ) + end + end +end diff --git a/app/services/notes/diff_position_update_service.rb b/app/services/notes/diff_position_update_service.rb deleted file mode 100644 index eff7b287269..00000000000 --- a/app/services/notes/diff_position_update_service.rb +++ /dev/null @@ -1,33 +0,0 @@ -module Notes - class DiffPositionUpdateService < BaseService - def execute(note) - results = tracer.trace(note.position) - return unless results - - position = results[:position] - outdated = results[:outdated] - - if outdated - note.change_position = position - - if note.persisted? && current_user - SystemNoteService.diff_discussion_outdated(note.to_discussion, project, current_user, position) - end - else - note.position = position - note.change_position = nil - end - end - - private - - def tracer - @tracer ||= Gitlab::Diff::PositionTracer.new( - project: project, - old_diff_refs: params[:old_diff_refs], - new_diff_refs: params[:new_diff_refs], - paths: params[:paths] - ) - end - end -end |