Possible dead (expensive) code for preparing notes for rendering for Projects::MergeRequests::DiffsController

I was able to remove these lines and notes are still rendered properly for MR diffs:

diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index 827d8a0f43a..91aed9b5b84 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -7,7 +7,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   before_action :apply_diff_view_cookie!
   before_action :commit, except: :diffs_batch
   before_action :define_diff_vars, except: :diffs_batch
-  before_action :define_diff_comment_vars, except: :diffs_batch

   def show
     render_diffs
@@ -41,15 +40,14 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   def render_diffs
     @environment = @merge_request.environments_for(current_user).last

-    note_positions = renderable_notes.map(&:position).compact
-    @diffs.unfold_diff_files(note_positions)
+    # note_positions = renderable_notes.map(&:position).compact
+    # @diffs.unfold_diff_files(note_positions)

     @diffs.write_cache

     request = {
       current_user: current_user,
       project: @merge_request.project,
-      render: ->(partial, locals) { view_to_html_string(partial, locals) }
     }

     render json: DiffsSerializer.new(request).represent(@diffs, additional_attributes)
@@ -114,27 +112,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
       latest_diff: @merge_request_diff&.latest?
     }
   end
-
-  def define_diff_comment_vars
-    @new_diff_note_attrs = {
-      noteable_type: 'MergeRequest',
-      noteable_id: @merge_request.id,
-      commit_id: @commit&.id
-    }
-
-    @diff_notes_disabled = false
-
-    @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?
-
-    @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@compare.diff_refs)
-    @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes), @merge_request)
-  end
-
-  def renderable_notes
-    define_diff_comment_vars unless @notes
-
-    @notes
-  end
 end

There's a lot of processing going on to prepare the notes for rendering, including memoizing all of them. Seems like now we request discussions.json to render notes, which seems to make this a dead code?

We use it for unfolding diffs, but we can make it quite cheaper.

Seems like it was introduced by 5a286eb7 @pslaughter. Is this being used somewhere in the FE that I'm not finding? 🤔

Edited Aug 14, 2020 by 🤖 GitLab Bot 🤖
Assignee Loading
Time tracking Loading