summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-07-07 20:45:03 +0000
committerDouwe Maan <douwe@gitlab.com>2016-07-07 20:45:03 +0000
commit86d238e4bda6424a79eb9d8ea7cfe41af714f49f (patch)
treee8d266bf7b22516ccfc2f83819ad858c08d1b0ad
parent91cf0387dd91b380647457c41edd948a357b620c (diff)
parentc66bcf34dd2ede698a784f55bd17346e85aeaf92 (diff)
downloadgitlab-ce-86d238e4bda6424a79eb9d8ea7cfe41af714f49f.tar.gz
Merge branch 'new-diff-notes' into 'master'
New diff notes Fixes #12732, #14731, #19375, #14783 Builds on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4110 To do: - [x] Get it mostly working - [x] Validate position validity - [x] Fix: Don’t link to `#` - [x] Fix: Base ref can be `nil`, potentially, when the MR has an oprhan source branch => Yep, doesn’t work. We need to store a `start_id` - [x] Optimize: Fewer duplicate `git diff` compares - [x] Optimize: Pass paths to `PositionTracer#diff` for faster diffs - [x] Refactor: Use `head_id` in `MergeRequest`/`MergeRequestDiff` instead of `source_sha` - [x] Refactor: Convert existing array-based diff refs to the DiffRefs model - [x] Tweak: Use `note_type` in `Autosave` key - [x] Tweak: Remove `line_code: note.line_code` from `link_to_reply_discussion` - [x] Update: `SentNotifications` and reply-by-email receiver - [x] Update: MR diff notification email - [x] Update: API (MR, Commit note creation and entity) - [x] Update: GitHub importer - [x] Address any other TODO comments - [x] Fix: Suppress "edited 4 minutes ago" - [x] Write tests - [x] `LineMapper` - [x] `PositionTracer` - [x] `Position` - [x] `DiffPositionUpdateService` - [x] `DiffNote` - [x] `MergeRequests::RefreshService` / `MergeRequest#update_diff_notes_positions` - [x] Make sure commits with diff notes don't get cleaned up, since this would prevent the diff notes from being rendered (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5062) Future improvements: - Display unresolved comments on files outside the diff, if the comment was added when that file _was_ part of the diff - Allow commenting on sections between hunks, when expanding the diff using `...` - (We'd need to generate line code based on Position if we have it, even if it falls outside bounds of diff) - `diff_hunk` on diff note API entity - Show diff hunk in notification email - Resolved line notes would have a boolean, and be inactive through `notes.any? { !active? || resolved? }` - Multi line notes would store a number of positions, and do the right thing (™) in grouping and then rendering if the first item is multiline? => true - Image diff notes could store x,y,width,height instead of old_line,new_line for similar grouping. Does it need a reference to say if it's on old or new? These can't have line_codes, clearly. Rendering would be interesting. - Show commit line comments in the MR diff - Comment on specific selected words - Comment on file header - Unfold top of discussion diff note - New diff notes API for commits and MRs /cc @rspeicher See merge request !4101
-rw-r--r--app/assets/javascripts/notes.js.coffee13
-rw-r--r--app/assets/stylesheets/framework/files.scss2
-rw-r--r--app/assets/stylesheets/pages/diff.scss10
-rw-r--r--app/controllers/projects/blob_controller.rb2
-rw-r--r--app/controllers/projects/commit_controller.rb1
-rw-r--r--app/controllers/projects/compare_controller.rb18
-rw-r--r--app/controllers/projects/merge_requests_controller.rb34
-rw-r--r--app/controllers/projects/notes_controller.rb21
-rw-r--r--app/helpers/application_helper.rb11
-rw-r--r--app/helpers/diff_helper.rb15
-rw-r--r--app/helpers/merge_requests_helper.rb2
-rw-r--r--app/helpers/notes_helper.rb73
-rw-r--r--app/helpers/workhorse_helper.rb9
-rw-r--r--app/mailers/emails/projects.rb3
-rw-r--r--app/models/commit.rb7
-rw-r--r--app/models/concerns/note_on_diff.rb52
-rw-r--r--app/models/deployment.rb6
-rw-r--r--app/models/diff_note.rb127
-rw-r--r--app/models/legacy_diff_note.rb69
-rw-r--r--app/models/merge_request.rb189
-rw-r--r--app/models/merge_request_diff.rb138
-rw-r--r--app/models/note.rb12
-rw-r--r--app/models/project_services/jira_service.rb2
-rw-r--r--app/models/repository.rb10
-rw-r--r--app/models/sent_notification.rb45
-rw-r--r--app/services/merge_requests/merge_service.rb2
-rw-r--r--app/services/merge_requests/merge_when_build_succeeds_service.rb2
-rw-r--r--app/services/merge_requests/refresh_service.rb12
-rw-r--r--app/services/merge_requests/reopen_service.rb2
-rw-r--r--app/services/notes/diff_position_update_service.rb30
-rw-r--r--app/views/notify/note_merge_request_email.html.haml4
-rw-r--r--app/views/notify/repository_push_email.html.haml5
-rw-r--r--app/views/projects/commit/show.html.haml3
-rw-r--r--app/views/projects/diffs/_diffs.html.haml4
-rw-r--r--app/views/projects/diffs/_file.html.haml51
-rw-r--r--app/views/projects/diffs/_file_header.html.haml25
-rw-r--r--app/views/projects/diffs/_image.html.haml11
-rw-r--r--app/views/projects/diffs/_line.html.haml16
-rw-r--r--app/views/projects/diffs/_parallel_view.html.haml18
-rw-r--r--app/views/projects/diffs/_text_file.html.haml13
-rw-r--r--app/views/projects/merge_requests/widget/_heading.html.haml10
-rw-r--r--app/views/projects/merge_requests/widget/open/_accept.html.haml2
-rw-r--r--app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml2
-rw-r--r--app/views/projects/notes/_diff_notes_with_reply.html.haml3
-rw-r--r--app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml6
-rw-r--r--app/views/projects/notes/_form.html.haml1
-rw-r--r--app/views/projects/notes/discussions/_diff_with_notes.html.haml35
-rw-r--r--app/views/projects/notes/discussions/_notes.html.haml3
-rw-r--r--app/workers/emails_on_push_worker.rb16
-rw-r--r--db/migrate/20160508202603_add_head_commit_id_to_merge_request_diffs.rb5
-rw-r--r--db/migrate/20160508215920_add_positions_to_diff_notes.rb6
-rw-r--r--db/migrate/20160516224534_add_start_commit_id_to_merge_request_diffs.rb5
-rw-r--r--db/migrate/20160522215720_add_note_type_and_position_to_sent_notification.rb22
-rw-r--r--db/schema.rb8
-rw-r--r--features/steps/project/merge_requests.rb5
-rw-r--r--features/steps/shared/diff_note.rb12
-rw-r--r--lib/api/entities.rb6
-rw-r--r--lib/api/merge_requests.rb4
-rw-r--r--lib/gitlab/diff/diff_refs.rb36
-rw-r--r--lib/gitlab/diff/file.rb101
-rw-r--r--lib/gitlab/diff/highlight.rb31
-rw-r--r--lib/gitlab/diff/line.rb16
-rw-r--r--lib/gitlab/diff/line_mapper.rb64
-rw-r--r--lib/gitlab/diff/parallel_diff.rb32
-rw-r--r--lib/gitlab/diff/position.rb155
-rw-r--r--lib/gitlab/diff/position_tracer.rb168
-rw-r--r--lib/gitlab/email/message/repository_push.rb8
-rw-r--r--lib/gitlab/email/receiver.rb10
-rw-r--r--lib/gitlab/github_import/pull_request_formatter.rb4
-rw-r--r--lib/gitlab/timeless.rb16
-rw-r--r--lib/gitlab/workhorse.rb12
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb8
-rw-r--r--spec/factories/notes.rb33
-rw-r--r--spec/features/merge_requests/created_from_fork_spec.rb2
-rw-r--r--spec/features/merge_requests/merge_when_build_succeeds_spec.rb4
-rw-r--r--spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb2
-rw-r--r--spec/features/notes_on_merge_requests_spec.rb6
-rw-r--r--spec/fixtures/parallel_diff_result.yml504
-rw-r--r--spec/helpers/diff_helper_spec.rb2
-rw-r--r--spec/lib/gitlab/diff/file_spec.rb2
-rw-r--r--spec/lib/gitlab/diff/highlight_spec.rb6
-rw-r--r--spec/lib/gitlab/diff/line_mapper_spec.rb137
-rw-r--r--spec/lib/gitlab/diff/parallel_diff_spec.rb3
-rw-r--r--spec/lib/gitlab/diff/position_spec.rb341
-rw-r--r--spec/lib/gitlab/diff/position_tracer_spec.rb1758
-rw-r--r--spec/lib/gitlab/github_import/pull_request_formatter_spec.rb12
-rw-r--r--spec/lib/gitlab/import_export/project_tree_saver_spec.rb2
-rw-r--r--spec/lib/gitlab/note_data_builder_spec.rb4
-rw-r--r--spec/lib/gitlab/url_builder_spec.rb8
-rw-r--r--spec/mailers/notify_spec.rb4
-rw-r--r--spec/models/diff_note_spec.rb191
-rw-r--r--spec/models/event_spec.rb4
-rw-r--r--spec/models/legacy_diff_note_spec.rb16
-rw-r--r--spec/models/merge_request_spec.rb69
-rw-r--r--spec/models/project_spec.rb2
-rw-r--r--spec/models/repository_spec.rb4
-rw-r--r--spec/requests/api/merge_requests_spec.rb4
-rw-r--r--spec/services/notes/diff_position_update_service_spec.rb175
-rw-r--r--spec/services/system_note_service_spec.rb2
99 files changed, 4622 insertions, 556 deletions
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 7c1d943667b..0b7d8f64456 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -240,12 +240,16 @@ class @Notes
@note_ids.push(note.id)
form = $("#new-discussion-note-form-#{note.discussion_id}")
+ if note.original_discussion_id? and form.length is 0
+ form = $("#new-discussion-note-form-#{note.original_discussion_id}")
row = form.closest("tr")
note_html = $(note.html)
note_html.syntaxHighlight()
# is this the first note of discussion?
discussionContainer = $(".notes[data-discussion-id='" + note.discussion_id + "']")
+ if note.original_discussion_id? and discussionContainer.length is 0
+ discussionContainer = $(".notes[data-discussion-id='" + note.original_discussion_id + "']")
if discussionContainer.length is 0
# insert the note and the reply button after the temp row
row.after note.discussion_html
@@ -318,6 +322,7 @@ class @Notes
form.addClass "js-main-target-form"
form.find("#note_line_code").remove()
+ form.find("#note_position").remove()
form.find("#note_type").remove()
###
@@ -335,10 +340,12 @@ class @Notes
new Autosave textarea, [
"Note"
- form.find("#note_commit_id").val()
- form.find("#note_line_code").val()
form.find("#note_noteable_type").val()
form.find("#note_noteable_id").val()
+ form.find("#note_commit_id").val()
+ form.find("#note_type").val()
+ form.find("#note_line_code").val()
+ form.find("#note_position").val()
]
###
@@ -512,10 +519,12 @@ class @Notes
setupDiscussionNoteForm: (dataHolder, form) =>
# setup note target
form.attr 'id', "new-discussion-note-form-#{dataHolder.data("discussionId")}"
+ form.attr "data-line-code", dataHolder.data("lineCode")
form.find("#note_type").val dataHolder.data("noteType")
form.find("#line_type").val dataHolder.data("lineType")
form.find("#note_commit_id").val dataHolder.data("commitId")
form.find("#note_line_code").val dataHolder.data("lineCode")
+ form.find("#note_position").val dataHolder.attr("data-position")
form.find("#note_noteable_type").val dataHolder.data("noteableType")
form.find("#note_noteable_id").val dataHolder.data("noteableId")
form.find('.js-note-discard')
diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss
index 71a9f79be3e..71e4b50f2af 100644
--- a/app/assets/stylesheets/framework/files.scss
+++ b/app/assets/stylesheets/framework/files.scss
@@ -50,7 +50,7 @@
}
a:not(.btn) {
- color: $gl-dark-link-color;
+ color: $gl-text-color;
}
.left-options {
diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss
index 5286b73cc50..21b1c223c88 100644
--- a/app/assets/stylesheets/pages/diff.scss
+++ b/app/assets/stylesheets/pages/diff.scss
@@ -434,13 +434,3 @@
}
}
}
-
-.discussion {
- .diff-content {
- .diff-line-num {
- &:before {
- content: attr(data-linenumber);
- }
- }
- }
-}
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 7599fec3cdf..5356fdf010d 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -57,7 +57,7 @@ class Projects::BlobController < Projects::ApplicationController
diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3', include_diff_info: true)
diff_lines = diffy.diff.scan(/.*\n/)[2..-1]
diff_lines = Gitlab::Diff::Parser.new.parse(diff_lines)
- @diff_lines = Gitlab::Diff::Highlight.new(diff_lines).highlight
+ @diff_lines = Gitlab::Diff::Highlight.new(diff_lines, repository: @repository).highlight
render layout: false
end
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index d162a5a3165..37d6521026c 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -121,7 +121,6 @@ class Projects::CommitController < Projects::ApplicationController
opts[:ignore_whitespace_change] = true if params[:format] == 'diff'
@diffs = commit.diffs(opts)
- @diff_refs = [commit.parent || commit, commit]
@notes_count = commit.notes.count
@statuses = CommitStatus.where(pipeline: pipelines)
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index af0b69a2442..d240b9fe989 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -14,14 +14,22 @@ class Projects::CompareController < Projects::ApplicationController
def show
compare = CompareService.new.
- execute(@project, @head_ref, @project, @base_ref, diff_options)
+ execute(@project, @head_ref, @project, @start_ref, diff_options)
if compare
@commits = Commit.decorate(compare.commits, @project)
+
+ @start_commit = @project.commit(@start_ref)
@commit = @project.commit(@head_ref)
- @base_commit = @project.merge_base_commit(@base_ref, @head_ref)
+ @base_commit = @project.merge_base_commit(@start_ref, @head_ref)
+
@diffs = compare.diffs(diff_options)
- @diff_refs = [@base_commit, @commit]
+ @diff_refs = Gitlab::Diff::DiffRefs.new(
+ base_sha: @base_commit.try(:sha),
+ start_sha: @start_commit.try(:sha),
+ head_sha: @commit.try(:sha)
+ )
+
@diff_notes_disabled = true
@grouped_diff_notes = {}
end
@@ -35,12 +43,12 @@ class Projects::CompareController < Projects::ApplicationController
private
def assign_ref_vars
- @base_ref = Addressable::URI.unescape(params[:from])
+ @start_ref = Addressable::URI.unescape(params[:from])
@ref = @head_ref = Addressable::URI.unescape(params[:to])
end
def merge_request
@merge_request ||= @project.merge_requests.opened.
- find_by(source_project: @project, source_branch: @head_ref, target_branch: @base_ref)
+ find_by(source_project: @project, source_branch: @head_ref, target_branch: @start_ref)
end
end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index dd86b940a08..df1943dd9bb 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -58,14 +58,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController
respond_to do |format|
format.html
- format.json { render json: @merge_request }
+
+ format.json do
+ render json: @merge_request
+ end
+
format.patch do
- headers.store(*Gitlab::Workhorse.send_git_patch(@project.repository,
- @merge_request.diff_base_commit.id,
- @merge_request.last_commit.id))
- headers['Content-Disposition'] = 'inline'
- head :ok
+ return render_404 unless @merge_request.diff_refs
+
+ send_git_patch @project.repository, @merge_request.diff_refs
end
+
format.diff do
return render_404 unless @merge_request.diff_refs
@@ -77,18 +80,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def diffs
apply_diff_view_cookie!
- @commit = @merge_request.last_commit
- @base_commit = @merge_request.diff_base_commit
-
- # MRs created before 8.4 don't have a diff_base_commit,
- # but we need it for the "View file @ ..." link by deleted files
- @base_commit ||= @merge_request.first_commit.parent || @merge_request.first_commit
+ @commit = @merge_request.diff_head_commit
+ @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit
@comments_target = {
noteable_type: 'MergeRequest',
noteable_id: @merge_request.id
}
+ @use_legacy_diff_notes = !@merge_request.support_new_diff_notes?
@grouped_diff_notes = @merge_request.notes.grouped_diff_notes
Banzai::NoteRenderer.render(
@@ -134,7 +134,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@target_project = merge_request.target_project
@source_project = merge_request.source_project
@commits = @merge_request.compare_commits.reverse
- @commit = @merge_request.last_commit
+ @commit = @merge_request.diff_head_commit
@base_commit = @merge_request.diff_base_commit
@diffs = @merge_request.compare.diffs(diff_options) if @merge_request.compare
@diff_notes_disabled = true
@@ -212,7 +212,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
return
end
- if params[:sha] != @merge_request.source_sha
+ if params[:sha] != @merge_request.diff_head_sha
@status = :sha_mismatch
return
end
@@ -274,16 +274,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController
status ||= "preparing"
else
ci_service = @merge_request.source_project.ci_service
- status = ci_service.commit_status(merge_request.last_commit.sha, merge_request.source_branch) if ci_service
+ status = ci_service.commit_status(merge_request.diff_head_sha, merge_request.source_branch) if ci_service
if ci_service.respond_to?(:commit_coverage)
- coverage = ci_service.commit_coverage(merge_request.last_commit.sha, merge_request.source_branch)
+ coverage = ci_service.commit_coverage(merge_request.diff_head_sha, merge_request.source_branch)
end
end
response = {
title: merge_request.title,
- sha: merge_request.last_commit_short_sha,
+ sha: merge_request.diff_head_commit.short_id,
status: status,
coverage: coverage
}
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index e14fe26dde7..3eacdbbd067 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -128,7 +128,7 @@ class Projects::NotesController < Projects::ApplicationController
elsif note.valid?
Banzai::NoteRenderer.render([note], @project, current_user)
- {
+ attrs = {
valid: true,
id: note.id,
discussion_id: note.discussion_id,
@@ -138,6 +138,23 @@ class Projects::NotesController < Projects::ApplicationController
discussion_html: note_to_discussion_html(note),
discussion_with_diff_html: note_to_discussion_with_diff_html(note)
}
+
+ # The discussion_id is used to add the comment to the correct discussion
+ # element on the merge request page. Among other things, the discussion_id
+ # contains the sha of head commit of the merge request.
+ # When new commits are pushed into the merge request after the initial
+ # load of the merge request page, the discussion elements will still have
+ # the old discussion_ids, with the old head commit sha. The new comment,
+ # however, will have the new discussion_id with the new commit sha.
+ # To ensure that these new comments will still end up in the correct
+ # discussion element, we also send the original discussion_id, with the
+ # old commit sha, along, and fall back on this value when no discussion
+ # element with the new discussion_id could be found.
+ if note.new_diff_note? && note.position != note.original_position
+ attrs[:original_discussion_id] = note.original_discussion_id
+ end
+
+ attrs
else
{
valid: false,
@@ -154,7 +171,7 @@ class Projects::NotesController < Projects::ApplicationController
def note_params
params.require(:note).permit(
:note, :noteable, :noteable_id, :noteable_type, :project_id,
- :attachment, :line_code, :commit_id, :type
+ :attachment, :line_code, :commit_id, :type, :position
)
end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 62d13a4b4f3..03495cf5ec4 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -306,4 +306,15 @@ module ApplicationHelper
def truncate_first_line(message, length = 50)
truncate(message.each_line.first.chomp, length: length) if message
end
+
+ # While similarly named to Rails's `link_to_if`, this method behaves quite differently.
+ # If `condition` is truthy, a link will be returned with the result of the block
+ # as its body. If `condition` is falsy, only the result of the block will be returned.
+ def conditional_link_to(condition, options, html_options = {}, &block)
+ if condition
+ link_to options, html_options, &block
+ else
+ capture(&block)
+ end
+ end
end
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index e22dce59d0f..eb57516247d 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -30,12 +30,8 @@ module DiffHelper
options
end
- def safe_diff_files(diffs, diff_refs)
- diffs.decorate! { |diff| Gitlab::Diff::File.new(diff, diff_refs) }
- end
-
- def generate_line_code(file_path, line)
- Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
+ def safe_diff_files(diffs, diff_refs: nil, repository: nil)
+ diffs.decorate! { |diff| Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
end
def unfold_bottom_class(bottom)
@@ -93,6 +89,8 @@ module DiffHelper
end
def commit_for_diff(diff_file)
+ return diff_file.content_commit if diff_file.content_commit
+
if diff_file.deleted_file
@base_commit || @commit.parent || @commit
else
@@ -100,10 +98,11 @@ module DiffHelper
end
end
- def diff_file_html_data(project, diff_commit, diff_file)
+ def diff_file_html_data(project, diff_file)
+ commit = commit_for_diff(diff_file)
{
blob_diff_path: namespace_project_blob_diff_path(project.namespace, project,
- tree_join(diff_commit.id, diff_file.file_path))
+ tree_join(commit.id, diff_file.file_path))
}
end
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 1dd07a2a220..c7dedfe9254 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -27,7 +27,7 @@ module MergeRequestsHelper
end
def ci_build_details_path(merge_request)
- build_url = merge_request.source_project.ci_service.build_page(merge_request.last_commit.sha, merge_request.source_branch)
+ build_url = merge_request.source_project.ci_service.build_page(merge_request.diff_head_sha, merge_request.source_branch)
return nil unless build_url
parsed_url = URI.parse(build_url)
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index e85ba76887d..2302e65c537 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -24,23 +24,55 @@ module NotesHelper
}.to_json
end
- def link_to_new_diff_note(line_code, line_type = nil)
- discussion_id = LegacyDiffNote.build_discussion_id(
- @comments_target[:noteable_type],
- @comments_target[:noteable_id] || @comments_target[:commit_id],
- line_code
- )
+ def link_to_new_diff_note(line_code, position, line_type = nil)
+ use_legacy_diff_note = @use_legacy_diff_notes
+ # If the controller doesn't force the use of legacy diff notes, we
+ # determine this on a line-by-line basis by seeing if there already exist
+ # active legacy diff notes at this line, in which case newly created notes
+ # will use the legacy technology as well.
+ # We do this because the discussion_id values of legacy and "new" diff
+ # notes, which are used to group notes on the merge request discussion tab,
+ # are incompatible.
+ # If we didn't, diff notes that would show for the same line on the changes
+ # tab, would show in different discussions on the discussion tab.
+ use_legacy_diff_note ||= begin
+ line_diff_notes = @grouped_diff_notes[line_code]
+ line_diff_notes && line_diff_notes.any?(&:legacy_diff_note?)
+ end
data = {
noteable_type: @comments_target[:noteable_type],
noteable_id: @comments_target[:noteable_id],
commit_id: @comments_target[:commit_id],
line_type: line_type,
- line_code: line_code,
- note_type: LegacyDiffNote.name,
- discussion_id: discussion_id
+ line_code: line_code
}
+ if use_legacy_diff_note
+ discussion_id = LegacyDiffNote.build_discussion_id(
+ @comments_target[:noteable_type],
+ @comments_target[:noteable_id] || @comments_target[:commit_id],
+ line_code
+ )
+
+ data.merge!(
+ note_type: LegacyDiffNote.name,
+ discussion_id: discussion_id
+ )
+ else
+ discussion_id = DiffNote.build_discussion_id(
+ @comments_target[:noteable_type],
+ @comments_target[:noteable_id] || @comments_target[:commit_id],
+ position
+ )
+
+ data.merge!(
+ position: position.to_json,
+ note_type: DiffNote.name,
+ discussion_id: discussion_id
+ )
+ end
+
button_tag(class: 'btn add-diff-note js-add-diff-note-button',
data: data,
title: 'Add a comment to this line') do
@@ -60,14 +92,15 @@ module NotesHelper
}
if note.diff_note?
- data.merge!(
- line_code: note.line_code,
- note_type: LegacyDiffNote.name
- )
+ data[:note_type] = note.type
+
+ data.merge!(note.diff_attributes)
end
- button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
- data: data, title: 'Add a reply'
+ content_tag(:div, class: "discussion-reply-holder") do
+ button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
+ data: data, title: 'Add a reply'
+ end
end
def note_max_access_for_user(note)
@@ -79,4 +112,14 @@ module NotesHelper
full_key = { project: note.project, user_id: note.author_id }
@max_access_by_user_id[full_key]
end
+
+ def diff_note_path(note)
+ return unless note.diff_note?
+
+ if note.for_merge_request? && note.active?
+ diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code)
+ elsif note.for_commit?
+ namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code)
+ end
+ end
end
diff --git a/app/helpers/workhorse_helper.rb b/app/helpers/workhorse_helper.rb
index 2bd0dbfd095..65598ad9ed3 100644
--- a/app/helpers/workhorse_helper.rb
+++ b/app/helpers/workhorse_helper.rb
@@ -1,4 +1,4 @@
-# Helpers to send Git blobs, diffs or archives through Workhorse.
+# Helpers to send Git blobs, diffs, patches or archives through Workhorse.
# Workhorse will also serve files when using `send_file`.
module WorkhorseHelper
# Send a Git blob through Workhorse
@@ -16,6 +16,13 @@ module WorkhorseHelper
head :ok
end
+ # Send a Git patch through Workhorse
+ def send_git_patch(repository, diff_refs)
+ headers.store(*Gitlab::Workhorse.send_git_patch(repository, diff_refs))
+ headers['Content-Disposition'] = 'inline'
+ head :ok
+ end
+
# Archive a Git repository and send it through Workhorse
def send_git_archive(repository, ref:, format:)
headers.store(*Gitlab::Workhorse.send_git_archive(repository, ref: ref, format: format))
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index e0af7081411..236b6ab00d8 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -29,8 +29,7 @@ module Emails
# used in notify layout
@target_url = @message.target_url
@project = Project.find(project_id)
- @diff_notes_disabled = true
-
+
add_project_headers
headers['X-GitLab-Author'] = @message.author_username
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 174ccbaea6c..2ef3973c160 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -214,6 +214,13 @@ class Commit
@raw.short_id(7)
end
+ def diff_refs
+ Gitlab::Diff::DiffRefs.new(
+ base_sha: self.parent_id || self.sha,
+ head_sha: self.sha
+ )
+ end
+
def pipelines
@pipeline ||= project.pipelines.where(sha: sha)
end
diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb
new file mode 100644
index 00000000000..2785fbb21c9
--- /dev/null
+++ b/app/models/concerns/note_on_diff.rb
@@ -0,0 +1,52 @@
+module NoteOnDiff
+ extend ActiveSupport::Concern
+
+ NUMBER_OF_TRUNCATED_DIFF_LINES = 16
+
+ included do
+ delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true
+ end
+
+ def diff_note?
+ true
+ end
+
+ def diff_file
+ raise NotImplementedError
+ end
+
+ def diff_line
+ raise NotImplementedError
+ end
+
+ def for_line?(line)
+ raise NotImplementedError
+ end
+
+ def diff_attributes
+ raise NotImplementedError
+ end
+
+ def can_be_award_emoji?
+ false
+ end
+
+ # Returns an array of at most 16 highlighted lines above a diff note
+ def truncated_diff_lines
+ prev_lines = []
+
+ highlighted_diff_lines.each do |line|
+ if line.meta?
+ prev_lines.clear
+ else
+ prev_lines << line
+
+ break if for_line?(line)
+
+ prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
+ end
+ end
+
+ prev_lines
+ end
+end
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index e498ca96e3c..520026c18dd 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -11,6 +11,8 @@ class Deployment < ActiveRecord::Base
delegate :name, to: :environment, prefix: true
+ after_save :keep_around_commit
+
def commit
project.commit(sha)
end
@@ -26,4 +28,8 @@ class Deployment < ActiveRecord::Base
def last?
self == environment.last_deployment
end
+
+ def keep_around_commit
+ project.repository.keep_around(self.sha)
+ end
end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
new file mode 100644
index 00000000000..9671955db36
--- /dev/null
+++ b/app/models/diff_note.rb
@@ -0,0 +1,127 @@
+class DiffNote < Note
+ include NoteOnDiff
+
+ serialize :original_position, Gitlab::Diff::Position
+ serialize :position, Gitlab::Diff::Position
+
+ validates :original_position, presence: true
+ validates :position, presence: true
+ validates :diff_line, presence: true
+ validates :line_code, presence: true, line_code: true
+ validates :noteable_type, inclusion: { in: ['Commit', 'MergeRequest'] }
+ validate :positions_complete
+ validate :verify_supported
+
+ before_validation :set_original_position, :update_position, on: :create
+ before_validation :set_line_code
+ after_save :keep_around_commits
+
+ class << self
+ def build_discussion_id(noteable_type, noteable_id, position)
+ [super(noteable_type, noteable_id), *position.key].join("-")
+ end
+ end
+
+ def new_diff_note?
+ true
+ end
+
+ def diff_attributes
+ { position: position.to_json }
+ end
+
+ def discussion_id
+ @discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, position)
+ end
+
+ def original_discussion_id
+ @original_discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, original_position)
+ end
+
+ def position=(new_position)
+ if new_position.is_a?(String)
+ new_position = JSON.parse(new_position) rescue nil
+ end
+
+ if new_position.is_a?(Hash)
+ new_position = new_position.with_indifferent_access
+ new_position = Gitlab::Diff::Position.new(new_position)
+ end
+
+ super(new_position)
+ end
+
+ def diff_file
+ @diff_file ||= self.original_position.diff_file(self.project.repository)
+ end
+
+ def diff_line
+ @diff_line ||= diff_file.line_for_position(self.original_position) if diff_file
+ end
+
+ def for_line?(line)
+ diff_file.position(line) == self.original_position
+ end
+
+ def active?(diff_refs = nil)
+ return false unless supported?
+ return true if for_commit?
+
+ diff_refs ||= self.noteable.diff_refs
+
+ self.position.diff_refs == diff_refs
+ end
+
+ private
+
+ def supported?
+ !self.for_merge_request? || self.noteable.support_new_diff_notes?
+ end
+
+ def set_original_position
+ self.original_position = self.position.dup
+ end
+
+ def set_line_code
+ self.line_code = self.position.line_code(self.project.repository)
+ 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 verify_supported
+ return if supported?
+
+ errors.add(:noteable, "doesn't support new-style diff notes")
+ end
+
+ def positions_complete
+ return if self.original_position.complete? && self.position.complete?
+
+ errors.add(:position, "is invalid")
+ end
+
+ def keep_around_commits
+ project.repository.keep_around(self.original_position.base_sha)
+ project.repository.keep_around(self.original_position.start_sha)
+ project.repository.keep_around(self.original_position.head_sha)
+
+ if self.position != self.original_position
+ project.repository.keep_around(self.position.base_sha)
+ project.repository.keep_around(self.position.start_sha)
+ project.repository.keep_around(self.position.head_sha)
+ end
+ end
+end
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 33d2a69ebaf..790dfd4d480 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -1,4 +1,6 @@
class LegacyDiffNote < Note
+ include NoteOnDiff
+
serialize :st_diff
validates :line_code, presence: true, line_code: true
@@ -11,12 +13,12 @@ class LegacyDiffNote < Note
end
end
- def diff_note?
+ def legacy_diff_note?
true
end
- def legacy_diff_note?
- true
+ def diff_attributes
+ { line_code: line_code }
end
def discussion_id
@@ -27,61 +29,20 @@ class LegacyDiffNote < Note
line_code.split('_')[0] if line_code
end
- def diff_old_line
- line_code.split('_')[1].to_i if line_code
- end
-
- def diff_new_line
- line_code.split('_')[2].to_i if line_code
- end
-
def diff
@diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
end
- def diff_file_path
- diff.new_path.presence || diff.old_path
- end
-
- def diff_lines
- @diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
+ def diff_file
+ @diff_file ||= Gitlab::Diff::File.new(diff, repository: self.project.repository) if diff
end
def diff_line
- @diff_line ||= diff_lines.find { |line| generate_line_code(line) == self.line_code }
+ @diff_line ||= diff_file.line_for_line_code(self.line_code)
end
- def diff_line_text
- diff_line.try(:text)
- end
-
- def diff_line_type
- diff_line.try(:type)
- end
-
- def highlighted_diff_lines
- Gitlab::Diff::Highlight.new(diff_lines).highlight
- end
-
- def truncated_diff_lines
- max_number_of_lines = 16
- prev_match_line = nil
- prev_lines = []
-
- highlighted_diff_lines.each do |line|
- if line.type == "match"
- prev_lines.clear
- prev_match_line = line
- else
- prev_lines << line
-
- break if generate_line_code(line) == self.line_code
-
- prev_lines.shift if prev_lines.length >= max_number_of_lines
- end
- end
-
- prev_lines
+ def for_line?(line)
+ !line.meta? && diff_file.line_code(line) == self.line_code
end
# Check if this note is part of an "active" discussion
@@ -102,7 +63,7 @@ class LegacyDiffNote < Note
if noteable_diff
parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line)
- @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line_text }
+ @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line.text }
else
@active = false
end
@@ -110,10 +71,6 @@ class LegacyDiffNote < Note
@active
end
- def award_emoji_supported?
- false
- end
-
private
def find_diff
@@ -149,10 +106,6 @@ class LegacyDiffNote < Note
self.class.where(attributes).last.try(:diff)
end
- def generate_line_code(line)
- Gitlab::Diff::LineCode.generate(diff_file_path, line.new_pos, line.old_pos)
- end
-
# Find the diff on noteable that matches our own
def find_noteable_diff
diffs = noteable.diffs(Commit.max_diff_options)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 4f7e1d2f302..083e93f1ee7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -16,7 +16,7 @@ class MergeRequest < ActiveRecord::Base
serialize :merge_params, Hash
- after_create :create_merge_request_diff, unless: :importing
+ after_create :create_merge_request_diff, unless: :importing?
after_update :update_merge_request_diff
delegate :commits, :diffs, :real_size, to: :merge_request_diff, prefix: nil
@@ -29,10 +29,6 @@ class MergeRequest < ActiveRecord::Base
# when creating new merge request
attr_accessor :can_be_created, :compare_commits, :compare
- # Temporary fields to store target_sha, and base_sha to
- # compare when importing pull requests from GitHub
- attr_accessor :base_target_sha, :head_source_sha
-
state_machine :state, initial: :opened do
event :close do
transition [:reopened, :opened] => :closed
@@ -89,12 +85,7 @@ class MergeRequest < ActiveRecord::Base
state :cannot_be_merged
around_transition do |merge_request, transition, block|
- merge_request.record_timestamps = false
- begin
- block.call
- ensure
- merge_request.record_timestamps = true
- end
+ Gitlab::Timeless.timeless(merge_request, &block)
end
end
@@ -169,10 +160,6 @@ class MergeRequest < ActiveRecord::Base
reference
end
- def last_commit
- merge_request_diff ? merge_request_diff.last_commit : compare_commits.last
- end
-
def first_commit
merge_request_diff ? merge_request_diff.first_commit : compare_commits.first
end
@@ -182,15 +169,86 @@ class MergeRequest < ActiveRecord::Base
end
def diff_base_commit
- if merge_request_diff
+ if persisted?
merge_request_diff.base_commit
- elsif source_sha
- self.target_project.merge_base_commit(self.source_sha, self.target_branch)
+ elsif diff_start_commit && diff_head_commit
+ self.target_project.merge_base_commit(diff_start_sha, diff_head_sha)
+ end
+ end
+
+ # MRs created before 8.4 don't store a MergeRequestDiff#base_commit_sha,
+ # but we need to get a commit for the "View file @ ..." link by deleted files,
+ # so we find the likely one if we can't get the actual one.
+ # This will not be the actual base commit if the target branch was merged into
+ # the source branch after the merge request was created, but it is good enough
+ # for the specific purpose of linking to a commit.
+ # It is not good enough for use in `Gitlab::Git::DiffRefs`, which needs the
+ # true base commit, so we can't simply have `#diff_base_commit` fall back on
+ # this method.
+ def likely_diff_base_commit
+ first_commit.parent || first_commit
+ end
+
+ def diff_start_commit
+ if persisted?
+ merge_request_diff.start_commit
+ else
+ target_branch_head
+ end
+ end
+
+ def diff_head_commit
+ if persisted?
+ merge_request_diff.head_commit
+ else
+ source_branch_head
end
end
- def last_commit_short_sha
- last_commit.short_id
+ def diff_start_sha
+ diff_start_commit.try(:sha)
+ end
+
+ def diff_base_sha
+ diff_base_commit.try(:sha)
+ end
+
+ def diff_head_sha
+ diff_head_commit.try(:sha)
+ end
+
+ # When importing a pull request from GitHub, the old and new branches may no
+ # longer actually exist by those names, but we need to recreate the merge
+ # request diff with the right source and target shas.
+ # We use these attributes to force these to the intended values.
+ attr_writer :target_branch_sha, :source_branch_sha
+
+ def source_branch_head
+ source_branch_ref = @source_branch_sha || source_branch
+ source_project.repository.commit(source_branch) if source_branch_ref
+ end
+
+ def target_branch_head
+ target_branch_ref = @target_branch_sha || target_branch
+ target_project.repository.commit(target_branch) if target_branch_ref
+ end
+
+ def target_branch_sha
+ target_branch_head.try(:sha)
+ end
+
+ def source_branch_sha
+ source_branch_head.try(:sha)
+ end
+
+ def diff_refs
+ return unless diff_start_commit || diff_base_commit
+
+ Gitlab::Diff::DiffRefs.new(
+ base_sha: diff_base_sha,
+ start_sha: diff_start_sha,
+ head_sha: diff_head_sha
+ )
end
def validate_branches
@@ -227,21 +285,30 @@ 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
return unless unchecked?
can_be_merged =
- !broken? && project.repository.can_be_merged?(source_sha, target_branch)
+ !broken? && project.repository.can_be_merged?(diff_head_sha, target_branch)
if can_be_merged
mark_as_mergeable
@@ -293,7 +360,7 @@ class MergeRequest < ActiveRecord::Base
!source_project.protected_branch?(source_branch) &&
!source_project.root_ref?(source_branch) &&
Ability.abilities.allowed?(current_user, :push_code, source_project) &&
- last_commit == source_project.commit(source_branch)
+ diff_head_commit == source_branch_head
end
def should_remove_source_branch?
@@ -331,8 +398,8 @@ class MergeRequest < ActiveRecord::Base
work_in_progress: work_in_progress?
}
- if last_commit
- attrs.merge!(last_commit: last_commit.hook_attrs)
+ if diff_head_commit
+ attrs.merge!(last_commit: diff_head_commit.hook_attrs)
end
attributes.merge!(attrs)
@@ -510,22 +577,6 @@ class MergeRequest < ActiveRecord::Base
end
end
- def target_sha
- return @base_target_sha if defined?(@base_target_sha)
-
- target_project.repository.commit(target_branch).try(:sha)
- end
-
- def source_sha
- return @head_source_sha if defined?(@head_source_sha)
-
- last_commit.try(:sha) || source_tip.try(:sha)
- end
-
- def source_tip
- source_branch && source_project.repository.commit(source_branch)
- end
-
def fetch_ref
target_project.repository.fetch_ref(
source_project.repository.path_to_repo,
@@ -558,10 +609,10 @@ class MergeRequest < ActiveRecord::Base
def diverged_commits_count
cache = Rails.cache.read(:"merge_request_#{id}_diverged_commits")
- if cache.blank? || cache[:source_sha] != source_sha || cache[:target_sha] != target_sha
+ if cache.blank? || cache[:source_sha] != source_branch_sha || cache[:target_sha] != target_branch_sha
cache = {
- source_sha: source_sha,
- target_sha: target_sha,
+ source_sha: source_branch_sha,
+ target_sha: target_branch_sha,
diverged_commits_count: compute_diverged_commits_count
}
Rails.cache.write(:"merge_request_#{id}_diverged_commits", cache)
@@ -571,9 +622,9 @@ class MergeRequest < ActiveRecord::Base
end
def compute_diverged_commits_count
- return 0 unless source_sha && target_sha
+ return 0 unless source_branch_sha && target_branch_sha
- Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_sha, target_sha).size
+ Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_branch_sha, target_branch_sha).size
end
private :compute_diverged_commits_count
@@ -582,13 +633,7 @@ class MergeRequest < ActiveRecord::Base
end
def pipeline
- @pipeline ||= source_project.pipeline(last_commit.id, source_branch) if last_commit && source_project
- end
-
- def diff_refs
- return nil unless diff_base_commit
-
- [diff_base_commit, last_commit]
+ @pipeline ||= source_project.pipeline(diff_head_sha, source_branch) if diff_head_sha && source_project
end
def merge_commit
@@ -603,6 +648,36 @@ class MergeRequest < ActiveRecord::Base
merge_commit
end
+ def support_new_diff_notes?
+ 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/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 0fcde6fc8f1..ba235750aeb 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -7,7 +7,7 @@ class MergeRequestDiff < ActiveRecord::Base
belongs_to :merge_request
- delegate :head_source_sha, :target_branch, :source_branch, to: :merge_request, prefix: nil
+ delegate :source_branch_sha, :target_branch_sha, :target_branch, :source_branch, to: :merge_request, prefix: nil
state_machine :state, initial: :empty do
state :collected
@@ -24,7 +24,7 @@ class MergeRequestDiff < ActiveRecord::Base
serialize :st_diffs
after_create :reload_content, unless: :importing?
- after_save :keep_around_commit
+ after_save :keep_around_commits, unless: :importing?
def reload_content
reload_commits
@@ -39,9 +39,9 @@ class MergeRequestDiff < ActiveRecord::Base
if options[:ignore_whitespace_change]
@diffs_no_whitespace ||= begin
compare = Gitlab::Git::Compare.new(
- self.repository.raw_repository,
- self.base,
- self.head,
+ repository.raw_repository,
+ self.start_commit_sha || self.target_branch_sha,
+ self.head_commit_sha || self.source_branch_sha,
)
compare.diffs(options)
end
@@ -63,37 +63,39 @@ class MergeRequestDiff < ActiveRecord::Base
end
def base_commit
- return nil unless self.base_commit_sha
+ return unless self.base_commit_sha
- merge_request.target_project.commit(self.base_commit_sha)
+ project.commit(self.base_commit_sha)
end
- def last_commit_short_sha
- @last_commit_short_sha ||= last_commit.short_id
- end
+ def start_commit
+ return unless self.start_commit_sha
- def dump_commits(commits)
- commits.map(&:to_hash)
+ project.commit(self.start_commit_sha)
end
- def load_commits(array)
- array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash), merge_request.source_project) }
- end
+ def head_commit
+ return last_commit unless self.head_commit_sha
- def dump_diffs(diffs)
- if diffs.respond_to?(:map)
- diffs.map(&:to_hash)
- end
+ project.commit(self.head_commit_sha)
end
- def load_diffs(raw, options)
- if raw.respond_to?(:each)
- Gitlab::Git::DiffCollection.new(raw, options)
- else
- Gitlab::Git::DiffCollection.new([])
- end
+ def compare
+ @compare ||=
+ begin
+ # Update ref for merge request
+ merge_request.fetch_ref
+
+ Gitlab::Git::Compare.new(
+ repository.raw_repository,
+ self.target_branch_sha,
+ self.source_branch_sha
+ )
+ end
end
+ private
+
# Collect array of Git::Commit objects
# between target and source branches
def unmerged_commits
@@ -106,6 +108,14 @@ class MergeRequestDiff < ActiveRecord::Base
commits
end
+ def dump_commits(commits)
+ commits.map(&:to_hash)
+ end
+
+ def load_commits(array)
+ array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash), merge_request.source_project) }
+ end
+
# Reload all commits related to current merge request from repo
# and save it as array of hashes in st_commits db field
def reload_commits
@@ -120,6 +130,26 @@ class MergeRequestDiff < ActiveRecord::Base
update_columns_serialized(new_attributes)
end
+ # Collect array of Git::Diff objects
+ # between target and source branches
+ def unmerged_diffs
+ compare.diffs(Commit.max_diff_options)
+ end
+
+ def dump_diffs(diffs)
+ if diffs.respond_to?(:map)
+ diffs.map(&:to_hash)
+ end
+ end
+
+ def load_diffs(raw, options)
+ if raw.respond_to?(:each)
+ Gitlab::Git::DiffCollection.new(raw, options)
+ else
+ Gitlab::Git::DiffCollection.new([])
+ end
+ end
+
# Reload diffs between branches related to current merge request from repo
# and save it as array of hashes in st_diffs db field
def reload_diffs
@@ -147,59 +177,33 @@ class MergeRequestDiff < ActiveRecord::Base
new_attributes[:st_diffs] = new_diffs
- base_commit_sha = self.repository.merge_base(self.head, self.base)
- new_attributes[:base_commit_sha] = base_commit_sha
-
- self.repository.keep_around(base_commit_sha)
+ new_attributes[:start_commit_sha] = self.target_branch_sha
+ new_attributes[:head_commit_sha] = self.source_branch_sha
+ new_attributes[:base_commit_sha] = branch_base_sha
update_columns_serialized(new_attributes)
- end
- # Collect array of Git::Diff objects
- # between target and source branches
- def unmerged_diffs
- compare.diffs(Commit.max_diff_options)
+ keep_around_commits
end
- def repository
- merge_request.target_project.repository
+ def project
+ merge_request.target_project
end
- def source_sha
- return head_source_sha if head_source_sha.present?
-
- source_commit = merge_request.source_project.commit(source_branch)
- source_commit.try(:sha)
+ def repository
+ project.repository
end
- def target_sha
- merge_request.target_sha
- end
+ def branch_base_commit
+ return unless self.source_branch_sha && self.target_branch_sha
- def base
- self.target_sha || self.target_branch
+ project.merge_base_commit(self.source_branch_sha, self.target_branch_sha)
end
- def head
- self.source_sha
+ def branch_base_sha
+ branch_base_commit.try(:sha)
end
- def compare
- @compare ||=
- begin
- # Update ref for merge request
- merge_request.fetch_ref
-
- Gitlab::Git::Compare.new(
- self.repository.raw_repository,
- self.base,
- self.head
- )
- end
- end
-
- private
-
#
# #save or #update_attributes providing changes on serialized attributes do a lot of
# serialization and deserialization calls resulting in bad performance.
@@ -223,7 +227,9 @@ class MergeRequestDiff < ActiveRecord::Base
reload
end
- def keep_around_commit
- self.repository.keep_around(self.base_commit_sha)
+ def keep_around_commits
+ repository.keep_around(target_branch_sha)
+ repository.keep_around(source_branch_sha)
+ repository.keep_around(branch_base_sha)
end
end
diff --git a/app/models/note.rb b/app/models/note.rb
index 81b5c47b738..ffffd0c0838 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -56,7 +56,7 @@ class Note < ActiveRecord::Base
scope :inc_author, ->{ includes(:author) }
scope :inc_author_project_award_emoji, ->{ includes(:project, :author, :award_emoji) }
- scope :legacy_diff_notes, ->{ where(type: 'LegacyDiffNote') }
+ scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) }
scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
scope :with_associations, -> do
@@ -82,7 +82,7 @@ class Note < ActiveRecord::Base
end
def grouped_diff_notes
- legacy_diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code)
+ diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code)
end
# Searches for notes matching the given query.
@@ -115,6 +115,10 @@ class Note < ActiveRecord::Base
false
end
+ def new_diff_note?
+ false
+ end
+
def active?
true
end
@@ -193,7 +197,7 @@ class Note < ActiveRecord::Base
end
def award_emoji?
- award_emoji_supported? && contains_emoji_only?
+ can_be_award_emoji? && contains_emoji_only?
end
def emoji_awardable?
@@ -204,7 +208,7 @@ class Note < ActiveRecord::Base
self.line_code = nil if self.line_code.blank?
end
- def award_emoji_supported?
+ def can_be_award_emoji?
noteable.is_a?(Awardable)
end
diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index 27bf08bf7d9..97bcbacf2b9 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -144,7 +144,7 @@ class JiraService < IssueTrackerService
commit_id = if entity.is_a?(Commit)
entity.id
elsif entity.is_a?(MergeRequest)
- entity.last_commit.id
+ entity.diff_head_sha
end
commit_url = build_entity_url(:commit, commit_id)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index d232d422195..ba66bc47c29 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -653,16 +653,6 @@ class Repository
end
end
- def blob_for_diff(commit, diff)
- blob_at(commit.id, diff.file_path)
- end
-
- def prev_blob_for_diff(commit, diff)
- if commit.parent_id
- blob_at(commit.parent_id, diff.old_path)
- end
- end
-
def refs_contains_sha(ref_type, sha)
args = %W(#{Gitlab.config.git.bin_path} #{ref_type} --contains #{sha})
names = Gitlab::Popen.popen(args, path_to_repo).first
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index a2df899d012..016172c6d7e 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -1,4 +1,6 @@
class SentNotification < ActiveRecord::Base
+ serialize :position, Gitlab::Diff::Position
+
belongs_to :project
belongs_to :noteable, polymorphic: true
belongs_to :recipient, class_name: "User"
@@ -7,7 +9,7 @@ class SentNotification < ActiveRecord::Base
validates :reply_key, uniqueness: true
validates :noteable_id, presence: true, unless: :for_commit?
validates :commit_id, presence: true, if: :for_commit?
- validates :line_code, line_code: true, allow_blank: true
+ validate :note_valid
after_save :keep_around_commit
@@ -20,7 +22,7 @@ class SentNotification < ActiveRecord::Base
find_by(reply_key: reply_key)
end
- def record(noteable, recipient_id, reply_key, params = {})
+ def record(noteable, recipient_id, reply_key, attrs = {})
return unless reply_key
noteable_id = nil
@@ -31,7 +33,7 @@ class SentNotification < ActiveRecord::Base
noteable_id = noteable.id
end
- params.reverse_merge!(
+ attrs.reverse_merge!(
project: noteable.project,
noteable_type: noteable.class.name,
noteable_id: noteable_id,
@@ -40,13 +42,17 @@ class SentNotification < ActiveRecord::Base
reply_key: reply_key
)
- create(params)
+ create(attrs)
end
- def record_note(note, recipient_id, reply_key, params = {})
- params[:line_code] = note.line_code
+ def record_note(note, recipient_id, reply_key, attrs = {})
+ if note.diff_note?
+ attrs[:note_type] = note.type
+
+ attrs.merge!(note.diff_attributes)
+ end
- record(note.noteable, recipient_id, reply_key, params)
+ record(note.noteable, recipient_id, reply_key, attrs)
end
end
@@ -70,8 +76,33 @@ class SentNotification < ActiveRecord::Base
self.reply_key
end
+ def note_attributes
+ {
+ project: self.project,
+ author: self.recipient,
+ type: self.note_type,
+ noteable_type: self.noteable_type,
+ noteable_id: self.noteable_id,
+ commit_id: self.commit_id,
+ line_code: self.line_code,
+ position: self.position.to_json
+ }
+ end
+
+ def create_note(note)
+ Notes::CreateService.new(
+ self.project,
+ self.recipient,
+ self.note_attributes.merge(note: note)
+ ).execute
+ end
+
private
+ def note_valid
+ Note.new(note_attributes.merge(note: "Test")).valid?
+ end
+
def keep_around_commit
project.repository.keep_around(self.commit_id)
end
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 3bec66cea88..f1b1d90c457 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -34,7 +34,7 @@ module MergeRequests
committer: committer
}
- commit_id = repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
+ commit_id = repository.merge(current_user, merge_request.diff_head_sha, merge_request.target_branch, options)
merge_request.update(merge_commit_sha: commit_id)
rescue GitHooksService::PreReceiveError => e
merge_request.update(merge_error: e.message)
diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb
index 870f5705184..4ad5fb08311 100644
--- a/app/services/merge_requests/merge_when_build_succeeds_service.rb
+++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb
@@ -12,7 +12,7 @@ module MergeRequests
merge_request.merge_when_build_succeeds = true
merge_request.merge_user = @current_user
- SystemNoteService.merge_when_build_succeeds(merge_request, @project, @current_user, merge_request.last_commit)
+ SystemNoteService.merge_when_build_succeeds(merge_request, @project, @current_user, merge_request.diff_head_commit)
end
merge_request.save
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index fe0579744b4..21490ac77ea 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -34,10 +34,10 @@ module MergeRequests
def close_merge_requests
commit_ids = @commits.map(&:id)
merge_requests = @project.merge_requests.opened.where(target_branch: @branch_name).to_a
- merge_requests = merge_requests.select(&:last_commit)
+ merge_requests = merge_requests.select(&:diff_head_commit)
merge_requests = merge_requests.select do |merge_request|
- commit_ids.include?(merge_request.last_commit.id)
+ commit_ids.include?(merge_request.diff_head_sha)
end
merge_requests.uniq.select(&:source_project).each do |merge_request|
@@ -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
@@ -94,12 +94,10 @@ module MergeRequests
merge_request = merge_requests_for_source_branch.first
return unless merge_request
- last_commit = merge_request.last_commit
-
begin
# Since any number of commits could have been made to the restored branch,
# find the common root to see what has been added.
- common_ref = @project.repository.merge_base(last_commit.id, @newrev)
+ common_ref = @project.repository.merge_base(merge_request.diff_head_sha, @newrev)
# If the a commit no longer exists in this repo, gitlab_git throws
# a Rugged::OdbError. This is fixed in https://gitlab.com/gitlab-org/gitlab_git/merge_requests/52
@commits = @project.repository.commits_between(common_ref, @newrev) if common_ref
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
diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml
index a3643a00cfe..35c4b862bb7 100644
--- a/app/views/notify/note_merge_request_email.html.haml
+++ b/app/views/notify/note_merge_request_email.html.haml
@@ -1,7 +1,7 @@
-- if @note.legacy_diff_note?
+- if @note.diff_note?
%p.details
New comment on diff for
- = link_to @note.diff_file_path, @target_url
+ = link_to @note.diff_file.file_path, @target_url
\:
= render 'note_message'
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index f1532371b2e..c161ecc3463 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -72,12 +72,11 @@
The diff for this file was not included because it is too large.
- else
%hr
- - diff_commit = diff_file.deleted_file ? @message.diff_refs.first : @message.diff_refs.last
- - blob = @message.project.repository.blob_for_diff(diff_commit, diff_file)
+ - blob = diff_file.blob
- if blob && blob.respond_to?(:text?) && blob_text_viewable?(blob)
%table.code.white
- diff_file.highlighted_diff_lines.each do |line|
- = render "projects/diffs/line", {line: line, diff_file: diff_file, line_code: nil, plain: true}
+ = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
- else
No preview for this file type
%br
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index 401cb4f7e30..d0da2606587 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/app/views/projects/commit/show.html.haml
@@ -7,8 +7,7 @@
= render "ci_menu"
- else
%div.block-connector
-= render "projects/diffs/diffs", diffs: @diffs, project: @project,
- diff_refs: @diff_refs
+= render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @commit.diff_refs
= render "projects/notes/notes_with_form"
- if can_collaborate_with_project?
- %w(revert cherry-pick).each do |type|
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index f18bc8c41b3..1975287faee 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -2,7 +2,7 @@
- if diff_view == 'parallel'
- fluid_layout true
-- diff_files = safe_diff_files(diffs, diff_refs)
+- diff_files = safe_diff_files(diffs, diff_refs: diff_refs, repository: project.repository)
.content-block.oneline-block.files-changed
.inline-parallel-buttons
@@ -24,7 +24,7 @@
.files
- diff_files.each_with_index do |diff_file, index|
- diff_commit = commit_for_diff(diff_file)
- - blob = project.repository.blob_for_diff(diff_commit, diff_file)
+ - blob = diff_file.blob(diff_commit)
- next unless blob
- blob.load_all_data!(project.repository) unless blob.only_display_raw?
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index 2395ea3c275..3b758a1ec4e 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -1,29 +1,8 @@
-.diff-file.file-holder{id: "diff-#{i}", data: diff_file_html_data(project, diff_commit, diff_file)}
+.diff-file.file-holder{id: "diff-#{i}", data: diff_file_html_data(project, diff_file)}
.file-title{id: "file-path-#{hexdigest(diff_file.file_path)}"}
- - if diff_file.diff.submodule?
- %span
- = icon('archive fw')
- %span
- = submodule_link(blob, @commit.id, project.repository)
- - else
- = blob_icon blob.mode, blob.name
-
- = link_to "#diff-#{i}" do
- - if diff_file.renamed_file
- - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
- = old_path
- &rarr;
- = new_path
- - else
- %span
- = diff_file.new_path
- - if diff_file.deleted_file
- deleted
-
- - if diff_file.mode_changed?
- %small
- = "#{diff_file.diff.a_mode} → #{diff_file.diff.b_mode}"
+ = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_commit, project: project, url: "#diff-#{i}"
+ - unless diff_file.submodule?
.file-actions.hidden-xs
- if blob_text_viewable?(blob)
= link_to '#', class: 'js-toggle-diff-comments btn active has-tooltip btn-file-option', title: "Toggle comments for this file" do
@@ -42,17 +21,23 @@
- return unless blob.respond_to?(:text?)
- if diff_file.too_large?
.nothing-here-block This diff could not be displayed because it is too large.
- - elsif blob_text_viewable?(blob) && !project.repository.diffable?(blob)
- .nothing-here-block This diff was suppressed by a .gitattributes entry.
- - elsif blob_text_viewable?(blob)
- - if diff_view == 'parallel'
- = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
- - else
- = render "projects/diffs/text_file", diff_file: diff_file, index: i
- elsif blob.only_display_raw?
.nothing-here-block This file is too large to display.
+ - elsif blob_text_viewable?(blob)
+ - if !project.repository.diffable?(blob)
+ .nothing-here-block This diff was suppressed by a .gitattributes entry.
+ - elsif diff_file.diff_lines.length > 0
+ - if diff_view == 'parallel'
+ = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
+ - else
+ = render "projects/diffs/text_file", diff_file: diff_file, index: i
+ - else
+ - if diff_file.mode_changed?
+ .nothing-here-block File mode changed
+ - elsif diff_file.renamed_file
+ .nothing-here-block File moved
- elsif blob.image?
- - old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file)
- = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs
+ - old_blob = diff_file.old_blob(diff_commit)
+ = render "projects/diffs/image", diff_file: diff_file, old_file: old_blob, file: blob, index: i
- else
.nothing-here-block No preview for this file type
diff --git a/app/views/projects/diffs/_file_header.html.haml b/app/views/projects/diffs/_file_header.html.haml
new file mode 100644
index 00000000000..95a2772fd0b
--- /dev/null
+++ b/app/views/projects/diffs/_file_header.html.haml
@@ -0,0 +1,25 @@
+- if defined?(blob) && blob && diff_file.submodule?
+ %span
+ = icon('archive fw')
+ %span
+ = submodule_link(blob, diff_commit.id, project.repository)
+- else
+ = conditional_link_to url.present?, url do
+ = blob_icon diff_file.b_mode, diff_file.file_path
+
+ - if diff_file.renamed_file
+ - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
+ %strong
+ = old_path
+ &rarr;
+ %strong
+ = new_path
+ - else
+ %strong
+ = diff_file.new_path
+ - if diff_file.deleted_file
+ deleted
+
+ - if diff_file.mode_changed?
+ %small
+ = "#{diff_file.a_mode} → #{diff_file.b_mode}"
diff --git a/app/views/projects/diffs/_image.html.haml b/app/views/projects/diffs/_image.html.haml
index 2731219ccad..9ec6a7aa5cd 100644
--- a/app/views/projects/diffs/_image.html.haml
+++ b/app/views/projects/diffs/_image.html.haml
@@ -1,9 +1,8 @@
- diff = diff_file.diff
-- file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(@commit.id, diff.new_path))
+- file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(diff_file.new_ref, diff.new_path))
// diff_refs will be nil for orphaned commits (e.g. first commit in repo)
-- if diff_refs
- - old_commit_id = diff_refs.first.id
- - old_file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(old_commit_id, diff.old_path))
+- if diff_file.old_ref
+ - old_file_raw_path = namespace_project_raw_path(@project.namespace, @project, tree_join(diff_file.old_ref, diff.old_path))
- if diff.renamed_file || diff.new_file || diff.deleted_file
.image
@@ -16,7 +15,7 @@
%div.two-up.view
%span.wrap
.frame.deleted
- %a{href: namespace_project_blob_path(@project.namespace, @project, tree_join(old_commit_id, diff.old_path))}
+ %a{href: namespace_project_blob_path(@project.namespace, @project, tree_join(diff_file.old_ref, diff.old_path))}
%img{src: old_file_raw_path}
%p.image-info.hide
%span.meta-filesize= "#{number_to_human_size old_file.size}"
@@ -28,7 +27,7 @@
%span.meta-height
%span.wrap
.frame.added
- %a{href: namespace_project_blob_path(@project.namespace, @project, tree_join(@commit.id, diff.new_path))}
+ %a{href: namespace_project_blob_path(@project.namespace, @project, tree_join(diff_file.new_ref, diff.new_path))}
%img{src: file_raw_path}
%p.image-info.hide
%span.meta-filesize= "#{number_to_human_size file.size}"
diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml
index f1577e8a47b..22cad00240a 100644
--- a/app/views/projects/diffs/_line.html.haml
+++ b/app/views/projects/diffs/_line.html.haml
@@ -1,3 +1,6 @@
+- plain = local_assigns.fetch(:plain, false)
+- line_code = diff_file.line_code(line)
+- position = diff_file.position(line)
- type = line.type
%tr.line_holder{ id: line_code, class: type }
- case type
@@ -9,18 +12,19 @@
%td.new_line.diff-line-num
%td.line_content.match= line.text
- else
- %td.old_line.diff-line-num{ class: type, data: { linenumber: line.new_pos } }
+ %td.old_line.diff-line-num{ class: type, data: { linenumber: line.old_pos } }
- link_text = type == "new" ? "&nbsp;".html_safe : line.old_pos
- - if defined?(plain) && plain
+ - if plain
= link_text
- else
= link_to "", "##{line_code}", id: line_code, data: { linenumber: link_text }
- - if !@diff_notes_disabled && can?(current_user, :create_note, @project)
- = link_to_new_diff_note(line_code)
+
+ - if !plain && !@diff_notes_disabled && can?(current_user, :create_note, @project)
+ = link_to_new_diff_note(line_code, position)
%td.new_line.diff-line-num{ class: type, data: { linenumber: line.new_pos } }
- link_text = type == "old" ? "&nbsp;".html_safe : line.new_pos
- - if defined?(plain) && plain
+ - if plain
= link_text
- else
= link_to "", "##{line_code}", id: line_code, data: { linenumber: link_text }
- %td.line_content{ class: ['noteable_line', type, line_code], data: { line_code: line_code } }= diff_line_content(line.text, type)
+ %td.line_content.noteable_line{ class: [type, line_code], data: { line_code: line_code, position: position.to_json } }= diff_line_content(line.text, type)
diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml
index 4ecc9528bd2..51f207dce94 100644
--- a/app/views/projects/diffs/_parallel_view.html.haml
+++ b/app/views/projects/diffs/_parallel_view.html.haml
@@ -14,30 +14,28 @@
%td.new_line.diff-line-num
%td.line_content.parallel.match= left[:text]
- else
- %td.old_line.diff-line-num{id: left[:line_code], class: "#{left[:type]} #{'empty-cell' if !left[:number]}"}
+ %td.old_line.diff-line-num{id: left[:line_code], class: [left[:type], ('empty-cell' unless left[:number])]}
= link_to raw(left[:number]), "##{left[:line_code]}", id: left[:line_code]
- if !@diff_notes_disabled && can?(current_user, :create_note, @project)
- = link_to_new_diff_note(left[:line_code], 'old')
- %td.line_content{class: "parallel noteable_line #{left[:type]} #{left[:line_code]} #{'empty-cell' if left[:text].empty?}", data: { line_code: left[:line_code] }}= diff_line_content(left[:text])
+ = link_to_new_diff_note(left[:line_code], left[:position], 'old')
+ %td.line_content.parallel.noteable_line{class: [left[:type], left[:line_code], ('empty-cell' if left[:text].empty?)], data: { line_code: left[:line_code], position: left[:position].to_json }}= diff_line_content(left[:text])
- if right[:type] == 'new'
- new_line_class = 'new'
- new_line_code = right[:line_code]
+ - new_position = right[:position]
- else
- new_line_class = nil
- new_line_code = left[:line_code]
+ - new_position = left[:position]
- %td.new_line.diff-line-num{id: new_line_code, class: "#{new_line_class} #{'empty-cell' if !right[:number]}", data: { linenumber: right[:number] }}
+ %td.new_line.diff-line-num{id: new_line_code, class: [new_line_class, ('empty-cell' unless right[:number])], data: { linenumber: right[:number] }}
= link_to raw(right[:number]), "##{new_line_code}", id: new_line_code
- if !@diff_notes_disabled && can?(current_user, :create_note, @project)
- = link_to_new_diff_note(new_line_code, 'new')
- %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code} #{'empty-cell' if right[:text].empty?}", data: { line_code: new_line_code }}= diff_line_content(right[:text])
+ = link_to_new_diff_note(new_line_code, new_position, 'new')
+ %td.line_content.parallel.noteable_line{class: [new_line_class, new_line_code, ('empty-cell' if right[:text].empty?)], data: { line_code: new_line_code, position: new_position.to_json }}= diff_line_content(right[:text])
- unless @diff_notes_disabled
- notes_left, notes_right = organize_comments(left, right)
- if notes_left.present? || notes_right.present?
= render "projects/notes/diff_notes_with_reply_parallel", notes_left: notes_left, notes_right: notes_right
-
-- if diff_file.diff.diff.blank? && diff_file.mode_changed?
- .file-mode-changed
- File mode changed
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index 068593a7dd1..192093d1273 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -4,22 +4,17 @@
%a.show-suppressed-diff.js-show-suppressed-diff Changes suppressed. Click to show.
%table.text-file.code.js-syntax-highlight{ class: too_big ? 'hide' : '' }
-
- last_line = 0
- - diff_file.highlighted_diff_lines.each_with_index do |line, index|
- - line_code = generate_line_code(diff_file.file_path, line)
+ - diff_file.highlighted_diff_lines.each do |line|
- last_line = line.new_pos
- = render "projects/diffs/line", {line: line, diff_file: diff_file, line_code: line_code}
+ = render "projects/diffs/line", line: line, diff_file: diff_file
- unless @diff_notes_disabled
- - diff_notes = @grouped_diff_notes[line_code]
+ - line_code = diff_file.line_code(line)
+ - diff_notes = @grouped_diff_notes[line_code] if line_code
- if diff_notes
= render "projects/notes/diff_notes_with_reply", notes: diff_notes
- if last_line > 0
= render "projects/diffs/match_line", { line: "",
line_old: last_line, line_new: last_line, bottom: true, new_file: diff_file.new_file }
-
-- if diff_file.diff.blank? && diff_file.mode_changed?
- .file-mode-changed
- File mode changed
diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml
index 08a38d283d2..489c632ae22 100644
--- a/app/views/projects/merge_requests/widget/_heading.html.haml
+++ b/app/views/projects/merge_requests/widget/_heading.html.haml
@@ -7,7 +7,7 @@
CI build
= ci_label_for_status(status)
for
- - commit = @merge_request.last_commit
+ - commit = @merge_request.diff_head_commit
= succeed "." do
= link_to @pipeline.short_sha, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, @pipeline.sha), class: "monospace"
%span.ci-coverage
@@ -24,7 +24,7 @@
CI build
= ci_label_for_status(status)
for
- - commit = @merge_request.last_commit
+ - commit = @merge_request.diff_head_commit
= succeed "." do
= link_to commit.short_id, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, commit), class: "monospace"
%span.ci-coverage
@@ -33,12 +33,12 @@
.ci_widget
= icon("spinner spin")
- Checking CI status for #{@merge_request.last_commit_short_sha}&hellip;
+ Checking CI status for #{@merge_request.diff_head_commit.short_id}&hellip;
.ci_widget.ci-not_found{style: "display:none"}
= icon("times-circle")
- Could not find CI status for #{@merge_request.last_commit_short_sha}.
+ Could not find CI status for #{@merge_request.diff_head_commit.short_id}.
.ci_widget.ci-error{style: "display:none"}
= icon("times-circle")
- Could not connect to the CI server. Please check your settings and try again. \ No newline at end of file
+ Could not connect to the CI server. Please check your settings and try again.
diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml
index 941513febbd..bf2e76f0083 100644
--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -2,7 +2,7 @@
= form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f|
= hidden_field_tag :authenticity_token, form_authenticity_token
- = hidden_field_tag :sha, @merge_request.source_sha
+ = hidden_field_tag :sha, @merge_request.diff_head_sha
.accept-merge-holder.clearfix.js-toggle-container
.clearfix
.accept-action
diff --git a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
index ad898ff153b..2b6b5e05e86 100644
--- a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
@@ -16,7 +16,7 @@
- if remove_source_branch_button || user_can_cancel_automatic_merge
.clearfix.prepend-top-10
- if remove_source_branch_button
- = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.source_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
+ = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.diff_head_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
= icon('times')
Remove Source Branch When Merged
diff --git a/app/views/projects/notes/_diff_notes_with_reply.html.haml b/app/views/projects/notes/_diff_notes_with_reply.html.haml
index 8144c1ba49e..ec6c4938efc 100644
--- a/app/views/projects/notes/_diff_notes_with_reply.html.haml
+++ b/app/views/projects/notes/_diff_notes_with_reply.html.haml
@@ -4,5 +4,4 @@
%td.notes_content
%ul.notes{ data: { discussion_id: note.discussion_id } }
= render partial: "projects/notes/note", collection: notes, as: :note
- .discussion-reply-holder
- = link_to_reply_discussion(note)
+ = link_to_reply_discussion(note)
diff --git a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
index 45986b0d1e8..e50a4f86d03 100644
--- a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
+++ b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
@@ -8,8 +8,7 @@
%ul.notes{ data: { discussion_id: note_left.discussion_id } }
= render partial: "projects/notes/note", collection: notes_left, as: :note
- .discussion-reply-holder
- = link_to_reply_discussion(note_left, 'old')
+ = link_to_reply_discussion(note_left, 'old')
- else
%td.notes_line.old= ""
%td.notes_content.parallel.old= ""
@@ -20,8 +19,7 @@
%ul.notes{ data: { discussion_id: note_right.discussion_id } }
= render partial: "projects/notes/note", collection: notes_right, as: :note
- .discussion-reply-holder
- = link_to_reply_discussion(note_right, 'new')
+ = link_to_reply_discussion(note_right, 'new')
- else
%td.notes_line.new= ""
%td.notes_content.parallel.new= ""
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index 03b3f6935d1..7c61ba750fe 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -7,6 +7,7 @@
= f.hidden_field :noteable_id
= f.hidden_field :noteable_type
= f.hidden_field :type
+ = f.hidden_field :position
= render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do
= render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text', placeholder: "Write a comment or drag your files here..."
diff --git a/app/views/projects/notes/discussions/_diff_with_notes.html.haml b/app/views/projects/notes/discussions/_diff_with_notes.html.haml
index 6401245bf73..4a69b8f8840 100644
--- a/app/views/projects/notes/discussions/_diff_with_notes.html.haml
+++ b/app/views/projects/notes/discussions/_diff_with_notes.html.haml
@@ -1,30 +1,17 @@
- note = discussion_notes.first
-- diff = note.diff
-- return unless diff
+- diff_file = note.diff_file
+- return unless diff_file
+
+- blob = note.blob
+
+.diff-file.file-holder
+ .file-title
+ = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: note.project, url: diff_note_path(note)
-.diff-file
- .diff-header
- %span
- - if diff.deleted_file
- = diff.old_path
- - else
- = diff.new_path
- - if diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
- %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}"
.diff-content.code.js-syntax-highlight
%table
- note.truncated_diff_lines.each do |line|
- - type = line.type
- - line_code = generate_line_code(note.diff_file_path, line)
- %tr.line_holder{ id: line_code, class: "#{type}" }
- - if type == "match"
- %td.old_line.diff-line-num= "..."
- %td.new_line.diff-line-num= "..."
- %td.line_content.match= line.text
- - else
- %td.old_line.diff-line-num{ data: { linenumber: type == "new" ? "&nbsp;".html_safe : line.old_pos } }
- %td.new_line.diff-line-num{ data: { linenumber: type == "old" ? "&nbsp;".html_safe : line.new_pos } }
- %td.line_content{ class: ['noteable_line', type, line_code], line_code: line_code }= diff_line_content(line.text, type)
+ = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
- - if line_code == note.line_code
- = render "projects/notes/diff_notes_with_reply", notes: discussion_notes
+ - if note.for_line?(line)
+ = render "projects/notes/diff_notes_with_reply", notes: discussion_notes
diff --git a/app/views/projects/notes/discussions/_notes.html.haml b/app/views/projects/notes/discussions/_notes.html.haml
index e598e3c7c63..a785149549d 100644
--- a/app/views/projects/notes/discussions/_notes.html.haml
+++ b/app/views/projects/notes/discussions/_notes.html.haml
@@ -3,5 +3,4 @@
.notes{ data: { discussion_id: note.discussion_id } }
%ul.notes.timeline
= render partial: "projects/notes/note", collection: discussion_notes, as: :note
- .discussion-reply-holder
- = link_to_reply_discussion(note)
+ = link_to_reply_discussion(note)
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index 971f969e25e..8551288e2f2 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -28,18 +28,30 @@ class EmailsOnPushWorker
:push
end
+ merge_base_sha = project.merge_base_commit(before_sha, after_sha).try(:sha)
+
diff_refs = nil
compare = nil
reverse_compare = false
if action == :push
compare = Gitlab::Git::Compare.new(project.repository.raw_repository, before_sha, after_sha)
- diff_refs = [project.merge_base_commit(before_sha, after_sha), project.commit(after_sha)]
+
+ diff_refs = Gitlab::Diff::DiffRefs.new(
+ base_sha: merge_base_sha,
+ start_sha: before_sha,
+ head_sha: after_sha
+ )
return false if compare.same
if compare.commits.empty?
compare = Gitlab::Git::Compare.new(project.repository.raw_repository, after_sha, before_sha)
- diff_refs = [project.merge_base_commit(after_sha, before_sha), project.commit(before_sha)]
+
+ diff_refs = Gitlab::Diff::DiffRefs.new(
+ base_sha: merge_base_sha,
+ start_sha: after_sha,
+ head_sha: before_sha
+ )
reverse_compare = true
diff --git a/db/migrate/20160508202603_add_head_commit_id_to_merge_request_diffs.rb b/db/migrate/20160508202603_add_head_commit_id_to_merge_request_diffs.rb
new file mode 100644
index 00000000000..1c4d60e7234
--- /dev/null
+++ b/db/migrate/20160508202603_add_head_commit_id_to_merge_request_diffs.rb
@@ -0,0 +1,5 @@
+class AddHeadCommitIdToMergeRequestDiffs < ActiveRecord::Migration
+ def change
+ add_column :merge_request_diffs, :head_commit_sha, :string
+ end
+end
diff --git a/db/migrate/20160508215920_add_positions_to_diff_notes.rb b/db/migrate/20160508215920_add_positions_to_diff_notes.rb
new file mode 100644
index 00000000000..2952c25004e
--- /dev/null
+++ b/db/migrate/20160508215920_add_positions_to_diff_notes.rb
@@ -0,0 +1,6 @@
+class AddPositionsToDiffNotes < ActiveRecord::Migration
+ def change
+ add_column :notes, :position, :text
+ add_column :notes, :original_position, :text
+ end
+end
diff --git a/db/migrate/20160516224534_add_start_commit_id_to_merge_request_diffs.rb b/db/migrate/20160516224534_add_start_commit_id_to_merge_request_diffs.rb
new file mode 100644
index 00000000000..b7fd76ee84b
--- /dev/null
+++ b/db/migrate/20160516224534_add_start_commit_id_to_merge_request_diffs.rb
@@ -0,0 +1,5 @@
+class AddStartCommitIdToMergeRequestDiffs < ActiveRecord::Migration
+ def change
+ add_column :merge_request_diffs, :start_commit_sha, :string
+ end
+end
diff --git a/db/migrate/20160522215720_add_note_type_and_position_to_sent_notification.rb b/db/migrate/20160522215720_add_note_type_and_position_to_sent_notification.rb
new file mode 100644
index 00000000000..4eef16c9408
--- /dev/null
+++ b/db/migrate/20160522215720_add_note_type_and_position_to_sent_notification.rb
@@ -0,0 +1,22 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddNoteTypeAndPositionToSentNotification < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # When using the methods "add_concurrent_index" or "add_column_with_default"
+ # you must disable the use of transactions as these methods can not run in an
+ # existing transaction. When using "add_concurrent_index" make sure that this
+ # method is the _only_ method called in the migration, any other changes
+ # should go in a separate migration. This ensures that upon failure _only_ the
+ # index creation fails and can be retried or reverted easily.
+ #
+ # To disable transactions uncomment the following line and remove these
+ # comments:
+ # disable_ddl_transaction!
+
+ def change
+ add_column :sent_notifications, :note_type, :string
+ add_column :sent_notifications, :position, :text
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f6465136e6a..68b9425253c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -593,6 +593,8 @@ ActiveRecord::Schema.define(version: 20160705163108) do
t.datetime "updated_at"
t.string "base_commit_sha"
t.string "real_size"
+ t.string "head_commit_sha"
+ t.string "start_commit_sha"
end
add_index "merge_request_diffs", ["merge_request_id"], name: "index_merge_request_diffs_on_merge_request_id", unique: true, using: :btree
@@ -689,10 +691,12 @@ ActiveRecord::Schema.define(version: 20160705163108) do
t.string "line_code"
t.string "commit_id"
t.integer "noteable_id"
- t.boolean "system", default: false, null: false
+ t.boolean "system", default: false, null: false
t.text "st_diff"
t.integer "updated_by_id"
t.string "type"
+ t.text "position"
+ t.text "original_position"
end
add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree
@@ -881,6 +885,8 @@ ActiveRecord::Schema.define(version: 20160705163108) do
t.string "commit_id"
t.string "reply_key", null: false
t.string "line_code"
+ t.string "note_type"
+ t.text "position"
end
add_index "sent_notifications", ["reply_key"], name: "index_sent_notifications_on_reply_key", unique: true, using: :btree
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index 640f1720a6c..da848afd48e 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -272,10 +272,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
step 'user "John Doe" leaves a comment like "Line is wrong" on diff' do
mr = MergeRequest.find_by(title: "Bug NS-05")
- create(:note_on_merge_request_diff, project: project,
+ create(:diff_note_on_merge_request, project: project,
noteable: mr,
author: user_exists("John Doe"),
- line_code: sample_commit.line_code,
note: 'Line is wrong')
end
@@ -519,7 +518,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
step '"Bug NS-05" has CI status' do
project = merge_request.source_project
project.enable_ci
- pipeline = create :ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch
+ pipeline = create :ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch
create :ci_build, pipeline: pipeline
end
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index e8b1e4b4879..56ef44ec969 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -23,7 +23,7 @@ module SharedDiffNote
page.within(diff_file_selector) do
click_diff_line(sample_commit.line_code)
- page.within("form[id$='#{sample_commit.line_code}-true']") do
+ page.within("form[data-line-code='#{sample_commit.line_code}']") do
fill_in "note[note]", with: "Typo, please fix"
find(".js-comment-button").trigger("click")
sleep 0.05
@@ -33,7 +33,7 @@ module SharedDiffNote
step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do
click_parallel_diff_line(sample_commit.line_code, 'old')
- page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do
+ page.within("#{diff_file_selector} form[data-line-code='#{sample_commit.line_code}']") do
fill_in "note[note]", with: "Old comment"
find(".js-comment-button").trigger("click")
end
@@ -41,7 +41,7 @@ module SharedDiffNote
step 'I leave a diff comment in a parallel view on the right side like "New comment"' do
click_parallel_diff_line(sample_commit.line_code, 'new')
- page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do
+ page.within("#{diff_file_selector} form[data-line-code='#{sample_commit.line_code}']") do
fill_in "note[note]", with: "New comment"
find(".js-comment-button").trigger("click")
end
@@ -51,7 +51,7 @@ module SharedDiffNote
page.within(diff_file_selector) do
click_diff_line(sample_commit.line_code)
- page.within("form[id$='#{sample_commit.line_code}-true']") do
+ page.within("form[data-line-code='#{sample_commit.line_code}']") do
fill_in "note[note]", with: "Should fix it :smile:"
find('.js-md-preview-button').click
end
@@ -62,7 +62,7 @@ module SharedDiffNote
page.within(diff_file_selector) do
click_diff_line(sample_commit.del_line_code)
- page.within("form[id$='#{sample_commit.del_line_code}-true']") do
+ page.within("form[data-line-code='#{sample_commit.del_line_code}']") do
fill_in "note[note]", with: "DRY this up"
find('.js-md-preview-button').click
end
@@ -91,7 +91,7 @@ module SharedDiffNote
page.within(diff_file_selector) do
click_diff_line(sample_commit.line_code)
- page.within("form[id$='#{sample_commit.line_code}-true']") do
+ page.within("form[data-line-code='#{sample_commit.line_code}']") do
fill_in 'note[note]', with: ':smile:'
click_button('Comment')
end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 8cc4368b5c2..db877d2eeb0 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -240,9 +240,9 @@ module API
class CommitNote < Grape::Entity
expose :note
- expose(:path) { |note| note.diff_file_path if note.legacy_diff_note? }
- expose(:line) { |note| note.diff_new_line if note.legacy_diff_note? }
- expose(:line_type) { |note| note.diff_line_type if note.legacy_diff_note? }
+ expose(:path) { |note| note.diff_file.try(:file_path) if note.diff_note? }
+ expose(:line) { |note| note.diff_line.try(:new_line) if note.diff_note? }
+ expose(:line_type) { |note| note.diff_line.try(:type) if note.diff_note? }
expose :author, using: Entities::UserBasic
expose :created_at
end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 0e94efd4acd..4fcdf8968c9 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -233,8 +233,8 @@ module API
render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable?
- if params[:sha] && merge_request.source_sha != params[:sha]
- render_api_error!("SHA does not match HEAD of source branch: #{merge_request.source_sha}", 409)
+ if params[:sha] && merge_request.diff_head_sha != params[:sha]
+ render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
end
merge_params = {
diff --git a/lib/gitlab/diff/diff_refs.rb b/lib/gitlab/diff/diff_refs.rb
new file mode 100644
index 00000000000..8406ca4269c
--- /dev/null
+++ b/lib/gitlab/diff/diff_refs.rb
@@ -0,0 +1,36 @@
+module Gitlab
+ module Diff
+ class DiffRefs
+ attr_reader :base_sha
+ attr_reader :start_sha
+ attr_reader :head_sha
+
+ def initialize(base_sha:, start_sha: base_sha, head_sha:)
+ @base_sha = base_sha
+ @start_sha = start_sha
+ @head_sha = head_sha
+ end
+
+ def ==(other)
+ other.is_a?(self.class) &&
+ base_sha == other.base_sha &&
+ start_sha == other.start_sha &&
+ head_sha == other.head_sha
+ end
+
+ # There is only one case in which we will have `start_sha` and `head_sha`,
+ # but not `base_sha`, which is when a diff is generated between an
+ # orphaned branch and another branch, which means there _is_ no base, but
+ # we're still able to highlight it, and to create diff notes, which are
+ # the primary things `DiffRefs` are used for.
+ # `DiffRefs` are "complete" when they have `start_sha` and `head_sha`,
+ # because `base_sha` can always be derived from this, to return an actual
+ # sha, or `nil`.
+ # We have `base_sha` directly available on `DiffRefs` because it's faster#
+ # than having to look it up in the repo every time.
+ def complete?
+ start_sha && head_sha
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index d2e85cabf72..b0c50edba59 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -1,47 +1,83 @@
module Gitlab
module Diff
class File
- attr_reader :diff, :diff_refs
+ attr_reader :diff, :repository, :diff_refs
delegate :new_file, :deleted_file, :renamed_file,
- :old_path, :new_path, to: :diff, prefix: false
+ :old_path, :new_path, :a_mode, :b_mode,
+ :submodule?, :too_large?, to: :diff, prefix: false
- def initialize(diff, diff_refs)
+ def initialize(diff, repository:, diff_refs: nil)
@diff = diff
+ @repository = repository
@diff_refs = diff_refs
end
+ def position(line)
+ return unless diff_refs
+
+ Position.new(
+ old_path: old_path,
+ new_path: new_path,
+ old_line: line.old_line,
+ new_line: line.new_line,
+ diff_refs: diff_refs
+ )
+ end
+
+ def line_code(line)
+ return if line.meta?
+
+ Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
+ end
+
+ def line_for_line_code(code)
+ diff_lines.find { |line| line_code(line) == code }
+ end
+
+ def line_for_position(pos)
+ diff_lines.find { |line| position(line) == pos }
+ end
+
+ def position_for_line_code(code)
+ line = line_for_line_code(code)
+ position(line) if line
+ end
+
+ def line_code_for_position(pos)
+ line = line_for_position(pos)
+ line_code(line) if line
+ end
+
+ def content_commit
+ return unless diff_refs
+
+ repository.commit(deleted_file ? old_ref : new_ref)
+ end
+
def old_ref
- diff_refs[0] if diff_refs
+ diff_refs.try(:base_sha)
end
def new_ref
- diff_refs[1] if diff_refs
+ diff_refs.try(:head_sha)
end
- # Array of Gitlab::DIff::Line objects
+ # Array of Gitlab::Diff::Line objects
def diff_lines
- @lines ||= parser.parse(raw_diff.each_line).to_a
- end
-
- def too_large?
- diff.too_large?
+ @lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a
end
def highlighted_diff_lines
- Gitlab::Diff::Highlight.new(self).highlight
+ @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
end
def parallel_diff_lines
- Gitlab::Diff::ParallelDiff.new(self).parallelize
+ @parallel_diff_lines ||= Gitlab::Diff::ParallelDiff.new(self).parallelize
end
def mode_changed?
- !!(diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode)
- end
-
- def parser
- Gitlab::Diff::Parser.new
+ a_mode && b_mode && a_mode != b_mode
end
def raw_diff
@@ -53,17 +89,15 @@ module Gitlab
end
def prev_line(index)
- if index > 0
- diff_lines[index - 1]
- end
+ diff_lines[index - 1] if index > 0
+ end
+
+ def paths
+ [old_path, new_path].compact
end
def file_path
- if diff.new_path.present?
- diff.new_path
- elsif diff.old_path.present?
- diff.old_path
- end
+ new_path.presence || old_path
end
def added_lines
@@ -73,6 +107,21 @@ module Gitlab
def removed_lines
diff_lines.count(&:removed?)
end
+
+ def old_blob(commit = content_commit)
+ return unless commit
+
+ parent_id = commit.parent_id
+ return unless parent_id
+
+ repository.blob_at(parent_id, old_path)
+ end
+
+ def blob(commit = content_commit)
+ return unless commit
+
+ repository.blob_at(commit.id, file_path)
+ end
end
end
end
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 9429b3ff88d..649a265a02c 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -1,11 +1,13 @@
module Gitlab
module Diff
class Highlight
- attr_reader :diff_file, :diff_lines, :raw_lines
+ attr_reader :diff_file, :diff_lines, :raw_lines, :repository
delegate :old_path, :new_path, :old_ref, :new_ref, to: :diff_file, prefix: :diff
- def initialize(diff_lines)
+ def initialize(diff_lines, repository: nil)
+ @repository = repository
+
if diff_lines.is_a?(Gitlab::Diff::File)
@diff_file = diff_lines
@diff_lines = @diff_file.diff_lines
@@ -19,7 +21,7 @@ module Gitlab
@diff_lines.map.with_index do |diff_line, i|
diff_line = diff_line.dup
# ignore highlighting for "match" lines
- next diff_line if diff_line.type == 'match' || diff_line.type == 'nonewline'
+ next diff_line if diff_line.meta?
rich_line = highlight_line(diff_line) || diff_line.text
@@ -40,12 +42,12 @@ module Gitlab
line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' '
- case diff_line.type
- when 'new', nil
- rich_line = new_lines[diff_line.new_pos - 1]
- when 'old'
- rich_line = old_lines[diff_line.old_pos - 1]
- end
+ rich_line =
+ if diff_line.unchanged? || diff_line.added?
+ new_lines[diff_line.new_pos - 1]
+ elsif diff_line.removed?
+ old_lines[diff_line.old_pos - 1]
+ end
# Only update text if line is found. This will prevent
# issues with submodules given the line only exists in diff content.
@@ -58,19 +60,12 @@ module Gitlab
def old_lines
return unless diff_file
- @old_lines ||= Gitlab::Highlight.highlight_lines(*processing_args(:old))
+ @old_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_old_ref, diff_old_path)
end
def new_lines
return unless diff_file
- @new_lines ||= Gitlab::Highlight.highlight_lines(*processing_args(:new))
- end
-
- def processing_args(version)
- ref = send("diff_#{version}_ref")
- path = send("diff_#{version}_path")
-
- [ref.project.repository, ref.id, path]
+ @new_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_new_ref, diff_new_path)
end
end
end
diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb
index 03730b435ad..c6189d660c2 100644
--- a/lib/gitlab/diff/line.rb
+++ b/lib/gitlab/diff/line.rb
@@ -9,6 +9,18 @@ module Gitlab
@old_pos, @new_pos = old_pos, new_pos
end
+ def old_line
+ old_pos unless added? || meta?
+ end
+
+ def new_line
+ new_pos unless removed? || meta?
+ end
+
+ def unchanged?
+ type.nil?
+ end
+
def added?
type == 'new'
end
@@ -16,6 +28,10 @@ module Gitlab
def removed?
type == 'old'
end
+
+ def meta?
+ type == 'match' || type == 'nonewline'
+ end
end
end
end
diff --git a/lib/gitlab/diff/line_mapper.rb b/lib/gitlab/diff/line_mapper.rb
new file mode 100644
index 00000000000..576a761423e
--- /dev/null
+++ b/lib/gitlab/diff/line_mapper.rb
@@ -0,0 +1,64 @@
+# When provided a diff for a specific file, maps old line numbers to new line
+# numbers and back, to find out where a specific line in a file was moved by the
+# changes.
+module Gitlab
+ module Diff
+ class LineMapper
+ attr_accessor :diff_file
+
+ def initialize(diff_file)
+ @diff_file = diff_file
+ end
+
+ # Find new line number for old line number.
+ def old_to_new(old_line)
+ map_line_number(old_line, from: :old_line, to: :new_line)
+ end
+
+ # Find old line number for new line number.
+ def new_to_old(new_line)
+ map_line_number(new_line, from: :new_line, to: :old_line)
+ end
+
+ private
+
+ def diff_lines
+ @diff_lines ||= @diff_file.diff_lines
+ end
+
+ # Find old/new line number based on its old/new counterpart line number.
+ def map_line_number(from_line, from:, to:)
+ # If no diff file could be found, the file wasn't changed, and the
+ # mapped line number is the same as the specified line number.
+ return from_line unless diff_file
+
+ # To find the mapped line number for the specified line number,
+ # we need to find:
+ # - The diff line with that exact line number, if it is in the diff context
+ # - The first diff line with a higher line number, if it falls between diff contexts
+ # - The last known diff line, if it falls after the last diff context
+ diff_line = diff_lines.find do |diff_line|
+ diff_from_line = diff_line.send(from)
+ diff_from_line && diff_from_line >= from_line
+ end
+ diff_line ||= diff_lines.last
+
+ # If no diff line could be found, the file wasn't changed, and the
+ # mapped line number is the same as the specified line number.
+ return from_line unless diff_line
+
+ diff_from_line = diff_line.send(from)
+ diff_to_line = diff_line.send(to)
+
+ # If the line was removed, there is no mapped line number.
+ return unless diff_to_line
+
+ # Because we may not have the diff line with the exact line number
+ # we were looking for, we need to adjust the mapped line number.
+ distance = diff_from_line - from_line
+
+ diff_to_line - distance
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/diff/parallel_diff.rb b/lib/gitlab/diff/parallel_diff.rb
index 74f9b3c050a..1c1fc148123 100644
--- a/lib/gitlab/diff/parallel_diff.rb
+++ b/lib/gitlab/diff/parallel_diff.rb
@@ -15,17 +15,19 @@ module Gitlab
highlighted_diff_lines.each do |line|
full_line = line.text
type = line.type
- line_code = generate_line_code(diff_file.file_path, line)
+ line_code = diff_file.line_code(line)
line_new = line.new_pos
line_old = line.old_pos
+ position = diff_file.position(line)
next_line = diff_file.next_line(line.index)
if next_line
next_line = highlighted_diff_lines[next_line.index]
- next_line_code = generate_line_code(diff_file.file_path, next_line)
+ full_next_line = next_line.text
+ next_line_code = diff_file.line_code(next_line)
next_type = next_line.type
- next_line = next_line.text
+ next_position = diff_file.position(next_line)
end
case type
@@ -37,12 +39,14 @@ module Gitlab
number: line_old,
text: full_line,
line_code: line_code,
+ position: position
},
right: {
type: type,
number: line_new,
text: full_line,
- line_code: line_code
+ line_code: line_code,
+ position: position
}
}
when 'old'
@@ -55,12 +59,14 @@ module Gitlab
number: line_old,
text: full_line,
line_code: line_code,
+ position: position
},
right: {
type: next_type,
number: line_new,
- text: next_line,
- line_code: next_line_code
+ text: full_next_line,
+ line_code: next_line_code,
+ position: next_position,
}
}
skip_next = true
@@ -73,12 +79,14 @@ module Gitlab
number: line_old,
text: full_line,
line_code: line_code,
+ position: position
},
right: {
type: next_type,
number: nil,
text: "",
- line_code: nil
+ line_code: nil,
+ position: nil
}
}
end
@@ -95,12 +103,14 @@ module Gitlab
number: nil,
text: "",
line_code: line_code,
+ position: position
},
right: {
type: type,
number: line_new,
text: full_line,
- line_code: line_code
+ line_code: line_code,
+ position: position
}
}
end
@@ -108,12 +118,6 @@ module Gitlab
end
lines
end
-
- private
-
- def generate_line_code(file_path, line)
- Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
- end
end
end
end
diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
new file mode 100644
index 00000000000..989fff8918e
--- /dev/null
+++ b/lib/gitlab/diff/position.rb
@@ -0,0 +1,155 @@
+# Defines a specific location, identified by paths and line numbers,
+# within a specific diff, identified by start, head and base commit ids.
+module Gitlab
+ module Diff
+ class Position
+ attr_reader :old_path
+ attr_reader :new_path
+ attr_reader :old_line
+ attr_reader :new_line
+ attr_reader :base_sha
+ attr_reader :start_sha
+ attr_reader :head_sha
+
+ def initialize(attrs = {})
+ @old_path = attrs[:old_path]
+ @new_path = attrs[:new_path]
+ @old_line = attrs[:old_line]
+ @new_line = attrs[:new_line]
+
+ if attrs[:diff_refs]
+ @base_sha = attrs[:diff_refs].base_sha
+ @start_sha = attrs[:diff_refs].start_sha
+ @head_sha = attrs[:diff_refs].head_sha
+ else
+ @base_sha = attrs[:base_sha]
+ @start_sha = attrs[:start_sha]
+ @head_sha = attrs[:head_sha]
+ end
+ end
+
+ # `Gitlab::Diff::Position` objects are stored as serialized attributes in
+ # `DiffNote`, which use YAML to encode and decode objects.
+ # `#init_with` and `#encode_with` can be used to customize the en/decoding
+ # behavior. In this case, we override these to prevent memoized instance
+ # variables like `@diff_file` and `@diff_line` from being serialized.
+ def init_with(coder)
+ initialize(coder['attributes'])
+
+ self
+ end
+
+ def encode_with(coder)
+ coder['attributes'] = self.to_h
+ end
+
+ def key
+ @key ||= [base_sha, start_sha, head_sha, Digest::SHA1.hexdigest(old_path || ""), Digest::SHA1.hexdigest(new_path || ""), old_line, new_line]
+ end
+
+ def ==(other)
+ other.is_a?(self.class) && key == other.key
+ end
+
+ def to_h
+ {
+ old_path: old_path,
+ new_path: new_path,
+ old_line: old_line,
+ new_line: new_line,
+ base_sha: base_sha,
+ start_sha: start_sha,
+ head_sha: head_sha
+ }
+ end
+
+ def inspect
+ %(#<#{self.class}:#{object_id} #{to_h}>)
+ end
+
+ def complete?
+ file_path.present? &&
+ (old_line || new_line) &&
+ diff_refs.complete?
+ end
+
+ def to_json
+ JSON.generate(self.to_h)
+ end
+
+ def type
+ if old_line && new_line
+ nil
+ elsif new_line
+ 'new'
+ else
+ 'old'
+ end
+ end
+
+ def unchanged?
+ type.nil?
+ end
+
+ def added?
+ type == 'new'
+ end
+
+ def removed?
+ type == 'old'
+ end
+
+ def paths
+ [old_path, new_path].compact.uniq
+ end
+
+ def file_path
+ new_path.presence || old_path
+ end
+
+ def diff_refs
+ @diff_refs ||= DiffRefs.new(base_sha: base_sha, start_sha: start_sha, head_sha: head_sha)
+ end
+
+ def diff_file(repository)
+ @diff_file ||= begin
+ if RequestStore.active?
+ key = {
+ project_id: repository.project.id,
+ start_sha: start_sha,
+ head_sha: head_sha,
+ path: file_path
+ }
+
+ RequestStore.fetch(key) { find_diff_file(repository) }
+ else
+ find_diff_file(repository)
+ end
+ end
+ end
+
+ def diff_line(repository)
+ @diff_line ||= diff_file(repository).line_for_position(self)
+ end
+
+ def line_code(repository)
+ @line_code ||= diff_file(repository).line_code_for_position(self)
+ end
+
+ private
+
+ def find_diff_file(repository)
+ diffs = Gitlab::Git::Compare.new(
+ repository.raw_repository,
+ start_sha,
+ head_sha
+ ).diffs(paths: paths)
+
+ diff = diffs.first
+ return unless diff
+
+ Gitlab::Diff::File.new(diff, repository: repository, diff_refs: diff_refs)
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/diff/position_tracer.rb b/lib/gitlab/diff/position_tracer.rb
new file mode 100644
index 00000000000..4d04f867268
--- /dev/null
+++ b/lib/gitlab/diff/position_tracer.rb
@@ -0,0 +1,168 @@
+# Finds the diff position in the new diff that corresponds to the same location
+# specified by the provided position in the old diff.
+module Gitlab
+ module Diff
+ class PositionTracer
+ attr_accessor :repository
+ attr_accessor :old_diff_refs
+ attr_accessor :new_diff_refs
+ attr_accessor :paths
+
+ def initialize(repository:, old_diff_refs:, new_diff_refs:, paths: nil)
+ @repository = repository
+ @old_diff_refs = old_diff_refs
+ @new_diff_refs = new_diff_refs
+ @paths = paths
+ end
+
+ def trace(old_position)
+ return unless old_diff_refs.complete? && new_diff_refs.complete?
+ return unless old_position.diff_refs == old_diff_refs
+
+ # Suppose we have an MR with source branch `feature` and target branch `master`.
+ # When the MR was created, the head of `master` was commit A, and the
+ # head of `feature` was commit B, resulting in the original diff A->B.
+ # Since creation, `master` was updated to C.
+ # Now `feature` is being updated to D, and the newly generated MR diff is C->D.
+ # It is possible that C and D are direct decendants of A and B respectively,
+ # but this isn't necessarily the case as rebases and merges come into play.
+ #
+ # Suppose we have a diff note on the original diff A->B. Now that the MR
+ # is updated, we need to find out what line in C->D corresponds to the
+ # line the note was originally created on, so that we can update the diff note's
+ # records and continue to display it in the right place in the diffs.
+ # If we cannot find this line in the new diff, this means the diff note is now
+ # outdated, and we will display that fact to the user.
+ #
+ # In the new diff, the file the diff note was originally created on may
+ # have been renamed, deleted or even created, if the file existed in A and B,
+ # but was removed in C, and restored in D.
+ #
+ # Every diff note stores a Position object that defines a specific location,
+ # identified by paths and line numbers, within a specific diff, identified
+ # by start, head and base commit ids.
+ #
+ # For diff notes for diff A->B, the position looks like this:
+ # Position
+ # base_sha - ID of commit A
+ # head_sha - ID of commit B
+ # old_path - path as of A (nil if file was newly created)
+ # new_path - path as of B (nil if file was deleted)
+ # old_line - line number as of A (nil if file was newly created)
+ # new_line - line number as of B (nil if file was deleted)
+ #
+ # We can easily update `base_sha` and `head_sha` to hold the IDs of commits C and D,
+ # but need to find the paths and line numbers as of C and D.
+ #
+ # If the file was unchanged or newly created in A->B, the path as of D can be found
+ # by generating diff B->D ("head to head"), finding the diff file with
+ # `diff_file.old_path == position.new_path`, and taking `diff_file.new_path`.
+ # The path as of C can be found by taking diff C->D, finding the diff file
+ # with that same `new_path` and taking `diff_file.old_path`.
+ # The line number as of D can be found by using the LineMapper on diff B->D
+ # and providing the line number as of B.
+ # The line number as of C can be found by using the LineMapper on diff C->D
+ # and providing the line number as of D.
+ #
+ # If the file was deleted in A->B, the path as of C can be found
+ # by generating diff A->C ("base to base"), finding the diff file with
+ # `diff_file.old_path == position.old_path`, and taking `diff_file.new_path`.
+ # The path as of D can be found by taking diff C->D, finding the diff file
+ # with that same `old_path` and taking `diff_file.new_path`.
+ # The line number as of C can be found by using the LineMapper on diff A->C
+ # and providing the line number as of A.
+ # The line number as of D can be found by using the LineMapper on diff C->D
+ # and providing the line number as of C.
+
+ results = nil
+ results ||= trace_added_line(old_position) if old_position.added? || old_position.unchanged?
+ results ||= trace_removed_line(old_position) if old_position.removed? || old_position.unchanged?
+
+ return unless results
+
+ file_diff, old_line, new_line = results
+
+ Position.new(
+ old_path: file_diff.old_path,
+ new_path: file_diff.new_path,
+ head_sha: new_diff_refs.head_sha,
+ start_sha: new_diff_refs.start_sha,
+ base_sha: new_diff_refs.base_sha,
+ old_line: old_line,
+ new_line: new_line
+ )
+ end
+
+ private
+
+ def trace_added_line(old_position)
+ file_path = old_position.new_path
+
+ return unless diff_head_to_head
+
+ file_head_to_head = diff_head_to_head.find { |diff_file| diff_file.old_path == file_path }
+
+ file_path = file_head_to_head.new_path if file_head_to_head
+
+ new_line = LineMapper.new(file_head_to_head).old_to_new(old_position.new_line)
+
+ return unless new_line
+
+ file_diff = new_diffs.find { |diff_file| diff_file.new_path == file_path }
+ return unless file_diff
+
+ old_line = LineMapper.new(file_diff).new_to_old(new_line)
+
+ [file_diff, old_line, new_line]
+ end
+
+ def trace_removed_line(old_position)
+ file_path = old_position.old_path
+
+ return unless diff_base_to_base
+
+ file_base_to_base = diff_base_to_base.find { |diff_file| diff_file.old_path == file_path }
+
+ file_path = file_base_to_base.old_path if file_base_to_base
+
+ old_line = LineMapper.new(file_base_to_base).old_to_new(old_position.old_line)
+
+ return unless old_line
+
+ file_diff = new_diffs.find { |diff_file| diff_file.old_path == file_path }
+ return unless file_diff
+
+ new_line = LineMapper.new(file_diff).old_to_new(old_line)
+
+ [file_diff, old_line, new_line]
+ end
+
+ def diff_base_to_base
+ @diff_base_to_base ||= diff_files(old_diff_refs.base_sha || old_diff_refs.start_sha, new_diff_refs.base_sha || new_diff_refs.start_sha)
+ end
+
+ def diff_head_to_head
+ @diff_head_to_head ||= diff_files(old_diff_refs.head_sha, new_diff_refs.head_sha)
+ end
+
+ def new_diffs
+ @new_diffs ||= diff_files(new_diff_refs.start_sha, new_diff_refs.head_sha, use_base: true)
+ end
+
+ def diff_files(start_sha, head_sha, use_base: false)
+ base_sha = self.repository.merge_base(start_sha, head_sha) || start_sha
+
+ diffs = self.repository.raw_repository.diff(
+ use_base ? base_sha : start_sha,
+ head_sha,
+ {},
+ *paths
+ )
+
+ diffs.decorate! do |diff|
+ Gitlab::Diff::File.new(diff, repository: self.repository)
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb
index 047c77c6fc2..97701b0cd42 100644
--- a/lib/gitlab/email/message/repository_push.rb
+++ b/lib/gitlab/email/message/repository_push.rb
@@ -33,11 +33,15 @@ module Gitlab
end
def commits
- @commits ||= (Commit.decorate(compare.commits, project) if compare)
+ return unless compare
+
+ @commits ||= Commit.decorate(compare.commits, project)
end
def diffs
- @diffs ||= (safe_diff_files(compare.diffs(max_files: 30), diff_refs) if compare)
+ return unless compare
+
+ @diffs ||= safe_diff_files(compare.diffs(max_files: 30), diff_refs: diff_refs, repository: project.repository)
end
def diffs_count
diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index 97ef9851d71..1c671a7487b 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -104,15 +104,7 @@ module Gitlab
end
def create_note(reply)
- Notes::CreateService.new(
- sent_notification.project,
- sent_notification.recipient,
- note: reply,
- noteable_type: sent_notification.noteable_type,
- noteable_id: sent_notification.noteable_id,
- commit_id: sent_notification.commit_id,
- line_code: sent_notification.line_code
- ).execute
+ sent_notification.create_note(reply)
end
end
end
diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb
index 498b00cb658..a4ea2210abd 100644
--- a/lib/gitlab/github_import/pull_request_formatter.rb
+++ b/lib/gitlab/github_import/pull_request_formatter.rb
@@ -11,10 +11,10 @@ module Gitlab
description: description,
source_project: source_branch_project,
source_branch: source_branch_name,
- head_source_sha: source_branch_sha,
+ source_branch_sha: source_branch_sha,
target_project: target_branch_project,
target_branch: target_branch_name,
- base_target_sha: target_branch_sha,
+ target_branch_sha: target_branch_sha,
state: state,
milestone: milestone,
author_id: author_id,
diff --git a/lib/gitlab/timeless.rb b/lib/gitlab/timeless.rb
new file mode 100644
index 00000000000..b290c716f97
--- /dev/null
+++ b/lib/gitlab/timeless.rb
@@ -0,0 +1,16 @@
+module Gitlab
+ module Timeless
+ def self.timeless(model, &block)
+ original_record_timestamps = model.record_timestamps
+ model.record_timestamps = false
+
+ if block.arity.abs == 1
+ block.call(model)
+ else
+ block.call
+ end
+ ensure
+ model.record_timestamps = original_record_timestamps
+ end
+ end
+end
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index ef1241f8600..bc0193a6c32 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -38,12 +38,10 @@ module Gitlab
end
def send_git_diff(repository, diff_refs)
- from, to = diff_refs
-
params = {
'RepoPath' => repository.path_to_repo,
- 'ShaFrom' => from.sha,
- 'ShaTo' => to.sha
+ 'ShaFrom' => diff_refs.start_sha,
+ 'ShaTo' => diff_refs.head_sha
}
[
@@ -52,11 +50,11 @@ module Gitlab
]
end
- def send_git_patch(repository, from, to)
+ def send_git_patch(repository, diff_refs)
params = {
'RepoPath' => repository.path_to_repo,
- 'ShaFrom' => from,
- 'ShaTo' => to
+ 'ShaFrom' => diff_refs.start_sha,
+ 'ShaTo' => diff_refs.head_sha
}
[
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index eff74e12869..c4b57e77804 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -103,7 +103,7 @@ describe Projects::MergeRequestsController do
id: merge_request.iid,
format: :patch)
- expect(response.headers['Gitlab-Workhorse-Send-Data']).to start_with("git-format-patch:")
+ expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:")
end
end
end
@@ -212,7 +212,7 @@ describe Projects::MergeRequestsController do
context 'when the sha parameter matches the source SHA' do
def merge_with_sha
- post :merge, base_params.merge(sha: merge_request.source_sha)
+ post :merge, base_params.merge(sha: merge_request.diff_head_sha)
end
it 'returns :success' do
@@ -229,11 +229,11 @@ describe Projects::MergeRequestsController do
context 'when merge_when_build_succeeds is passed' do
def merge_when_build_succeeds
- post :merge, base_params.merge(sha: merge_request.source_sha, merge_when_build_succeeds: '1')
+ post :merge, base_params.merge(sha: merge_request.diff_head_sha, merge_when_build_succeeds: '1')
end
before do
- create(:ci_empty_pipeline, project: project, sha: merge_request.source_sha, ref: merge_request.source_branch)
+ create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
end
it 'returns :merge_when_build_succeeds' do
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index 696cf276e57..83e38095feb 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -10,21 +10,46 @@ FactoryGirl.define do
on_issue
factory :note_on_commit, traits: [:on_commit]
- factory :note_on_commit_diff, traits: [:on_commit, :on_diff], class: LegacyDiffNote
factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note]
factory :note_on_merge_request, traits: [:on_merge_request]
- factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff], class: LegacyDiffNote
factory :note_on_project_snippet, traits: [:on_project_snippet]
factory :system_note, traits: [:system]
+ factory :legacy_diff_note_on_commit, traits: [:on_commit, :legacy_diff_note], class: LegacyDiffNote
+ factory :legacy_diff_note_on_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote
+
+ factory :diff_note_on_merge_request, traits: [:on_merge_request], class: DiffNote do
+ position do
+ Gitlab::Diff::Position.new(
+ old_path: "files/ruby/popen.rb",
+ new_path: "files/ruby/popen.rb",
+ old_line: nil,
+ new_line: 14,
+ diff_refs: noteable.diff_refs
+ )
+ end
+ end
+
+ factory :diff_note_on_commit, traits: [:on_commit], class: DiffNote do
+ position do
+ Gitlab::Diff::Position.new(
+ old_path: "files/ruby/popen.rb",
+ new_path: "files/ruby/popen.rb",
+ old_line: nil,
+ new_line: 14,
+ diff_refs: project.commit(commit_id).try(:diff_refs)
+ )
+ end
+ end
+
trait :on_commit do
noteable nil
- noteable_id nil
noteable_type 'Commit'
+ noteable_id nil
commit_id RepoHelpers.sample_commit.id
end
- trait :on_diff do
+ trait :legacy_diff_note do
line_code "0_184_184"
end
diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb
index b4d2201c729..f676200ecf3 100644
--- a/spec/features/merge_requests/created_from_fork_spec.rb
+++ b/spec/features/merge_requests/created_from_fork_spec.rb
@@ -30,7 +30,7 @@ feature 'Merge request created from fork' do
given(:pipeline) do
create(:ci_pipeline_with_two_job, project: fork_project,
- sha: merge_request.last_commit.id,
+ sha: merge_request.diff_head_sha,
ref: merge_request.source_branch)
end
diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
index c5e6412d7bf..96f7b8c9932 100644
--- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
+++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
@@ -12,7 +12,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
end
context "Active build for Merge Request" do
- let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) }
+ let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) }
let!(:ci_build) { create(:ci_build, pipeline: pipeline) }
before do
@@ -47,7 +47,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
merge_user: user, title: "MepMep", merge_when_build_succeeds: true)
end
- let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) }
+ let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) }
let!(:ci_build) { create(:ci_build, pipeline: pipeline) }
before do
diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb
index 65e9185ec24..80e8b8fc642 100644
--- a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb
+++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb
@@ -19,7 +19,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
end
context 'when project has CI enabled' do
- let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) }
+ let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) }
context 'when merge requests can only be merged if the build succeeds' do
before do
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index 737efcef45d..5174168713c 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -166,12 +166,14 @@ describe 'Comments', feature: true do
click_diff_line
is_expected.
- to have_css("tr[id='#{line_code}'] + .js-temp-notes-holder form",
+ to have_css("form[data-line-code='#{line_code}']",
count: 1)
end
it 'should be removed when canceled' do
- page.within(".diff-file form[id$='#{line_code}-true']") do
+ is_expected.to have_css('.js-temp-notes-holder')
+
+ page.within("form[data-line-code='#{line_code}']") do
find('.js-close-discussion-note-form').trigger('click')
end
diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml
index a8b7907d4ba..7d01183e3ef 100644
--- a/spec/fixtures/parallel_diff_result.yml
+++ b/spec/fixtures/parallel_diff_result.yml
@@ -3,322 +3,818 @@
:type: match
:number: 6
:text: "@@ -6,12 +6,18 @@ module Popen"
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
+ :line_code:
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: match
:number: 6
:text: "@@ -6,12 +6,18 @@ module Popen"
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
+ :line_code:
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 6
:text: |2
<span id="LC6" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 6
+ :new_line: 6
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 6
:text: |2
<span id="LC6" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 6
+ :new_line: 6
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 7
:text: |2
<span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 7
+ :new_line: 7
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 7
:text: |2
<span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 7
+ :new_line: 7
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 8
:text: |2
<span id="LC8" class="line"> <span class="k">unless</span> <span class="n">cmd</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Array</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 8
+ :new_line: 8
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 8
:text: |2
<span id="LC8" class="line"> <span class="k">unless</span> <span class="n">cmd</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Array</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 8
+ :new_line: 8
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type: old
:number: 9
:text: |
-<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 9
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 9
:text: |
+<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff left'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff right'> </span><span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 9
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 10
:text: |2
<span id="LC10" class="line"> <span class="k">end</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 10
+ :new_line: 10
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 10
:text: |2
<span id="LC10" class="line"> <span class="k">end</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 10
+ :new_line: 10
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 11
:text: |2
<span id="LC11" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 11
+ :new_line: 11
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 11
:text: |2
<span id="LC11" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 11
+ :new_line: 11
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 12
:text: |2
<span id="LC12" class="line"> <span class="n">path</span> <span class="o">||=</span> <span class="no">Dir</span><span class="p">.</span><span class="nf">pwd</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 12
+ :new_line: 12
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 12
:text: |2
<span id="LC12" class="line"> <span class="n">path</span> <span class="o">||=</span> <span class="no">Dir</span><span class="p">.</span><span class="nf">pwd</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 12
+ :new_line: 12
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type: old
:number: 13
:text: |
-<span id="LC13" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span> <span class="s2">&quot;PWD&quot;</span> <span class="o">=&gt;</span> <span class="n">path</span> <span class="p">}</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 13
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: old
:number:
:text: ''
:line_code:
+ :position:
- :left:
:type: old
:number: 14
:text: |
-<span id="LC14" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span> <span class="ss">chdir: </span><span class="n">path</span> <span class="p">}</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 14
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 13
:text: |
+<span id="LC13" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 13
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 14
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 14
:text: |
+<span id="LC14" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 14
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 15
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 15
:text: |
+<span id="LC15" class="line"> <span class="s2">&quot;PWD&quot;</span> <span class="o">=&gt;</span> <span class="n">path</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 15
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 16
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 16
:text: |
+<span id="LC16" class="line"> <span class="p">}</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 16
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 17
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 17
:text: |
+<span id="LC17" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 17
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 18
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 18
:text: |
+<span id="LC18" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 18
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 19
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 19
:text: |
+<span id="LC19" class="line"> <span class="ss">chdir: </span><span class="n">path</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 19
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 20
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 20
:text: |
+<span id="LC20" class="line"> <span class="p">}</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 20
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 15
:text: |2
<span id="LC21" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 15
+ :new_line: 21
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 21
:text: |2
<span id="LC21" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 15
+ :new_line: 21
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 16
:text: |2
<span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 16
+ :new_line: 22
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 22
:text: |2
<span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 16
+ :new_line: 22
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 17
:text: |2
<span id="LC23" class="line"> <span class="no">FileUtils</span><span class="p">.</span><span class="nf">mkdir_p</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 17
+ :new_line: 23
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 23
:text: |2
<span id="LC23" class="line"> <span class="no">FileUtils</span><span class="p">.</span><span class="nf">mkdir_p</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 17
+ :new_line: 23
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type: match
:number: 19
:text: "@@ -19,6 +25,7 @@ module Popen"
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
+ :line_code:
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: match
:number: 25
:text: "@@ -19,6 +25,7 @@ module Popen"
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
+ :line_code:
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line:
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 19
:text: |2
<span id="LC25" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 19
+ :new_line: 25
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 25
:text: |2
<span id="LC25" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 19
+ :new_line: 25
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 20
:text: |2
<span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">&quot;&quot;</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 20
+ :new_line: 26
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 26
:text: |2
<span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">&quot;&quot;</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 20
+ :new_line: 26
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 21
:text: |2
<span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 21
+ :new_line: 27
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 27
:text: |2
<span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 21
+ :new_line: 27
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number:
:text: ''
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 28
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type: new
:number: 28
:text: |
+<span id="LC28" class="line"></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line:
+ :new_line: 28
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 22
:text: |2
<span id="LC29" class="line"> <span class="no">Open3</span><span class="p">.</span><span class="nf">popen3</span><span class="p">(</span><span class="n">vars</span><span class="p">,</span> <span class="o">*</span><span class="n">cmd</span><span class="p">,</span> <span class="n">options</span><span class="p">)</span> <span class="k">do</span> <span class="o">|</span><span class="n">stdin</span><span class="p">,</span> <span class="n">stdout</span><span class="p">,</span> <span class="n">stderr</span><span class="p">,</span> <span class="n">wait_thr</span><span class="o">|</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 22
+ :new_line: 29
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 29
:text: |2
<span id="LC29" class="line"> <span class="no">Open3</span><span class="p">.</span><span class="nf">popen3</span><span class="p">(</span><span class="n">vars</span><span class="p">,</span> <span class="o">*</span><span class="n">cmd</span><span class="p">,</span> <span class="n">options</span><span class="p">)</span> <span class="k">do</span> <span class="o">|</span><span class="n">stdin</span><span class="p">,</span> <span class="n">stdout</span><span class="p">,</span> <span class="n">stderr</span><span class="p">,</span> <span class="n">wait_thr</span><span class="o">|</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 22
+ :new_line: 29
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 23
:text: |2
<span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 23
+ :new_line: 30
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 30
:text: |2
<span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 23
+ :new_line: 30
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
- :left:
:type:
:number: 24
:text: |2
<span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 24
+ :new_line: 31
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
:right:
:type:
:number: 31
:text: |2
<span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31
+ :position: !ruby/object:Gitlab::Diff::Position
+ attributes:
+ :old_path: files/ruby/popen.rb
+ :new_path: files/ruby/popen.rb
+ :old_line: 24
+ :new_line: 31
+ :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+ :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d
diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb
index 52764f41e0d..e2db33d8345 100644
--- a/spec/helpers/diff_helper_spec.rb
+++ b/spec/helpers/diff_helper_spec.rb
@@ -9,7 +9,7 @@ describe DiffHelper do
let(:diffs) { commit.diffs }
let(:diff) { diffs.first }
let(:diff_refs) { [commit.parent, commit] }
- let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) }
+ let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
describe 'diff_view' do
it 'returns a valid value when cookie is set' do
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index a0cbef6e6a4..1cb513d5229 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -6,7 +6,7 @@ describe Gitlab::Diff::File, lib: true do
let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) }
let(:diff) { commit.diffs.first }
- let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) }
+ let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
describe :diff_lines do
let(:diff_lines) { diff_file.diff_lines }
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index d19bf4ac84b..fb5d50a5c68 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -6,11 +6,11 @@ describe Gitlab::Diff::Highlight, lib: true do
let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) }
let(:diff) { commit.diffs.first }
- let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) }
+ let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
describe '#highlight' do
context "with a diff file" do
- let(:subject) { Gitlab::Diff::Highlight.new(diff_file).highlight }
+ let(:subject) { Gitlab::Diff::Highlight.new(diff_file, repository: project.repository).highlight }
it 'should return Gitlab::Diff::Line elements' do
expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
@@ -41,7 +41,7 @@ describe Gitlab::Diff::Highlight, lib: true do
end
context "with diff lines" do
- let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines).highlight }
+ let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines, repository: project.repository).highlight }
it 'should return Gitlab::Diff::Line elements' do
expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
diff --git a/spec/lib/gitlab/diff/line_mapper_spec.rb b/spec/lib/gitlab/diff/line_mapper_spec.rb
new file mode 100644
index 00000000000..4e50e03bb7e
--- /dev/null
+++ b/spec/lib/gitlab/diff/line_mapper_spec.rb
@@ -0,0 +1,137 @@
+require 'spec_helper'
+
+describe Gitlab::Diff::LineMapper, lib: true do
+ include RepoHelpers
+
+ let(:project) { create(:project) }
+ let(:repository) { project.repository }
+ let(:commit) { project.commit(sample_commit.id) }
+ let(:diffs) { commit.diffs }
+ let(:diff) { diffs.first }
+ let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) }
+ subject { described_class.new(diff_file) }
+
+ describe '#old_to_new' do
+ context "with a diff file" do
+ let(:mapping) do
+ {
+ 1 => 1,
+ 2 => 2,
+ 3 => 3,
+ 4 => 4,
+ 5 => 5,
+ 6 => 6,
+ 7 => 7,
+ 8 => 8,
+ 9 => nil,
+ # nil => 9,
+ 10 => 10,
+ 11 => 11,
+ 12 => 12,
+ 13 => nil,
+ 14 => nil,
+ # nil => 15,
+ # nil => 16,
+ # nil => 17,
+ # nil => 18,
+ # nil => 19,
+ # nil => 20,
+ 15 => 21,
+ 16 => 22,
+ 17 => 23,
+ 18 => 24,
+ 19 => 25,
+ 20 => 26,
+ 21 => 27,
+ # nil => 28,
+ 22 => 29,
+ 23 => 30,
+ 24 => 31,
+ 25 => 32,
+ 26 => 33,
+ 27 => 34,
+ 28 => 35,
+ 29 => 36,
+ 30 => 37
+ }
+ end
+
+ it 'returns the new line number for the old line number' do
+ mapping.each do |old_line, new_line|
+ expect(subject.old_to_new(old_line)).to eq(new_line)
+ end
+ end
+ end
+
+ context "without a diff file" do
+ let(:diff_file) { nil }
+
+ it "returns the same line number" do
+ expect(subject.old_to_new(100)).to eq(100)
+ end
+ end
+ end
+
+ describe '#new_to_old' do
+ context "with a diff file" do
+ let(:mapping) do
+ {
+ 1 => 1,
+ 2 => 2,
+ 3 => 3,
+ 4 => 4,
+ 5 => 5,
+ 6 => 6,
+ 7 => 7,
+ 8 => 8,
+ # nil => 9,
+ 9 => nil,
+ 10 => 10,
+ 11 => 11,
+ 12 => 12,
+ # nil => 13,
+ # nil => 14,
+ 13 => nil,
+ 14 => nil,
+ 15 => nil,
+ 16 => nil,
+ 17 => nil,
+ 18 => nil,
+ 19 => nil,
+ 20 => nil,
+ 21 => 15,
+ 22 => 16,
+ 23 => 17,
+ 24 => 18,
+ 25 => 19,
+ 26 => 20,
+ 27 => 21,
+ 28 => nil,
+ 29 => 22,
+ 30 => 23,
+ 31 => 24,
+ 32 => 25,
+ 33 => 26,
+ 34 => 27,
+ 35 => 28,
+ 36 => 29,
+ 37 => 30
+ }
+ end
+
+ it 'returns the old line number for the new line number' do
+ mapping.each do |new_line, old_line|
+ expect(subject.new_to_old(new_line)).to eq(old_line)
+ end
+ end
+ end
+
+ context "without a diff file" do
+ let(:diff_file) { nil }
+
+ it "returns the same line number" do
+ expect(subject.new_to_old(100)).to eq(100)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/diff/parallel_diff_spec.rb b/spec/lib/gitlab/diff/parallel_diff_spec.rb
index 1c5bbc47120..5f76b70c6f5 100644
--- a/spec/lib/gitlab/diff/parallel_diff_spec.rb
+++ b/spec/lib/gitlab/diff/parallel_diff_spec.rb
@@ -8,8 +8,7 @@ describe Gitlab::Diff::ParallelDiff, lib: true do
let(:commit) { project.commit(sample_commit.id) }
let(:diffs) { commit.diffs }
let(:diff) { diffs.first }
- let(:diff_refs) { [commit.parent, commit] }
- let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) }
+ let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) }
subject { described_class.new(diff_file) }
let(:parallel_diff_result_array) { YAML.load_file("#{Rails.root}/spec/fixtures/parallel_diff_result.yml") }
diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb
new file mode 100644
index 00000000000..cf28628cb96
--- /dev/null
+++ b/spec/lib/gitlab/diff/position_spec.rb
@@ -0,0 +1,341 @@
+require 'spec_helper'
+
+describe Gitlab::Diff::Position, lib: true do
+ include RepoHelpers
+
+ let(:project) { create(:project) }
+
+ describe "position for an added file" do
+ let(:commit) { project.commit("2ea1f3dec713d940208fb5ce4a38765ecb5d3f73") }
+
+ subject do
+ described_class.new(
+ old_path: "files/images/wm.svg",
+ new_path: "files/images/wm.svg",
+ old_line: nil,
+ new_line: 5,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.new_file).to be true
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.added?).to be true
+ expect(diff_line.new_line).to eq(subject.new_line)
+ expect(diff_line.text).to eq("+ <desc>Created with Sketch.</desc>")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, 0)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+
+ describe "position for a changed file" do
+ let(:commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") }
+
+ describe "position for an added line" do
+ subject do
+ described_class.new(
+ old_path: "files/ruby/popen.rb",
+ new_path: "files/ruby/popen.rb",
+ old_line: nil,
+ new_line: 14,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.added?).to be true
+ expect(diff_line.new_line).to eq(subject.new_line)
+ expect(diff_line.text).to eq("+ vars = {")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, 15)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+
+ describe "position for an unchanged line" do
+ subject do
+ described_class.new(
+ old_path: "files/ruby/popen.rb",
+ new_path: "files/ruby/popen.rb",
+ old_line: 16,
+ new_line: 22,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.unchanged?).to be true
+ expect(diff_line.old_line).to eq(subject.old_line)
+ expect(diff_line.new_line).to eq(subject.new_line)
+ expect(diff_line.text).to eq(" unless File.directory?(path)")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, subject.old_line)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+
+ describe "position for a removed line" do
+ subject do
+ described_class.new(
+ old_path: "files/ruby/popen.rb",
+ new_path: "files/ruby/popen.rb",
+ old_line: 14,
+ new_line: nil,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.removed?).to be true
+ expect(diff_line.old_line).to eq(subject.old_line)
+ expect(diff_line.text).to eq("- options = { chdir: path }")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, 13, subject.old_line)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+ end
+
+ describe "position for a renamed file" do
+ let(:commit) { project.commit("6907208d755b60ebeacb2e9dfea74c92c3449a1f") }
+
+ describe "position for an added line" do
+ subject do
+ described_class.new(
+ old_path: "files/js/commit.js.coffee",
+ new_path: "files/js/commit.coffee",
+ old_line: nil,
+ new_line: 4,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.added?).to be true
+ expect(diff_line.new_line).to eq(subject.new_line)
+ expect(diff_line.text).to eq("+ new CommitFile(@)")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, 5)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+
+ describe "position for an unchanged line" do
+ subject do
+ described_class.new(
+ old_path: "files/js/commit.js.coffee",
+ new_path: "files/js/commit.coffee",
+ old_line: 3,
+ new_line: 3,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.unchanged?).to be true
+ expect(diff_line.old_line).to eq(subject.old_line)
+ expect(diff_line.new_line).to eq(subject.new_line)
+ expect(diff_line.text).to eq(" $('.files .diff-file').each ->")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, subject.old_line)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+
+ describe "position for a removed line" do
+ subject do
+ described_class.new(
+ old_path: "files/js/commit.js.coffee",
+ new_path: "files/js/commit.coffee",
+ old_line: 4,
+ new_line: nil,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.new_path).to eq(subject.new_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.removed?).to be true
+ expect(diff_line.old_line).to eq(subject.old_line)
+ expect(diff_line.text).to eq("- new CommitFile(this)")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, 4, subject.old_line)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+ end
+
+ describe "position for a deleted file" do
+ let(:commit) { project.commit("8634272bfad4cf321067c3e94d64d5a253f8321d") }
+
+ subject do
+ described_class.new(
+ old_path: "LICENSE",
+ new_path: "LICENSE",
+ old_line: 3,
+ new_line: nil,
+ diff_refs: commit.diff_refs
+ )
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file(project.repository)
+
+ expect(diff_file.deleted_file).to be true
+ expect(diff_file.old_path).to eq(subject.old_path)
+ expect(diff_file.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line(project.repository)
+
+ expect(diff_line.removed?).to be true
+ expect(diff_line.old_line).to eq(subject.old_line)
+ expect(diff_line.text).to eq("-Copyright (c) 2014 gitlabhq")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(subject.file_path, 0, subject.old_line)
+
+ expect(subject.line_code(project.repository)).to eq(line_code)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb
new file mode 100644
index 00000000000..08312e60f4a
--- /dev/null
+++ b/spec/lib/gitlab/diff/position_tracer_spec.rb
@@ -0,0 +1,1758 @@
+require 'spec_helper'
+
+describe Gitlab::Diff::PositionTracer, lib: true do
+ # Douwe's diary New York City, 2016-06-28
+ # --------------------------------------------------------------------------
+ #
+ # Dear diary,
+ #
+ # Ideally, we would have a test for every single diff scenario that can
+ # occur and that the PositionTracer should correctly trace a position
+ # through, across the following variables:
+ #
+ # - Old diff file type: created, changed, renamed, deleted, unchanged (5)
+ # - Old diff line type: added, removed, unchanged (3)
+ # - New diff file type: created, changed, renamed, deleted, unchanged (5)
+ # - New diff line type: added, removed, unchanged (3)
+ # - Old-to-new diff line change: kept, moved, undone (3)
+ #
+ # This adds up to 5 * 3 * 5 * 3 * 3 = 675 different potential scenarios,
+ # and 675 different tests to cover them all. In reality, it would be fewer,
+ # since one cannot have a removed line in a created file diff, for example,
+ # but for the sake of this diary entry, let's be pessimistic.
+ #
+ # Writing these tests is a manual and time consuming process, as every test
+ # requires the manual construction or finding of a combination of diffs that
+ # create the exact diff scenario we are looking for, and can take between
+ # 1 and 10 minutes, depending on the farfetchedness of the scenario and
+ # complexity of creating it.
+ #
+ # This means that writing tests to cover all of these scenarios would end up
+ # taking between 11 and 112 hours in total, which I do not believe is the
+ # best use of my time.
+ #
+ # A better course of action would be to think of scenarios that are likely
+ # to occur, but also potentially tricky to trace correctly, and only cover
+ # those, with a few more obvious scenarios thrown in to cover our bases.
+ #
+ # Unfortunately, I only came to the above realization once I was about
+ # 1/5th of the way through the process of writing ALL THE SPECS, having
+ # already wasted about 3 hours trying to be thorough.
+ #
+ # I did find 2 bugs while writing those though, so that's good.
+ #
+ # In any case, all of this means that the tests below will be extremely
+ # (excessively, unjustifiably) thorough for scenarios where "the file was
+ # created in the old diff" and then drop off to comparitively lackluster
+ # testing of other scenarios.
+ #
+ # I did still try to cover most of the obvious and potentially tricky
+ # scenarios, though.
+
+ include RepoHelpers
+
+ let(:project) { create(:project) }
+ let(:current_user) { project.owner }
+ let(:repository) { project.repository }
+ let(:file_name) { "test-file" }
+ let(:new_file_name) { "#{file_name}-new" }
+ let(:second_file_name) { "#{file_name}-2" }
+ let(:branch_name) { "position-tracer-test" }
+
+ let(:old_diff_refs) { raise NotImplementedError }
+ let(:new_diff_refs) { raise NotImplementedError }
+ let(:old_position) { raise NotImplementedError }
+
+ let(:position_tracer) { described_class.new(repository: project.repository, old_diff_refs: old_diff_refs, new_diff_refs: new_diff_refs) }
+ subject { position_tracer.trace(old_position) }
+
+ def diff_refs(base_commit, head_commit)
+ Gitlab::Diff::DiffRefs.new(base_sha: base_commit.id, head_sha: head_commit.id)
+ end
+
+ def position(attrs = {})
+ attrs.reverse_merge!(
+ diff_refs: old_diff_refs
+ )
+ Gitlab::Diff::Position.new(attrs)
+ end
+
+ def expect_new_position(attrs, new_position = subject)
+ if attrs.nil?
+ expect(new_position).to be_nil
+ else
+ expect(new_position).not_to be_nil
+
+ expect(new_position.diff_refs).to eq(new_diff_refs)
+
+ attrs.each do |attr, value|
+ expect(new_position.send(attr)).to eq(value)
+ end
+ end
+ end
+
+ def create_branch(new_name, branch_name)
+ CreateBranchService.new(project, current_user).execute(new_name, branch_name)
+ end
+
+ def create_file(branch_name, file_name, content)
+ Files::CreateService.new(
+ project,
+ current_user,
+ source_branch: branch_name,
+ target_branch: branch_name,
+ commit_message: "Create file",
+ file_path: file_name,
+ file_content: content
+ ).execute
+ project.commit(branch_name)
+ end
+
+ def update_file(branch_name, file_name, content)
+ Files::UpdateService.new(
+ project,
+ current_user,
+ source_branch: branch_name,
+ target_branch: branch_name,
+ commit_message: "Update file",
+ file_path: file_name,
+ file_content: content
+ ).execute
+ project.commit(branch_name)
+ end
+
+ def delete_file(branch_name, file_name)
+ Files::DeleteService.new(
+ project,
+ current_user,
+ source_branch: branch_name,
+ target_branch: branch_name,
+ commit_message: "Delete file",
+ file_path: file_name
+ ).execute
+ project.commit(branch_name)
+ end
+
+ let(:initial_commit) do
+ create_branch(branch_name, "master")[:branch].name
+ project.commit(branch_name)
+ end
+
+ describe "#trace" do
+ describe "diff scenarios" do
+ let(:create_file_commit) do
+ initial_commit
+
+ create_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ B
+ C
+ CONTENT
+ )
+ end
+
+ let(:create_second_file_commit) do
+ create_file_commit
+
+ create_file(
+ branch_name,
+ second_file_name,
+ <<-CONTENT.strip_heredoc
+ D
+ E
+ CONTENT
+ )
+ end
+
+ let(:update_line_commit) do
+ create_second_file_commit
+
+ update_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ BB
+ C
+ CONTENT
+ )
+ end
+
+ let(:update_second_file_line_commit) do
+ update_line_commit
+
+ update_file(
+ branch_name,
+ second_file_name,
+ <<-CONTENT.strip_heredoc
+ D
+ EE
+ CONTENT
+ )
+ end
+
+ let(:move_line_commit) do
+ update_second_file_line_commit
+
+ update_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ BB
+ A
+ C
+ CONTENT
+ )
+ end
+
+ let(:add_second_file_line_commit) do
+ move_line_commit
+
+ update_file(
+ branch_name,
+ second_file_name,
+ <<-CONTENT.strip_heredoc
+ D
+ EE
+ F
+ CONTENT
+ )
+ end
+
+ let(:move_second_file_line_commit) do
+ add_second_file_line_commit
+
+ update_file(
+ branch_name,
+ second_file_name,
+ <<-CONTENT.strip_heredoc
+ D
+ F
+ EE
+ CONTENT
+ )
+ end
+
+ let(:delete_line_commit) do
+ move_second_file_line_commit
+
+ update_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ BB
+ A
+ CONTENT
+ )
+ end
+
+ let(:delete_second_file_line_commit) do
+ delete_line_commit
+
+ update_file(
+ branch_name,
+ second_file_name,
+ <<-CONTENT.strip_heredoc
+ D
+ F
+ CONTENT
+ )
+ end
+
+ let(:delete_file_commit) do
+ delete_second_file_line_commit
+
+ delete_file(branch_name, file_name)
+ end
+
+ let(:rename_file_commit) do
+ delete_file_commit
+
+ create_file(
+ branch_name,
+ new_file_name,
+ <<-CONTENT.strip_heredoc
+ BB
+ A
+ CONTENT
+ )
+ end
+
+ let(:update_line_again_commit) do
+ rename_file_commit
+
+ update_file(
+ branch_name,
+ new_file_name,
+ <<-CONTENT.strip_heredoc
+ BB
+ AA
+ CONTENT
+ )
+ end
+
+ let(:move_line_again_commit) do
+ update_line_again_commit
+
+ update_file(
+ branch_name,
+ new_file_name,
+ <<-CONTENT.strip_heredoc
+ AA
+ BB
+ CONTENT
+ )
+ end
+
+ let(:delete_line_again_commit) do
+ move_line_again_commit
+
+ update_file(
+ branch_name,
+ new_file_name,
+ <<-CONTENT.strip_heredoc
+ AA
+ CONTENT
+ )
+ end
+
+ context "when the file was created in the old diff" do
+ context "when the file is created in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, create_second_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 + BB
+ # 2 + A
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ new_line: 1
+ )
+ end
+ end
+
+ context "when that line was changed between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 3) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+ end
+
+ context "when the file is changed in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(update_line_commit, move_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 3) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 - A
+ # 2 1 BB
+ # 2 + A
+ # 3 3 C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(update_line_commit, move_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 - A
+ # 2 1 BB
+ # 2 + A
+ # 3 3 C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ new_line: 1
+ )
+ end
+ end
+
+ context "when that line was changed between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(move_line_commit, delete_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 3) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 BB
+ # 2 2 A
+ # 3 - C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+ end
+
+ context "when the file is renamed in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, rename_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ #
+ # new diff:
+ # file_name -> new_file_name
+ # 1 1 BB
+ # 2 2 A
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: file_name,
+ new_path: new_file_name,
+ old_line: old_position.new_line,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, update_line_again_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ #
+ # new diff:
+ # file_name -> new_file_name
+ # 1 1 BB
+ # 2 - A
+ # 2 + AA
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: file_name,
+ new_path: new_file_name,
+ old_line: old_position.new_line,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, move_line_again_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ #
+ # new diff:
+ # file_name -> new_file_name
+ # 1 + AA
+ # 1 2 BB
+ # 2 - A
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: file_name,
+ new_path: new_file_name,
+ old_line: 1,
+ new_line: 2
+ )
+ end
+ end
+
+ context "when that line was changed between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, update_line_again_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ #
+ # new diff:
+ # file_name -> new_file_name
+ # 1 1 BB
+ # 2 - A
+ # 2 + AA
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, delete_line_again_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ #
+ # new diff:
+ # file_name -> new_file_name
+ # 1 - BB
+ # 2 - A
+ # 1 + AA
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+ end
+
+ context "when the file is deleted in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, delete_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ #
+ # new diff:
+ # 1 - BB
+ # 2 - A
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, delete_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ # 3 + C
+ #
+ # new diff:
+ # 1 - BB
+ # 2 - A
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(move_line_commit, delete_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 - BB
+ # 2 - A
+ # 3 - C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was changed between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(update_line_commit, delete_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 - A
+ # 2 - BB
+ # 3 - C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, delete_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 3) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ # 3 + C
+ #
+ # new diff:
+ # 1 - BB
+ # 2 - A
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+ end
+
+ context "when the file is unchanged in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, create_second_file_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 2 B
+ # 3 3 C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(update_line_commit, update_second_file_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 2 BB
+ # 3 3 C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(move_line_commit, move_second_file_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 BB
+ # 2 2 A
+ # 3 3 C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was changed between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:new_diff_refs) { diff_refs(update_line_commit, update_second_file_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 2 BB
+ # 3 3 C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when that line was deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(delete_line_commit, delete_second_file_line_commit) }
+ let(:old_position) { position(new_path: file_name, new_line: 3) }
+
+ # old diff:
+ # 1 + BB
+ # 2 + A
+ # 3 + C
+ #
+ # new diff:
+ # 1 1 BB
+ # 2 2 A
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+ end
+ end
+
+ context "when the file was changed in the old diff" do
+ context "when the file is created in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 1 2 A
+ # 2 - B
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + BB
+ # 2 + A
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + BB
+ # 2 + A
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: 1
+ )
+ end
+ end
+
+ context "when that line was changed or deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 1 2 A
+ # 2 - B
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + B
+ # 3 + C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+
+ context "when the position pointed at a deleted line in the old diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 2) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+
+ context "when the position pointed at an unchanged line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 1, new_line: 1) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 1, new_line: 2) }
+
+ # old diff:
+ # 1 + BB
+ # 1 2 A
+ # 2 - B
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + BB
+ # 2 + A
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(move_line_commit, delete_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 2, new_line: 2) }
+
+ # old diff:
+ # 1 1 BB
+ # 2 2 A
+ # 3 - C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + BB
+ # 3 + C
+
+ it "returns the new position" do
+ expect_new_position(
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: 1
+ )
+ end
+ end
+
+ context "when that line was changed or deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(initial_commit, delete_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 3, new_line: 3) }
+
+ # old diff:
+ # 1 + BB
+ # 1 2 A
+ # 2 - B
+ # 3 3 C
+ #
+ # new diff:
+ # 1 + A
+ # 2 + B
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+ end
+
+ context "when the file is changed in the new diff" do
+ context "when the position pointed at an added line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_second_file_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: old_position.old_path,
+ new_path: old_position.new_path,
+ old_line: nil,
+ new_line: old_position.new_line
+ )
+ end
+ end
+
+ context "when the file's content was changed between the old and the new diff" do
+ context "when that line was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(move_line_commit, delete_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 1 2 A
+ # 2 - B
+ # 3 3 C
+ #
+ # new diff:
+ # 1 1 BB
+ # 2 2 A
+ # 3 - C
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: old_position.old_path,
+ new_path: old_position.new_path,
+ old_line: 1,
+ new_line: 1
+ )
+ end
+ end
+
+ context "when that line was moved between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(update_line_commit, move_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 - A
+ # 2 1 BB
+ # 2 + A
+ # 3 3 C
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: old_position.old_path,
+ new_path: old_position.new_path,
+ old_line: 2,
+ new_line: 1
+ )
+ end
+ end
+
+ context "when that line was changed or deleted between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) }
+
+ # old diff:
+ # 1 + BB
+ # 1 2 A
+ # 2 - B
+ # 3 3 C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+
+ it "returns nil" do
+ expect(subject).to be_nil
+ end
+ end
+ end
+ end
+
+ context "when the position pointed at a deleted line in the old diff" do
+ context "when the file's content was unchanged between the old and the new diff" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_second_file_line_commit) }
+ let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 2) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+
+ it "returns the new position" do
+ expect_new_position(
+ old_path: old_position.old_path,
+ new_path: old_position.new_path,
+ old_line: old_position.old_line,
+ new_line: nil
+ )
+ end
+ end
+ end
+ end
+ end
+ end
+ end
+
+ describe "typical use scenarios" do
+ let(:second_branch_name) { "#{branch_name}-2" }
+
+ def expect_positions(old_attrs, new_attrs)
+ old_positions = old_attrs.map do |old_attrs|
+ position(old_attrs)
+ end
+
+ new_positions = old_positions.map do |old_position|
+ position_tracer.trace(old_position)
+ end
+
+ new_positions.zip(new_attrs).each do |new_position, new_attrs|
+ expect_new_position(new_attrs, new_position)
+ end
+ end
+
+ let(:create_file_commit) do
+ initial_commit
+
+ create_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ B
+ C
+ D
+ E
+ F
+ CONTENT
+ )
+ end
+
+ let(:second_create_file_commit) do
+ create_file_commit
+
+ create_branch(second_branch_name, branch_name)
+
+ update_file(
+ second_branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ Z
+ Z
+ Z
+ A
+ B
+ C
+ D
+ E
+ F
+ CONTENT
+ )
+ end
+
+ let(:update_file_commit) do
+ second_create_file_commit
+
+ update_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ C
+ DD
+ E
+ F
+ G
+ CONTENT
+ )
+ end
+
+ let(:update_file_again_commit) do
+ update_file_commit
+
+ update_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ BB
+ C
+ D
+ E
+ FF
+ G
+ CONTENT
+ )
+ end
+
+ describe "simple push of new commit" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 3 2 C
+ # 4 - D
+ # 3 + DD
+ # 5 4 E
+ # 6 5 F
+ # 6 + G
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ # 4 4 D
+ # 5 5 E
+ # 6 - F
+ # 6 + FF
+ # 7 + G
+
+ it "returns the new positions" do
+ old_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, # C
+ { old_path: file_name, old_line: 4 }, # - D
+ { new_path: file_name, new_line: 3 }, # + DD
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, # E
+ { old_path: file_name, new_path: file_name, old_line: 6, new_line: 5 }, # F
+ { new_path: file_name, new_line: 6 }, # + G
+ ]
+
+ new_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 },
+ { old_path: file_name, old_line: 2 },
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 },
+ { old_path: file_name, old_line: 4, new_line: 4 },
+ nil,
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 },
+ { old_path: file_name, old_line: 6 },
+ { new_path: file_name, new_line: 7 },
+ ]
+
+ expect_positions(old_position_attrs, new_position_attrs)
+ end
+ end
+
+ describe "force push to overwrite last commit" do
+ let(:second_create_file_commit) do
+ create_file_commit
+
+ create_branch(second_branch_name, branch_name)
+
+ update_file(
+ second_branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ BB
+ C
+ D
+ E
+ FF
+ G
+ CONTENT
+ )
+ end
+
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, second_create_file_commit) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 3 2 C
+ # 4 - D
+ # 3 + DD
+ # 5 4 E
+ # 6 5 F
+ # 6 + G
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ # 4 4 D
+ # 5 5 E
+ # 6 - F
+ # 6 + FF
+ # 7 + G
+
+ it "returns the new positions" do
+ old_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, # C
+ { old_path: file_name, old_line: 4 }, # - D
+ { new_path: file_name, new_line: 3 }, # + DD
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, # E
+ { old_path: file_name, new_path: file_name, old_line: 6, new_line: 5 }, # F
+ { new_path: file_name, new_line: 6 }, # + G
+ ]
+
+ new_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 },
+ { old_path: file_name, old_line: 2 },
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 },
+ { old_path: file_name, old_line: 4, new_line: 4 },
+ nil,
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 },
+ { old_path: file_name, old_line: 6 },
+ { new_path: file_name, new_line: 7 },
+ ]
+
+ expect_positions(old_position_attrs, new_position_attrs)
+ end
+ end
+
+ describe "force push to delete last commit" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ # 4 4 D
+ # 5 5 E
+ # 6 - F
+ # 6 + FF
+ # 7 + G
+ #
+ # new diff:
+ # 1 1 A
+ # 2 - B
+ # 3 2 C
+ # 4 - D
+ # 3 + DD
+ # 5 4 E
+ # 6 5 F
+ # 6 + G
+
+ it "returns the new positions" do
+ old_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { new_path: file_name, new_line: 2 }, # + BB
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E
+ { old_path: file_name, old_line: 6 }, # - F
+ { new_path: file_name, new_line: 6 }, # + FF
+ { new_path: file_name, new_line: 7 }, # + G
+ ]
+
+ new_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 },
+ { old_path: file_name, old_line: 2 },
+ nil,
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 },
+ { old_path: file_name, old_line: 4 },
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 },
+ { old_path: file_name, new_path: file_name, old_line: 6, new_line: 5 },
+ nil,
+ { new_path: file_name, new_line: 6 },
+ ]
+
+ expect_positions(old_position_attrs, new_position_attrs)
+ end
+ end
+
+ describe "rebase on top of target branch" do
+ let(:second_update_file_commit) do
+ update_file_commit
+
+ update_file(
+ second_branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ Z
+ Z
+ Z
+ A
+ C
+ DD
+ E
+ F
+ G
+ CONTENT
+ )
+ end
+
+ let(:update_file_again_commit) do
+ second_update_file_commit
+
+ update_file(
+ branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ A
+ BB
+ C
+ D
+ E
+ FF
+ G
+ CONTENT
+ )
+ end
+
+ let(:overwrite_update_file_again_commit) do
+ update_file_again_commit
+
+ update_file(
+ second_branch_name,
+ file_name,
+ <<-CONTENT.strip_heredoc
+ Z
+ Z
+ Z
+ A
+ BB
+ C
+ D
+ E
+ FF
+ G
+ CONTENT
+ )
+ end
+
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, overwrite_update_file_again_commit) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ # 4 4 D
+ # 5 5 E
+ # 6 - F
+ # 6 + FF
+ # 7 + G
+ #
+ # new diff:
+ # 1 + Z
+ # 2 + Z
+ # 3 + Z
+ # 1 4 A
+ # 2 - B
+ # 5 + BB
+ # 3 6 C
+ # 4 7 D
+ # 5 8 E
+ # 6 - F
+ # 9 + FF
+ # 0 + G
+
+ it "returns the new positions" do
+ old_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { new_path: file_name, new_line: 2 }, # + BB
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E
+ { old_path: file_name, old_line: 6 }, # - F
+ { new_path: file_name, new_line: 6 }, # + FF
+ { new_path: file_name, new_line: 7 }, # + G
+ ]
+
+ new_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 4 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { new_path: file_name, new_line: 5 }, # + BB
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 6 }, # C
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 7 }, # D
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 8 }, # E
+ { old_path: file_name, old_line: 6 }, # - F
+ { new_path: file_name, new_line: 9 }, # + FF
+ { new_path: file_name, new_line: 10 }, # + G
+ ]
+
+ expect_positions(old_position_attrs, new_position_attrs)
+ end
+ end
+
+ describe "merge of target branch" do
+ let(:merge_commit) do
+ update_file_again_commit
+
+ committer = repository.user_to_committer(current_user)
+
+ options = {
+ message: "Merge branches",
+ author: committer,
+ committer: committer
+ }
+
+ repository.merge(current_user, second_create_file_commit.sha, branch_name, options)
+ project.commit(branch_name)
+ end
+
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
+ let(:new_diff_refs) { diff_refs(create_file_commit, merge_commit) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ # 4 4 D
+ # 5 5 E
+ # 6 - F
+ # 6 + FF
+ # 7 + G
+ #
+ # new diff:
+ # 1 + Z
+ # 2 + Z
+ # 3 + Z
+ # 1 4 A
+ # 2 - B
+ # 5 + BB
+ # 3 6 C
+ # 4 7 D
+ # 5 8 E
+ # 6 - F
+ # 9 + FF
+ # 0 + G
+
+ it "returns the new positions" do
+ old_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { new_path: file_name, new_line: 2 }, # + BB
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E
+ { old_path: file_name, old_line: 6 }, # - F
+ { new_path: file_name, new_line: 6 }, # + FF
+ { new_path: file_name, new_line: 7 }, # + G
+ ]
+
+ new_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 4 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { new_path: file_name, new_line: 5 }, # + BB
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 6 }, # C
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 7 }, # D
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 8 }, # E
+ { old_path: file_name, old_line: 6 }, # - F
+ { new_path: file_name, new_line: 9 }, # + FF
+ { new_path: file_name, new_line: 10 }, # + G
+ ]
+
+ expect_positions(old_position_attrs, new_position_attrs)
+ end
+ end
+
+ describe "changing target branch" do
+ let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
+ let(:new_diff_refs) { diff_refs(update_file_commit, update_file_again_commit) }
+
+ # old diff:
+ # 1 1 A
+ # 2 - B
+ # 2 + BB
+ # 3 3 C
+ # 4 4 D
+ # 5 5 E
+ # 6 - F
+ # 6 + FF
+ # 7 + G
+ #
+ # new diff:
+ # 1 1 A
+ # 2 + BB
+ # 2 3 C
+ # 3 - DD
+ # 4 + D
+ # 4 5 E
+ # 5 - F
+ # 6 + FF
+ # 7 G
+
+ it "returns the new positions" do
+ old_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A
+ { old_path: file_name, old_line: 2 }, # - B
+ { new_path: file_name, new_line: 2 }, # + BB
+ { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D
+ { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E
+ { old_path: file_name, old_line: 6 }, # - F
+ { new_path: file_name, new_line: 6 }, # + FF
+ { new_path: file_name, new_line: 7 }, # + G
+ ]
+
+ new_position_attrs = [
+ { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 },
+ nil,
+ { new_path: file_name, new_line: 2 },
+ { old_path: file_name, new_path: file_name, old_line: 2, new_line: 3 },
+ { new_path: file_name, new_line: 4 },
+ { old_path: file_name, new_path: file_name, old_line: 4, new_line: 5 },
+ { old_path: file_name, old_line: 5 },
+ { new_path: file_name, new_line: 6 },
+ { new_path: file_name, new_line: 7 },
+ ]
+
+ expect_positions(old_position_attrs, new_position_attrs)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
index 9587252b990..79931ecd134 100644
--- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
+++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
@@ -43,10 +43,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
description: "*Created by: octocat*\n\nPlease pull these awesome changes",
source_project: project,
source_branch: 'feature',
- head_source_sha: source_sha,
+ source_branch_sha: source_sha,
target_project: project,
target_branch: 'master',
- base_target_sha: target_sha,
+ target_branch_sha: target_sha,
state: 'opened',
milestone: nil,
author_id: project.creator_id,
@@ -70,10 +70,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
description: "*Created by: octocat*\n\nPlease pull these awesome changes",
source_project: project,
source_branch: 'feature',
- head_source_sha: source_sha,
+ source_branch_sha: source_sha,
target_project: project,
target_branch: 'master',
- base_target_sha: target_sha,
+ target_branch_sha: target_sha,
state: 'closed',
milestone: nil,
author_id: project.creator_id,
@@ -97,10 +97,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
description: "*Created by: octocat*\n\nPlease pull these awesome changes",
source_project: project,
source_branch: 'feature',
- head_source_sha: source_sha,
+ source_branch_sha: source_sha,
target_project: project,
target_branch: 'master',
- base_target_sha: target_sha,
+ target_branch_sha: target_sha,
state: 'merged',
milestone: nil,
author_id: project.creator_id,
diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
index a75eaa4d51f..1424de9e60b 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -125,7 +125,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
ci_pipeline = create(:ci_pipeline,
project: project,
- sha: merge_request.last_commit.id,
+ sha: merge_request.diff_head_sha,
ref: merge_request.source_branch,
statuses: [commit_status])
diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/note_data_builder_spec.rb
index e848d88182f..3d6bcdfd873 100644
--- a/spec/lib/gitlab/note_data_builder_spec.rb
+++ b/spec/lib/gitlab/note_data_builder_spec.rb
@@ -27,7 +27,7 @@ describe 'Gitlab::NoteDataBuilder', lib: true do
end
describe 'When asking for a note on commit diff' do
- let(:note) { create(:note_on_commit_diff, project: project) }
+ let(:note) { create(:diff_note_on_commit, project: project) }
it 'returns the note and commit-specific data' do
expect(data).to have_key(:commit)
@@ -90,7 +90,7 @@ describe 'Gitlab::NoteDataBuilder', lib: true do
end
let(:note) do
- create(:note_on_merge_request_diff, noteable: merge_request,
+ create(:diff_note_on_merge_request, noteable: merge_request,
project: project)
end
diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb
index bf11472407a..a826b24419a 100644
--- a/spec/lib/gitlab/url_builder_spec.rb
+++ b/spec/lib/gitlab/url_builder_spec.rb
@@ -43,9 +43,9 @@ describe Gitlab::UrlBuilder, lib: true do
end
end
- context 'on a CommitDiff' do
+ context 'on a Commit Diff' do
it 'returns a proper URL' do
- note = build_stubbed(:note_on_commit_diff)
+ note = build_stubbed(:diff_note_on_commit)
url = described_class.build(note)
@@ -75,10 +75,10 @@ describe Gitlab::UrlBuilder, lib: true do
end
end
- context 'on a MergeRequestDiff' do
+ context 'on a MergeRequest Diff' do
it 'returns a proper URL' do
merge_request = create(:merge_request, iid: 42)
- note = build_stubbed(:note_on_merge_request_diff, noteable: merge_request)
+ note = build_stubbed(:diff_note_on_merge_request, noteable: merge_request)
url = described_class.build(note)
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index aa382f930d7..0a9b10bebea 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -948,7 +948,7 @@ describe Notify do
let(:commits) { Commit.decorate(compare.commits, nil) }
let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: Commit.new(compare.base, project), to: Commit.new(compare.head, project)) }
let(:send_from_committer_email) { false }
- let(:diff_refs) { [project.merge_base_commit(sample_image_commit.id, sample_commit.id), project.commit(sample_commit.id)] }
+ let(:diff_refs) { Gitlab::Diff::DiffRefs.new(base_sha: project.merge_base_commit(sample_image_commit.id, sample_commit.id).id, head_sha: sample_commit.id) }
subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, diff_refs: diff_refs, send_from_committer_email: send_from_committer_email) }
@@ -1049,7 +1049,7 @@ describe Notify do
let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_commit.parent_id, sample_commit.id) }
let(:commits) { Commit.decorate(compare.commits, nil) }
let(:diff_path) { namespace_project_commit_path(project.namespace, project, commits.first) }
- let(:diff_refs) { [project.merge_base_commit(sample_commit.parent_id, sample_commit.id), project.commit(sample_commit.id)] }
+ let(:diff_refs) { Gitlab::Diff::DiffRefs.new(base_sha: project.merge_base_commit(sample_image_commit.id, sample_commit.id).id, head_sha: sample_commit.id) }
subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, diff_refs: diff_refs) }
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
new file mode 100644
index 00000000000..af8e890ca95
--- /dev/null
+++ b/spec/models/diff_note_spec.rb
@@ -0,0 +1,191 @@
+require 'spec_helper'
+
+describe DiffNote, models: true do
+ include RepoHelpers
+
+ let(:project) { create(:project) }
+ let(:merge_request) { create(:merge_request, source_project: project) }
+ let(:commit) { project.commit(sample_commit.id) }
+
+ let(:path) { "files/ruby/popen.rb" }
+
+ let!(:position) do
+ Gitlab::Diff::Position.new(
+ old_path: path,
+ new_path: path,
+ old_line: nil,
+ new_line: 14,
+ diff_refs: merge_request.diff_refs
+ )
+ end
+
+ let!(:new_position) do
+ Gitlab::Diff::Position.new(
+ old_path: path,
+ new_path: path,
+ old_line: 16,
+ new_line: 22,
+ diff_refs: merge_request.diff_refs
+ )
+ end
+
+ subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
+
+ describe "#position=" do
+ context "when provided a string" do
+ it "sets the position" do
+ subject.position = new_position.to_json
+
+ expect(subject.position).to eq(new_position)
+ end
+ end
+
+ context "when provided a hash" do
+ it "sets the position" do
+ subject.position = new_position.to_h
+
+ expect(subject.position).to eq(new_position)
+ end
+ end
+
+ context "when provided a position object" do
+ it "sets the position" do
+ subject.position = new_position
+
+ expect(subject.position).to eq(new_position)
+ end
+ end
+ end
+
+ describe "#diff_file" do
+ it "returns the correct diff file" do
+ diff_file = subject.diff_file
+
+ expect(diff_file.old_path).to eq(position.old_path)
+ expect(diff_file.new_path).to eq(position.new_path)
+ expect(diff_file.diff_refs).to eq(position.diff_refs)
+ end
+ end
+
+ describe "#diff_line" do
+ it "returns the correct diff line" do
+ diff_line = subject.diff_line
+
+ expect(diff_line.added?).to be true
+ expect(diff_line.new_line).to eq(position.new_line)
+ expect(diff_line.text).to eq("+ vars = {")
+ end
+ end
+
+ describe "#line_code" do
+ it "returns the correct line code" do
+ line_code = Gitlab::Diff::LineCode.generate(position.file_path, position.new_line, 15)
+
+ expect(subject.line_code).to eq(line_code)
+ end
+ end
+
+ describe "#for_line?" do
+ context "when provided the correct diff line" do
+ it "returns true" do
+ expect(subject.for_line?(subject.diff_line)).to be true
+ end
+ end
+
+ context "when provided a different diff line" do
+ it "returns false" do
+ some_line = subject.diff_file.diff_lines.first
+
+ expect(subject.for_line?(some_line)).to be false
+ end
+ end
+ end
+
+ describe "#active?" do
+ context "when noteable is a commit" do
+ subject { create(:diff_note_on_commit, project: project, position: position) }
+
+ it "returns true" do
+ expect(subject.active?).to be true
+ end
+ end
+
+ context "when noteable is a merge request" do
+ context "when the merge request's diff refs match that of the diff note" do
+ it "returns true" do
+ expect(subject.active?).to be true
+ end
+ end
+
+ context "when the merge request's diff refs don't match that of the diff note" do
+ before do
+ allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs)
+ end
+
+ it "returns false" do
+ expect(subject.active?).to be false
+ end
+ end
+ end
+ end
+
+ describe "creation" do
+ describe "updating of position" do
+ context "when noteable is a commit" do
+ let(:diff_note) { create(:diff_note_on_commit, project: project, position: position) }
+
+ it "doesn't use the DiffPositionUpdateService" do
+ expect(Notes::DiffPositionUpdateService).not_to receive(:new)
+
+ diff_note
+ end
+
+ it "doesn't update the position" do
+ diff_note
+
+ expect(diff_note.original_position).to eq(position)
+ expect(diff_note.position).to eq(position)
+ end
+ end
+
+ context "when noteable is a merge request" do
+ let(:diff_note) { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
+
+ context "when the note is active" do
+ it "doesn't use the DiffPositionUpdateService" do
+ expect(Notes::DiffPositionUpdateService).not_to receive(:new)
+
+ diff_note
+ end
+
+ it "doesn't update the position" do
+ diff_note
+
+ expect(diff_note.original_position).to eq(position)
+ expect(diff_note.position).to eq(position)
+ end
+ end
+
+ context "when the note is outdated" do
+ before do
+ allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs)
+ end
+
+ it "uses the DiffPositionUpdateService" do
+ service = instance_double("Notes::DiffPositionUpdateService")
+ expect(Notes::DiffPositionUpdateService).to receive(:new).with(
+ project,
+ nil,
+ old_diff_refs: position.diff_refs,
+ new_diff_refs: commit.diff_refs,
+ paths: [path]
+ ).and_return(service)
+ expect(service).to receive(:execute)
+
+ diff_note
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 6ac19756f15..b5d0d79e14e 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -56,7 +56,7 @@ describe Event, models: true do
end
context 'merge request diff note event' do
- let(:target) { create(:note_on_merge_request_diff) }
+ let(:target) { create(:legacy_diff_note_on_merge_request) }
it { is_expected.to be_note }
end
@@ -132,7 +132,7 @@ describe Event, models: true do
context 'merge request diff note event' do
let(:project) { create(:project, :public) }
let(:merge_request) { create(:merge_request, source_project: project, author: author, assignee: assignee) }
- let(:note_on_merge_request) { create(:note_on_merge_request_diff, noteable: merge_request, project: project) }
+ let(:note_on_merge_request) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project) }
let(:target) { note_on_merge_request }
it { expect(event.visible_to_user?(non_member)).to eq true }
diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb
index b2d06853886..d64d89edbd3 100644
--- a/spec/models/legacy_diff_note_spec.rb
+++ b/spec/models/legacy_diff_note_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
describe LegacyDiffNote, models: true do
describe "Commit diff line notes" do
- let!(:note) { create(:note_on_commit_diff, note: "+1 from me") }
+ let!(:note) { create(:legacy_diff_note_on_commit, note: "+1 from me") }
let!(:commit) { note.noteable }
it "should save a valid note" do
@@ -17,7 +17,7 @@ describe LegacyDiffNote, models: true do
describe '#active?' do
it 'is always true when the note has no associated diff' do
- note = build(:note_on_merge_request_diff)
+ note = build(:legacy_diff_note_on_merge_request)
expect(note).to receive(:diff).and_return(nil)
@@ -25,7 +25,7 @@ describe LegacyDiffNote, models: true do
end
it 'is never true when the note has no noteable associated' do
- note = build(:note_on_merge_request_diff)
+ note = build(:legacy_diff_note_on_merge_request)
expect(note).to receive(:diff).and_return(double)
expect(note).to receive(:noteable).and_return(nil)
@@ -34,7 +34,7 @@ describe LegacyDiffNote, models: true do
end
it 'returns the memoized value if defined' do
- note = build(:note_on_merge_request_diff)
+ note = build(:legacy_diff_note_on_merge_request)
note.instance_variable_set(:@active, 'foo')
expect(note).not_to receive(:find_noteable_diff)
@@ -45,7 +45,7 @@ describe LegacyDiffNote, models: true do
context 'for a merge request noteable' do
it 'is false when noteable has no matching diff' do
merge = build_stubbed(:merge_request, :simple)
- note = build(:note_on_merge_request_diff, noteable: merge)
+ note = build(:legacy_diff_note_on_merge_request, noteable: merge)
allow(note).to receive(:diff).and_return(double)
expect(note).to receive(:find_noteable_diff).and_return(nil)
@@ -63,9 +63,9 @@ describe LegacyDiffNote, models: true do
code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
# We're persisting in order to trigger the set_diff callback
- note = create(:note_on_merge_request_diff, noteable: merge,
- line_code: code,
- project: merge.source_project)
+ note = create(:legacy_diff_note_on_merge_request, noteable: merge,
+ line_code: code,
+ project: merge.source_project)
# Make sure we don't get a false positive from a guard clause
expect(note).to receive(:find_noteable_diff).and_call_original
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index ceb4d64698f..a4b6ff8f8ad 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe MergeRequest, models: true do
+ include RepoHelpers
+
subject { create(:merge_request) }
describe 'associations' do
@@ -62,7 +64,7 @@ describe MergeRequest, models: true do
end
end
- describe '#target_sha' do
+ describe '#target_branch_sha' do
context 'when the target branch does not exist anymore' do
let(:project) { create(:project) }
@@ -73,32 +75,32 @@ describe MergeRequest, models: true do
end
it 'returns nil' do
- expect(subject.target_sha).to be_nil
+ expect(subject.target_branch_sha).to be_nil
end
end
end
- describe '#source_sha' do
+ describe '#source_branch_sha' do
let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) }
context 'with diffs' do
subject { create(:merge_request, :with_diffs) }
it 'returns the sha of the source branch last commit' do
- expect(subject.source_sha).to eq(last_branch_commit.sha)
+ expect(subject.source_branch_sha).to eq(last_branch_commit.sha)
end
end
context 'without diffs' do
subject { create(:merge_request, :without_diffs) }
it 'returns the sha of the source branch last commit' do
- expect(subject.source_sha).to eq(last_branch_commit.sha)
+ expect(subject.source_branch_sha).to eq(last_branch_commit.sha)
end
end
context 'when the merge request is being created' do
subject { build(:merge_request, source_branch: nil, compare_commits: []) }
it 'returns nil' do
- expect(subject.source_sha).to be_nil
+ expect(subject.source_branch_sha).to be_nil
end
end
end
@@ -252,12 +254,14 @@ describe MergeRequest, models: true do
end
it "can be removed if the last commit is the head of the source branch" do
- allow(subject.source_project).to receive(:commit).and_return(subject.last_commit)
+ allow(subject.source_project).to receive(:commit).and_return(subject.diff_head_commit)
expect(subject.can_remove_source_branch?(user)).to be_truthy
end
it "cannot be removed if the last commit is not also the head of the source branch" do
+ subject.source_branch = "lfs"
+
expect(subject.can_remove_source_branch?(user)).to be_falsey
end
end
@@ -363,7 +367,7 @@ describe MergeRequest, models: true do
and_return(2)
subject.diverged_commits_count
- allow(subject).to receive(:source_sha).and_return('123abc')
+ allow(subject).to receive(:source_branch_sha).and_return('123abc')
subject.diverged_commits_count
end
@@ -373,7 +377,7 @@ describe MergeRequest, models: true do
and_return(2)
subject.diverged_commits_count
- allow(subject).to receive(:target_sha).and_return('123abc')
+ allow(subject).to receive(:target_branch_sha).and_return('123abc')
subject.diverged_commits_count
end
end
@@ -392,11 +396,10 @@ describe MergeRequest, models: true do
describe '#pipeline' do
describe 'when the source project exists' do
- it 'returns the latest commit' do
- commit = double(:commit, id: '123abc')
+ it 'returns the latest pipeline' do
pipeline = double(:ci_pipeline, ref: 'master')
- allow(subject).to receive(:last_commit).and_return(commit)
+ allow(subject).to receive(:diff_head_sha).and_return('123abc')
expect(subject.source_project).to receive(:pipeline).
with('123abc', 'master').
@@ -464,7 +467,7 @@ describe MergeRequest, models: true do
context 'when it is not broken and has no conflicts' do
it 'is marked as mergeable' do
allow(subject).to receive(:broken?) { false }
- allow(project.repository).to receive(:can_be_merged?) { true }
+ allow(project.repository).to receive(:can_be_merged?).and_return(true)
expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged')
end
@@ -481,7 +484,7 @@ describe MergeRequest, models: true do
context 'when it has conflicts' do
before do
allow(subject).to receive(:broken?) { false }
- allow(project.repository).to receive(:can_be_merged?) { false }
+ allow(project.repository).to receive(:can_be_merged?).and_return(false)
end
it 'becomes unmergeable' do
@@ -608,4 +611,42 @@ describe MergeRequest, models: true do
end
end
end
+
+ describe "#reload_diff" do
+ let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) }
+
+ let(:commit) { subject.project.commit(sample_commit.id) }
+
+ it "reloads the diff content" do
+ expect(subject.merge_request_diff).to receive(:reload_content)
+
+ subject.reload_diff
+ end
+
+ it "updates diff note positions" do
+ old_diff_refs = subject.diff_refs
+
+ merge_request_diff = subject.merge_request_diff
+
+ # Update merge_request_diff so that #diff_refs will return commit.diff_refs
+ allow(merge_request_diff).to receive(:reload_content) do
+ merge_request_diff.base_commit_sha = commit.parent_id
+ merge_request_diff.start_commit_sha = commit.parent_id
+ merge_request_diff.head_commit_sha = commit.sha
+ end
+
+ expect(Notes::DiffPositionUpdateService).to receive(:new).with(
+ subject.project,
+ nil,
+ old_diff_refs: old_diff_refs,
+ new_diff_refs: commit.diff_refs,
+ paths: note.position.paths
+ ).and_call_original
+ expect_any_instance_of(Notes::DiffPositionUpdateService).to receive(:execute).with(note)
+
+ expect_any_instance_of(DiffNote).to receive(:save).once
+
+ subject.reload_diff
+ end
+ end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 2e89d6de3a2..1b434a726dc 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -312,7 +312,7 @@ describe Project, models: true do
it 'should update merge request commits with new one if pushed to source branch' do
project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user)
merge_request.reload
- expect(merge_request.last_commit.id).to eq(commit_id)
+ expect(merge_request.diff_head_sha).to eq(commit_id)
end
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 7975fc64e59..24e49c8def3 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -12,8 +12,8 @@ describe Repository, models: true do
end
let(:merge_commit) do
source_sha = repository.find_branch('feature').target
- merge_commit_id = repository.merge(user, source_sha, 'master', commit_options)
- repository.commit(merge_commit_id)
+ merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options)
+ repository.commit(merge_commit_sha)
end
describe :branch_names_contains do
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 5d81844fb84..4a1b5600bdf 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -439,14 +439,14 @@ describe API::API, api: true do
end
it "returns 409 if the SHA parameter doesn't match" do
- put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha.succ
+ put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha.reverse
expect(response).to have_http_status(409)
expect(json_response['message']).to start_with('SHA does not match HEAD of source branch')
end
it "succeeds if the SHA parameter matches" do
- put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha
+ put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha
expect(response).to have_http_status(200)
end
diff --git a/spec/services/notes/diff_position_update_service_spec.rb b/spec/services/notes/diff_position_update_service_spec.rb
new file mode 100644
index 00000000000..110efb54fa0
--- /dev/null
+++ b/spec/services/notes/diff_position_update_service_spec.rb
@@ -0,0 +1,175 @@
+require 'spec_helper'
+
+describe Notes::DiffPositionUpdateService, services: true do
+ let(:project) { create(:project) }
+ let(:create_commit) { project.commit("913c66a37b4a45b9769037c55c2d238bd0942d2e") }
+ let(:modify_commit) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") }
+ let(:edit_commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") }
+
+ let(:path) { "files/ruby/popen.rb" }
+
+ let(:old_diff_refs) do
+ Gitlab::Diff::DiffRefs.new(
+ base_sha: create_commit.parent_id,
+ head_sha: modify_commit.sha
+ )
+ end
+
+ let(:new_diff_refs) do
+ Gitlab::Diff::DiffRefs.new(
+ base_sha: create_commit.parent_id,
+ head_sha: edit_commit.sha
+ )
+ end
+
+ subject do
+ described_class.new(
+ project,
+ nil,
+ old_diff_refs: old_diff_refs,
+ new_diff_refs: new_diff_refs,
+ paths: [path]
+ )
+ end
+
+ # old diff:
+ # 1 + require 'fileutils'
+ # 2 + require 'open3'
+ # 3 +
+ # 4 + module Popen
+ # 5 + extend self
+ # 6 +
+ # 7 + def popen(cmd, path=nil)
+ # 8 + unless cmd.is_a?(Array)
+ # 9 + raise "System commands must be given as an array of strings"
+ # 10 + end
+ # 11 +
+ # 12 + path ||= Dir.pwd
+ # 13 + vars = { "PWD" => path }
+ # 14 + options = { chdir: path }
+ # 15 +
+ # 16 + unless File.directory?(path)
+ # 17 + FileUtils.mkdir_p(path)
+ # 18 + end
+ # 19 +
+ # 20 + @cmd_output = ""
+ # 21 + @cmd_status = 0
+ # 22 + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+ # 23 + @cmd_output << stdout.read
+ # 24 + @cmd_output << stderr.read
+ # 25 + @cmd_status = wait_thr.value.exitstatus
+ # 26 + end
+ # 27 +
+ # 28 + return @cmd_output, @cmd_status
+ # 29 + end
+ # 30 + end
+ #
+ # new diff:
+ # 1 + require 'fileutils'
+ # 2 + require 'open3'
+ # 3 +
+ # 4 + module Popen
+ # 5 + extend self
+ # 6 +
+ # 7 + def popen(cmd, path=nil)
+ # 8 + unless cmd.is_a?(Array)
+ # 9 + raise RuntimeError, "System commands must be given as an array of strings"
+ # 10 + end
+ # 11 +
+ # 12 + path ||= Dir.pwd
+ # 13 +
+ # 14 + vars = {
+ # 15 + "PWD" => path
+ # 16 + }
+ # 17 +
+ # 18 + options = {
+ # 19 + chdir: path
+ # 20 + }
+ # 21 +
+ # 22 + unless File.directory?(path)
+ # 23 + FileUtils.mkdir_p(path)
+ # 24 + end
+ # 25 +
+ # 26 + @cmd_output = ""
+ # 27 + @cmd_status = 0
+ # 28 +
+ # 29 + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+ # 30 + @cmd_output << stdout.read
+ # 31 + @cmd_output << stderr.read
+ # 32 + @cmd_status = wait_thr.value.exitstatus
+ # 33 + end
+ # 34 +
+ # 35 + return @cmd_output, @cmd_status
+ # 36 + end
+ # 37 + end
+ #
+ # old->new diff:
+ # .. .. @@ -6,12 +6,18 @@ module Popen
+ # 6 6
+ # 7 7 def popen(cmd, path=nil)
+ # 8 8 unless cmd.is_a?(Array)
+ # 9 - raise "System commands must be given as an array of strings"
+ # 9 + raise RuntimeError, "System commands must be given as an array of strings"
+ # 10 10 end
+ # 11 11
+ # 12 12 path ||= Dir.pwd
+ # 13 - vars = { "PWD" => path }
+ # 14 - options = { chdir: path }
+ # 13 +
+ # 14 + vars = {
+ # 15 + "PWD" => path
+ # 16 + }
+ # 17 +
+ # 18 + options = {
+ # 19 + chdir: path
+ # 20 + }
+ # 15 21
+ # 16 22 unless File.directory?(path)
+ # 17 23 FileUtils.mkdir_p(path)
+ # 18 24 end
+ # 19 25
+ # 20 26 @cmd_output = ""
+ # 21 27 @cmd_status = 0
+ # 28 +
+ # 22 29 Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+ # 23 30 @cmd_output << stdout.read
+ # 24 31 @cmd_output << stderr.read
+ # .. ..
+
+ describe "#execute" do
+ let(:note) { create(:diff_note_on_merge_request, project: project, position: old_position) }
+
+ let(:old_position) do
+ Gitlab::Diff::Position.new(
+ old_path: path,
+ new_path: path,
+ old_line: nil,
+ new_line: line,
+ diff_refs: old_diff_refs
+ )
+ end
+
+ context "when the diff line is the same" do
+ let(:line) { 16 }
+
+ it "updates the position" do
+ subject.execute(note)
+
+ expect(note.original_position).to eq(old_position)
+ expect(note.position).not_to eq(old_position)
+ expect(note.position.new_line).to eq(22)
+ end
+ end
+
+ context "when the diff line has changed" do
+ let(:line) { 9 }
+
+ it "doesn't update the position" do
+ subject.execute(note)
+
+ expect(note.original_position).to eq(old_position)
+ expect(note.position).to eq(old_position)
+ end
+ end
+ end
+end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 85dd30bf48c..43693441450 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -213,7 +213,7 @@ describe SystemNoteService, services: true do
create(:merge_request, source_project: project, target_project: project)
end
- subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.last_commit) }
+ subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.diff_head_commit) }
it_behaves_like 'a system note'