summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2018-10-08 18:49:41 +0000
committerCindy Pallares <cindy@gitlab.com>2018-10-11 16:33:14 -0500
commitd64bc275d814c0ff26e4282677a39eaa4a668f80 (patch)
tree9421712ecf9c475dae4c634bdc0924b9960374ae
parent37308a9a9b6ba05fb6e06a9188e947ada2318ba9 (diff)
downloadgitlab-ce-d64bc275d814c0ff26e4282677a39eaa4a668f80.tar.gz
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
-rw-r--r--app/controllers/projects/merge_requests_controller.rb6
-rw-r--r--app/helpers/notes_helper.rb2
-rw-r--r--changelogs/unreleased/osw-remove-dead-code-on-mr-show.yml5
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb22
4 files changed, 6 insertions, 29 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 6a5da9b8292..8bc3a81d771 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -44,12 +44,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@noteable = @merge_request
@commits_count = @merge_request.commits_count
- # TODO cleanup- Fatih Simon Create an issue to remove these after the refactoring
- # we no longer render notes here. I see it will require a small frontend refactoring,
- # since we gather some data from this collection.
- @discussions = @merge_request.discussions
- @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
-
labels
set_pipeline_variables
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index e0905584803..033686823a2 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -142,7 +142,7 @@ module NotesHelper
def initial_notes_data(autocomplete)
{
notesUrl: notes_url,
- notesIds: @notes.map(&:id),
+ notesIds: @noteable.notes.pluck(:id), # rubocop: disable CodeReuse/ActiveRecord
now: Time.now.to_i,
diffView: diff_view,
enableGFM: {
diff --git a/changelogs/unreleased/osw-remove-dead-code-on-mr-show.yml b/changelogs/unreleased/osw-remove-dead-code-on-mr-show.yml
new file mode 100644
index 00000000000..d4e2641daf5
--- /dev/null
+++ b/changelogs/unreleased/osw-remove-dead-code-on-mr-show.yml
@@ -0,0 +1,5 @@
+---
+title: Removes expensive dead code on main MR page request
+merge_request: 22153
+author:
+type: performance
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index f2467bfd525..73b62dc1151 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -76,28 +76,6 @@ describe Projects::MergeRequestsController do
expect(response).to be_success
end
- context "loads notes" do
- let(:first_contributor) { create(:user) }
- let(:contributor) { create(:user) }
- let(:merge_request) { create(:merge_request, author: first_contributor, target_project: project, source_project: project) }
- let(:contributor_merge_request) { create(:merge_request, :merged, author: contributor, target_project: project, source_project: project) }
- # the order here is important
- # as the controller reloads these from DB, references doesn't correspond after
- let!(:first_contributor_note) { create(:note, author: first_contributor, noteable: merge_request, project: project) }
- let!(:contributor_note) { create(:note, author: contributor, noteable: merge_request, project: project) }
- let!(:owner_note) { create(:note, author: user, noteable: merge_request, project: project) }
-
- it "with special_role FIRST_TIME_CONTRIBUTOR" do
- go(format: :html)
-
- notes = assigns(:notes)
- expect(notes).to match(a_collection_containing_exactly(an_object_having_attributes(special_role: Note::SpecialRole::FIRST_TIME_CONTRIBUTOR),
- an_object_having_attributes(special_role: nil),
- an_object_having_attributes(special_role: nil)
- ))
- end
- end
-
context "that is invalid" do
let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) }