perf: fix notifications duplicate query and add composite index (#12110)

Database CPU utilization was spiking due to expensive notification COUNT
queries. Analysis revealed two critical issues:

1. Missing database index: Notification count queries were performing
table scans without proper indexing
2. Duplicate WHERE clauses: SQL queries contained redundant read_at IS
NULL conditions, causing unnecessary query complexity

 ### Root Cause Analysis

  The expensive queries were:
```
  -- 41.61 calls/sec with duplicate condition
  SELECT COUNT(*) FROM "notifications"
  WHERE "notifications"."user_id" = $1
    AND "notifications"."account_id" = $2
    AND "notifications"."snoozed_until" IS NULL
    AND "notifications"."read_at" IS NULL
    AND "notifications"."read_at" IS NULL  -- Duplicate!
```
This was caused by a logic error in NotificationFinder#unread_count
introduced in commit cd06b2b33 (PR #8907). The method assumed
@notifications contained all notifications, but @notifications was
already filtered to unread notifications in most cases.

###  The Default Query Flow:

1. Frontend calls: NotificationsAPI.getUnreadCount() →
/notifications/unread_count
  2. No parameters sent, so params = {}
  3. NotificationFinder setup:
    - find_all_notifications: WHERE user_id = ? AND account_id = ?
    - filter_snoozed_notifications: WHERE snoozed_until IS NULL
- filter_read_notifications: WHERE read_at IS NULL (because
type_included?('read') is false)
  4. unread_count called: Adds another WHERE read_at IS NULL
----
### Solution

  1. Added Missing Database Index
  - Index: (user_id, account_id, snoozed_until, read_at)
  2. Fixed Duplicate WHERE Clause Logic

app/finders/notification_finder.rb

@@ -15,7 +15,13 @@ class NotificationFinder
   end
 
   def unread_count
-    @notifications.where(read_at: nil).count
+    if type_included?('read')
+      # If we're including read notifications, filter to unread
+      @notifications.where(read_at: nil).count
+    else
+      # Already filtered to unread notifications, just count
+      @notifications.count
+    end
   end
 
   def count
@@ -27,7 +33,7 @@ class NotificationFinder
   def set_up
     find_all_notifications
     filter_snoozed_notifications
-    fitler_read_notifications
+    filter_read_notifications
   end
 
   def find_all_notifications
@@ -38,7 +44,7 @@ class NotificationFinder
     @notifications = @notifications.where(snoozed_until: nil) unless type_included?('snoozed')
   end
 
-  def fitler_read_notifications
+  def filter_read_notifications
     @notifications = @notifications.where(read_at: nil) unless type_included?('read')
   end
 

db/migrate/20250805160307_add_notifications_performance_index.rb

@@ -0,0 +1,11 @@
+class AddNotificationsPerformanceIndex < ActiveRecord::Migration[7.1]
+  disable_ddl_transaction!
+
+  def change
+    # Add composite index to optimize notification count queries
+    # This covers the common query pattern: WHERE user_id = ? AND account_id = ? AND snoozed_until IS NULL AND read_at IS NULL
+    add_index :notifications, [:user_id, :account_id, :snoozed_until, :read_at],
+              name: 'idx_notifications_performance',
+              algorithm: :concurrently
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2025_07_22_152516) do
+ActiveRecord::Schema[7.1].define(version: 2025_08_05_160307) do
   # These extensions should be enabled to support this database
   enable_extension "pg_stat_statements"
   enable_extension "pg_trgm"
@@ -909,6 +909,7 @@ ActiveRecord::Schema[7.1].define(version: 2025_07_22_152516) do
     t.index ["last_activity_at"], name: "index_notifications_on_last_activity_at"
     t.index ["primary_actor_type", "primary_actor_id"], name: "uniq_primary_actor_per_account_notifications"
     t.index ["secondary_actor_type", "secondary_actor_id"], name: "uniq_secondary_actor_per_account_notifications"
+    t.index ["user_id", "account_id", "snoozed_until", "read_at"], name: "idx_notifications_performance"
     t.index ["user_id"], name: "index_notifications_on_user_id"
   end
 

spec/finders/notification_finder_spec.rb

@@ -66,14 +66,31 @@ RSpec.describe NotificationFinder do
         expect(subject.unread_count).to eq(3)
         expect(subject.count).to eq(3)
       end
+
+      it 'avoids duplicate filtering in unread_count method' do
+        # Test the logical fix: when no 'read' filter is included,
+        # @notifications is already filtered to unread, so unread_count
+        # should just count without adding another read_at filter
+
+        allow(subject.instance_variable_get(:@notifications)).to receive(:where).and_call_original
+        allow(subject.instance_variable_get(:@notifications)).to receive(:count).and_call_original
+
+        result = subject.unread_count
+
+        # Should return correct count without additional where clause
+        expect(result).to eq(3)
+
+        # The fix ensures that when params[:includes] doesn't contain 'read',
+        # unread_count uses @notifications.count instead of @notifications.where(read_at: nil).count
+      end
     end
 
     context 'with filters applied' do
       let(:params) { { includes: %w[read snoozed] } }
 
       it 'adjusts counts based on included statuses' do
-        expect(subject.unread_count).to eq(4)
-        expect(subject.count).to eq(6)
+        expect(subject.unread_count).to eq(4) # 3 unread + 1 snoozed (which is unread)
+        expect(subject.count).to eq(6) # all notifications including read and snoozed
       end
     end
   end