Change the way old merge request diff handled

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

2 files changed, 19 insertions, 12 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 7e3444882ea..b7c2afb0920 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -261,7 +261,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def diff_refs
-    if merge_request_diff
+    if persisted?
       merge_request_diff.diff_refs
     else
       start_sha = target_branch_sha
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 24e09c4d57c..4c18775c44a 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -22,10 +22,6 @@ class MergeRequestDiff < ActiveRecord::Base
   serialize :st_commits
   serialize :st_diffs
 
-  # For compatibility with old MergeRequestDiff which
-  # does not store those variables in database
-  after_initialize :ensure_commits_sha, if: :persisted?
-
   # All diff information is collected from repository after object is created.
   # It allows you to override variables like head_commit_sha before getting diff.
   after_create :save_git_content, unless: :importing?
@@ -50,6 +46,20 @@ class MergeRequestDiff < ActiveRecord::Base
     self.base_commit_sha  ||= find_base_sha
   end
 
+  # This method will rely on repository branch sha
+  # in case start_commit_sha is nil. Its necesarry for old merge request diff
+  # created before version 8.4 to work
+  def safe_start_commit_sha
+    start_commit_sha || merge_request.target_branch_sha
+  end
+
+  # This method will rely on repository branch sha
+  # in case head_commit_sha is nil. Its necesarry for old merge request diff
+  # created before version 8.4 to work
+  def safe_head_commit_sha
+    head_commit_sha || last_commit.try(:sha) || merge_request.source_branch_sha
+  end
+
   def size
     real_size.presence || diffs.size
   end
@@ -60,8 +70,8 @@ class MergeRequestDiff < ActiveRecord::Base
         begin
           compare = Gitlab::Git::Compare.new(
             repository.raw_repository,
-            start_commit_sha,
-            head_commit_sha
+            safe_start_commit_sha,
+            safe_head_commit_sha
           )
           compare.diffs(options)
         end
@@ -126,8 +136,8 @@ class MergeRequestDiff < ActiveRecord::Base
 
         Gitlab::Git::Compare.new(
           repository.raw_repository,
-          start_commit_sha,
-          head_commit_sha
+          safe_start_commit_sha,
+          safe_head_commit_sha
         )
       end
   end
@@ -216,9 +226,6 @@ class MergeRequestDiff < ActiveRecord::Base
     return unless head_commit_sha && start_commit_sha
 
     project.merge_base_commit(head_commit_sha, start_commit_sha).try(:sha)
-  rescue Rugged::OdbError
-    # In case head or start commit does not exist in the repository any more
-    nil
   end
 
   def utf8_st_diffs