Merge branch 'main' into dependabot/bundler/aws-sdk-s3-1.199.0

Author: rsmithlal

app/controllers/better_together/notifications_controller.rb

@@ -13,32 +13,84 @@ def index
       @unread_count = helpers.current_person.notifications.unread.size
     end
 
+    def dropdown # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
+      # Get basic info needed for cache key (minimal queries)
+      max_updated_at = helpers.current_person.notifications.maximum(:updated_at)
+      unread_count = helpers.current_person.notifications.unread.size
+
+      # Create cache key based on max updated_at of user's notifications
+      cache_key = "notifications_dropdown/#{helpers.current_person.id}/#{max_updated_at&.to_i}/#{unread_count}"
+
+      cached_content = Rails.cache.fetch(cache_key, expires_in: 1.hour) do
+        # Only fetch detailed data when cache misses
+        notifications = helpers.recent_notifications
+
+        # Warm up fragment caches for individual notifications in background
+        warm_notification_fragment_caches(notifications) if Rails.env.production?
+
+        render_to_string(
+          partial: 'better_together/notifications/dropdown_content',
+          locals: { notifications: notifications, unread_count: unread_count }
+        )
+      end
+
+      render html: cached_content.html_safe
+    end
+
     # TODO: Make a Stimulus controller to dispatch this action async when messages are viewed
-    def mark_as_read # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
+    def mark_as_read
+      process_mark_as_read_request
+
+      respond_to do |format|
+        format.html { redirect_to notifications_path }
+        format.turbo_stream { render_turbo_stream_response }
+      end
+    end
+
+    private
+
+    def process_mark_as_read_request
       if params[:id]
         mark_notification_as_read(params[:id])
       elsif params[:record_id]
         mark_record_notification_as_read(params[:record_id])
       else
         helpers.current_person.notifications.unread.update_all(read_at: Time.current)
       end
+    end
 
-      respond_to do |format|
-        format.html { redirect_to notifications_path }
-        format.turbo_stream do
-          if @notification
-            render turbo_stream: turbo_stream.replace(helpers.dom_id(@notification), @notification)
-          else
-            render turbo_stream: turbo_stream.replace(
-              'notifications',
-              partial: 'better_together/notifications/notifications',
-              locals: { notifications: helpers.current_person.notifications, unread_count: 0 }
-            )
-          end
-        end
+    def render_turbo_stream_response
+      if @notification
+        render_notification_turbo_stream
+      else
+        render_notifications_list_turbo_stream
       end
     end
 
+    def render_notification_turbo_stream
+      render turbo_stream: turbo_stream.replace(
+        helpers.dom_id(@notification),
+        partial: 'better_together/notifications/notification',
+        locals: notification_locals
+      )
+    end
+
+    def render_notifications_list_turbo_stream
+      render turbo_stream: turbo_stream.replace(
+        'notifications',
+        partial: 'better_together/notifications/notifications',
+        locals: { notifications: helpers.current_person.notifications, unread_count: 0 }
+      )
+    end
+
+    def notification_locals
+      {
+        notification: @notification,
+        notification_title: @notification.event.record&.try(:title) || 'Notification',
+        notification_url: @notification.try(:url) || '#'
+      }
+    end
+
     def mark_notification_as_read(id)
       @notification = helpers.current_person.notifications.find(id)
       @notification.update(read_at: Time.current)
@@ -47,5 +99,22 @@ def mark_notification_as_read(id)
     def mark_record_notification_as_read(id)
       mark_notifications_read_for_record_id(id)
     end
+
+    # Warm fragment caches for notifications to improve subsequent renders
+    def warm_notification_fragment_caches(notifications)
+      notifications.each do |notification|
+        next unless helpers.should_cache_notification?(notification)
+
+        # Pre-warm the fragment cache keys
+        fragment_key = helpers.notification_fragment_cache_key(notification)
+        type_key = helpers.notification_type_fragment_cache_key(notification)
+
+        # Check if fragments are already cached to avoid unnecessary work
+        unless Rails.cache.exist?(fragment_key) && Rails.cache.exist?(type_key)
+          # This could be moved to a background job for better performance
+          Rails.logger.debug "Warming fragment cache for notification #{notification.id}"
+        end
+      end
+    end
   end
 end

app/helpers/better_together/notifications_helper.rb

@@ -24,5 +24,67 @@ def unread_notification_count
     def recent_notifications
       current_person.notifications.joins(:event).order(created_at: :desc).limit(5)
     end
+
+    # Fragment cache key for notification types
+    def notification_fragment_cache_key(notification)
+      # Base cache key with notification's cache_key_with_version
+      base_key = [notification.cache_key_with_version]
+
+      # Add related record version if it exists and has cache_key_with_version
+      if notification.event&.record.respond_to?(:cache_key_with_version)
+        base_key << notification.event.record.cache_key_with_version
+      end
+
+      # Add event cache key for consistency
+      base_key << notification.event.cache_key_with_version if notification.event.respond_to?(:cache_key_with_version)
+
+      # Add locale for i18n support
+      base_key << I18n.locale
+
+      base_key
+    end
+
+    # Type-specific fragment cache key for notification patterns
+    def notification_type_fragment_cache_key(notification)
+      type_key = [
+        notification.type || notification.class.name,
+        notification.cache_key_with_version,
+        I18n.locale
+      ]
+
+      # Add related record cache key if available
+      if notification.event&.record.respond_to?(:cache_key_with_version)
+        type_key << notification.event.record.cache_key_with_version
+      end
+
+      type_key
+    end
+
+    # Cache expiration utilities for notifications
+    def expire_notification_fragments(notification)
+      # Expire all fragments related to this notification
+      expire_fragment(notification_fragment_cache_key(notification))
+      expire_fragment(notification_type_fragment_cache_key(notification))
+
+      # Expire header/content/footer fragments
+      expire_fragment(['notification_header', notification.cache_key_with_version])
+      expire_fragment(['notification_content', notification.cache_key_with_version])
+      expire_fragment(['notification_footer', notification.cache_key_with_version])
+    end
+
+    # Expire fragments for a specific notification type pattern
+    def expire_notification_type_fragments(notification_type)
+      # This is useful when you want to expire all cached fragments for a specific notification type
+      # Note: This requires more specific cache management based on your needs
+      Rails.cache.delete_matched("*#{notification_type}*")
+    end
+
+    # Check if a notification should use fragment caching
+    def should_cache_notification?(notification)
+      # Only cache if the notification and its record are present and have cache keys
+      notification.present? &&
+        notification.event&.record.present? &&
+        notification.respond_to?(:cache_key_with_version)
+    end
   end
 end

app/javascript/controllers/better_together/lazy_notifications_controller.js

@@ -0,0 +1,79 @@
+// app/assets/javascripts/controllers/better_together/lazy_notifications_controller.js
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  static targets = [ "dropdown", "content" ]
+  static values = { 
+    loaded: Boolean,
+    url: String
+  }
+
+  connect() {
+    this.loadedValue = false
+    console.log('Lazy notifications controller connected with URL:', this.urlValue)
+    
+    // Find the dropdown toggle element (the <a> tag with data-bs-toggle="dropdown")
+    const dropdownToggle = this.element.querySelector('[data-bs-toggle="dropdown"]')
+    
+    if (dropdownToggle) {
+      console.log('Found dropdown toggle, adding event listener')
+      // Listen for Bootstrap dropdown show event on the dropdown toggle
+      dropdownToggle.addEventListener('show.bs.dropdown', (event) => {
+        console.log('Bootstrap dropdown show event triggered')
+        if (!this.loadedValue) {
+          this.loadContent()
+        }
+      })
+    } else {
+      console.log('No dropdown toggle found')
+    }
+  }
+
+  toggle(event) {
+    // This method can be removed or kept for debugging
+    console.log('Dropdown toggle clicked')
+  }
+
+  async loadContent() {
+    if (this.loadedValue) return
+    
+    console.log('Loading notifications from:', this.urlValue)
+    
+    try {
+      // Show loading state
+      this.contentTarget.innerHTML = `
+        <div class="text-center p-3">
+          <div class="spinner-border spinner-border-sm" role="status">
+            <span class="visually-hidden">Loading...</span>
+          </div>
+          <div class="mt-2">Loading notifications...</div>
+        </div>
+      `
+      
+      const response = await fetch(this.urlValue, {
+        headers: {
+          'Accept': 'text/html'
+        }
+      })
+      
+      if (response.ok) {
+        const html = await response.text()
+        this.contentTarget.innerHTML = html
+        this.loadedValue = true
+      } else {
+        this.contentTarget.innerHTML = `
+          <div class="text-danger text-center p-3">
+            Failed to load notifications
+          </div>
+        `
+      }
+    } catch (error) {
+      console.error('Error loading notifications:', error)
+      this.contentTarget.innerHTML = `
+        <div class="text-danger text-center p-3">
+          Error loading notifications
+        </div>
+      `
+    }
+  }
+}

app/jobs/better_together/notification_cache_warming_job.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+module BetterTogether
+  # Background job for warming notification fragment caches
+  class NotificationCacheWarmingJob < ApplicationJob
+    queue_as :low_priority
+
+    def perform(notification_ids)
+      notifications = Noticed::Notification.where(id: notification_ids).includes(event: :record)
+
+      notifications.find_each do |notification|
+        warm_notification_fragments(notification) if should_warm_cache?(notification)
+      end
+    end
+
+    private
+
+    def warm_notification_fragments(notification)
+      # Generate cache keys without actually rendering to check existence
+      fragment_key = notification_fragment_cache_key(notification)
+      type_key = notification_type_fragment_cache_key(notification)
+
+      # Only warm if not already cached
+      return if Rails.cache.exist?(fragment_key) && Rails.cache.exist?(type_key)
+
+      # Render the notification to warm the cache
+      ApplicationController.renderer.render(
+        partial: notification,
+        locals: {},
+        formats: [:html]
+      )
+    rescue StandardError => e
+      Rails.logger.warn "Failed to warm cache for notification #{notification.id}: #{e.message}"
+    end
+
+    def should_warm_cache?(notification)
+      notification.event&.record.present? &&
+        notification.respond_to?(:cache_key_with_version) &&
+        notification.created_at > 1.week.ago # Only warm recent notifications
+    end
+
+    def notification_fragment_cache_key(notification)
+      base_key = [notification.cache_key_with_version]
+      if notification.event&.record.respond_to?(:cache_key_with_version)
+        base_key << notification.event.record.cache_key_with_version
+      end
+      base_key << notification.event.cache_key_with_version if notification.event.respond_to?(:cache_key_with_version)
+      base_key << I18n.locale
+      base_key
+    end
+
+    def notification_type_fragment_cache_key(notification)
+      type_key = [
+        notification.type || notification.class.name,
+        notification.cache_key_with_version,
+        I18n.locale
+      ]
+      if notification.event&.record.respond_to?(:cache_key_with_version)
+        type_key << notification.event.record.cache_key_with_version
+      end
+      type_key
+    end
+  end
+end

app/models/concerns/better_together/notification_cache_management.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module BetterTogether
+  # Concern for managing notification fragment caches
+  module NotificationCacheManagement
+    extend ActiveSupport::Concern
+
+    included do
+      # Automatically expire fragment caches when notification changes
+      after_update :expire_notification_caches, if: :should_expire_caches?
+      after_destroy :expire_notification_caches
+    end
+
+    private
+
+    def expire_notification_caches
+      # Use the helper methods to expire relevant fragments
+      ApplicationController.helpers.expire_notification_fragments(self) if respond_to_cache_methods?
+    end
+
+    def should_expire_caches?
+      # Expire caches on status changes (read/unread) or content changes
+      saved_change_to_read_at? || saved_change_to_created_at? || saved_change_to_updated_at?
+    end
+
+    def respond_to_cache_methods?
+      ApplicationController.helpers.respond_to?(:expire_notification_fragments)
+    rescue StandardError
+      false
+    end
+  end
+end

app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb

@@ -1,10 +1,12 @@
 <!-- app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb -->
-<!-- app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb -->
 <%= render layout: 'better_together/notifications/notification',
            locals: { notification: notification,
                      notification_title: notification.title,
                      notification_url: notification.invitable } do %>
-  <p class="mb-1">
-    <%= notification.body %>
-  </p>
+  <% # Fragment cache for event invitation-specific content %>
+  <% cache ["event_invitation_content", notification.cache_key_with_version, notification.invitable.cache_key_with_version, I18n.locale] do %>
+    <p class="mb-1">
+      <%= notification.body %>
+    </p>
+  <% end %>
 <% end %>

app/views/better_together/joatu/agreement_notifier/notifications/_notification.html.erb

@@ -3,7 +3,10 @@
            locals: { notification: notification,
                      notification_title: notification.title,
                      notification_url: notification.agreement } do %>
-  <p class="mb-1">
-    <%= notification.body %>
-  </p>
+  <% # Fragment cache for agreement-specific content %>
+  <% cache ["agreement_notification_content", notification.cache_key_with_version, notification.agreement.cache_key_with_version, I18n.locale] do %>
+    <p class="mb-1">
+      <%= notification.body %>
+    </p>
+  <% end %>
 <% end %>