Commit 66ec9255 authored by Paco Guzman's avatar Paco Guzman

Preload notes/discussions associations (award_emoji: :user)

parent e7a27946
...@@ -4,6 +4,7 @@ v 8.10.0 (unreleased) ...@@ -4,6 +4,7 @@ v 8.10.0 (unreleased)
- Wrap code blocks on Activies and Todos page. !4783 (winniehell) - Wrap code blocks on Activies and Todos page. !4783 (winniehell)
- Add Sidekiq queue duration to transaction metrics. - Add Sidekiq queue duration to transaction metrics.
- Fix MR-auto-close text added to description. !4836 - Fix MR-auto-close text added to description. !4836
- Eager load award emoji on notes
- Fix pagination when sorting by columns with lots of ties (like priority) - Fix pagination when sorting by columns with lots of ties (like priority)
- Implement Subresource Integrity for CSS and JavaScript assets. This prevents malicious assets from loading in the case of a CDN compromise. - Implement Subresource Integrity for CSS and JavaScript assets. This prevents malicious assets from loading in the case of a CDN compromise.
- Fix user creation with stronger minimum password requirements !4054 (nathan-pmt) - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt)
......
...@@ -320,8 +320,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -320,8 +320,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def define_show_vars def define_show_vars
# Build a note object for comment form # Build a note object for comment form
@note = @project.notes.new(noteable: @merge_request) @note = @project.notes.new(noteable: @merge_request)
@notes = @merge_request.mr_and_commit_notes.inc_author.fresh @discussions = @merge_request.mr_and_commit_notes.inc_author_project_award_emoji.fresh.discussions
@discussions = @notes.discussions @notes = @discussions.flatten
@noteable = @merge_request @noteable = @merge_request
# Get commits from repository # Get commits from repository
......
...@@ -2,10 +2,11 @@ module Awardable ...@@ -2,10 +2,11 @@ module Awardable
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
has_many :award_emoji, as: :awardable, dependent: :destroy has_many :award_emoji, -> { includes(:user) }, as: :awardable, dependent: :destroy
if self < Participable if self < Participable
participant :award_emoji_with_associations # By default we always load award_emoji user association
participant :award_emoji
end end
end end
...@@ -34,12 +35,9 @@ module Awardable ...@@ -34,12 +35,9 @@ module Awardable
end end
end end
def award_emoji_with_associations
award_emoji.includes(:user)
end
def grouped_awards(with_thumbs: true) def grouped_awards(with_thumbs: true)
awards = award_emoji_with_associations.group_by(&:name) # By default we always load award_emoji user association
awards = award_emoji.group_by(&:name)
if with_thumbs if with_thumbs
awards[AwardEmoji::UPVOTE_NAME] ||= [] awards[AwardEmoji::UPVOTE_NAME] ||= []
......
...@@ -19,9 +19,14 @@ module Issuable ...@@ -19,9 +19,14 @@ module Issuable
belongs_to :milestone belongs_to :milestone
has_many :notes, as: :noteable, dependent: :destroy do has_many :notes, as: :noteable, dependent: :destroy do
def authors_loaded? def authors_loaded?
# We check first if we're loaded to not load unnecesarily. # We check first if we're loaded to not load unnecessarily.
loaded? && to_a.all? { |note| note.association(:author).loaded? } loaded? && to_a.all? { |note| note.association(:author).loaded? }
end end
def award_emojis_loaded?
# We check first if we're loaded to not load unnecessarily.
loaded? && to_a.all? { |note| note.association(:award_emoji).loaded? }
end
end end
has_many :label_links, as: :target, dependent: :destroy has_many :label_links, as: :target, dependent: :destroy
has_many :labels, through: :label_links has_many :labels, through: :label_links
...@@ -49,7 +54,7 @@ module Issuable ...@@ -49,7 +54,7 @@ module Issuable
scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) } scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) }
scope :join_project, -> { joins(:project) } scope :join_project, -> { joins(:project) }
scope :inc_notes_with_associations, -> { includes(notes: :author) } scope :inc_notes_with_associations, -> { includes(notes: [ :project, :author, :award_emoji ]) }
scope :references_project, -> { references(:project) } scope :references_project, -> { references(:project) }
scope :non_archived, -> { join_project.where(projects: { archived: false }) } scope :non_archived, -> { join_project.where(projects: { archived: false }) }
...@@ -260,7 +265,14 @@ module Issuable ...@@ -260,7 +265,14 @@ module Issuable
# already have their authors loaded (possibly because the scope # already have their authors loaded (possibly because the scope
# `inc_notes_with_associations` was used) and skip the inclusion if that's # `inc_notes_with_associations` was used) and skip the inclusion if that's
# the case. # the case.
notes.authors_loaded? ? notes : notes.includes(:author) includes = []
includes << :author unless notes.authors_loaded?
includes << :award_emoji unless notes.award_emojis_loaded?
if includes.any?
notes.includes(includes)
else
notes
end
end end
def updated_tasks def updated_tasks
......
...@@ -309,7 +309,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -309,7 +309,7 @@ class MergeRequest < ActiveRecord::Base
commits_for_notes_limit = 100 commits_for_notes_limit = 100
commit_ids = commits.last(commits_for_notes_limit).map(&:id) commit_ids = commits.last(commits_for_notes_limit).map(&:id)
Note.includes(:award_emoji).where( Note.where(
"(project_id = :target_project_id AND noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR" + "(project_id = :target_project_id AND noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR" +
"((project_id = :source_project_id OR project_id = :target_project_id) AND noteable_type = 'Commit' AND commit_id IN (:commit_ids))", "((project_id = :source_project_id OR project_id = :target_project_id) AND noteable_type = 'Commit' AND commit_id IN (:commit_ids))",
mr_id: id, mr_id: id,
......
...@@ -49,12 +49,14 @@ class Note < ActiveRecord::Base ...@@ -49,12 +49,14 @@ class Note < ActiveRecord::Base
scope :fresh, ->{ order(created_at: :asc, id: :asc) } scope :fresh, ->{ order(created_at: :asc, id: :asc) }
scope :inc_author_project, ->{ includes(:project, :author) } scope :inc_author_project, ->{ includes(:project, :author) }
scope :inc_author, ->{ includes(:author) } scope :inc_author, ->{ includes(:author) }
scope :inc_author_project_award_emoji, ->{ includes(:project, :author, :award_emoji) }
scope :legacy_diff_notes, ->{ where(type: 'LegacyDiffNote') } scope :legacy_diff_notes, ->{ where(type: 'LegacyDiffNote') }
scope :non_diff_notes, ->{ where(type: ['Note', nil]) } scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
scope :with_associations, -> do scope :with_associations, -> do
includes(:author, :noteable, :updated_by, :award_emoji, # FYI noteable cannot be loaded for LegacyDiffNote for commits
includes(:author, :noteable, :updated_by,
project: [:project_members, { group: [:group_members] }]) project: [:project_members, { group: [:group_members] }])
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment