diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2019-02-21 00:20:08 +0300 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2019-02-26 23:19:17 +0300 |
commit | e116fcab4151d61087afae17b6bf6cac58be29fc (patch) | |
tree | f641fd7f6fa036f93bcf171ffae370d29e57c5d7 /app | |
parent | 3395eacb57285424b7b8d49bdf836f638af31e8c (diff) |
Always fetch MR latest version when creating suggestions
This is an issue that can only be seen through EE. Further
details can be seen on
https://gitlab.com/gitlab-org/gitlab-ee/issues/9876. In general
we should always use the latest diff version of a file in order
to both create and apply suggestions.
Diffstat (limited to 'app')
-rw-r--r-- | app/models/diff_note.rb | 9 | ||||
-rw-r--r-- | app/models/suggestion.rb | 5 | ||||
-rw-r--r-- | app/services/suggestions/apply_service.rb | 12 | ||||
-rw-r--r-- | app/services/suggestions/create_service.rb | 10 |
4 files changed, 25 insertions, 11 deletions
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 279603496b0..bd9e146a40f 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -41,6 +41,15 @@ class DiffNote < Note create_note_diff_file(creation_params) end + # Returns the diff file from `position` + def latest_diff_file + strong_memoize(:latest_diff_file) do + repository = project.repository + position.diff_file(repository) + end + end + + # Returns the diff file from `original_position` def diff_file strong_memoize(:diff_file) do enqueue_diff_file_creation_job if should_create_diff_file? diff --git a/app/models/suggestion.rb b/app/models/suggestion.rb index 7eee4fbbe5f..09034646bff 100644 --- a/app/models/suggestion.rb +++ b/app/models/suggestion.rb @@ -19,11 +19,6 @@ class Suggestion < ApplicationRecord position.file_path end - def diff_file - repository = project.repository - position.diff_file(repository) - end - # For now, suggestions only serve as a way to send patches that # will change a single line (being able to apply multiple in the same place), # which explains `from_line` and `to_line` being the same line. diff --git a/app/services/suggestions/apply_service.rb b/app/services/suggestions/apply_service.rb index 1f720fc835f..f778c5aa5f5 100644 --- a/app/services/suggestions/apply_service.rb +++ b/app/services/suggestions/apply_service.rb @@ -15,7 +15,13 @@ module Suggestions return error('The file has been changed') end - params = file_update_params(suggestion) + diff_file = suggestion.note.latest_diff_file + + unless diff_file + return error('The file was not found') + end + + params = file_update_params(suggestion, diff_file) result = ::Files::UpdateService.new(suggestion.project, @current_user, params).execute if result[:status] == :success @@ -38,8 +44,8 @@ module Suggestions suggestion.position.head_sha == suggestion.noteable.source_branch_sha end - def file_update_params(suggestion) - blob = suggestion.diff_file.new_blob + def file_update_params(suggestion, diff_file) + blob = diff_file.new_blob file_path = suggestion.file_path branch_name = suggestion.branch file_content = new_file_content(suggestion, blob) diff --git a/app/services/suggestions/create_service.rb b/app/services/suggestions/create_service.rb index 77e958cbe0c..c7ac2452c53 100644 --- a/app/services/suggestions/create_service.rb +++ b/app/services/suggestions/create_service.rb @@ -9,6 +9,10 @@ module Suggestions def execute return unless @note.supports_suggestion? + diff_file = @note.latest_diff_file + + return unless diff_file + suggestions = Banzai::SuggestionsParser.parse(@note.note) # For single line suggestion we're only looking forward to @@ -20,7 +24,7 @@ module Suggestions rows = suggestions.map.with_index do |suggestion, index| - from_content = changing_lines(comment_line, comment_line) + from_content = changing_lines(diff_file, comment_line, comment_line) # The parsed suggestion doesn't have information about the correct # ending characters (we may have a line break, or not), so we take @@ -44,8 +48,8 @@ module Suggestions private - def changing_lines(from_line, to_line) - @note.diff_file.new_blob_lines_between(from_line, to_line).join + def changing_lines(diff_file, from_line, to_line) + diff_file.new_blob_lines_between(from_line, to_line).join end def line_break_chars(line) |