Merge branch 'id-preload-all-user-callouts' into 'master'

Preload all user callouts in a single request

See merge request gitlab-org/gitlab!57679

app/models/user.rb

@@ -1851,10 +1851,12 @@ class User < ApplicationRecord
   end
 
   def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil)
-    callouts = self.callouts.with_feature_name(feature_name)
-    callouts = callouts.with_dismissed_after(ignore_dismissal_earlier_than) if ignore_dismissal_earlier_than
+    callout = callouts_by_feature_name[feature_name]
 
-    callouts.any?
+    return false unless callout
+    return callout.dismissed_after?(ignore_dismissal_earlier_than) if ignore_dismissal_earlier_than
+
+    true
   end
 
   # Load the current highest access by looking directly at the user's memberships
@@ -1917,6 +1919,10 @@ class User < ApplicationRecord
 
   private
 
+  def callouts_by_feature_name
+    @callouts_by_feature_name ||= callouts.index_by(&:feature_name)
+  end
+
   def authorized_groups_without_shared_membership
     Group.from_union([
       groups,

app/models/user_callout.rb

@@ -38,6 +38,7 @@ class UserCallout < ApplicationRecord
     uniqueness: { scope: :user_id },
     inclusion: { in: UserCallout.feature_names.keys }
 
-  scope :with_feature_name, -> (feature_name) { where(feature_name: UserCallout.feature_names[feature_name]) }
-  scope :with_dismissed_after, -> (dismissed_after) { where('dismissed_at > ?', dismissed_after) }
+  def dismissed_after?(dismissed_after)
+    dismissed_at > dismissed_after
+  end
 end

changelogs/unreleased/id-preload-all-user-callouts.yml

+---
+title: Preload all user callouts in a single request
+merge_request: 57679
+author:
+type: performance

ee/spec/helpers/ee/ci/runners_helper_spec.rb

@@ -3,7 +3,7 @@
 require "spec_helper"
 
 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(:project) { create(:project, namespace: namespace) }
 

spec/models/user_callout_spec.rb

@@ -18,36 +18,14 @@ RSpec.describe UserCallout do
     it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity }
   end
 
-  describe 'scopes' 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(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )}
-
-      it 'does not return callouts dismissed before specified date' do
-        callouts = described_class.with_dismissed_after(15.days.ago)
-
-        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
+  describe '#dismissed_after?' do
+    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_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )}
+
+    it 'returns whether a callout dismissed after specified date' do
+      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)
     end
   end
 end