Skip to content

Commit dbab210

Browse files
author
Robert Speicher
committed
Merge branch 'osw-remove-dead-code-on-mr-show' into 'master'
Removes expensive dead code on main MR page request Closes #51172 See merge request gitlab-org/gitlab-ce!22153
2 parents 1540d51 + 72273d3 commit dbab210

File tree

4 files changed

+6
-29
lines changed

4 files changed

+6
-29
lines changed

app/controllers/projects/merge_requests_controller.rb

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,6 @@ def show
4444
@noteable = @merge_request
4545
@commits_count = @merge_request.commits_count
4646

47-
# TODO cleanup- Fatih Simon Create an issue to remove these after the refactoring
48-
# we no longer render notes here. I see it will require a small frontend refactoring,
49-
# since we gather some data from this collection.
50-
@discussions = @merge_request.discussions
51-
@notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
52-
5347
labels
5448

5549
set_pipeline_variables

app/helpers/notes_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ def can_create_note?
142142
def initial_notes_data(autocomplete)
143143
{
144144
notesUrl: notes_url,
145-
notesIds: @notes.map(&:id),
145+
notesIds: @noteable.notes.pluck(:id), # rubocop: disable CodeReuse/ActiveRecord
146146
now: Time.now.to_i,
147147
diffView: diff_view,
148148
enableGFM: {
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
title: Removes expensive dead code on main MR page request
3+
merge_request: 22153
4+
author:
5+
type: performance

spec/controllers/projects/merge_requests_controller_spec.rb

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -76,28 +76,6 @@ def go(extra_params = {})
7676
expect(response).to be_success
7777
end
7878

79-
context "loads notes" do
80-
let(:first_contributor) { create(:user) }
81-
let(:contributor) { create(:user) }
82-
let(:merge_request) { create(:merge_request, author: first_contributor, target_project: project, source_project: project) }
83-
let(:contributor_merge_request) { create(:merge_request, :merged, author: contributor, target_project: project, source_project: project) }
84-
# the order here is important
85-
# as the controller reloads these from DB, references doesn't correspond after
86-
let!(:first_contributor_note) { create(:note, author: first_contributor, noteable: merge_request, project: project) }
87-
let!(:contributor_note) { create(:note, author: contributor, noteable: merge_request, project: project) }
88-
let!(:owner_note) { create(:note, author: user, noteable: merge_request, project: project) }
89-
90-
it "with special_role FIRST_TIME_CONTRIBUTOR" do
91-
go(format: :html)
92-
93-
notes = assigns(:notes)
94-
expect(notes).to match(a_collection_containing_exactly(an_object_having_attributes(special_role: Note::SpecialRole::FIRST_TIME_CONTRIBUTOR),
95-
an_object_having_attributes(special_role: nil),
96-
an_object_having_attributes(special_role: nil)
97-
))
98-
end
99-
end
100-
10179
context "that is invalid" do
10280
let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) }
10381

0 commit comments

Comments
 (0)