Merge branch 'fix-removed-file-in-diff' into 'master'

Fix bug when removed file was not appearing in merge request diff

Finally we can see removed files in merge request diff :sparkles: 

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Fixes #2533 

See merge request !1471

6 files changed, 70 insertions, 51 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a499dda2bb3..492e4b9aebf 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,7 @@ v 8.1.0 (unreleased)
   - Move CI variables page to project settings area
   - Move CI triggers page to project settings area
   - Move CI project settings page to CE project settings area
+  - Fix bug when removed file was not appearing in merge request diff
 
 v 8.0.3
   - Fix URL shown in Slack notifications
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 6ffa1a7121d..9d718e13b85 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -167,4 +167,23 @@ module DiffHelper
       content_tag(:span, commit_id, class: 'monospace'),
     ].join(' ').html_safe
   end
+
+  def commit_for_diff(diff)
+    if diff.deleted_file
+      @merge_request ? @merge_request.commits.last : @commit.parent_id
+    else
+      @commit
+    end
+  end
+
+  def diff_file_html_data(project, diff_commit, diff_file)
+    {
+      blob_diff_path: namespace_project_blob_diff_path(project.namespace, project,
+                                                       tree_join(diff_commit.id, diff_file.file_path))
+    }
+  end
+
+  def editable_diff?(diff)
+    !diff.deleted_file && @merge_request && @merge_request.source_project
+  end
 end
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index 2f24dc7c909..c5acafa2630 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -15,7 +15,12 @@
 
 .files
   - diff_files.each_with_index do |diff_file, index|
-    = render 'projects/diffs/file', diff_file: diff_file, i: index, project: project
+    - diff_commit = commit_for_diff(diff_file.diff)
+    - blob = project.repository.blob_for_diff(diff_commit, diff_file.diff)
+    - next unless blob
+
+    = render 'projects/diffs/file', i: index, project: project,
+      diff_file: diff_file, diff_commit: diff_commit, blob: blob
 
 - if @diff_timeout
   .alert.alert-danger
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index 99ee23a1ddc..4617b188150 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -1,24 +1,18 @@
-- blob = project.repository.blob_for_diff(@commit, diff_file.diff)
-- return unless blob
-- blob_diff_path = namespace_project_blob_diff_path(project.namespace, project, tree_join(@commit.id, diff_file.file_path))
-.diff-file{id: "diff-#{i}", data: {blob_diff_path: blob_diff_path }}
+.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)}"}
-    - if diff_file.deleted_file
-      %span="#{diff_file.old_path} deleted"
-
-      .diff-btn-group
-        - if @commit.parent_ids.present?
-          = view_file_btn(@commit.parent_id, diff_file, project)
-    - elsif diff_file.diff.submodule?
+    - if diff_file.diff.submodule?
       %span
         - submodule_item = project.repository.blob_at(@commit.id, diff_file.file_path)
         = submodule_link(submodule_item, @commit.id, project.repository)
     - else
       %span
-        - if diff_file.renamed_file
+        - if diff_file.deleted_file
+          = "#{diff_file.old_path} deleted"
+        - elsif diff_file.renamed_file
           = "#{diff_file.old_path} renamed to #{diff_file.new_path}"
         - else
           = diff_file.new_path
+
         - if diff_file.mode_changed?
           %span.file-mode= "#{diff_file.diff.a_mode} → #{diff_file.diff.b_mode}"
 
@@ -28,12 +22,12 @@
             %i.fa.fa-comments
            
 
-        - if @merge_request && @merge_request.source_project
+        - if editable_diff?(diff_file)
           = edit_blob_link(@merge_request.source_project,
               @merge_request.source_branch, diff_file.new_path,
               after: ' ', from_merge_request_id: @merge_request.id)
 
-        = view_file_btn(@commit.id, diff_file, project)
+        = view_file_btn(diff_commit.id, diff_file, project)
 
   .diff-content.diff-wrap-lines
     -# Skipp all non non-supported blobs
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index 947f668e432..83055188bac 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -115,40 +115,40 @@ Feature: Project Merge Requests
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
     And I click on the Changes tab
-    And I leave a comment like "Line is wrong" on line 39 of the second file
-    And I click link "Hide inline discussion" of the second file
-    Then I should not see a comment like "Line is wrong here" in the second file
+    And I leave a comment like "Line is wrong" on line 39 of the third file
+    And I click link "Hide inline discussion" of the third file
+    Then I should not see a comment like "Line is wrong here" in the third file
 
   @javascript
   Scenario: I show comments on a merge request diff with comments in a single file
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
     And I click on the Changes tab
-    And I leave a comment like "Line is wrong" on line 39 of the second file
-    Then I should see a comment like "Line is wrong" in the second file
+    And I leave a comment like "Line is wrong" on line 39 of the third file
+    Then I should see a comment like "Line is wrong" in the third file
 
   @javascript
   Scenario: I hide comments on a merge request diff with comments in multiple files
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
     And I click on the Changes tab
-    And I leave a comment like "Line is correct" on line 12 of the first file
-    And I leave a comment like "Line is wrong" on line 39 of the second file
-    And I click link "Hide inline discussion" of the second file
-    Then I should not see a comment like "Line is wrong here" in the second file
-    And I should still see a comment like "Line is correct" in the first file
+    And I leave a comment like "Line is correct" on line 12 of the second file
+    And I leave a comment like "Line is wrong" on line 39 of the third file
+    And I click link "Hide inline discussion" of the third file
+    Then I should not see a comment like "Line is wrong here" in the third file
+    And I should still see a comment like "Line is correct" in the second file
 
   @javascript
   Scenario: I show comments on a merge request diff with comments in multiple files
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
     And I click on the Changes tab
-    And I leave a comment like "Line is correct" on line 12 of the first file
-    And I leave a comment like "Line is wrong" on line 39 of the second file
-    And I click link "Hide inline discussion" of the second file
-    And I click link "Show inline discussion" of the second file
-    Then I should see a comment like "Line is wrong" in the second file
-    And I should still see a comment like "Line is correct" in the first file
+    And I leave a comment like "Line is correct" on line 12 of the second file
+    And I leave a comment like "Line is wrong" on line 39 of the third file
+    And I click link "Hide inline discussion" of the third file
+    And I click link "Show inline discussion" of the third file
+    Then I should see a comment like "Line is wrong" in the third file
+    And I should still see a comment like "Line is correct" in the second file
 
   @javascript
   Scenario: I unfold diff
@@ -163,8 +163,8 @@ Feature: Project Merge Requests
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
     And I click on the Changes tab
-    And I leave a comment like "Line is correct" on line 12 of the first file
-    And I leave a comment like "Line is wrong" on line 39 of the second file
+    And I leave a comment like "Line is correct" on line 12 of the second file
+    And I leave a comment like "Line is wrong" on line 39 of the third file
     And I click Side-by-side Diff tab
     Then I should see comments on the side-by-side diff page
 
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index c92998631ff..875bf6c4676 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -224,43 +224,43 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
     end
   end
 
-  step 'I click link "Hide inline discussion" of the second file' do
-    page.within '.files [id^=diff]:nth-child(2)' do
+  step 'I click link "Hide inline discussion" of the third file' do
+    page.within '.files [id^=diff]:nth-child(3)' do
       find('.js-toggle-diff-comments').trigger('click')
     end
   end
 
-  step 'I click link "Show inline discussion" of the second file' do
-    page.within '.files [id^=diff]:nth-child(2)' do
+  step 'I click link "Show inline discussion" of the third file' do
+    page.within '.files [id^=diff]:nth-child(3)' do
       find('.js-toggle-diff-comments').trigger('click')
     end
   end
 
-  step 'I should not see a comment like "Line is wrong" in the second file' do
-    page.within '.files [id^=diff]:nth-child(2)' do
+  step 'I should not see a comment like "Line is wrong" in the third file' do
+    page.within '.files [id^=diff]:nth-child(3)' do
       expect(page).not_to have_visible_content "Line is wrong"
     end
   end
 
-  step 'I should see a comment like "Line is wrong" in the second file' do
-    page.within '.files [id^=diff]:nth-child(2) .note-body > .note-text' do
+  step 'I should see a comment like "Line is wrong" in the third file' do
+    page.within '.files [id^=diff]:nth-child(3) .note-body > .note-text' do
       expect(page).to have_visible_content "Line is wrong"
     end
   end
 
-  step 'I should not see a comment like "Line is wrong here" in the second file' do
-    page.within '.files [id^=diff]:nth-child(2)' do
+  step 'I should not see a comment like "Line is wrong here" in the third file' do
+    page.within '.files [id^=diff]:nth-child(3)' do
       expect(page).not_to have_visible_content "Line is wrong here"
     end
   end
 
-  step 'I should see a comment like "Line is wrong here" in the second file' do
-    page.within '.files [id^=diff]:nth-child(2) .note-body > .note-text' do
+  step 'I should see a comment like "Line is wrong here" in the third file' do
+    page.within '.files [id^=diff]:nth-child(3) .note-body > .note-text' do
       expect(page).to have_visible_content "Line is wrong here"
     end
   end
 
-  step 'I leave a comment like "Line is correct" on line 12 of the first file' do
+  step 'I leave a comment like "Line is correct" on line 12 of the second file' do
     init_diff_note_first_file
 
     page.within(".js-discussion-note-form") do
@@ -268,12 +268,12 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
       click_button "Add Comment"
     end
 
-    page.within ".files [id^=diff]:nth-child(1) .note-body > .note-text" do
+    page.within ".files [id^=diff]:nth-child(2) .note-body > .note-text" do
       expect(page).to have_content "Line is correct"
     end
   end
 
-  step 'I leave a comment like "Line is wrong" on line 39 of the second file' do
+  step 'I leave a comment like "Line is wrong" on line 39 of the third file' do
     init_diff_note_second_file
 
     page.within(".js-discussion-note-form") do
@@ -282,8 +282,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
     end
   end
 
-  step 'I should still see a comment like "Line is correct" in the first file' do
-    page.within '.files [id^=diff]:nth-child(1) .note-body > .note-text' do
+  step 'I should still see a comment like "Line is correct" in the second file' do
+    page.within '.files [id^=diff]:nth-child(2) .note-body > .note-text' do
       expect(page).to have_visible_content "Line is correct"
     end
   end
@@ -303,7 +303,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I should see comments on the side-by-side diff page' do
-    page.within '.files [id^=diff]:nth-child(1) .parallel .note-body > .note-text' do
+    page.within '.files [id^=diff]:nth-child(2) .parallel .note-body > .note-text' do
       expect(page).to have_visible_content "Line is correct"
     end
   end