Commit 8c9ad563 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch '196250' into 'master'

#196250 adding preload to prevent N+1

See merge request gitlab-org/gitlab!22845
parents 4671a965 f9eb6c29
...@@ -4,13 +4,14 @@ class Profiles::NotificationsController < Profiles::ApplicationController ...@@ -4,13 +4,14 @@ class Profiles::NotificationsController < Profiles::ApplicationController
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def show def show
@user = current_user @user = current_user
@group_notifications = current_user.notification_settings.for_groups.order(:id) @group_notifications = current_user.notification_settings.preload_source_route.for_groups.order(:id)
@group_notifications += GroupsFinder.new( @group_notifications += GroupsFinder.new(
current_user, current_user,
all_available: false, all_available: false,
exclude_group_ids: @group_notifications.select(:source_id) exclude_group_ids: @group_notifications.select(:source_id)
).execute.map { |group| current_user.notification_settings_for(group, inherit: true) } ).execute.map { |group| current_user.notification_settings_for(group, inherit: true) }
@project_notifications = current_user.notification_settings.for_projects.order(:id) @project_notifications = current_user.notification_settings.for_projects.order(:id)
.preload_source_route
.select { |notification| current_user.can?(:read_project, notification.source) } .select { |notification| current_user.can?(:read_project, notification.source) }
@global_notification_setting = current_user.global_notification_setting @global_notification_setting = current_user.global_notification_setting
end end
......
...@@ -27,6 +27,8 @@ class NotificationSetting < ApplicationRecord ...@@ -27,6 +27,8 @@ class NotificationSetting < ApplicationRecord
.where.not(projects: { pending_delete: true }) .where.not(projects: { pending_delete: true })
end end
scope :preload_source_route, -> { preload(source: [:route]) }
EMAIL_EVENTS = [ EMAIL_EVENTS = [
:new_release, :new_release,
:new_note, :new_note,
......
---
title: Remove N+1 query for profile notifications
merge_request: 22845
author: Ohad Dahan
type: performance
...@@ -24,7 +24,6 @@ describe Profiles::NotificationsController do ...@@ -24,7 +24,6 @@ describe Profiles::NotificationsController do
context 'with groups that do not have notification preferences' do context 'with groups that do not have notification preferences' do
set(:group) { create(:group) } set(:group) { create(:group) }
set(:subgroup) { create(:group, parent: group) } set(:subgroup) { create(:group, parent: group) }
before do before do
group.add_developer(user) group.add_developer(user)
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe 'view user notifications' do
let(:user) do
create(:user) do |user|
user.emails.create(email: 'original@example.com')
user.emails.create(email: 'new@example.com')
user.notification_email = 'original@example.com'
user.save!
end
end
before do
login_as(user)
create_list(:group, 2) do |group|
group.add_developer(user)
end
end
def get_profile_notifications
get profile_notifications_path
end
describe 'GET /profile/notifications' do
it 'avoid N+1 due to an additional groups (with no parent group)' do
get_profile_notifications
control = ActiveRecord::QueryRecorder.new do
get_profile_notifications
end
create_list(:group, 2) { |group| group.add_developer(user) }
expect do
get_profile_notifications
end.not_to exceed_query_limit(control)
end
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