Commit 1e7990d1 authored by Igor Drozdov's avatar Igor Drozdov

Preload all user callouts in a single request

The maximum number of user callouts for a single user
is limited by the number of different types of callouts

At the moment we have 28, so it ok to load all these
lightweight records instead of making a separate request
for every feature
parent 6f6aaaa5
...@@ -1851,10 +1851,12 @@ class User < ApplicationRecord ...@@ -1851,10 +1851,12 @@ class User < ApplicationRecord
end end
def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil) def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil)
callouts = self.callouts.with_feature_name(feature_name) callout = callouts_by_feature_name[feature_name]
callouts = callouts.with_dismissed_after(ignore_dismissal_earlier_than) if ignore_dismissal_earlier_than
callouts.any? return false unless callout
return callout.dismissed_after?(ignore_dismissal_earlier_than) if ignore_dismissal_earlier_than
true
end end
# Load the current highest access by looking directly at the user's memberships # Load the current highest access by looking directly at the user's memberships
...@@ -1917,6 +1919,10 @@ class User < ApplicationRecord ...@@ -1917,6 +1919,10 @@ class User < ApplicationRecord
private private
def callouts_by_feature_name
@callouts_by_feature_name ||= callouts.index_by(&:feature_name)
end
def authorized_groups_without_shared_membership def authorized_groups_without_shared_membership
Group.from_union([ Group.from_union([
groups, groups,
......
...@@ -38,6 +38,7 @@ class UserCallout < ApplicationRecord ...@@ -38,6 +38,7 @@ class UserCallout < ApplicationRecord
uniqueness: { scope: :user_id }, uniqueness: { scope: :user_id },
inclusion: { in: UserCallout.feature_names.keys } inclusion: { in: UserCallout.feature_names.keys }
scope :with_feature_name, -> (feature_name) { where(feature_name: UserCallout.feature_names[feature_name]) } def dismissed_after?(dismissed_after)
scope :with_dismissed_after, -> (dismissed_after) { where('dismissed_at > ?', dismissed_after) } dismissed_at > dismissed_after
end
end end
---
title: Preload all user callouts in a single request
merge_request: 57679
author:
type: performance
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require "spec_helper" require "spec_helper"
RSpec.describe EE::Ci::RunnersHelper do RSpec.describe EE::Ci::RunnersHelper do
let_it_be(:user) { create(:user) } let_it_be(:user, refind: true) { create(:user) }
let_it_be(:namespace) { create(:namespace, owner: user) } let_it_be(:namespace) { create(:namespace, owner: user) }
let_it_be(:project) { create(:project, namespace: namespace) } let_it_be(:project) { create(:project, namespace: namespace) }
......
...@@ -18,36 +18,14 @@ RSpec.describe UserCallout do ...@@ -18,36 +18,14 @@ RSpec.describe UserCallout do
it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity } it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity }
end end
describe 'scopes' do describe '#dismissed_after?' do
describe '.with_feature_name' do
let(:second_feature_name) { described_class.feature_names.keys.second }
let(:last_feature_name) { described_class.feature_names.keys.last }
it 'returns callout for requested feature name only' do
callout1 = create(:user_callout, feature_name: second_feature_name )
create(:user_callout, feature_name: last_feature_name )
callouts = described_class.with_feature_name(second_feature_name)
expect(callouts).to match_array([callout1])
end
end
describe '.with_dismissed_after' do
let(:some_feature_name) { described_class.feature_names.keys.second } let(:some_feature_name) { described_class.feature_names.keys.second }
let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )} let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )}
let(:callout_dismissed_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )}
it 'does not return callouts dismissed before specified date' do it 'returns whether a callout dismissed after specified date' do
callouts = described_class.with_dismissed_after(15.days.ago) expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false)
expect(callout_dismissed_day_ago.dismissed_after?(15.days.ago)).to eq(true)
expect(callouts).to match_array([])
end
it 'returns callouts dismissed after specified date' do
callouts = described_class.with_dismissed_after(2.months.ago)
expect(callouts).to match_array([callout_dismissed_month_ago])
end
end end
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