diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-05-21 23:38:33 +0300 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-05-24 00:27:30 +0300 |
commit | ab91f76e8b275d0fc1d78b38b95d4b2cd8e73bdb (patch) | |
tree | e22910b5a3fc8dee98c5d674ee030b46421a4727 /app/models | |
parent | 52527be4387cb978402a330f2e4de96e586a62db (diff) |
Add system note with link to diff comparison when MR discussion becomes outdated
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/diff_discussion.rb | 16 | ||||
-rw-r--r-- | app/models/diff_note.rb | 7 | ||||
-rw-r--r-- | app/models/merge_request.rb | 20 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 5 | ||||
-rw-r--r-- | app/models/note.rb | 17 | ||||
-rw-r--r-- | app/models/system_note_metadata.rb | 1 |
6 files changed, 35 insertions, 31 deletions
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 14ddd2fcc88..800574d8be0 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -19,21 +19,9 @@ class DiffDiscussion < Discussion def merge_request_version_params return unless for_merge_request? + return {} if active? - if active? - {} - else - diff_refs = position.diff_refs - - if diff = noteable.merge_request_diff_for(diff_refs) - { diff_id: diff.id } - elsif diff = noteable.merge_request_diff_for(diff_refs.head_sha) - { - diff_id: diff.id, - start_sha: diff_refs.start_sha - } - end - end + noteable.version_params_for(position.diff_refs) end def reply_attributes diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 76c59199afd..1764004078e 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -8,6 +8,7 @@ class DiffNote < Note serialize :original_position, Gitlab::Diff::Position serialize :position, Gitlab::Diff::Position + serialize :change_position, Gitlab::Diff::Position validates :original_position, presence: true validates :position, presence: true @@ -25,7 +26,7 @@ class DiffNote < Note DiffDiscussion end - %i(original_position position).each do |meth| + %i(original_position position change_position).each do |meth| define_method "#{meth}=" do |new_position| if new_position.is_a?(String) new_position = JSON.parse(new_position) rescue nil @@ -36,6 +37,8 @@ class DiffNote < Note new_position = Gitlab::Diff::Position.new(new_position) end + return if new_position == read_attribute(meth) + super(new_position) end end @@ -45,7 +48,7 @@ class DiffNote < Note end def diff_line - @diff_line ||= diff_file.line_for_position(self.original_position) if diff_file + @diff_line ||= diff_file&.line_for_position(self.original_position) end def for_line?(line) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9be00880438..2eec013fa9d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -416,13 +416,24 @@ class MergeRequest < ActiveRecord::Base @merge_request_diffs_by_diff_refs_or_sha[diff_refs_or_sha] end + def version_params_for(diff_refs) + if diff = merge_request_diff_for(diff_refs) + { diff_id: diff.id } + elsif diff = merge_request_diff_for(diff_refs.head_sha) + { + diff_id: diff.id, + start_sha: diff_refs.start_sha + } + end + end + def reload_diff_if_branch_changed if source_branch_changed? || target_branch_changed? reload_diff end end - def reload_diff + def reload_diff(current_user = nil) return unless open? old_diff_refs = self.diff_refs @@ -432,7 +443,8 @@ class MergeRequest < ActiveRecord::Base update_diff_notes_positions( old_diff_refs: old_diff_refs, - new_diff_refs: new_diff_refs + new_diff_refs: new_diff_refs, + current_user: current_user ) end @@ -861,7 +873,7 @@ class MergeRequest < ActiveRecord::Base diff_sha_refs && diff_sha_refs.complete? end - def update_diff_notes_positions(old_diff_refs:, new_diff_refs:) + def update_diff_notes_positions(old_diff_refs:, new_diff_refs:, current_user: nil) return unless has_complete_diff_refs? return if new_diff_refs == old_diff_refs @@ -875,7 +887,7 @@ class MergeRequest < ActiveRecord::Base service = Notes::DiffPositionUpdateService.new( self.project, - nil, + current_user, old_diff_refs: old_diff_refs, new_diff_refs: new_diff_refs, paths: paths diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index f0a3c30ea74..6e3917a10a3 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -175,12 +175,11 @@ class MergeRequestDiff < ActiveRecord::Base self == merge_request.merge_request_diff end - def compare_with(sha, straight: true) + def compare_with(sha) # When compare merge request versions we want diff A..B instead of A...B # so we handle cases when user does squash and rebase of the commits between versions. # For this reason we set straight to true by default. - CompareService.new(project, head_commit_sha) - .execute(project, sha, straight: straight) + CompareService.new(project, head_commit_sha).execute(project, sha, straight: true) end def commits_count diff --git a/app/models/note.rb b/app/models/note.rb index 46d0a4f159f..b4228cc6492 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -121,16 +121,17 @@ class Note < ActiveRecord::Base end def grouped_diff_discussions(diff_refs = nil) - groups = {} + groups = Hash.new { |h, k| h[k] = [] } diff_notes.fresh.discussions.each do |discussion| - if discussion.active?(diff_refs) - discussions = groups[discussion.line_code] ||= [] - elsif diff_refs && discussion.created_at_diff?(diff_refs) - discussions = groups[discussion.original_line_code] ||= [] - end - - discussions << discussion if discussions + line_code = + if discussion.active?(diff_refs) + discussion.line_code + elsif diff_refs && discussion.created_at_diff?(diff_refs) + discussion.original_line_code + end + + groups[line_code] << discussion if line_code end groups diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index b44f4fe000c..414c95f7705 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -2,6 +2,7 @@ class SystemNoteMetadata < ActiveRecord::Base ICON_TYPES = %w[ commit description merge confidential visible label assignee cross_reference title time_tracking branch milestone discussion task moved opened closed merged + outdated ].freeze validates :note, presence: true |