From a236b5e6a22cf762354a9bb3460e6bad80ab5b3f Mon Sep 17 00:00:00 2001
From: Filipa Lacerda <filipa@gitlab.com>
Date: Fri, 13 Apr 2018 09:16:06 +0100
Subject: [PATCH] Render only one error message per diff

Move html to haml file instead of JS
---
 app/assets/javascripts/notes.js               | 27 ++++++++-----------
 app/assets/stylesheets/framework/buttons.scss |  2 +-
 .../discussions/_diff_with_notes.html.haml    |  5 +++-
 .../user_scrolls_to_note_on_load_spec.rb      |  2 +-
 4 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 184f335809d..9f1db70b06e 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -1431,31 +1431,21 @@ export default class Notes {
     syntaxHighlight(fileHolder);
   }
 
-  static renderDiffError($container) {
-    $container.find('.line_content').html(
-      $(`
-        <div class="js-error-load-lazy-diff nothing-here-block">
-          ${sprintf(__('Unable to load the diff.%{buttonStartTag}Try again%{buttonEndTag}?'), {
-            buttonStartTag: '<button type="button" class="btn-link btn-no-padding js-toggle-lazy-diff">',
-            buttonEndTag: '</button>'
-          }, false)}
-        </div>
-      `),
-    );
-  }
-
   loadLazyDiff(e) {
     const $container = $(e.currentTarget).closest('.js-toggle-container');
     Notes.renderPlaceholderComponent($container);
 
     $container.find('.js-toggle-lazy-diff').removeClass('js-toggle-lazy-diff');
 
-    const tableEl = $container.find('tbody');
-    if (tableEl.length === 0) return;
+    const $tableEl = $container.find('tbody');
+    if ($tableEl.length === 0) return;
 
     const fileHolder = $container.find('.file-holder');
     const url = fileHolder.data('linesPath');
 
+    const $errorContainer = $container.find('.js-error-lazy-load-diff');
+    const $successContainer = $container.find('.js-success-lazy-load');
+
     /**
      * We only fetch resolved discussions.
      * Unresolved discussions don't have an endpoint being provided.
@@ -1464,10 +1454,15 @@ export default class Notes {
       axios
       .get(url)
       .then(({ data }) => {
+        // Reset state in case last request returned error
+        $successContainer.removeClass('hidden');
+        $errorContainer.addClass('hidden');
+
         Notes.renderDiffContent($container, data);
       })
       .catch(() => {
-        Notes.renderDiffError($container);
+        $successContainer.addClass('hidden');
+        $errorContainer.removeClass('hidden');
       });
     }
   }
diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index 9a8fe12e4e8..0be86c36738 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -488,4 +488,4 @@ fieldset[disabled] .btn,
 
 .btn-no-padding {
   padding: 0;
-}
\ No newline at end of file
+}
diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml
index b382bc201b5..c13a128835a 100644
--- a/app/views/discussions/_diff_with_notes.html.haml
+++ b/app/views/discussions/_diff_with_notes.html.haml
@@ -28,8 +28,11 @@
           %tr.line_holder.line-holder-placeholder
             %td.old_line.diff-line-num
             %td.new_line.diff-line-num
-            %td.line_content
+            %td.line_content.js-success-lazy-load
               .js-code-placeholder
+            %td.line_content.js-error-lazy-load-diff.hidden
+              - button = button_tag(_("Try again"), class: "btn-link btn-no-padding js-toggle-lazy-diff")
+              = _("Unable to load the diff. %{button_try_again}").html_safe % { button_try_again: button}
           = render "discussions/diff_discussion", discussions: [discussion], expanded: true
   - else
     - partial = (diff_file.new_file? || diff_file.deleted_file?) ? 'single_image_diff' : 'replaced_image_diff'
diff --git a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb
index be888bf1a55..71cd1ed0776 100644
--- a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb
+++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb
@@ -40,7 +40,7 @@ describe 'Merge request > User scrolls to note on load', :js do
     expect(note_element.visible?).to eq true
 
     page.within note_container do
-      expect(page).not_to have_selector('.js-error-load-lazy-diff')
+      expect(page).not_to have_selector('.js-error-lazy-load-diff')
     end
 
   end