Commit 44d35a82 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets Committed by Robert Speicher

Merge branch 'merge-request-deleted-file' into 'master'

Correctly find last known blob for file deleted in MR.

Fixes #3092.

When building a new MR, `@merge_request.commits.last` would fail because this delegates to `merge_request_diff` which is still `nil` at that point. I fixed that, and changed some of the logic because showing deleted blob contents didn't previously work for the Compare page, and the UI would show the wrong commit sha for "View File @...".

See merge request !1647
parent de27df7c
......@@ -17,9 +17,10 @@ class Projects::CompareController < Projects::ApplicationController
execute(@project, head_ref, @project, base_ref)
if compare_result
@commits = compare_result.commits
@commits = Commit.decorate(compare_result.commits, @project)
@diffs = compare_result.diffs
@commit = @commits.last
@first_commit = @commits.first
@line_notes = []
end
end
......
......@@ -56,6 +56,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def diffs
@commit = @merge_request.last_commit
@first_commit = @merge_request.first_commit
@comments_allowed = @reply_allowed = true
@comments_target = {
noteable_type: 'MergeRequest',
......@@ -89,7 +91,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@target_project = merge_request.target_project
@source_project = merge_request.source_project
@commits = @merge_request.compare_commits
@commit = @merge_request.compare_commits.last
@commit = @merge_request.last_commit
@first_commit = @merge_request.first_commit
@diffs = @merge_request.compare_diffs
@note_counts = Note.where(commit_id: @commits.map(&:id)).
group(:commit_id).count
......
......@@ -170,7 +170,8 @@ module DiffHelper
def commit_for_diff(diff)
if diff.deleted_file
@merge_request ? @merge_request.commits.last : @commit.parents.first
first_commit = @first_commit || @commit
first_commit.parent
else
@commit
end
......
......@@ -164,6 +164,14 @@ class Commit
@committer ||= User.find_by_any_email(committer_email)
end
def parents
@parents ||= parent_ids.map { |id| project.commit(id) }
end
def parent
@parent ||= project.commit(self.parent_id) if self.parent_id
end
def notes
project.notes.for_commit_id(self.id)
end
......@@ -181,10 +189,6 @@ class Commit
@raw.short_id(7)
end
def parents
@parents ||= Commit.decorate(super, project)
end
def ci_commit
project.ci_commit(sha)
end
......
......@@ -40,7 +40,7 @@ class MergeRequest < ActiveRecord::Base
after_create :create_merge_request_diff
after_update :update_merge_request_diff
delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil
delegate :commits, :diffs, to: :merge_request_diff, prefix: nil
# When this attribute is true some MR validation is ignored
# It allows us to close or modify broken merge requests
......@@ -157,6 +157,18 @@ 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
def last_commit_short_sha
last_commit.short_id
end
def validate_branches
if target_project == source_project && target_branch == source_branch
errors.add :branch_conflict, "You can not use same project/branch for source and target"
......
......@@ -55,6 +55,10 @@ class MergeRequestDiff < ActiveRecord::Base
commits.first
end
def first_commit
commits.last
end
def last_commit_short_sha
@last_commit_short_sha ||= last_commit.short_id
end
......
......@@ -312,13 +312,7 @@ class Repository
end
def blob_for_diff(commit, diff)
file = blob_at(commit.id, diff.new_path)
unless file
file = prev_blob_for_diff(commit, diff)
end
file
blob_at(commit.id, diff.file_path)
end
def prev_blob_for_diff(commit, diff)
......
......@@ -15,8 +15,8 @@
.files
- diff_files.each_with_index do |diff_file, index|
- diff_commit = commit_for_diff(diff_file.diff)
- blob = project.repository.blob_for_diff(diff_commit, diff_file.diff)
- diff_commit = commit_for_diff(diff_file)
- blob = project.repository.blob_for_diff(diff_commit, diff_file)
- next unless blob
= render 'projects/diffs/file', i: index, project: project,
......
.diff-file{id: "diff-#{i}", data: diff_file_html_data(project, diff_commit, diff_file)}
.diff-header{id: "file-path-#{hexdigest(diff_file.new_path || diff_file.old_path)}"}
.diff-header{id: "file-path-#{hexdigest(diff_file.file_path)}"}
- if diff_file.diff.submodule?
%span
- submodule_item = project.repository.blob_at(@commit.id, diff_file.file_path)
......@@ -38,7 +38,7 @@
- else
= render "projects/diffs/text_file", diff_file: diff_file, index: i
- elsif blob.image?
- old_file = project.repository.prev_blob_for_diff(@commit, diff_file)
- 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
- else
.nothing-here-block No preview for this file type
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment