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 by 🤖 GitLab Bot 🤖