Fixing image lfs bug and also displaying text lfs

This commit, introduced in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23812,
fixes a problem creating a displaying image diff notes when the image
is stored in LFS. The main problem was that `Gitlab::Diff::File` was
returning an invalid valid in `text?` for this kind of files.

It also fixes a rendering problem with other LFS files, like text
ones. They LFS pointer shouldn't be shown when LFS is enabled
for the project, but they were.

5 files changed, 49 insertions, 8 deletions

diff --git a/app/models/diff_viewer/base.rb b/app/models/diff_viewer/base.rb
index 1176861a827..527ee33b83b 100644
--- a/app/models/diff_viewer/base.rb
+++ b/app/models/diff_viewer/base.rb
@@ -18,7 +18,7 @@ module DiffViewer
 
     def initialize(diff_file)
       @diff_file = diff_file
-      @initially_binary = diff_file.binary?
+      @initially_binary = diff_file.binary_in_repo?
     end
 
     def self.partial_path
@@ -48,7 +48,7 @@ module DiffViewer
 
     def self.can_render_blob?(blob, verify_binary: true)
       return true if blob.nil?
-      return false if verify_binary && binary? != blob.binary?
+      return false if verify_binary && binary? != blob.binary_in_repo?
       return true if extensions&.include?(blob.extension)
       return true if file_types&.include?(blob.file_type)
 
@@ -70,20 +70,49 @@ module DiffViewer
     end
 
     def binary_detected_after_load?
-      !@initially_binary && diff_file.binary?
+      !@initially_binary && diff_file.binary_in_repo?
     end
 
     # This method is used on the server side to check whether we can attempt to
-    # render the diff_file at all. Human-readable error messages are found in the
-    # `BlobHelper#diff_render_error_reason` helper.
+    # render the diff_file at all. The human-readable error message can be
+    # retrieved by #render_error_message.
     def render_error
       if too_large?
         :too_large
       end
     end
 
+    def render_error_message
+      return unless render_error
+
+      _("This %{viewer} could not be displayed because %{reason}. You can %{options} instead.") %
+        {
+          viewer: switcher_title,
+          reason: render_error_reason,
+          options: render_error_options.to_sentence(two_words_connector: _(' or '), last_word_connector: _(', or '))
+        }
+    end
+
     def prepare!
       # To be overridden by subclasses
     end
+
+    private
+
+    def render_error_options
+      options = []
+
+      blob_url = Gitlab::Routing.url_helpers.project_blob_path(diff_file.repository.project,
+                                                               File.join(diff_file.content_sha, diff_file.file_path))
+      options << ActionController::Base.helpers.link_to(_('view the blob'), blob_url)
+
+      options
+    end
+
+    def render_error_reason
+      if render_error == :too_large
+        _("it is too large")
+      end
+    end
   end
 end
diff --git a/app/models/diff_viewer/image.rb b/app/models/diff_viewer/image.rb
index c356c2ca50e..350bef1d42a 100644
--- a/app/models/diff_viewer/image.rb
+++ b/app/models/diff_viewer/image.rb
@@ -9,6 +9,6 @@ module DiffViewer
     self.extensions = UploaderHelper::IMAGE_EXT
     self.binary = true
     self.switcher_icon = 'picture-o'
-    self.switcher_title = 'image diff'
+    self.switcher_title = _('image diff')
   end
 end
diff --git a/app/models/diff_viewer/rich.rb b/app/models/diff_viewer/rich.rb
index 2faa1be6567..5caefa2031c 100644
--- a/app/models/diff_viewer/rich.rb
+++ b/app/models/diff_viewer/rich.rb
@@ -7,7 +7,7 @@ module DiffViewer
     included do
       self.type = :rich
       self.switcher_icon = 'file-text-o'
-      self.switcher_title = 'rendered diff'
+      self.switcher_title = _('rendered diff')
     end
   end
 end
diff --git a/app/models/diff_viewer/server_side.rb b/app/models/diff_viewer/server_side.rb
index 977204e6c97..0877c9dddec 100644
--- a/app/models/diff_viewer/server_side.rb
+++ b/app/models/diff_viewer/server_side.rb
@@ -24,5 +24,17 @@ module DiffViewer
 
       super
     end
+
+    private
+
+    def render_error_reason
+      return super unless render_error == :server_side_but_stored_externally
+
+      if diff_file.external_storage == :lfs
+        _('it is stored in LFS')
+      else
+        _('it is stored externally')
+      end
+    end
   end
 end
diff --git a/app/models/diff_viewer/simple.rb b/app/models/diff_viewer/simple.rb
index 8d28ca5239a..929d8ad5a7e 100644
--- a/app/models/diff_viewer/simple.rb
+++ b/app/models/diff_viewer/simple.rb
@@ -7,7 +7,7 @@ module DiffViewer
     included do
       self.type = :simple
       self.switcher_icon = 'code'
-      self.switcher_title = 'source diff'
+      self.switcher_title = _('source diff')
     end
   end
 end