summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2016-08-27 20:59:48 -0700
committerRémy Coutable <remy@rymai.me>2016-11-14 15:57:01 +0100
commit0f61bb2dc5298a7e06f8e211adcf46dd282b9c86 (patch)
tree3e5d555919ae02c30bb8519cc898d8a256f75e9a
parent36fa5d660df96c49484cd27695da1df6e361eb8f (diff)
downloadgitlab-ce-0f61bb2dc5298a7e06f8e211adcf46dd282b9c86.tar.gz
Fix Error 500 when creating a merge request that contains an image that was deleted and addedstanhu/gitlab-ce-fix-error-500-with-mr-images
Steps to reproduce: 1. Start with a repo with an image 2. Add a commit to delete the image 3. Add another commit to replace the image with another image In a diff comparison, we really just compare about what the image was before the diff, not the direct parent of the last commit. This MR fixes that. Closes #3893, gitlab-org/gitlab-ee#678 Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--app/views/projects/diffs/_content.html.haml2
-rw-r--r--changelogs/unreleased/stanhu-gitlab-ce-fix-error-500-with-mr-images.yml4
-rw-r--r--lib/gitlab/diff/file.rb13
-rw-r--r--spec/features/merge_requests/create_new_mr_spec.rb10
-rw-r--r--spec/lib/gitlab/diff/file_spec.rb24
-rw-r--r--spec/support/test_env.rb1
6 files changed, 48 insertions, 6 deletions
diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml
index c3d2f80544b..6120b2191dd 100644
--- a/app/views/projects/diffs/_content.html.haml
+++ b/app/views/projects/diffs/_content.html.haml
@@ -25,7 +25,7 @@
- elsif diff_file.renamed_file
.nothing-here-block File moved
- elsif blob.image?
- - old_blob = diff_file.old_blob(diff_commit)
+ - old_blob = diff_file.old_blob(diff_file.old_content_commit || @base_commit)
= render "projects/diffs/image", diff_file: diff_file, old_file: old_blob, file: blob
- else
.nothing-here-block No preview for this file type
diff --git a/changelogs/unreleased/stanhu-gitlab-ce-fix-error-500-with-mr-images.yml b/changelogs/unreleased/stanhu-gitlab-ce-fix-error-500-with-mr-images.yml
new file mode 100644
index 00000000000..7ca0d5fb19e
--- /dev/null
+++ b/changelogs/unreleased/stanhu-gitlab-ce-fix-error-500-with-mr-images.yml
@@ -0,0 +1,4 @@
+---
+title: Fix Error 500 when creating a merge request that contains an image that was deleted and added
+merge_request: 7457
+author:
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 5110bfbf898..c6bf25b5874 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -55,6 +55,12 @@ module Gitlab
repository.commit(deleted_file ? old_ref : new_ref)
end
+ def old_content_commit
+ return unless diff_refs
+
+ repository.commit(old_ref)
+ end
+
def old_ref
diff_refs.try(:base_sha)
end
@@ -111,13 +117,10 @@ module Gitlab
diff_lines.count(&:removed?)
end
- def old_blob(commit = content_commit)
+ def old_blob(commit = old_content_commit)
return unless commit
- parent_id = commit.parent_id
- return unless parent_id
-
- repository.blob_at(parent_id, old_path)
+ repository.blob_at(commit.id, old_path)
end
def blob(commit = content_commit)
diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb
index c68e1ea4af9..584574cc91a 100644
--- a/spec/features/merge_requests/create_new_mr_spec.rb
+++ b/spec/features/merge_requests/create_new_mr_spec.rb
@@ -67,4 +67,14 @@ feature 'Create New Merge Request', feature: true, js: true do
expect(page).to have_content('Source branch "non-exist-source" does not exist')
expect(page).to have_content('Target branch "non-exist-target" does not exist')
end
+
+ context 'when a branch contains commits that both delete and add the same image' do
+ it 'renders the diff successfully' do
+ visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { target_branch: 'master', source_branch: 'deleted-image-test' })
+
+ click_link "Changes"
+
+ expect(page).to have_content "6049019_460s.jpg"
+ end
+ end
end
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index 0650cb291e5..38475792d93 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -46,4 +46,28 @@ describe Gitlab::Diff::File, lib: true do
expect(diff_file.collapsed?).to eq(false)
end
end
+
+ describe '#old_content_commit' do
+ it 'returns base commit' do
+ old_content_commit = diff_file.old_content_commit
+
+ expect(old_content_commit.id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+ end
+ end
+
+ describe '#old_blob' do
+ it 'returns blob of commit of base commit' do
+ old_data = diff_file.old_blob.data
+
+ expect(old_data).to include('raise "System commands must be given as an array of strings"')
+ end
+ end
+
+ describe '#blob' do
+ it 'returns blob of new commit' do
+ data = diff_file.blob.data
+
+ expect(data).to include('raise RuntimeError, "System commands must be given as an array of strings"')
+ end
+ end
end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 103f7542286..4cf81be3adc 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -35,6 +35,7 @@ module TestEnv
'conflict-missing-side' => 'eb227b3',
'conflict-non-utf8' => 'd0a293c',
'conflict-too-large' => '39fa04f',
+ 'deleted-image-test' => '6c17798'
}
# gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily