Commit 379d6391 authored by Justin Ho Tuan Duong's avatar Justin Ho Tuan Duong Committed by Luke Duncalfe

Only include fields used by the frontend

To avoid loading tags and project_import_data
even though it's not used,
we now only select the id and name of the project which
are the only 2 attributes used on the frontend.

This prevents fetching unused data and an N+1 query
on projects.

https://gitlab.com/gitlab-org/gitlab/-/issues/21044
parent 17c4304e
---
title: Reduce number of SQL queries in Profiles::SlacksController#edit
merge_request: 57674
author:
type: performance
...@@ -10,7 +10,7 @@ class Profiles::SlacksController < Profiles::ApplicationController ...@@ -10,7 +10,7 @@ class Profiles::SlacksController < Profiles::ApplicationController
feature_category :users feature_category :users
def edit def edit
@projects = disabled_projects if current_user @projects = disabled_projects.select([:id, :name]) if current_user
end end
def slack_link def slack_link
......
...@@ -36,7 +36,7 @@ module EE ...@@ -36,7 +36,7 @@ module EE
def add_to_slack_data(projects) def add_to_slack_data(projects)
{ {
projects: projects, projects: projects.as_json(only: [:id, :name]),
sign_in_path: new_session_path(:user, redirect_to_referer: 'yes'), sign_in_path: new_session_path(:user, redirect_to_referer: 'yes'),
is_signed_in: current_user.present?, is_signed_in: current_user.present?,
slack_link_profile_slack_path: slack_link_profile_slack_path, slack_link_profile_slack_path: slack_link_profile_slack_path,
......
...@@ -3,9 +3,12 @@ ...@@ -3,9 +3,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe EE::ServicesHelper do RSpec.describe EE::ServicesHelper do
include Devise::Test::ControllerHelpers
let(:controller_class) do let(:controller_class) do
Class.new(ActionController::Base) do Class.new(ActionController::Base) do
include EE::ServicesHelper include EE::ServicesHelper
include ActionView::Helpers::AssetUrlHelper
def slack_auth_project_settings_slack_url(project) def slack_auth_project_settings_slack_url(project)
"http://some-path/project/1" "http://some-path/project/1"
...@@ -111,4 +114,54 @@ RSpec.describe EE::ServicesHelper do ...@@ -111,4 +114,54 @@ RSpec.describe EE::ServicesHelper do
is_expected.to include(:issues_show_path) is_expected.to include(:issues_show_path)
end end
end end
describe '#add_to_slack_data' do
let_it_be(:projects) { create_list(:project, 3) }
def relation
Project.id_in(projects.pluck(:id))
end
let(:request) do
double(:Request,
optional_port: nil,
path_parameters: {},
protocol: 'https',
routes: nil,
env: { 'warden' => warden },
engine_script_name: nil,
original_script_name: nil,
host: 'example.com')
end
before do
allow(subject).to receive(:request).and_return(request)
end
it 'includes the required keys' do
additions = Gitlab::Json.parse(subject.add_to_slack_data(relation))
expect(additions.keys).to match_array %w[
projects
sign_in_path
is_signed_in
slack_link_profile_slack_path
gitlab_for_slack_gif_path
gitlab_logo_path
slack_logo_path
docs_path
]
end
it 'does not suffer from N+1 performance issues' do
baseline = ActiveRecord::QueryRecorder.new { subject.add_to_slack_data(relation.limit(1)) }
expect do
subject.add_to_slack_data(relation)
end.not_to exceed_query_limit(baseline)
end
it 'serializes nil projects without error' do
expect(subject.add_to_slack_data(nil)).to include('"projects":null')
end
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